[libvirt PATCH] nodedev: Handle inactive mdevs with the same UUID

Jonathon Jongsma posted 1 patch 2 years, 8 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210706190517.1604513-1-jjongsma@redhat.com
src/node_device/node_device_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[libvirt PATCH] nodedev: Handle inactive mdevs with the same UUID
Posted by Jonathon Jongsma 2 years, 8 months ago
Unfortunately, mdevctl supports defining more than one mdev with the
same UUID as long as they have different parent devices. So we can't use
the UUID alone as a way to uniquely identify these node devices. Append
the parent name to ensure uniqueness. For example:

    Before: mdev_88a6b868_46bd_4015_8e5b_26107f82da38
    After:  mdev_88a6b868_46bd_4015_8e5b_26107f82da38_0000_00_02_0

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1979440

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
QUESTION: Is there any expectation of stability in these device names? I'm not
sure to what extent these changes might affect users. Given that support for
persistent mdevs is fairly new, there are likely not many users yet.

 src/node_device/node_device_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index b4dd57e5f4..89dc704162 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1038,7 +1038,7 @@ nodeDeviceGetMdevctlListCommand(bool defined,
 
 static void mdevGenerateDeviceName(virNodeDeviceDef *dev)
 {
-    nodeDeviceGenerateName(dev, "mdev", dev->caps->data.mdev.uuid, NULL);
+    nodeDeviceGenerateName(dev, "mdev", dev->caps->data.mdev.uuid, dev->parent);
 }
 
 
-- 
2.31.1

Re: [libvirt PATCH] nodedev: Handle inactive mdevs with the same UUID
Posted by Boris Fiuczynski 2 years, 8 months ago
On 7/6/21 9:05 PM, Jonathon Jongsma wrote:
> Unfortunately, mdevctl supports defining more than one mdev with the
> same UUID as long as they have different parent devices. So we can't use
> the UUID alone as a way to uniquely identify these node devices. Append
> the parent name to ensure uniqueness. For example:
> 
>      Before: mdev_88a6b868_46bd_4015_8e5b_26107f82da38
>      After:  mdev_88a6b868_46bd_4015_8e5b_26107f82da38_0000_00_02_0
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1979440
> 
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
> QUESTION: Is there any expectation of stability in these device names? I'm not
> sure to what extent these changes might affect users. Given that support for
> persistent mdevs is fairly new, there are likely not many users yet.

People using mdevs with libvirt will certainly be impacted by this but I 
am not sure how many there are.

I am wondering if the scenario is even useful since I just tried it out 
and starting the second mdev with the uuid fails.

# mdevctl list -d
e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0033 vfio_ccw-io manual (active)
e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0034 vfio_ccw-io manual
# mdevctl start -u e60cef97-3f6b-485e-ac46-0520f9f66ac2 -p 0.0.0034
Device exists under different parent


> 
>   src/node_device/node_device_driver.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index b4dd57e5f4..89dc704162 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -1038,7 +1038,7 @@ nodeDeviceGetMdevctlListCommand(bool defined,
>   
>   static void mdevGenerateDeviceName(virNodeDeviceDef *dev)
>   {
> -    nodeDeviceGenerateName(dev, "mdev", dev->caps->data.mdev.uuid, NULL);
> +    nodeDeviceGenerateName(dev, "mdev", dev->caps->data.mdev.uuid, dev->parent);
>   }
>   
>   
> 


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


Re: [libvirt PATCH] nodedev: Handle inactive mdevs with the same UUID
Posted by Jonathon Jongsma 2 years, 8 months ago
On Thu, Jul 8, 2021 at 3:47 PM Boris Fiuczynski <fiuczy@linux.ibm.com> wrote:
>
> On 7/6/21 9:05 PM, Jonathon Jongsma wrote:
> > Unfortunately, mdevctl supports defining more than one mdev with the
> > same UUID as long as they have different parent devices. So we can't use
> > the UUID alone as a way to uniquely identify these node devices. Append
> > the parent name to ensure uniqueness. For example:
> >
> >      Before: mdev_88a6b868_46bd_4015_8e5b_26107f82da38
> >      After:  mdev_88a6b868_46bd_4015_8e5b_26107f82da38_0000_00_02_0
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1979440
> >
> > Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> > ---
> > QUESTION: Is there any expectation of stability in these device names? I'm not
> > sure to what extent these changes might affect users. Given that support for
> > persistent mdevs is fairly new, there are likely not many users yet.
>
> People using mdevs with libvirt will certainly be impacted by this but I
> am not sure how many there are.
>
> I am wondering if the scenario is even useful since I just tried it out
> and starting the second mdev with the uuid fails.
>
> # mdevctl list -d
> e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0033 vfio_ccw-io manual (active)
> e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0034 vfio_ccw-io manual
> # mdevctl start -u e60cef97-3f6b-485e-ac46-0520f9f66ac2 -p 0.0.0034
> Device exists under different parent
>

Yes, it is expected that you can only have a single device with the
same UUID active at one time. I asked Alex about why mdevctl supported
this feature, and he said he thought it was so that a single VM xml
could run with different mdevs depending on which one was
instantiated. I'm not sure that anybody is actually using this
"feature", but libvirt still has to deal with the possibility that we
could get such a config from mdevctl.

Jonathon

Re: [libvirt PATCH] nodedev: Handle inactive mdevs with the same UUID
Posted by Jonathon Jongsma 2 years, 8 months ago
On Thu, Jul 8, 2021 at 4:28 PM Jonathon Jongsma <jjongsma@redhat.com> wrote:
>
> On Thu, Jul 8, 2021 at 3:47 PM Boris Fiuczynski <fiuczy@linux.ibm.com> wrote:
> >
> > On 7/6/21 9:05 PM, Jonathon Jongsma wrote:
> > > Unfortunately, mdevctl supports defining more than one mdev with the
> > > same UUID as long as they have different parent devices. So we can't use
> > > the UUID alone as a way to uniquely identify these node devices. Append
> > > the parent name to ensure uniqueness. For example:
> > >
> > >      Before: mdev_88a6b868_46bd_4015_8e5b_26107f82da38
> > >      After:  mdev_88a6b868_46bd_4015_8e5b_26107f82da38_0000_00_02_0
> > >
> > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1979440
> > >
> > > Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> > > ---
> > > QUESTION: Is there any expectation of stability in these device names? I'm not
> > > sure to what extent these changes might affect users. Given that support for
> > > persistent mdevs is fairly new, there are likely not many users yet.
> >
> > People using mdevs with libvirt will certainly be impacted by this but I
> > am not sure how many there are.
> >
> > I am wondering if the scenario is even useful since I just tried it out
> > and starting the second mdev with the uuid fails.
> >
> > # mdevctl list -d
> > e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0033 vfio_ccw-io manual (active)
> > e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0034 vfio_ccw-io manual
> > # mdevctl start -u e60cef97-3f6b-485e-ac46-0520f9f66ac2 -p 0.0.0034
> > Device exists under different parent
> >
>
> Yes, it is expected that you can only have a single device with the
> same UUID active at one time. I asked Alex about why mdevctl supported
> this feature, and he said he thought it was so that a single VM xml
> could run with different mdevs depending on which one was
> instantiated. I'm not sure that anybody is actually using this
> "feature", but libvirt still has to deal with the possibility that we
> could get such a config from mdevctl.
>
> Jonathon

By the way, I'm self-NACKing this patch. I have a more comprehensive
patch series coming soon.

Jonathon