[libvirt] [PATCH] Fix regressions BlockStats/Info APIs in QEMU driver
From: "Daniel P. Berrange" <berrange@redhat.com> The change 18c2a592064d69499f70428e498f4a3cb5161cda caused some regressions in behaviour of virDomainBlockStats and virDomainBlockInfo in the QEMU driver. The virDomainBlockInfo API stopped working for inactive guests if querying a block device. The virDomainBlockStats API did not promptly report an error if the guest was not running in some cases. * src/qemu/qemu_driver.c: Fix inactive guest handling in BlockStats/Info APIs --- src/qemu/qemu_driver.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5632d62..3716b15 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5531,6 +5531,12 @@ qemudDomainBlockStats (virDomainPtr dom, goto cleanup; } + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(path, vm->def->disks[i]->dst)) { disk = vm->def->disks[i]; @@ -5994,7 +6000,8 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, highest allocated extent from QEMU */ if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK && format != VIR_STORAGE_FILE_RAW && - S_ISBLK(sb.st_mode)) { + S_ISBLK(sb.st_mode) && + virDomainObjIsActive(vm)) { qemuDomainObjPrivatePtr priv = vm->privateData; if ((priv->jobActive == QEMU_JOB_MIGRATION_OUT) @@ -6017,10 +6024,9 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, if (qemuDomainObjBeginJob(vm) < 0) goto cleanup; - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto endjob; + if (virDomainObjIsActive(vm)) { + ret = 0; + goto cleanup; } qemuDomainObjEnterMonitor(vm); @@ -6029,7 +6035,6 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, &info->allocation); qemuDomainObjExitMonitor(vm); -endjob: if (qemuDomainObjEndJob(vm) == 0) vm = NULL; } -- 1.7.5.2
On 06/02/2011 10:03 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The change 18c2a592064d69499f70428e498f4a3cb5161cda caused some regressions in behaviour of virDomainBlockStats and virDomainBlockInfo in the QEMU driver.
The virDomainBlockInfo API stopped working for inactive guests if querying a block device.
The virDomainBlockStats API did not promptly report an error if the guest was not running in some cases.
* src/qemu/qemu_driver.c: Fix inactive guest handling in BlockStats/Info APIs --- src/qemu/qemu_driver.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-)
@@ -6017,10 +6024,9 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, if (qemuDomainObjBeginJob(vm) < 0) goto cleanup;
- if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto endjob; + if (virDomainObjIsActive(vm)) { + ret = 0; + goto cleanup;
Oops - you lost the ! in that conditional. Also, 'goto cleanup' forgets to end the job condition that we hold. The real answer is that if the domain is not active, we set ret to 0 and short-circuit the attempt to query the guest. Conditional ACK if you change this hunk to be: if (!virDomainObjIsActive(vm)) { ret = 0; if (qemuDomainObjEndJob(vm) == 0) vm = NULL; goto cleanup; } -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Thu, Jun 02, 2011 at 01:27:46PM -0600, Eric Blake wrote:
On 06/02/2011 10:03 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The change 18c2a592064d69499f70428e498f4a3cb5161cda caused some regressions in behaviour of virDomainBlockStats and virDomainBlockInfo in the QEMU driver.
The virDomainBlockInfo API stopped working for inactive guests if querying a block device.
The virDomainBlockStats API did not promptly report an error if the guest was not running in some cases.
* src/qemu/qemu_driver.c: Fix inactive guest handling in BlockStats/Info APIs --- src/qemu/qemu_driver.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-)
@@ -6017,10 +6024,9 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, if (qemuDomainObjBeginJob(vm) < 0) goto cleanup;
- if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto endjob; + if (virDomainObjIsActive(vm)) { + ret = 0; + goto cleanup;
Oops - you lost the ! in that conditional. Also, 'goto cleanup' forgets to end the job condition that we hold. The real answer is that if the domain is not active, we set ret to 0 and short-circuit the attempt to query the guest.
Conditional ACK if you change this hunk to be:
if (!virDomainObjIsActive(vm)) { ret = 0; if (qemuDomainObjEndJob(vm) == 0) vm = NULL; goto cleanup; }
We can actually remove the goto entirely, so I changed it to this: if (qemuDomainObjBeginJob(vm) < 0) goto cleanup; if (virDomainObjIsActive(vm)) { qemuDomainObjEnterMonitor(vm); ret = qemuMonitorGetBlockExtent(priv->mon, disk->info.alias, &info->allocation); qemuDomainObjExitMonitor(vm); } else { ret = 0; } if (qemuDomainObjEndJob(vm) == 0) vm = NULL; Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Eric Blake