Re: [libvirt] [PATCH 0/9] Set the SCSI controller model during post parse
On 01/31/2018 04:52 AM, Ján Tomko wrote:
On Tue, Jan 30, 2018 at 06:04:54PM -0500, John Ferlan wrote:
> Fallout or a pile yak shavings from the series to move the controller
> validation from command line building into domain xml validation:
>
> https://www.redhat.com/archives/libvir-list/2018-January/msg00188.html
>
> This series grabs the first patch from the other series that was
> essentially already ACK'd, but instead of heading down the path of
> "working around" the fact that SCSI controller model may not be
> set after domain post processing, this series builds up a series
> of changes to implement altering the SCSI controller model during
> device post processing rather than waiting for the validation phase
> to "cheat" and alter a local model value temporarily.
>
> The first 4 patches are relatively straightforward and don't change
> any of the outputs. Starting with patch 5, things get a bit more
> interesting. Patch 6 is where the conversion to set the default
> model for SCSI controllers starts... Patch 7 is where things got
> a bit tricky w/r/t the implicit controller... Patches 8 and 9 just
> perform cleanup from that setting.
>
> If this is accepted - I'll go back to the other series to adjust
> and repost; otherwise, we can determine whether the other series
> is necessary or if we're just happy with the way things are.
>
> John Ferlan (9):
> qemu: Split qemuDomainSetSCSIControllerModel
> conf: Rework and rename virDomainDeviceFindControllerModel
> qemu: Introduce qemuDomainFindSCSIControllerModel
> qemu: Introduce qemuDomainGetSCSIControllerModel
> qemu: Fetch/save the default SCSI controller model during hotplug
> qemu: Introduce qemuDomainSetSCSIControllerModel
> conf: Allow configuration of implicit controller model
Could be nicer to just add an implicit controller with the correct
model.
Understood; however, how do you propose to accomplish that in
domain_conf code that's supposed to be hypervisor agnostic?
It's not like we can pass around a qemuCaps so that when
virDomainDefMaybeAddController can call the get default model function.
This ended up being the least offensive way I could determine would
accomplish the task even though it.
The other thought I had was jiggling the order of the call to
virDomainDefPostParseCommon to before the iteration of the devices
(virDomainDeviceInfoIterateInternal), but doing that scared me more than
taking the option to follow how serials and videos handled similar
adjustments after post parse was called.
In any case, thanks for the speedy review. I'll push what's here and
maybe someone else will propose some other brilliant suggestion.
Tks -
John
> qemu: Reduce need to call qemuDomainGetSCSIControllerModel
> qemu: Update qemuDomainFindSCSIControllerModel return
>
> src/conf/domain_conf.c | 41 ++++--
> src/conf/domain_conf.h | 6 +-
> src/libvirt_private.syms | 2 +-
> src/qemu/qemu_alias.c | 14 +-
> src/qemu/qemu_alias.h | 3 +-
> src/qemu/qemu_command.c | 88 +++++++++---
> src/qemu/qemu_command.h | 3 +-
> src/qemu/qemu_domain.c | 8 +-
> src/qemu/qemu_domain_address.c | 149
> +++++++++++----------
> src/qemu/qemu_domain_address.h | 11 +-
> src/qemu/qemu_hotplug.c | 14 +-
> src/vbox/vbox_common.c | 8 +-
> tests/qemuargv2xmldata/nomachine-ppc64.xml | 2 +-
> tests/qemuargv2xmldata/pseries-disk.xml | 2 +-
> .../qemuhotplug-base-with-scsi-controller-live.xml | 8 +-
> ...se-without-scsi-controller-live+disk-scsi-2.xml | 8 +-
> tests/qemuxml2xmloutdata/disk-scsi-device-auto.xml | 2 +-
> .../hostdev-scsi-lsi-iscsi-auth.xml | 2 +-
> .../qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml | 2 +-
> tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml | 2 +-
> 20 files changed, 229 insertions(+), 146 deletions(-)
>
Either way:
ACK series
Jan