[libvirt] [PATCH] virsh: fixed handling of targetless disks (e.g. empty CDROM) in 'domblkinfo' cmd

Ján Tomko jtomko at redhat.com
Thu Oct 3 09:54:44 UTC 2019


On Wed, Oct 02, 2019 at 04:21:52PM +0200, Pavel Mores wrote:
>On Wed, Oct 02, 2019 at 08:34:10AM -0300, Daniel Henrique Barboza wrote:
>>
>>
>> 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.
>
>Was that the consensus from the migration debate?  If so I'm happy to fix my
>patch.

IIUC the consensus was that we will switch to g_auto eventually,
but it's a simple string replace that will be dealt with by whoever
pushes the patch.

Also, if you go with any of the two approaches I suggested (ignoring all
errors or using the XPathBoolean), you don't need to introduce a new
allocated variable

Jano

>
>	pvl
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191003/45e20a42/attachment-0001.sig>


More information about the libvir-list mailing list