This public API is implemented for almost all other objects that have
a concept of persistent definition and activatability. Node devices
(mdevs) that can be defined and inactive, it will be useful to be
able to update defined and active node devices as well.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
---
include/libvirt/libvirt-nodedev.h | 18 +++++++++++++
src/access/viraccessperm.c | 1 +
src/access/viraccessperm.h | 6 +++++
src/conf/virnodedeviceobj.c | 42 +++++++++++++++++++++++++++++
src/conf/virnodedeviceobj.h | 3 +++
src/driver-nodedev.h | 6 +++++
src/libvirt-nodedev.c | 45 +++++++++++++++++++++++++++++++
src/libvirt_private.syms | 1 +
src/libvirt_public.syms | 5 ++++
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 17 +++++++++++-
src/remote_protocol-structs | 6 +++++
12 files changed, 150 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h
index f7ddbfa4ad..de1d5bcbc1 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -188,6 +188,24 @@ int virNodeDeviceIsPersistent(virNodeDevicePtr dev);
int virNodeDeviceIsActive(virNodeDevicePtr dev);
+/**
+ * virNodeDeviceUpdateXMLFlags:
+ *
+ * Flags to control options for virNodeDeviceUpdateXML()
+ *
+ * Since: 10.1.0
+ */
+typedef enum {
+ VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CURRENT = 0, /* affect live if node device is active,
+ config if it's not active (Since: 10.1.0) */
+ VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE = 1 << 0, /* affect live state of node device only (Since: 10.1.0) */
+ VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG = 1 << 1, /* affect persistent config only (Since: 10.1.0) */
+} virNodeDeviceUpdateXMLFlags;
+
+int virNodeDeviceUpdateXML(virNodeDevicePtr dev,
+ const char *xmlDesc,
+ unsigned int flags);
+
/**
* VIR_NODE_DEVICE_EVENT_CALLBACK:
*
diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c
index d4a0c98b9b..702bee761d 100644
--- a/src/access/viraccessperm.c
+++ b/src/access/viraccessperm.c
@@ -71,6 +71,7 @@ VIR_ENUM_IMPL(virAccessPermNodeDevice,
"getattr", "read", "write",
"start", "stop",
"detach", "delete",
+ "save",
);
VIR_ENUM_IMPL(virAccessPermNWFilter,
diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h
index 2f04459ed9..6cc2140c67 100644
--- a/src/access/viraccessperm.h
+++ b/src/access/viraccessperm.h
@@ -507,6 +507,12 @@ typedef enum {
*/
VIR_ACCESS_PERM_NODE_DEVICE_DELETE,
+ /**
+ * @desc: Save node device
+ * @message: Saving node device driver requires authorization
+ */
+ VIR_ACCESS_PERM_NODE_DEVICE_SAVE,
+
VIR_ACCESS_PERM_NODE_DEVICE_LAST
} virAccessPermNodeDevice;
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 31ec4249d8..a8b1b625a7 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -1137,3 +1137,45 @@ virNodeDeviceObjListFind(virNodeDeviceObjList *devs,
virNodeDeviceObjListFindHelper,
&data);
}
+
+
+/**
+ * virNodeDeviceObjUpdateModificationImpact:
+ * @obj: Pointer to node device object
+ * @flags: flags to update the modification impact on
+ *
+ * Resolves virNodeDeviceObjUpdateModificationImpact flags in @flags so that
+ * they correctly apply to the actual state of @obj. @flags may be modified
+ * after call to this function.
+ *
+ * Returns 0 on success if @flags point to a valid combination for @obj or -1
+ * on error.
+ */
+int
+virNodeDeviceObjUpdateModificationImpact(virNodeDeviceObj *obj,
+ unsigned int *flags)
+{
+ bool isActive = virNodeDeviceObjIsActive(obj);
+
+ if ((*flags & (VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE | VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG)) ==
+ VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CURRENT) {
+ if (isActive)
+ *flags |= VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE;
+ else
+ *flags |= VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG;
+ }
+
+ if (!isActive && (*flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("mdev is not active"));
+ return -1;
+ }
+
+ if (!virNodeDeviceObjIsPersistent(obj) && (*flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("transient mdevs do not have any persistent config"));
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index ba2e424998..43889b8dc8 100644
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -159,3 +159,6 @@ virNodeDeviceObj *
virNodeDeviceObjListFind(virNodeDeviceObjList *devs,
virNodeDeviceObjListPredicate callback,
void *opaque);
+
+int virNodeDeviceObjUpdateModificationImpact(virNodeDeviceObj *obj,
+ unsigned int *flags);
diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h
index 167a8166dd..1c6a731f4e 100644
--- a/src/driver-nodedev.h
+++ b/src/driver-nodedev.h
@@ -101,6 +101,11 @@ typedef int
typedef int
(*virDrvNodeDeviceIsActive)(virNodeDevicePtr dev);
+typedef int
+(*virDrvNodeDeviceUpdateXML)(virNodeDevicePtr dev,
+ const char *xmlDesc,
+ unsigned int flags);
+
typedef int
(*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn,
virNodeDevicePtr dev,
@@ -146,4 +151,5 @@ struct _virNodeDeviceDriver {
virDrvNodeDeviceGetAutostart nodeDeviceGetAutostart;
virDrvNodeDeviceIsPersistent nodeDeviceIsPersistent;
virDrvNodeDeviceIsActive nodeDeviceIsActive;
+ virDrvNodeDeviceUpdateXML nodeDeviceUpdateXML;
};
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index c683b2eef9..064ee48650 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -1174,3 +1174,48 @@ int virNodeDeviceIsActive(virNodeDevicePtr dev)
virDispatchError(dev->conn);
return -1;
}
+
+
+/**
+ * virNodeDeviceUpdateXML:
+ * @dev: pointer to the node device object
+ * @xmlDesc: string containing an XML description of the device to be defined
+ * @flags: bitwise OR of virNodeDeviceUpdateXMLFlags
+ *
+ * Update the definition of an existing node device, either its live
+ * running configuration, its persistent configuration, or both.
+ *
+ * Returns 0 in case of success, -1 in case of error
+ *
+ * Since: 10.1.0
+ */
+int
+virNodeDeviceUpdateXML(virNodeDevicePtr dev,
+ const char *xmlDesc,
+ unsigned int flags)
+{
+ VIR_DEBUG("nodeDevice=%p, xmlDesc=%s, flags=0x%x",
+ dev, NULLSTR(xmlDesc), flags);
+
+ virResetLastError();
+
+ virCheckNodeDeviceReturn(dev, -1);
+
+ virCheckReadOnlyGoto(dev->conn->flags, error);
+ virCheckNonNullArgGoto(xmlDesc, error);
+
+ if (dev->conn->nodeDeviceDriver &&
+ dev->conn->nodeDeviceDriver->nodeDeviceUpdateXML) {
+ int retval = dev->conn->nodeDeviceDriver->nodeDeviceUpdateXML(dev, xmlDesc, flags);
+ if (retval < 0)
+ goto error;
+
+ return 0;
+ }
+
+ virReportUnsupportedError();
+
+ error:
+ virDispatchError(dev->conn);
+ return -1;
+}
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c398371734..fae873aba0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1373,6 +1373,7 @@ virNodeDeviceObjListRemoveLocked;
virNodeDeviceObjSetActive;
virNodeDeviceObjSetAutostart;
virNodeDeviceObjSetPersistent;
+virNodeDeviceObjUpdateModificationImpact;
# conf/virnwfilterbindingdef.h
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index bd1e916d2a..8b53eeb2ee 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -938,4 +938,9 @@ LIBVIRT_9.7.0 {
virNetworkSetMetadata;
} LIBVIRT_9.0.0;
+LIBVIRT_10.1.0 {
+ global:
+ virNodeDeviceUpdateXML;
+} LIBVIRT_9.7.0;
+
# .... define new API here using predicted next version number ....
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index bedf2cb833..ca7e125141 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -7983,6 +7983,7 @@ static virNodeDeviceDriver node_device_driver = {
.nodeDeviceSetAutostart = remoteNodeDeviceSetAutostart, /* 7.8.0 */
.nodeDeviceIsPersistent = remoteNodeDeviceIsPersistent, /* 7.8.0 */
.nodeDeviceIsActive = remoteNodeDeviceIsActive, /* 7.8.0 */
+ .nodeDeviceUpdateXML = remoteNodeDeviceUpdateXML, /* 10.1.0 */
};
static virNWFilterDriver nwfilter_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index e295b0acc3..e95f672daf 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2237,6 +2237,12 @@ struct remote_node_device_is_active_ret {
int active;
};
+struct remote_node_device_update_xml_args {
+ remote_nonnull_string name;
+ remote_nonnull_string xml_desc;
+ unsigned int flags;
+};
+
/*
* Events Register/Deregister:
@@ -7021,5 +7027,14 @@ enum remote_procedure {
* @generate: both
* @acl: none
*/
- REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446
+ REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446,
+
+ /**
+ * @generate: both
+ * @priority: high
+ * @acl: node_device:write
+ * @acl: node_device:save:!VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG|VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE
+ * @acl: node_device:save:VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG
+ */
+ REMOTE_PROC_NODE_DEVICE_UPDATE_XML = 447
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 924ca41825..9a1b881caf 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -1673,6 +1673,11 @@ struct remote_node_device_is_active_args {
struct remote_node_device_is_active_ret {
int active;
};
+struct remote_node_device_update_xml_args {
+ remote_nonnull_string name;
+ remote_nonnull_string xml_desc;
+ u_int flags;
+};
struct remote_connect_domain_event_register_ret {
int cb_registered;
};
@@ -3743,4 +3748,5 @@ enum remote_procedure {
REMOTE_PROC_NETWORK_SET_METADATA = 444,
REMOTE_PROC_NETWORK_GET_METADATA = 445,
REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446,
+ REMOTE_PROC_NODE_DEVICE_UPDATE_XML = 447,
};
--
2.42.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On 2/7/24 7:39 AM, Boris Fiuczynski wrote: > This public API is implemented for almost all other objects that have > a concept of persistent definition and activatability. Node devices > (mdevs) that can be defined and inactive, it will be useful to be > able to update defined and active node devices as well. It looks like my previous comment on this patch might not have been sent to the list, so I'll repeat it here: In the commit summary, you talk about virNodeDeviceUpdate(), however, in the code you actually name the function virNodeDeviceUpdateXML(). There are no other public objects that provide a $(OBJECT)UpdateXML() function. The only other object that has a similar API is virNetworkUpdate(). That doesn't seem to match your claim that "this public API is implemented for almost all other objects...". What API were you referring to exactly? I think for API consistency, perhaps the Update() name is better than UpdateXML() to match virNetworkUpdate()? Regardless, the commit message should match the function name. Tangentially related: Almost all of the other API objects can update their persistent definition from an XML file by calling the $(OBJECT)DefineXML command to replace the existing definition. The Node device API does not yet support doing that, maybe we should strive for a bit more API consistency here and implement that as well. Clearly the DefineXML() API will only operate on a persistent definition, so if we want to provide a way to update a live device, we'll need a new API, which is presumably where this one comes in... > > Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> > --- > include/libvirt/libvirt-nodedev.h | 18 +++++++++++++ > src/access/viraccessperm.c | 1 + > src/access/viraccessperm.h | 6 +++++ > src/conf/virnodedeviceobj.c | 42 +++++++++++++++++++++++++++++ > src/conf/virnodedeviceobj.h | 3 +++ > src/driver-nodedev.h | 6 +++++ > src/libvirt-nodedev.c | 45 +++++++++++++++++++++++++++++++ > src/libvirt_private.syms | 1 + > src/libvirt_public.syms | 5 ++++ > src/remote/remote_driver.c | 1 + > src/remote/remote_protocol.x | 17 +++++++++++- > src/remote_protocol-structs | 6 +++++ > 12 files changed, 150 insertions(+), 1 deletion(-) > > diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h > index f7ddbfa4ad..de1d5bcbc1 100644 > --- a/include/libvirt/libvirt-nodedev.h > +++ b/include/libvirt/libvirt-nodedev.h > @@ -188,6 +188,24 @@ int virNodeDeviceIsPersistent(virNodeDevicePtr dev); > > int virNodeDeviceIsActive(virNodeDevicePtr dev); > > +/** > + * virNodeDeviceUpdateXMLFlags: > + * > + * Flags to control options for virNodeDeviceUpdateXML() > + * > + * Since: 10.1.0 > + */ > +typedef enum { > + VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CURRENT = 0, /* affect live if node device is active, > + config if it's not active (Since: 10.1.0) */ > + VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE = 1 << 0, /* affect live state of node device only (Since: 10.1.0) */ > + VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG = 1 << 1, /* affect persistent config only (Since: 10.1.0) */ > +} virNodeDeviceUpdateXMLFlags; > + > +int virNodeDeviceUpdateXML(virNodeDevicePtr dev, > + const char *xmlDesc, > + unsigned int flags); > + > /** > * VIR_NODE_DEVICE_EVENT_CALLBACK: > * > diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c > index d4a0c98b9b..702bee761d 100644 > --- a/src/access/viraccessperm.c > +++ b/src/access/viraccessperm.c > @@ -71,6 +71,7 @@ VIR_ENUM_IMPL(virAccessPermNodeDevice, > "getattr", "read", "write", > "start", "stop", > "detach", "delete", > + "save", > ); > > VIR_ENUM_IMPL(virAccessPermNWFilter, > diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h > index 2f04459ed9..6cc2140c67 100644 > --- a/src/access/viraccessperm.h > +++ b/src/access/viraccessperm.h > @@ -507,6 +507,12 @@ typedef enum { > */ > VIR_ACCESS_PERM_NODE_DEVICE_DELETE, > > + /** > + * @desc: Save node device > + * @message: Saving node device driver requires authorization > + */ > + VIR_ACCESS_PERM_NODE_DEVICE_SAVE, > + > VIR_ACCESS_PERM_NODE_DEVICE_LAST > } virAccessPermNodeDevice; > > diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c > index 31ec4249d8..a8b1b625a7 100644 > --- a/src/conf/virnodedeviceobj.c > +++ b/src/conf/virnodedeviceobj.c > @@ -1137,3 +1137,45 @@ virNodeDeviceObjListFind(virNodeDeviceObjList *devs, > virNodeDeviceObjListFindHelper, > &data); > } > + > + > +/** > + * virNodeDeviceObjUpdateModificationImpact: > + * @obj: Pointer to node device object > + * @flags: flags to update the modification impact on > + * > + * Resolves virNodeDeviceObjUpdateModificationImpact flags in @flags so that > + * they correctly apply to the actual state of @obj. @flags may be modified > + * after call to this function. > + * > + * Returns 0 on success if @flags point to a valid combination for @obj or -1 > + * on error. > + */ > +int > +virNodeDeviceObjUpdateModificationImpact(virNodeDeviceObj *obj, > + unsigned int *flags) > +{ > + bool isActive = virNodeDeviceObjIsActive(obj); > + > + if ((*flags & (VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE | VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG)) == > + VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CURRENT) { > + if (isActive) > + *flags |= VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE; > + else > + *flags |= VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG; > + } > + > + if (!isActive && (*flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE)) { > + virReportError(VIR_ERR_OPERATION_INVALID, "%s", > + _("mdev is not active")); > + return -1; The error message should be device-type-agnostic rather than mentioning mdev specifically > + } > + > + if (!virNodeDeviceObjIsPersistent(obj) && (*flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG)) { > + virReportError(VIR_ERR_OPERATION_INVALID, "%s", > + _("transient mdevs do not have any persistent config")); > + return -1; same > + } > + > + return 0; > +} > diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h > index ba2e424998..43889b8dc8 100644 > --- a/src/conf/virnodedeviceobj.h > +++ b/src/conf/virnodedeviceobj.h > @@ -159,3 +159,6 @@ virNodeDeviceObj * > virNodeDeviceObjListFind(virNodeDeviceObjList *devs, > virNodeDeviceObjListPredicate callback, > void *opaque); > + > +int virNodeDeviceObjUpdateModificationImpact(virNodeDeviceObj *obj, > + unsigned int *flags); > diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h > index 167a8166dd..1c6a731f4e 100644 > --- a/src/driver-nodedev.h > +++ b/src/driver-nodedev.h > @@ -101,6 +101,11 @@ typedef int > typedef int > (*virDrvNodeDeviceIsActive)(virNodeDevicePtr dev); > > +typedef int > +(*virDrvNodeDeviceUpdateXML)(virNodeDevicePtr dev, > + const char *xmlDesc, > + unsigned int flags); > + > typedef int > (*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn, > virNodeDevicePtr dev, > @@ -146,4 +151,5 @@ struct _virNodeDeviceDriver { > virDrvNodeDeviceGetAutostart nodeDeviceGetAutostart; > virDrvNodeDeviceIsPersistent nodeDeviceIsPersistent; > virDrvNodeDeviceIsActive nodeDeviceIsActive; > + virDrvNodeDeviceUpdateXML nodeDeviceUpdateXML; > }; > diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c > index c683b2eef9..064ee48650 100644 > --- a/src/libvirt-nodedev.c > +++ b/src/libvirt-nodedev.c > @@ -1174,3 +1174,48 @@ int virNodeDeviceIsActive(virNodeDevicePtr dev) > virDispatchError(dev->conn); > return -1; > } > + > + > +/** > + * virNodeDeviceUpdateXML: > + * @dev: pointer to the node device object > + * @xmlDesc: string containing an XML description of the device to be defined > + * @flags: bitwise OR of virNodeDeviceUpdateXMLFlags > + * > + * Update the definition of an existing node device, either its live > + * running configuration, its persistent configuration, or both. > + * > + * Returns 0 in case of success, -1 in case of error > + * > + * Since: 10.1.0 > + */ > +int > +virNodeDeviceUpdateXML(virNodeDevicePtr dev, > + const char *xmlDesc, > + unsigned int flags) > +{ > + VIR_DEBUG("nodeDevice=%p, xmlDesc=%s, flags=0x%x", > + dev, NULLSTR(xmlDesc), flags); > + > + virResetLastError(); > + > + virCheckNodeDeviceReturn(dev, -1); > + > + virCheckReadOnlyGoto(dev->conn->flags, error); > + virCheckNonNullArgGoto(xmlDesc, error); > + > + if (dev->conn->nodeDeviceDriver && > + dev->conn->nodeDeviceDriver->nodeDeviceUpdateXML) { > + int retval = dev->conn->nodeDeviceDriver->nodeDeviceUpdateXML(dev, xmlDesc, flags); > + if (retval < 0) > + goto error; > + > + return 0; > + } > + > + virReportUnsupportedError(); > + > + error: > + virDispatchError(dev->conn); > + return -1; > +} > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index c398371734..fae873aba0 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -1373,6 +1373,7 @@ virNodeDeviceObjListRemoveLocked; > virNodeDeviceObjSetActive; > virNodeDeviceObjSetAutostart; > virNodeDeviceObjSetPersistent; > +virNodeDeviceObjUpdateModificationImpact; > > > # conf/virnwfilterbindingdef.h > diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms > index bd1e916d2a..8b53eeb2ee 100644 > --- a/src/libvirt_public.syms > +++ b/src/libvirt_public.syms > @@ -938,4 +938,9 @@ LIBVIRT_9.7.0 { > virNetworkSetMetadata; > } LIBVIRT_9.0.0; > > +LIBVIRT_10.1.0 { > + global: > + virNodeDeviceUpdateXML; > +} LIBVIRT_9.7.0; > + > # .... define new API here using predicted next version number .... > diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c > index bedf2cb833..ca7e125141 100644 > --- a/src/remote/remote_driver.c > +++ b/src/remote/remote_driver.c > @@ -7983,6 +7983,7 @@ static virNodeDeviceDriver node_device_driver = { > .nodeDeviceSetAutostart = remoteNodeDeviceSetAutostart, /* 7.8.0 */ > .nodeDeviceIsPersistent = remoteNodeDeviceIsPersistent, /* 7.8.0 */ > .nodeDeviceIsActive = remoteNodeDeviceIsActive, /* 7.8.0 */ > + .nodeDeviceUpdateXML = remoteNodeDeviceUpdateXML, /* 10.1.0 */ > }; > > static virNWFilterDriver nwfilter_driver = { > diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x > index e295b0acc3..e95f672daf 100644 > --- a/src/remote/remote_protocol.x > +++ b/src/remote/remote_protocol.x > @@ -2237,6 +2237,12 @@ struct remote_node_device_is_active_ret { > int active; > }; > > +struct remote_node_device_update_xml_args { > + remote_nonnull_string name; > + remote_nonnull_string xml_desc; > + unsigned int flags; > +}; > + > > /* > * Events Register/Deregister: > @@ -7021,5 +7027,14 @@ enum remote_procedure { > * @generate: both > * @acl: none > */ > - REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446 > + REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446, > + > + /** > + * @generate: both > + * @priority: high > + * @acl: node_device:write > + * @acl: node_device:save:!VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG|VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE > + * @acl: node_device:save:VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG > + */ > + REMOTE_PROC_NODE_DEVICE_UPDATE_XML = 447 > }; > diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs > index 924ca41825..9a1b881caf 100644 > --- a/src/remote_protocol-structs > +++ b/src/remote_protocol-structs > @@ -1673,6 +1673,11 @@ struct remote_node_device_is_active_args { > struct remote_node_device_is_active_ret { > int active; > }; > +struct remote_node_device_update_xml_args { > + remote_nonnull_string name; > + remote_nonnull_string xml_desc; > + u_int flags; > +}; > struct remote_connect_domain_event_register_ret { > int cb_registered; > }; > @@ -3743,4 +3748,5 @@ enum remote_procedure { > REMOTE_PROC_NETWORK_SET_METADATA = 444, > REMOTE_PROC_NETWORK_GET_METADATA = 445, > REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446, > + REMOTE_PROC_NODE_DEVICE_UPDATE_XML = 447, > }; _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
On 2/9/24 23:13, Jonathon Jongsma wrote: > On 2/7/24 7:39 AM, Boris Fiuczynski wrote: >> This public API is implemented for almost all other objects that have >> a concept of persistent definition and activatability. Node devices >> (mdevs) that can be defined and inactive, it will be useful to be >> able to update defined and active node devices as well. > > It looks like my previous comment on this patch might not have been sent > to the list, so I'll repeat it here: > > In the commit summary, you talk about virNodeDeviceUpdate(), however, in > the code you actually name the function virNodeDeviceUpdateXML(). There > are no other public objects that provide a $(OBJECT)UpdateXML() > function. The only other object that has a similar API is > virNetworkUpdate(). That doesn't seem to match your claim that "this > public API is implemented for almost all other objects...". What API > were you referring to exactly? I think for API consistency, perhaps the > Update() name is better than UpdateXML() to match virNetworkUpdate()? > Regardless, the commit message should match the function name. Sorry, I forgot to change the method name in the summary as before I named it without the suffix XML. I will change the name back to match the networking which is the only method allowing to update live/active objects. I was trying to say that updating and/or modifying is possible on almost all other objects which have implemented persistent and active configurations. Most use *Define.. to update but this is an update of the persistent config as you already noted. As I wrote above already virNetworkUpdate is the only method which also supports update of the active config. > > Tangentially related: > > Almost all of the other API objects can update their persistent > definition from an XML file by calling the $(OBJECT)DefineXML command to > replace the existing definition. The Node device API does not yet > support doing that, maybe we should strive for a bit more API > consistency here and implement that as well. > > Clearly the DefineXML() API will only operate on a persistent > definition, so if we want to provide a way to update a live device, > we'll need a new API, which is presumably where this one comes in... Correct. I will take a look at how to align virNodeDeviceDefineXML > >> >> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> >> --- >> include/libvirt/libvirt-nodedev.h | 18 +++++++++++++ >> src/access/viraccessperm.c | 1 + >> src/access/viraccessperm.h | 6 +++++ >> src/conf/virnodedeviceobj.c | 42 +++++++++++++++++++++++++++++ >> src/conf/virnodedeviceobj.h | 3 +++ >> src/driver-nodedev.h | 6 +++++ >> src/libvirt-nodedev.c | 45 +++++++++++++++++++++++++++++++ >> src/libvirt_private.syms | 1 + >> src/libvirt_public.syms | 5 ++++ >> src/remote/remote_driver.c | 1 + >> src/remote/remote_protocol.x | 17 +++++++++++- >> src/remote_protocol-structs | 6 +++++ >> 12 files changed, 150 insertions(+), 1 deletion(-) >> >> diff --git a/include/libvirt/libvirt-nodedev.h >> b/include/libvirt/libvirt-nodedev.h >> index f7ddbfa4ad..de1d5bcbc1 100644 >> --- a/include/libvirt/libvirt-nodedev.h >> +++ b/include/libvirt/libvirt-nodedev.h >> @@ -188,6 +188,24 @@ int virNodeDeviceIsPersistent(virNodeDevicePtr dev); >> int virNodeDeviceIsActive(virNodeDevicePtr dev); >> +/** >> + * virNodeDeviceUpdateXMLFlags: >> + * >> + * Flags to control options for virNodeDeviceUpdateXML() >> + * >> + * Since: 10.1.0 >> + */ >> +typedef enum { >> + VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CURRENT = 0, /* affect >> live if node device is active, >> + config if >> it's not active (Since: 10.1.0) */ >> + VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE = 1 << 0, /* affect >> live state of node device only (Since: 10.1.0) */ >> + VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG = 1 << 1, /* affect >> persistent config only (Since: 10.1.0) */ >> +} virNodeDeviceUpdateXMLFlags; >> + >> +int virNodeDeviceUpdateXML(virNodeDevicePtr dev, >> + const char *xmlDesc, >> + unsigned int flags); >> + >> /** >> * VIR_NODE_DEVICE_EVENT_CALLBACK: >> * >> diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c >> index d4a0c98b9b..702bee761d 100644 >> --- a/src/access/viraccessperm.c >> +++ b/src/access/viraccessperm.c >> @@ -71,6 +71,7 @@ VIR_ENUM_IMPL(virAccessPermNodeDevice, >> "getattr", "read", "write", >> "start", "stop", >> "detach", "delete", >> + "save", >> ); >> VIR_ENUM_IMPL(virAccessPermNWFilter, >> diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h >> index 2f04459ed9..6cc2140c67 100644 >> --- a/src/access/viraccessperm.h >> +++ b/src/access/viraccessperm.h >> @@ -507,6 +507,12 @@ typedef enum { >> */ >> VIR_ACCESS_PERM_NODE_DEVICE_DELETE, >> + /** >> + * @desc: Save node device >> + * @message: Saving node device driver requires authorization >> + */ >> + VIR_ACCESS_PERM_NODE_DEVICE_SAVE, >> + >> VIR_ACCESS_PERM_NODE_DEVICE_LAST >> } virAccessPermNodeDevice; >> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c >> index 31ec4249d8..a8b1b625a7 100644 >> --- a/src/conf/virnodedeviceobj.c >> +++ b/src/conf/virnodedeviceobj.c >> @@ -1137,3 +1137,45 @@ virNodeDeviceObjListFind(virNodeDeviceObjList >> *devs, >> virNodeDeviceObjListFindHelper, >> &data); >> } >> + >> + >> +/** >> + * virNodeDeviceObjUpdateModificationImpact: >> + * @obj: Pointer to node device object >> + * @flags: flags to update the modification impact on >> + * >> + * Resolves virNodeDeviceObjUpdateModificationImpact flags in @flags >> so that >> + * they correctly apply to the actual state of @obj. @flags may be >> modified >> + * after call to this function. >> + * >> + * Returns 0 on success if @flags point to a valid combination for >> @obj or -1 >> + * on error. >> + */ >> +int >> +virNodeDeviceObjUpdateModificationImpact(virNodeDeviceObj *obj, >> + unsigned int *flags) >> +{ >> + bool isActive = virNodeDeviceObjIsActive(obj); >> + >> + if ((*flags & (VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE | >> VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG)) == >> + VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CURRENT) { >> + if (isActive) >> + *flags |= VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE; >> + else >> + *flags |= VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG; >> + } >> + >> + if (!isActive && (*flags & >> VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE)) { >> + virReportError(VIR_ERR_OPERATION_INVALID, "%s", >> + _("mdev is not active")); >> + return -1; > > > > The error message should be device-type-agnostic rather than mentioning > mdev specifically Changed "mdev" into "node device" > > >> + } >> + >> + if (!virNodeDeviceObjIsPersistent(obj) && (*flags & >> VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG)) { >> + virReportError(VIR_ERR_OPERATION_INVALID, "%s", >> + _("transient mdevs do not have any persistent >> config")); >> + return -1; > > > same Changed. > > >> + } >> + >> + return 0; >> +} >> diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h >> index ba2e424998..43889b8dc8 100644 >> --- a/src/conf/virnodedeviceobj.h >> +++ b/src/conf/virnodedeviceobj.h >> @@ -159,3 +159,6 @@ virNodeDeviceObj * >> virNodeDeviceObjListFind(virNodeDeviceObjList *devs, >> virNodeDeviceObjListPredicate callback, >> void *opaque); >> + >> +int virNodeDeviceObjUpdateModificationImpact(virNodeDeviceObj *obj, >> + unsigned int *flags); >> diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h >> index 167a8166dd..1c6a731f4e 100644 >> --- a/src/driver-nodedev.h >> +++ b/src/driver-nodedev.h >> @@ -101,6 +101,11 @@ typedef int >> typedef int >> (*virDrvNodeDeviceIsActive)(virNodeDevicePtr dev); >> +typedef int >> +(*virDrvNodeDeviceUpdateXML)(virNodeDevicePtr dev, >> + const char *xmlDesc, >> + unsigned int flags); >> + >> typedef int >> (*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn, >> virNodeDevicePtr dev, >> @@ -146,4 +151,5 @@ struct _virNodeDeviceDriver { >> virDrvNodeDeviceGetAutostart nodeDeviceGetAutostart; >> virDrvNodeDeviceIsPersistent nodeDeviceIsPersistent; >> virDrvNodeDeviceIsActive nodeDeviceIsActive; >> + virDrvNodeDeviceUpdateXML nodeDeviceUpdateXML; >> }; >> diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c >> index c683b2eef9..064ee48650 100644 >> --- a/src/libvirt-nodedev.c >> +++ b/src/libvirt-nodedev.c >> @@ -1174,3 +1174,48 @@ int virNodeDeviceIsActive(virNodeDevicePtr dev) >> virDispatchError(dev->conn); >> return -1; >> } >> + >> + >> +/** >> + * virNodeDeviceUpdateXML: >> + * @dev: pointer to the node device object >> + * @xmlDesc: string containing an XML description of the device to be >> defined >> + * @flags: bitwise OR of virNodeDeviceUpdateXMLFlags >> + * >> + * Update the definition of an existing node device, either its live >> + * running configuration, its persistent configuration, or both. >> + * >> + * Returns 0 in case of success, -1 in case of error >> + * >> + * Since: 10.1.0 >> + */ >> +int >> +virNodeDeviceUpdateXML(virNodeDevicePtr dev, >> + const char *xmlDesc, >> + unsigned int flags) >> +{ >> + VIR_DEBUG("nodeDevice=%p, xmlDesc=%s, flags=0x%x", >> + dev, NULLSTR(xmlDesc), flags); >> + >> + virResetLastError(); >> + >> + virCheckNodeDeviceReturn(dev, -1); >> + >> + virCheckReadOnlyGoto(dev->conn->flags, error); >> + virCheckNonNullArgGoto(xmlDesc, error); >> + >> + if (dev->conn->nodeDeviceDriver && >> + dev->conn->nodeDeviceDriver->nodeDeviceUpdateXML) { >> + int retval = >> dev->conn->nodeDeviceDriver->nodeDeviceUpdateXML(dev, xmlDesc, flags); >> + if (retval < 0) >> + goto error; >> + >> + return 0; >> + } >> + >> + virReportUnsupportedError(); >> + >> + error: >> + virDispatchError(dev->conn); >> + return -1; >> +} >> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >> index c398371734..fae873aba0 100644 >> --- a/src/libvirt_private.syms >> +++ b/src/libvirt_private.syms >> @@ -1373,6 +1373,7 @@ virNodeDeviceObjListRemoveLocked; >> virNodeDeviceObjSetActive; >> virNodeDeviceObjSetAutostart; >> virNodeDeviceObjSetPersistent; >> +virNodeDeviceObjUpdateModificationImpact; >> # conf/virnwfilterbindingdef.h >> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms >> index bd1e916d2a..8b53eeb2ee 100644 >> --- a/src/libvirt_public.syms >> +++ b/src/libvirt_public.syms >> @@ -938,4 +938,9 @@ LIBVIRT_9.7.0 { >> virNetworkSetMetadata; >> } LIBVIRT_9.0.0; >> +LIBVIRT_10.1.0 { >> + global: >> + virNodeDeviceUpdateXML; >> +} LIBVIRT_9.7.0; >> + >> # .... define new API here using predicted next version number .... >> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c >> index bedf2cb833..ca7e125141 100644 >> --- a/src/remote/remote_driver.c >> +++ b/src/remote/remote_driver.c >> @@ -7983,6 +7983,7 @@ static virNodeDeviceDriver node_device_driver = { >> .nodeDeviceSetAutostart = remoteNodeDeviceSetAutostart, /* 7.8.0 */ >> .nodeDeviceIsPersistent = remoteNodeDeviceIsPersistent, /* 7.8.0 */ >> .nodeDeviceIsActive = remoteNodeDeviceIsActive, /* 7.8.0 */ >> + .nodeDeviceUpdateXML = remoteNodeDeviceUpdateXML, /* 10.1.0 */ >> }; >> static virNWFilterDriver nwfilter_driver = { >> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x >> index e295b0acc3..e95f672daf 100644 >> --- a/src/remote/remote_protocol.x >> +++ b/src/remote/remote_protocol.x >> @@ -2237,6 +2237,12 @@ struct remote_node_device_is_active_ret { >> int active; >> }; >> +struct remote_node_device_update_xml_args { >> + remote_nonnull_string name; >> + remote_nonnull_string xml_desc; >> + unsigned int flags; >> +}; >> + >> /* >> * Events Register/Deregister: >> @@ -7021,5 +7027,14 @@ enum remote_procedure { >> * @generate: both >> * @acl: none >> */ >> - REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446 >> + REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446, >> + >> + /** >> + * @generate: both >> + * @priority: high >> + * @acl: node_device:write >> + * @acl: >> node_device:save:!VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG|VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE >> + * @acl: node_device:save:VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG >> + */ >> + REMOTE_PROC_NODE_DEVICE_UPDATE_XML = 447 >> }; >> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs >> index 924ca41825..9a1b881caf 100644 >> --- a/src/remote_protocol-structs >> +++ b/src/remote_protocol-structs >> @@ -1673,6 +1673,11 @@ struct remote_node_device_is_active_args { >> struct remote_node_device_is_active_ret { >> int active; >> }; >> +struct remote_node_device_update_xml_args { >> + remote_nonnull_string name; >> + remote_nonnull_string xml_desc; >> + u_int flags; >> +}; >> struct remote_connect_domain_event_register_ret { >> int cb_registered; >> }; >> @@ -3743,4 +3748,5 @@ enum remote_procedure { >> REMOTE_PROC_NETWORK_SET_METADATA = 444, >> REMOTE_PROC_NETWORK_GET_METADATA = 445, >> REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446, >> + REMOTE_PROC_NODE_DEVICE_UPDATE_XML = 447, >> }; > _______________________________________________ > Devel mailing list -- devel@lists.libvirt.org > To unsubscribe send an email to devel-leave@lists.libvirt.org -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
On 2/13/24 14:43, Boris Fiuczynski wrote: >> Tangentially related: >> >> Almost all of the other API objects can update their persistent >> definition from an XML file by calling the $(OBJECT)DefineXML command >> to replace the existing definition. The Node device API does not yet >> support doing that, maybe we should strive for a bit more API >> consistency here and implement that as well. >> >> Clearly the DefineXML() API will only operate on a persistent >> definition, so if we want to provide a way to update a live device, >> we'll need a new API, which is presumably where this one comes in... > > Correct. > I will take a look at how to align virNodeDeviceDefineXML The current libvirt code does not prevent to define an existing mdev and does call mdevctl which reports that mdevctl cowardly refuses to overwrite the existing config as an internal error. # virsh nodedev-define vfio-ccw-mdev-c60c_new-attr-set1.xml error: Failed to define node device from 'vfio-ccw-mdev-c60c_new-attr-set1.xml' error: internal error: Unable to define mediated device: Error: Cowardly refusing to overwrite existing config for 0.0.0008/c60cc60c-c60c-c60c-c60c-c60cc60cc60c Instead of handling this in libvirt by detecting on define if a persistent config already exists and instead of calling 'mdevctl define' using 'mdevctl modify' for the update should mdevctl become brave by removing its refusal? -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
On 2/14/24 6:59 AM, Boris Fiuczynski wrote: > On 2/13/24 14:43, Boris Fiuczynski wrote: >>> Tangentially related: >>> >>> Almost all of the other API objects can update their persistent >>> definition from an XML file by calling the $(OBJECT)DefineXML command >>> to replace the existing definition. The Node device API does not yet >>> support doing that, maybe we should strive for a bit more API >>> consistency here and implement that as well. >>> >>> Clearly the DefineXML() API will only operate on a persistent >>> definition, so if we want to provide a way to update a live device, >>> we'll need a new API, which is presumably where this one comes in... >> >> Correct. >> I will take a look at how to align virNodeDeviceDefineXML > > The current libvirt code does not prevent to define an existing mdev and > does call mdevctl which reports that mdevctl cowardly refuses to > overwrite the existing config as an internal error. > > # virsh nodedev-define vfio-ccw-mdev-c60c_new-attr-set1.xml > error: Failed to define node device from > 'vfio-ccw-mdev-c60c_new-attr-set1.xml' > error: internal error: Unable to define mediated device: Error: Cowardly > refusing to overwrite existing config for > 0.0.0008/c60cc60c-c60c-c60c-c60c-c60cc60cc60c > > Instead of handling this in libvirt by detecting on define if a > persistent config already exists and instead of calling 'mdevctl define' > using 'mdevctl modify' for the update should mdevctl become brave by > removing its refusal? > I don't think it would be a good idea to change the behavior of mdevctl at this point. But even if we did change mdevctl behavior, we'd still have to deal with older versions of mdevctl so we'd have to handle it in libvirt anyway. Jonathon _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
On 2/14/24 19:00, Jonathon Jongsma wrote: > On 2/14/24 6:59 AM, Boris Fiuczynski wrote: >> On 2/13/24 14:43, Boris Fiuczynski wrote: >>>> Tangentially related: >>>> >>>> Almost all of the other API objects can update their persistent >>>> definition from an XML file by calling the $(OBJECT)DefineXML >>>> command to replace the existing definition. The Node device API does >>>> not yet support doing that, maybe we should strive for a bit more >>>> API consistency here and implement that as well. >>>> >>>> Clearly the DefineXML() API will only operate on a persistent >>>> definition, so if we want to provide a way to update a live device, >>>> we'll need a new API, which is presumably where this one comes in... >>> >>> Correct. >>> I will take a look at how to align virNodeDeviceDefineXML >> >> The current libvirt code does not prevent to define an existing mdev >> and does call mdevctl which reports that mdevctl cowardly refuses to >> overwrite the existing config as an internal error. >> >> # virsh nodedev-define vfio-ccw-mdev-c60c_new-attr-set1.xml >> error: Failed to define node device from >> 'vfio-ccw-mdev-c60c_new-attr-set1.xml' >> error: internal error: Unable to define mediated device: Error: >> Cowardly refusing to overwrite existing config for >> 0.0.0008/c60cc60c-c60c-c60c-c60c-c60cc60cc60c >> >> Instead of handling this in libvirt by detecting on define if a >> persistent config already exists and instead of calling 'mdevctl >> define' using 'mdevctl modify' for the update should mdevctl become >> brave by removing its refusal? >> > > I don't think it would be a good idea to change the behavior of mdevctl > at this point. But even if we did change mdevctl behavior, we'd still > have to deal with older versions of mdevctl so we'd have to handle it in > libvirt anyway. > > Jonathon What would have to change in libvirt to handle the current versions not allowing to overwrite the config? The error above would remain for older medevctl versions. For versions which support overwrite the above error would disappear and the nodedev-define would succeed. There might be some changes required in libvirt to emit a different libvirt life cycle event for overwrites vs new defines. In case a event change is required in libvirt that would be missing if an older libvirt version is using a new mdevctl version. > _______________________________________________ > Devel mailing list -- devel@lists.libvirt.org > To unsubscribe send an email to devel-leave@lists.libvirt.org -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
On 2/15/24 3:29 AM, Boris Fiuczynski wrote: > On 2/14/24 19:00, Jonathon Jongsma wrote: >> On 2/14/24 6:59 AM, Boris Fiuczynski wrote: >>> On 2/13/24 14:43, Boris Fiuczynski wrote: >>>>> Tangentially related: >>>>> >>>>> Almost all of the other API objects can update their persistent >>>>> definition from an XML file by calling the $(OBJECT)DefineXML >>>>> command to replace the existing definition. The Node device API >>>>> does not yet support doing that, maybe we should strive for a bit >>>>> more API consistency here and implement that as well. >>>>> >>>>> Clearly the DefineXML() API will only operate on a persistent >>>>> definition, so if we want to provide a way to update a live device, >>>>> we'll need a new API, which is presumably where this one comes in... >>>> >>>> Correct. >>>> I will take a look at how to align virNodeDeviceDefineXML >>> >>> The current libvirt code does not prevent to define an existing mdev >>> and does call mdevctl which reports that mdevctl cowardly refuses to >>> overwrite the existing config as an internal error. >>> >>> # virsh nodedev-define vfio-ccw-mdev-c60c_new-attr-set1.xml >>> error: Failed to define node device from >>> 'vfio-ccw-mdev-c60c_new-attr-set1.xml' >>> error: internal error: Unable to define mediated device: Error: >>> Cowardly refusing to overwrite existing config for >>> 0.0.0008/c60cc60c-c60c-c60c-c60c-c60cc60cc60c >>> >>> Instead of handling this in libvirt by detecting on define if a >>> persistent config already exists and instead of calling 'mdevctl >>> define' using 'mdevctl modify' for the update should mdevctl become >>> brave by removing its refusal? >>> >> >> I don't think it would be a good idea to change the behavior of >> mdevctl at this point. But even if we did change mdevctl behavior, >> we'd still have to deal with older versions of mdevctl so we'd have to >> handle it in libvirt anyway. >> >> Jonathon > > What would have to change in libvirt to handle the current versions not > allowing to overwrite the config? > The error above would remain for older medevctl versions. You're right. If you don't care that older mdevctl versions don't work, you wouldn't need to do anything. But we should be able to very easily support the existing versions right now simply by checking whether the device is already defined and using 'modify' instead. And as I said, I don't currently intend to change mdevctl define behavior. Jonathon _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
© 2016 - 2024 Red Hat, Inc.