mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-12-23 22:25:25 +00:00
blockjob: allow for fast-finishing job
In my testing, I was able to provoke an odd block pull failure: $ virsh blockpull dom vda --bandwidth 10000 error: Requested operation is not valid: No active operation on device: drive-virtio-disk0 merely by using gdb to artifically wait to do the block job set speed until after the pull had already finished. But in reality, that should be a success, since the pull finished before we had a chance to set speed. Furthermore, using a double job lock is not only annoying, but a bug in itself - if you do parallel virDomainBlockRebase, and hit the race window just right, the first call grabs the VM job to start a fast block job, then the second call grabs the VM job to start a long-running job with unspecified speed, then the first call finally regrabs the VM job and sets the speed, which ends up running the second job under the speed from the first call. By consolidating things into a single job, we avoid opening that race, as well as reduce the time between starting the job and changing the speed, for less likelihood of the speed change happening after block job completion in the first place. * src/qemu/qemu_monitor.h (BLOCK_JOB_CMD): Add new mode. * src/qemu/qemu_driver.c (qemuDomainBlockRebase): Move secondary job call... (qemuDomainBlockJobImpl): ...here, for fewer locks. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockJob): Change return value on new internal mode.
This commit is contained in:
parent
a91ce852b5
commit
a9d3495e67
@ -11654,6 +11654,9 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
|
||||
* relying on qemu to do this. */
|
||||
ret = qemuMonitorBlockJob(priv->mon, device, base, bandwidth, info, mode,
|
||||
async);
|
||||
if (ret == 0 && mode == BLOCK_JOB_PULL && bandwidth)
|
||||
ret = qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
|
||||
BLOCK_JOB_SPEED_INTERNAL, async);
|
||||
qemuDomainObjExitMonitorWithDriver(driver, vm);
|
||||
if (ret < 0)
|
||||
goto endjob;
|
||||
@ -11749,15 +11752,9 @@ static int
|
||||
qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
|
||||
unsigned long bandwidth, unsigned int flags)
|
||||
{
|
||||
int ret;
|
||||
|
||||
virCheckFlags(0, -1);
|
||||
ret = qemuDomainBlockJobImpl(dom, path, base, bandwidth, NULL,
|
||||
BLOCK_JOB_PULL, flags);
|
||||
if (ret == 0 && bandwidth != 0)
|
||||
ret = qemuDomainBlockJobImpl(dom, path, NULL, bandwidth, NULL,
|
||||
BLOCK_JOB_SPEED, flags);
|
||||
return ret;
|
||||
return qemuDomainBlockJobImpl(dom, path, base, bandwidth, NULL,
|
||||
BLOCK_JOB_PULL, flags);
|
||||
}
|
||||
|
||||
static int
|
||||
|
@ -530,7 +530,8 @@ typedef enum {
|
||||
BLOCK_JOB_ABORT = 0,
|
||||
BLOCK_JOB_INFO = 1,
|
||||
BLOCK_JOB_SPEED = 2,
|
||||
BLOCK_JOB_PULL = 3,
|
||||
BLOCK_JOB_SPEED_INTERNAL = 3,
|
||||
BLOCK_JOB_PULL = 4,
|
||||
} BLOCK_JOB_CMD;
|
||||
|
||||
int qemuMonitorBlockJob(qemuMonitorPtr mon,
|
||||
|
@ -3453,6 +3453,7 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
|
||||
cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL);
|
||||
break;
|
||||
case BLOCK_JOB_SPEED:
|
||||
case BLOCK_JOB_SPEED_INTERNAL:
|
||||
cmd_name = async ? "block-job-set-speed" : "block_job_set_speed";
|
||||
cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device",
|
||||
device, "U:value",
|
||||
@ -3474,22 +3475,30 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
|
||||
ret = qemuMonitorJSONCommand(mon, cmd, &reply);
|
||||
|
||||
if (ret == 0 && virJSONValueObjectHasKey(reply, "error")) {
|
||||
if (qemuMonitorJSONHasError(reply, "DeviceNotActive"))
|
||||
qemuReportError(VIR_ERR_OPERATION_INVALID,
|
||||
_("No active operation on device: %s"), device);
|
||||
else if (qemuMonitorJSONHasError(reply, "DeviceInUse"))
|
||||
ret = -1;
|
||||
if (qemuMonitorJSONHasError(reply, "DeviceNotActive")) {
|
||||
/* If a job completes before we get a chance to set the
|
||||
* speed, we don't want to fail the original command. */
|
||||
if (mode == BLOCK_JOB_SPEED_INTERNAL)
|
||||
ret = 0;
|
||||
else
|
||||
qemuReportError(VIR_ERR_OPERATION_INVALID,
|
||||
_("No active operation on device: %s"),
|
||||
device);
|
||||
} else if (qemuMonitorJSONHasError(reply, "DeviceInUse")){
|
||||
qemuReportError(VIR_ERR_OPERATION_FAILED,
|
||||
_("Device %s in use"), device);
|
||||
else if (qemuMonitorJSONHasError(reply, "NotSupported"))
|
||||
_("Device %s in use"), device);
|
||||
} else if (qemuMonitorJSONHasError(reply, "NotSupported")) {
|
||||
qemuReportError(VIR_ERR_OPERATION_INVALID,
|
||||
_("Operation is not supported for device: %s"), device);
|
||||
else if (qemuMonitorJSONHasError(reply, "CommandNotFound"))
|
||||
_("Operation is not supported for device: %s"),
|
||||
device);
|
||||
} else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
|
||||
qemuReportError(VIR_ERR_OPERATION_INVALID,
|
||||
_("Command '%s' is not found"), cmd_name);
|
||||
else
|
||||
_("Command '%s' is not found"), cmd_name);
|
||||
} else {
|
||||
qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||||
_("Unexpected error"));
|
||||
ret = -1;
|
||||
}
|
||||
}
|
||||
|
||||
if (ret == 0 && mode == BLOCK_JOB_INFO)
|
||||
|
Loading…
Reference in New Issue
Block a user