Re: [libvirt] [REPOST PATCHv2 1/2] qemu: Check for existing hostdev address for cold attach device
On 07/13/2018 02:50 PM, John Ferlan wrote:
>
>
> On 07/09/2018 10:32 AM, Michal Privoznik wrote:
>> On 07/06/2018 08:50 PM, John Ferlan wrote:
>>> Prior to the hostdev being inserted in the hostdevs list,
>>> add a check during qemuDomainAttachDeviceConfig to determine
>>> whether the new/incoming device is providing
>>> the same as some existing hostdev on the list
>>> and if so fail the cold attach.
>>>
>>> This cannot be done during virDomainHostdevDefPostParse
>>> because that is called after virDomainDefParseXML would
>>> have inserted a hostdev onto the hostdevs list and thus
>>> would have a "conflict" with itself. Therefore, the post
>>> parse processing can only compare if the current hostdev
>>> address conflicts with a SCSI address.
>>>
>>> By comparison this is similar to the validation phase
>>> checks in virDomainDefCheckDuplicateDriveAddresses that
>>> occur during define/startup processing but are not run
>>> during config attach of a live guest.
>>>
>>> Signed-off-by: John Ferlan
>>> ---
>>> src/qemu/qemu_driver.c | 6 ++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>> index 9a35e04a85..ef1abe3f68 100644
>>> --- a/src/qemu/qemu_driver.c
>>> +++ b/src/qemu/qemu_driver.c
>>> @@ -8007,6 +8007,12 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
>>> _("device is already in the domain
>>> configuration"));
>>> return -1;
>>> }
>>> +if (dev->data.hostdev->info->type !=
>>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
>>> +virDomainDefHasDeviceAddress(vmdef, dev->data.hostdev->info)) {
>>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>> + _("a device with the same address already
>>> exists "));
>>> +return -1;
>>> +}
>>> if (virDomainHostdevInsert(vmdef, hostdev))
>>> return -1;
>>> dev->data.hostdev = NULL;
>>>
>>
>> I think hostdevs are not the only type of device suffering from this. In
>> fact, I've just tested disks and libvirt accepts attaching another disk
>> onto same happily.
>>
>> I wonder if this should go into virDomainDefCompatibleDevice() (now that
>> we have @action there ;-) ).
>>
>
> It's a case of myopia for the bug I'm working on as listed in patch2.
>
> What I did is no different than the VIR_DOMAIN_DEVICE_RNG check in the
> same function.
>
> Still, if I change this patch to add:
>
> if (action == VIR_DOMAIN_DEVICE_ACTION_ATTACH && !live &&
> data.newInfo &&
> data.newInfo->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> virDomainDefHasDeviceAddress(def, data.newInfo)) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>_("a device with the same address already exists "));
> return -1;
> }
>
> to virDomainDefCompatibleDevice and remove the (new)hostdev and
> (existing)rng checks, then I believe it covers the existing cases.
Yes, this looks reasonable.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [REPOST PATCHv2 1/2] qemu: Check for existing hostdev address for cold attach device
On 07/09/2018 10:32 AM, Michal Privoznik wrote:
> On 07/06/2018 08:50 PM, John Ferlan wrote:
>> Prior to the hostdev being inserted in the hostdevs list,
>> add a check during qemuDomainAttachDeviceConfig to determine
>> whether the new/incoming device is providing
>> the same as some existing hostdev on the list
>> and if so fail the cold attach.
>>
>> This cannot be done during virDomainHostdevDefPostParse
>> because that is called after virDomainDefParseXML would
>> have inserted a hostdev onto the hostdevs list and thus
>> would have a "conflict" with itself. Therefore, the post
>> parse processing can only compare if the current hostdev
>> address conflicts with a SCSI address.
>>
>> By comparison this is similar to the validation phase
>> checks in virDomainDefCheckDuplicateDriveAddresses that
>> occur during define/startup processing but are not run
>> during config attach of a live guest.
>>
>> Signed-off-by: John Ferlan
>> ---
>> src/qemu/qemu_driver.c | 6 ++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 9a35e04a85..ef1abe3f68 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -8007,6 +8007,12 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
>> _("device is already in the domain
>> configuration"));
>> return -1;
>> }
>> +if (dev->data.hostdev->info->type !=
>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
>> +virDomainDefHasDeviceAddress(vmdef, dev->data.hostdev->info)) {
>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("a device with the same address already exists
>> "));
>> +return -1;
>> +}
>> if (virDomainHostdevInsert(vmdef, hostdev))
>> return -1;
>> dev->data.hostdev = NULL;
>>
>
> I think hostdevs are not the only type of device suffering from this. In
> fact, I've just tested disks and libvirt accepts attaching another disk
> onto same happily.
>
> I wonder if this should go into virDomainDefCompatibleDevice() (now that
> we have @action there ;-) ).
>
It's a case of myopia for the bug I'm working on as listed in patch2.
What I did is no different than the VIR_DOMAIN_DEVICE_RNG check in the
same function.
Still, if I change this patch to add:
if (action == VIR_DOMAIN_DEVICE_ACTION_ATTACH && !live &&
data.newInfo &&
data.newInfo->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
virDomainDefHasDeviceAddress(def, data.newInfo)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("a device with the same address already exists "));
return -1;
}
to virDomainDefCompatibleDevice and remove the (new)hostdev and
(existing)rng checks, then I believe it covers the existing cases.
Tks -
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [REPOST PATCHv2 1/2] qemu: Check for existing hostdev address for cold attach device
On 07/06/2018 08:50 PM, John Ferlan wrote:
> Prior to the hostdev being inserted in the hostdevs list,
> add a check during qemuDomainAttachDeviceConfig to determine
> whether the new/incoming device is providing
> the same as some existing hostdev on the list
> and if so fail the cold attach.
>
> This cannot be done during virDomainHostdevDefPostParse
> because that is called after virDomainDefParseXML would
> have inserted a hostdev onto the hostdevs list and thus
> would have a "conflict" with itself. Therefore, the post
> parse processing can only compare if the current hostdev
> address conflicts with a SCSI address.
>
> By comparison this is similar to the validation phase
> checks in virDomainDefCheckDuplicateDriveAddresses that
> occur during define/startup processing but are not run
> during config attach of a live guest.
>
> Signed-off-by: John Ferlan
> ---
> src/qemu/qemu_driver.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9a35e04a85..ef1abe3f68 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8007,6 +8007,12 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
> _("device is already in the domain
> configuration"));
> return -1;
> }
> +if (dev->data.hostdev->info->type !=
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> +virDomainDefHasDeviceAddress(vmdef, dev->data.hostdev->info)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("a device with the same address already exists
> "));
> +return -1;
> +}
> if (virDomainHostdevInsert(vmdef, hostdev))
> return -1;
> dev->data.hostdev = NULL;
>
I think hostdevs are not the only type of device suffering from this. In
fact, I've just tested disks and libvirt accepts attaching another disk
onto same happily.
I wonder if this should go into virDomainDefCompatibleDevice() (now that
we have @action there ;-) ).
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [REPOST PATCHv2 1/2] qemu: Check for existing hostdev address for cold attach device
Prior to the hostdev being inserted in the hostdevs list,
add a check during qemuDomainAttachDeviceConfig to determine
whether the new/incoming device is providing
the same as some existing hostdev on the list
and if so fail the cold attach.
This cannot be done during virDomainHostdevDefPostParse
because that is called after virDomainDefParseXML would
have inserted a hostdev onto the hostdevs list and thus
would have a "conflict" with itself. Therefore, the post
parse processing can only compare if the current hostdev
address conflicts with a SCSI address.
By comparison this is similar to the validation phase
checks in virDomainDefCheckDuplicateDriveAddresses that
occur during define/startup processing but are not run
during config attach of a live guest.
Signed-off-by: John Ferlan
---
src/qemu/qemu_driver.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9a35e04a85..ef1abe3f68 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8007,6 +8007,12 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
_("device is already in the domain configuration"));
return -1;
}
+if (dev->data.hostdev->info->type !=
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+virDomainDefHasDeviceAddress(vmdef, dev->data.hostdev->info)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("a device with the same address already exists
"));
+return -1;
+}
if (virDomainHostdevInsert(vmdef, hostdev))
return -1;
dev->data.hostdev = NULL;
--
2.14.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
