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@xxxxxxxxxxxxx>
>> ---
>> 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)
>
> [...]
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
