[libvirt] [PATCH 0/9] Set the SCSI controller model during post parse

John Ferlan posted 9 patches 6 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180130230503.3820-1-jferlan@redhat.com
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(-)
[libvirt] [PATCH 0/9] Set the SCSI controller model during post parse
Posted by John Ferlan 6 years, 2 months ago
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
  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(-)

-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/9] Set the SCSI controller model during post parse
Posted by Ján Tomko 6 years, 2 months ago
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.

>  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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/9] Set the SCSI controller model during post parse
Posted by John Ferlan 6 years, 2 months ago

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list