[libvirt] [PATCH v3 3/3] qemu: Use the correct vm def on cold attach

[Ján Tomko]

On Wed, Jul 25, 2018 at 08:39:26AM -0400, John Ferlan wrote:
>
>
>On 07/25/2018 06:40 AM, Michal Privoznik wrote:
>> On 07/16/2018 11:14 PM, John Ferlan wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1559867
>>>
>>> When attaching a device to the domain we need to be sure
>>> to use the correct domain definition (vm->def or vm->newDef)
>>> when calling virDomainDeviceDefParse because the post parse
>>> processing algorithms that may assign an address for the
>>> device will use whatever domain definition was passed in.
>>>

This effectively reduces AttachDevice(AFFECT_LIVE | AFFECT_CONFIG) to
two subsequent AttachDevice calls, thus possibly attaching two
different devices, making the AFFECT_CONFIG option pointless.

E.g. when hotplugging a network interface, it might end up both on a
different PCI slot and with a different MAC address, which I'm afraid
might break some use cases.

>>> Additionally, some devices (SCSI hostdev and SCSI disk) use
>>> algorithms that rely on knowing what already exists of the
>>> other type when generating the new device's address. Using
>>> the wrong VM definition could result in duplicated addresses.
>>>

If suitablity for both live and persistent definition is a problem,
the address allocation code should take both domain definitions into
account and generate a single address for both device copies.

>>> In the case of the bz, two hostdev's with no domain address
>>> provided were added to the running domain's config only.
>>> However, the parsing algorithm used the live domain in
>>> order to figure out the host device address resulting in
>>> the same address being used and a subsequent start failing
>>> due to duplicate address.
>>>
>>> Fix this by separating the checks/code into CONFIG and LIVE
>>> processing using the correct definition for each block and
>>> performing cleanup for both options as necessary.
>>>
>>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>>> ---
>>>  src/qemu/qemu_driver.c | 52 +++++++++++++++++++-----------------------
>>>  1 file changed, 23 insertions(+), 29 deletions(-)

As I said in my review of v2 I still consider this a waste of energy and
we might possibly end up having to revert this change because it breaks
someone's use case.

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180725/4df94a8d/attachment-0001.sig>