Re: [libvirt] [PATCH v3 3/3] qemu: Use the correct vm def on cold attach
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(a)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
Attachments:
- signature.asc (application/pgp-signature — 833 bytes)