[libvirt] [PATCH] qemu: fix no error settings if fail to find a disk match path
When we use get blockjob info to a unexist disk path, we will get a error like this: # virsh blockjob r7 vdc error: An error occurred, but the cause is unknown This is because we do not set the error when jump to endjob. As virDomainDiskByName won't set the error, we need set them in the callers function. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 900740e..f134248 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16414,8 +16414,11 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, if (qemuDomainSupportsBlockJobs(vm, NULL) < 0) goto endjob; - if (!(disk = virDomainDiskByName(vm->def, path, true))) + if (!(disk = virDomainDiskByName(vm->def, path, true))) { + virReportError(VIR_ERR_INVALID_ARG, + _("invalid path %s not assigned to domain"), path); goto endjob; + } qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm), -- 1.8.3.1
On 07/09/2015 11:49 AM, Luyao Huang wrote:
When we use get blockjob info to a unexist disk path, we will get a error like this:
# virsh blockjob r7 vdc error: An error occurred, but the cause is unknown
This is because we do not set the error when jump to endjob. As virDomainDiskByName won't set the error, we need set them in the callers function.
Sorry for forget the bz: https://bugzilla.redhat.com/show_bug.cgi?id=1241355
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 900740e..f134248 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16414,8 +16414,11 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, if (qemuDomainSupportsBlockJobs(vm, NULL) < 0) goto endjob;
- if (!(disk = virDomainDiskByName(vm->def, path, true))) + if (!(disk = virDomainDiskByName(vm->def, path, true))) { + virReportError(VIR_ERR_INVALID_ARG, + _("invalid path %s not assigned to domain"), path); goto endjob; + }
qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm),
Luyao
On Thu, Jul 09, 2015 at 11:49:15AM +0800, Luyao Huang wrote:
When we use get blockjob info to a unexist disk path, we will get a error like this:
# virsh blockjob r7 vdc error: An error occurred, but the cause is unknown
This is because we do not set the error when jump to endjob. As virDomainDiskByName won't set the error, we need set them in the callers function.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 900740e..f134248 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16414,8 +16414,11 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, if (qemuDomainSupportsBlockJobs(vm, NULL) < 0) goto endjob;
- if (!(disk = virDomainDiskByName(vm->def, path, true))) + if (!(disk = virDomainDiskByName(vm->def, path, true))) { + virReportError(VIR_ERR_INVALID_ARG, + _("invalid path %s not assigned to domain"), path);
Since the 'path' parameter can be both path and a disk name (e.g. 'vdc' in your example), I wouldn't use "path" here because it looks weird when you get an error saying: invalid path vdc not assigned to domain I'd change it to something more abstract like: Disk '%s' not found in the domain or anything else but with such specification. If you're OK with that I'll push it with the modification (also fixing the commit message).
goto endjob; + }
qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm), -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/09/2015 02:37 PM, Martin Kletzander wrote:
On Thu, Jul 09, 2015 at 11:49:15AM +0800, Luyao Huang wrote:
When we use get blockjob info to a unexist disk path, we will get a error like this:
# virsh blockjob r7 vdc error: An error occurred, but the cause is unknown
This is because we do not set the error when jump to endjob. As virDomainDiskByName won't set the error, we need set them in the callers function.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 900740e..f134248 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16414,8 +16414,11 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, if (qemuDomainSupportsBlockJobs(vm, NULL) < 0) goto endjob;
- if (!(disk = virDomainDiskByName(vm->def, path, true))) + if (!(disk = virDomainDiskByName(vm->def, path, true))) { + virReportError(VIR_ERR_INVALID_ARG, + _("invalid path %s not assigned to domain"), path);
Since the 'path' parameter can be both path and a disk name (e.g. 'vdc' in your example), I wouldn't use "path" here because it looks weird when you get an error saying:
invalid path vdc not assigned to domain
I'd change it to something more abstract like:
Disk '%s' not found in the domain
or anything else but with such specification. If you're OK with that I'll push it with the modification (also fixing the commit message).
Okay, i think it will be ok for me as it sounds more friendly. Thanks a lot for your quick review and help. Luyao
goto endjob; + }
qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm), -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
lhuang -
Luyao Huang
-
Martin Kletzander