On Fri, Apr 24, 2009 at 01:08:30PM +0100, Daniel P. Berrange wrote:
As previously discussed, Dave Allen's reworking the NPIV patches
to
do creation/deleation of vports via the node device APIs. Thus, this
patch adds the 2 new APIs required for this approach.
I would really prefer to see this API in 0.6.3, even if there
is no backend implementation, this should ease our work and also
allow other people with distinct node storage implementation to
provide patches for this too. For this reason I would prefer to see
the API go in as earlier as possible in an official release to
allow people to work on those as needed.
diff -r 2d1278bdf31f include/libvirt/libvirt.h
--- a/include/libvirt/libvirt.h Fri Apr 24 11:01:11 2009 +0100
+++ b/include/libvirt/libvirt.h Fri Apr 24 13:06:12 2009 +0100
@@ -1124,6 +1124,12 @@ int virNodeDeviceDet
int virNodeDeviceReAttach (virNodeDevicePtr dev);
int virNodeDeviceReset (virNodeDevicePtr dev);
+virNodeDevicePtr virNodeDeviceCreateXML (virConnectPtr conn,
+ const char *xmlDesc,
+ unsigned int flags);
+
+int virNodeDeviceDestroy (virNodeDevicePtr dev);
+
/*
* Domain Event Notification
*/
Okay this API looks fine to me. I think we can push it as part of
0.6.3,
diff -r 2d1278bdf31f qemud/remote.c
--- a/qemud/remote.c Fri Apr 24 11:01:11 2009 +0100
+++ b/qemud/remote.c Fri Apr 24 13:06:12 2009 +0100
@@ -4323,6 +4323,54 @@ remoteDispatchNodeDeviceReset (struct qe
Okay this is directly dependant on the signature of the 2 functions.
diff -r 2d1278bdf31f qemud/remote_dispatch_args.h
--- a/qemud/remote_dispatch_args.h Fri Apr 24 11:01:11 2009 +0100
+++ b/qemud/remote_dispatch_args.h Fri Apr 24 13:06:12 2009 +0100
same here
diff -r 2d1278bdf31f qemud/remote_dispatch_prototypes.h
--- a/qemud/remote_dispatch_prototypes.h Fri Apr 24 11:01:11 2009 +0100
+++ b/qemud/remote_dispatch_prototypes.h Fri Apr 24 13:06:12 2009 +0100
@@ -527,6 +527,20 @@ static int remoteDispatchNetworkUndefine
and here
diff -r 2d1278bdf31f qemud/remote_dispatch_ret.h
--- a/qemud/remote_dispatch_ret.h Fri Apr 24 11:01:11 2009 +0100
+++ b/qemud/remote_dispatch_ret.h Fri Apr 24 13:06:12 2009 +0100
and here
diff -r 2d1278bdf31f qemud/remote_dispatch_table.h
--- a/qemud/remote_dispatch_table.h Fri Apr 24 11:01:11 2009 +0100
+++ b/qemud/remote_dispatch_table.h Fri Apr 24 13:06:12 2009 +0100
[...]
-{ /* DomainGetSecurityLabel => 118 */
+{ /* DomainGetSecurityLabel => 121 */
[...]
-{ /* NodeGetSecurityModel => 119 */
+{ /* NodeGetSecurityModel => 122 */
Any reason for the renumbering even though it seems harmless ?
skipping generated stuff...
diff -r 2d1278bdf31f qemud/remote_protocol.x
--- a/qemud/remote_protocol.x Fri Apr 24 11:01:11 2009 +0100
+++ b/qemud/remote_protocol.x Fri Apr 24 13:06:12 2009 +0100
@@ -1109,6 +1109,19 @@ struct remote_node_device_reset_args {
remote_nonnull_string name;
};
+struct remote_node_device_create_xml_args {
+ remote_nonnull_string xml_desc;
+ int flags;
+};
+
+struct remote_node_device_create_xml_ret {
+ remote_nonnull_node_device dev;
+};
+
+struct remote_node_device_destroy_args {
+ remote_nonnull_string name;
+};
+
/**
* Events Register/Deregister:
@@ -1270,7 +1283,10 @@ enum remote_procedure {
REMOTE_PROC_NODE_DEVICE_RESET = 120,
REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL = 121,
- REMOTE_PROC_NODE_GET_SECURITY_MODEL = 122
+ REMOTE_PROC_NODE_GET_SECURITY_MODEL = 122,
+
+ REMOTE_PROC_NODE_DEVICE_CREATE_XML = 123,
+ REMOTE_PROC_NODE_DEVICE_DESTROY = 124
};
looks fine
diff -r 2d1278bdf31f src/driver.h
--- a/src/driver.h Fri Apr 24 11:01:11 2009 +0100
+++ b/src/driver.h Fri Apr 24 13:06:12 2009 +0100
@@ -684,6 +684,11 @@ typedef int (*virDevMonDeviceListCaps)(v
char **const names,
int maxnames);
+typedef virNodeDevicePtr (*virDrvNodeDeviceCreateXML)(virConnectPtr conn,
+ const char *xmlDesc,
+ unsigned int flags);
+typedef int (*virDrvNodeDeviceDestroy)(virNodeDevicePtr dev);
+
/**
* _virDeviceMonitor:
*
@@ -702,6 +707,8 @@ struct _virDeviceMonitor {
virDevMonDeviceGetParent deviceGetParent;
virDevMonDeviceNumOfCaps deviceNumOfCaps;
virDevMonDeviceListCaps deviceListCaps;
+ virDrvNodeDeviceCreateXML deviceCreateXML;
+ virDrvNodeDeviceDestroy deviceDestroy;
};
okay
diff -r 2d1278bdf31f src/libvirt.c
--- a/src/libvirt.c Fri Apr 24 11:01:11 2009 +0100
+++ b/src/libvirt.c Fri Apr 24 13:06:12 2009 +0100
[...]
+/**
+ * virNodeDeviceCreateXML:
+ * @conn: pointer to the hypervisor connection
+ * @xmlDesc: string containing an XML description of the device to be created
+ * @flags: callers should always pass 0
+ *
+ * Create a new device on the VM host machine, for example, virtual
+ * HBAs created using vport_create.
+ *
+ * Returns a node device object if successful, NULL in case of failure
+ */
+virNodeDevicePtr
+virNodeDeviceCreateXML(virConnectPtr conn,
+ const char *xmlDesc,
+ unsigned int flags)
+{
+ VIR_DEBUG("conn=%p, xmlDesc=%s, flags=%d", conn, xmlDesc, flags);
+
+ virResetLastError();
+
+ if (!VIR_IS_CONNECT(conn)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return NULL;
+ }
+
+ if (conn->flags & VIR_CONNECT_RO) {
+ virLibConnError(conn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ goto error;
+ }
+
+ if (xmlDesc == NULL) {
+ virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ goto error;
+ }
+
+ if (conn->deviceMonitor->deviceCreateXML) {
are we always 100% sure conn->deviceMonitor is non NULL ?
+ virNodeDevicePtr dev =
conn->deviceMonitor->deviceCreateXML(conn, xmlDesc, flags);
+ if (dev == NULL)
+ goto error;
+ return dev;
+ }
+
+ virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+ /* Copy to connection error object for back compatability */
+ virSetConnError(conn);
+ return NULL;
+}
+
+
+/**
+ * virNodeDeviceDestroy:
+ * @dev: a device object
+ *
+ * Destroy the device object. The virtual device is removed from the host operating
system.
+ * This function may require privileged access
+ *
+ * Returns 0 in case of success and -1 in case of failure.
+ */
+int
+virNodeDeviceDestroy(virNodeDevicePtr dev)
+{
+ int retval = 0;
+
+ DEBUG("dev=%p", dev);
+
+ virResetLastError();
+
+ if (!VIR_IS_CONNECTED_NODE_DEVICE(dev)) {
+ virLibNodeDeviceError(NULL, VIR_ERR_INVALID_NODE_DEVICE, __FUNCTION__);
+ return (-1);
+ }
+
+ if (dev->conn->flags & VIR_CONNECT_RO) {
+ virLibConnError(dev->conn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ goto error;
+ }
+
+ if (dev->conn->deviceMonitor->deviceDestroy) {
+ retval = dev->conn->deviceMonitor->deviceDestroy(dev);
+ if (retval < 0) {
+ goto error;
+ }
+
+ return 0;
+ }
+
+ virLibConnError (dev->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+ /* Copy to connection error object for back compatability */
+ virSetConnError(dev->conn);
+ return -1;
+}
+
+
okay looks fine
diff -r 2d1278bdf31f src/libvirt_public.syms
--- a/src/libvirt_public.syms Fri Apr 24 11:01:11 2009 +0100
+++ b/src/libvirt_public.syms Fri Apr 24 13:06:12 2009 +0100
@@ -258,4 +258,10 @@ LIBVIRT_0.6.1 {
virNodeGetSecurityModel;
} LIBVIRT_0.6.0;
+LIBVIRT_0.6.3 {
+ global:
+ virNodeDeviceCreateXML;
+ virNodeDeviceDestroy;
+} LIBVIRT_0.6.1;
+
# .... define new API here using predicted next version number ....
fine too
diff -r 2d1278bdf31f src/remote_internal.c
--- a/src/remote_internal.c Fri Apr 24 11:01:11 2009 +0100
+++ b/src/remote_internal.c Fri Apr 24 13:06:12 2009 +0100
okay.
ACK, fine for commit, though if you could double check the couple
or questions raised,
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit
http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine
http://rpmfind.net/
http://veillard.com/ | virtualization library
http://libvirt.org/