[libvirt] [PATCH] virsh: fixed handling of targetless disks (e.g. empty CDROM) in 'domblkinfo' cmd
Daniel Henrique Barboza
danielhb413 at gmail.com
Wed Oct 2 11:34:10 UTC 2019
On 10/1/19 11:45 AM, Pavel Mores wrote:
> virDomainGetBlockInfo() returns error if called on a disk with no target
> (a targetless disk might be a removable media drive with no media in it,
> for instance an empty CDROM drive).
>
> So far this caused the virsh domblkinfo --all command to abort and ignore
> any remaining (not yet displayed) disk devices. This patch fixes it by
> ignoring virDomainGetBlockInfo() errors for CDROM and floppy drives,
> similar to how it's done for network drives.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1619625
>
> Signed-off-by: Pavel Mores <pmores at redhat.com>
> ---
> tools/virsh-domain-monitor.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 0e2c4191d7..0f495c1a3f 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -473,6 +473,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
> char *cap = NULL;
> char *alloc = NULL;
> char *phy = NULL;
> + char *device_type = NULL;
I believe you need to use VIR_AUTO(char *) here. Even considering that
the macro will be deprecated in the near future for glib stuff, by using
VIR_AUTO here:
- you'll make it easier to introduce the glib replacement, since
s/VIR_AUTO/<g_lib_macro> is a trivial change;
- you won't be adding more VIR_FREE() on top of existing cases that will
need to be addressed in the future.
Thanks,
DHB
> vshTablePtr table = NULL;
>
> if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> @@ -510,6 +511,8 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
> rc = virDomainGetBlockInfo(dom, target, &info, 0);
>
> if (rc < 0) {
> + device_type = virXPathString("string(./@device)", ctxt);
> +
> /* If protocol is present that's an indication of a networked
> * storage device which cannot provide statistics, so generate
> * 0 based data and get the next disk. */
> @@ -518,9 +521,16 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
> virGetLastErrorDomain() == VIR_FROM_STORAGE) {
> memset(&info, 0, sizeof(info));
> vshResetLibvirtError();
> + } else if (device_type != NULL &&
> + (STRCASEEQ(device_type, "cdrom") ||
> + STRCASEEQ(device_type, "floppy"))) {
> + memset(&info, 0, sizeof(info));
> + vshResetLibvirtError();
> } else {
> goto cleanup;
> }
> +
> + VIR_FREE(device_type);
> }
>
> if (!cmdDomblkinfoGet(ctl, &info, &cap, &alloc, &phy, human))
> @@ -556,6 +566,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
> VIR_FREE(target);
> VIR_FREE(protocol);
> VIR_FREE(disks);
> + VIR_FREE(device_type);
> xmlXPathFreeContext(ctxt);
> xmlFreeDoc(xmldoc);
> return ret;
More information about the libvir-list
mailing list