Re: [libvirt PATCH v3 06/21] nodedev: add STOPPED/STARTED lifecycle events

On Tue, 5 Jan 2021 17:25:50 +0100 Erik Skultety <eskultet(a)redhat.com&gt; wrote: > On Thu, Dec 24, 2020 at 08:14:30AM -0600, Jonathon Jongsma wrote: > > Since a mediated device can be persistently defined by the mdevctl > > backend, we need additional lifecycle events beyond CREATED/DELETED > > to indicate that e.g. the device has been stopped but the device > > definition still exists. > > > > Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; > > --- > > examples/c/misc/event-test.c | 4 ++++ > > include/libvirt/libvirt-nodedev.h | 2 ++ > > src/conf/node_device_conf.h | 1 + > > src/node_device/node_device_driver.c | 1 + > > src/node_device/node_device_udev.c | 25 +++++++++++++++++++++++-- > > tools/virsh-nodedev.c | 4 +++- > > 6 files changed, 34 insertions(+), 3 deletions(-) > > > > diff --git a/examples/c/misc/event-test.c > > b/examples/c/misc/event-test.c index f164e825e1..d6eec648ec 100644 > > --- a/examples/c/misc/event-test.c > > +++ b/examples/c/misc/event-test.c > > @@ -381,6 +381,10 @@ nodeDeviceEventToString(int event) > > return "Created"; > > case VIR_NODE_DEVICE_EVENT_DELETED: > > return "Deleted"; > > + case VIR_NODE_DEVICE_EVENT_STOPPED: > > + return "Stopped"; > > + case VIR_NODE_DEVICE_EVENT_STARTED: > > + return "Started"; > > case VIR_NODE_DEVICE_EVENT_LAST: > > break; > > } > > diff --git a/include/libvirt/libvirt-nodedev.h > > b/include/libvirt/libvirt-nodedev.h index d304283871..a473563857 > > 100644 --- a/include/libvirt/libvirt-nodedev.h > > +++ b/include/libvirt/libvirt-nodedev.h > > @@ -197,6 +197,8 @@ int > > virConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, typedef > > enum { VIR_NODE_DEVICE_EVENT_CREATED = 0, > > VIR_NODE_DEVICE_EVENT_DELETED = 1, > > + VIR_NODE_DEVICE_EVENT_STOPPED = 2, > > + VIR_NODE_DEVICE_EVENT_STARTED = 3, > > > > # ifdef VIR_ENUM_SENTINELS > > VIR_NODE_DEVICE_EVENT_LAST > > diff --git a/src/conf/node_device_conf.h > > b/src/conf/node_device_conf.h index 3d7872fd6e..bbc28cf2b9 100644 > > --- a/src/conf/node_device_conf.h > > +++ b/src/conf/node_device_conf.h > > @@ -157,6 +157,7 @@ struct _virNodeDevCapMdev { > > char *uuid; > > virMediatedDeviceAttrPtr *attributes; > > size_t nattributes; > > + bool persistent;

> > So, this patch is essentially unchanged since v2. In v2 I suggested > ^this attribute should be bound to the object not the capabability > and you haven't replied to those comments, is there a reason why that > is not the case? It's also consistent with what we do for other > objects, like domains and networks. hmm, I apparently missed that comment. But I guess the reason that I added it to the mdev caps rather than the object is because it really only applies to mdev devices. For instance, we don't really have a mechanism to keep around the definition of e.g. a PCI device that has been removed from the machine. And I don't really see any use case for doing so. I suppose we could just set this value to 'false' for all non-mdev node devices, but that doesn't seem very elegant to me.