Re: [libvirt] [PATCH v2] virsh: Don't break loop of domblkinfo for disks

On 08/22/2018 11:46 AM, Han Han wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1619625 > > --all option is added to cmdDomblkinfo since commit 62c39193 allowing to > show all block devices info. Reset error when empty source in case error > breaks the loop of domblkinfo for disks. > > Signed-off-by: Han Han <hhan(a)redhat.com&gt; > --- > tools/virsh-domain-monitor.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c > index b9b4f9739b..576610f005 100644 > --- a/tools/virsh-domain-monitor.c > +++ b/tools/virsh-domain-monitor.c > @@ -475,6 +475,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) > int ndisks; > size_t i; > xmlNodePtr *disks = NULL; > + char *source = NULL; > char *target = NULL; > char *protocol = NULL; > > @@ -505,16 +506,18 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) > > for (i = 0; i < ndisks; i++) { > ctxt->node = disks[i]; > + source = virXPathString("string(./source)", ctxt); > protocol = virXPathString("string(./source/@protocol)", ctxt); > target = virXPathString("string(./target/@dev)", ctxt); > > rc = virDomainGetBlockInfo(dom, target, &info, 0); > > if (rc < 0) { > - /* 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. */ > - if (protocol && !active && > + /* For the case of empty cdrom, networked disk which cannot > + * provide statistics, generate 0 based data and get the next > + * disk. > + */ > + if (!source && protocol && !active && Shouldn't this look like: if ((!source || protocol) && !active && I guess we want this to be: - either source is missing, or - disk is a network one Michal