On Thu, Jan 14, 2021 at 05:07:10PM -0600, Jonathon Jongsma wrote:
On Tue, 5 Jan 2021 17:25:50 +0100 Erik Skultety <eskultet@redhat.com> 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@redhat.com> --- 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;
There are some trailing whitespaces FWIW.
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.
I know, but I would not mind sacrificing a bit of elegance in exchange for remaining consistent across other drivers in libvirt - IOW persistence is a property of the XML definition, not the capability itself, regardless of the fact that for other types of devices we have no use for it - 'false' for everything else sounds like an acceptable trade-off to me. Unless you have some other compelling reason to break this consistency in which case I'm open to reconsider. Erik