On 27.02.2019 20:43, John Ferlan wrote:
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...
Thanx, great hint!
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- 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.
AFAIU tristate is nice when we have hypervisor-specific default values. In this case we not. So tristate won't be that useful. So tristate is implementation matter decoupled from RNG.
That way the printing would only be done *if* found specifically on input.
I think we should print "no" for PCI devices if @managed is omitted on input as we do usually after expanding default values. Nikolay
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)
[...]