Re: [libvirt] [PATCH] conf: don't output managed attr for non-pci hostdevs

On 2/22/19 4:35 AM, Nikolay Shirokovskiy wrote: > It is ignored on input for non pci hostdevs as written in docs. > I guess it would be better if the attr was disabled to be specified > in the first place instead but such behaviour is already documented. > At least let's not output it back. This will fix issue when > managed appeared on output for non pci hostdevs when it was not > specified on input. > > The tests are fixed by next commands: > grep -lRP 'hostdev.*mode=.subsystem.*type=.(?!pci).*managed' `find tests -name '*.xml'` > files > sed -i.bak -re 's/ managed=.(yes|no).//' `cat files` > To save yourself some effort in the future... VIR_TEST_REGENERATE_OUTPUT=1 tests/qemuxml2xmltest VIR_TEST_REGENERATE_OUTPUT=1 tests/xlconfigtest VIR_TEST_REGENERATE_OUTPUT=1 tests/qemuargv2xmltest Would have got a lot of them...

> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; > --- > src/conf/domain_conf.c | 6 ++++-- > tests/qemuargv2xmldata/hostdev-usb-address.xml | 2 +- > tests/qemuxml2argvdata/controller-order.xml | 2 +- > .../disk-hostdev-scsi-address-conflict.xml | 2 +- > .../disk-hostdev-scsi-virtio-iscsi-auth-AES.xml | 2 +- > .../hostdev-scsi-autogen-address.xml | 22 +++++++++++----------- > tests/qemuxml2argvdata/hostdev-scsi-boot.xml | 2 +- > tests/qemuxml2argvdata/hostdev-scsi-large-unit.xml | 2 +- > .../hostdev-scsi-lsi-iscsi-auth.xml | 4 ++-- > tests/qemuxml2argvdata/hostdev-scsi-lsi-iscsi.xml | 4 ++-- > tests/qemuxml2argvdata/hostdev-scsi-lsi.xml | 2 +- > tests/qemuxml2argvdata/hostdev-scsi-rawio.xml | 2 +- > tests/qemuxml2argvdata/hostdev-scsi-readonly.xml | 2 +- > tests/qemuxml2argvdata/hostdev-scsi-sgio.xml | 2 +- > tests/qemuxml2argvdata/hostdev-scsi-shareable.xml | 2 +- > .../hostdev-scsi-vhost-scsi-ccw.xml | 2 +- > .../hostdev-scsi-vhost-scsi-pci.xml | 2 +- > .../hostdev-scsi-vhost-scsi-pcie.xml | 2 +- > .../hostdev-scsi-virtio-iscsi-auth.xml | 4 ++-- > .../qemuxml2argvdata/hostdev-scsi-virtio-iscsi.xml | 4 ++-- > .../qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml | 2 +- > .../hostdev-usb-address-device-boot.xml | 2 +- > .../hostdev-usb-address-device.xml | 2 +- > tests/qemuxml2argvdata/hostdev-usb-address.xml | 2 +- > .../hostdevs-drive-address-conflict.xml | 4 ++-- > tests/qemuxml2argvdata/name-escape.xml | 2 +- > tests/qemuxml2argvdata/user-aliases-usb.xml | 4 ++-- > tests/qemuxml2xmloutdata/hostdev-mdev-display.xml | 2 +- > .../qemuxml2xmloutdata/hostdev-mdev-precreated.xml | 2 +- > .../hostdev-scsi-autogen-address.xml | 22 +++++++++++----------- > .../qemuxml2xmloutdata/hostdev-scsi-large-unit.xml | 2 +- > .../hostdev-scsi-lsi-iscsi-auth.xml | 4 ++-- > .../qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml | 4 ++-- > tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml | 2 +- > tests/qemuxml2xmloutdata/hostdev-scsi-rawio.xml | 2 +- > tests/qemuxml2xmloutdata/hostdev-scsi-readonly.xml | 2 +- > tests/qemuxml2xmloutdata/hostdev-scsi-sgio.xml | 2 +- > .../qemuxml2xmloutdata/hostdev-scsi-shareable.xml | 2 +- > .../hostdev-scsi-vhost-scsi-ccw.xml | 2 +- > .../hostdev-scsi-vhost-scsi-pci.xml | 2 +- > .../hostdev-scsi-vhost-scsi-pcie.xml | 2 +- > .../hostdev-scsi-virtio-iscsi-auth.xml | 4 ++-- > .../hostdev-scsi-virtio-iscsi.xml | 4 ++-- > .../hostdev-scsi-virtio-scsi.xml | 2 +- > .../hostdev-subsys-mdev-vfio-ccw.xml | 2 +- > tests/qemuxml2xmloutdata/hostdev-usb-address.xml | 2 +- > tests/xlconfigdata/test-usb.xml | 2 +- > 47 files changed, 80 insertions(+), 78 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index ceeb247..9ff659c 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -27330,8 +27330,10 @@ virDomainHostdevDefFormat(virBufferPtr buf, > virBufferAsprintf(buf, "<hostdev mode='%s' type='%s'", > mode, type); > if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { > - virBufferAsprintf(buf, " managed='%s'", > - def->managed ? "yes" : "no"); > + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { > + virBufferAsprintf(buf, " managed='%s'", > + def->managed ? "yes" : "no"); > + } Do you think perhaps @managed should be changed to a virTristateBool? After all the RNG attribute is a virYesNo like other Tristate's.

That way the printing would only be done *if* found specifically on input.

The problem with blindly removing the "no" even though it's listed as not being parsed on input is the differences it creates in output when someone for some reason does add "managed='{yes|no}'" and now we "remove" that... So, I'm conflicted on the best resolution for this. It is just a test and PCI is listed as the only one that would manage it, but it is also said the others would ignore it other than of course generating the output blindly which you're trying to change/fix. Perhaps there's some other opinions on this. John > > if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && > scsisrc->sgio) [...]