2:05 p.m.
This patch introduces a new virDomainUpdateDeviceFlags() API to allow
the configuration of a device to be changed on the fly. It ports the
existing CDROM media change code in all drivers to use this new API
(keeps old support too). It then adds VNC password change support
in the QEMU driver using this new API
2:05 p.m.
New subject: [libvirt] [PATCH 1/6] Introduce a new virDomainUpdateDeviceFlags public API
The current virDomainAttachDevice API can be (ab)used to change
the media of an existing CDROM/Floppy device. Going forward there
will be more devices that can be configured on the fly and overloading
virDomainAttachDevice for this is not too pleasant. This patch adds
a new virDomainUpdateDeviceFlags() explicitly just for modifying
existing devices.
* include/libvirt/libvirt.h.in: Add virDomainUpdateDeviceFlags
* src/driver.h: Internal API for virDomainUpdateDeviceFlags
* src/libvirt.c, src/libvirt_public.syms: Glue public API to
driver API
* src/esx/esx_driver.c, src/lxc/lxc_driver.c, src/opennebula/one_driver.c,
src/openvz/openvz_driver.c, src/phyp/phyp_driver.c, src/qemu/qemu_driver.c,
src/remote/remote_driver.c, src/test/test_driver.c, src/uml/uml_driver.c,
src/vbox/vbox_tmpl.c, src/xen/xen_driver.c, src/xenapi/xenapi_driver.c: Add
stubs for new driver entry point
---
include/libvirt/libvirt.h.in | 2 +
src/driver.h | 5 +++
src/esx/esx_driver.c | 1 +
src/libvirt.c | 66 ++++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 1 +
src/lxc/lxc_driver.c | 1 +
src/opennebula/one_driver.c | 1 +
src/openvz/openvz_driver.c | 1 +
src/phyp/phyp_driver.c | 1 +
src/qemu/qemu_driver.c | 1 +
src/remote/remote_driver.c | 1 +
src/test/test_driver.c | 1 +
src/uml/uml_driver.c | 1 +
src/vbox/vbox_tmpl.c | 1 +
src/xen/xen_driver.c | 1 +
src/xenapi/xenapi_driver.c | 1 +
16 files changed, 86 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 12b8ea1..28c1f4e 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -864,6 +864,8 @@ int virDomainAttachDeviceFlags(virDomainPtr domain,
const char *xml, unsigned int flags);
int virDomainDetachDeviceFlags(virDomainPtr domain,
const char *xml, unsigned int flags);
+int virDomainUpdateDeviceFlags(virDomainPtr domain,
+ const char *xml, unsigned int flags);
/*
* NUMA support
diff --git a/src/driver.h b/src/driver.h
index 362533f..bec5545 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -204,6 +204,10 @@ typedef int
const char *xml,
unsigned int flags);
typedef int
+ (*virDrvDomainUpdateDeviceFlags) (virDomainPtr domain,
+ const char *xml,
+ unsigned int flags);
+typedef int
(*virDrvDomainGetAutostart) (virDomainPtr domain,
int *autostart);
typedef int
@@ -448,6 +452,7 @@ struct _virDriver {
virDrvDomainAttachDeviceFlags domainAttachDeviceFlags;
virDrvDomainDetachDevice domainDetachDevice;
virDrvDomainDetachDeviceFlags domainDetachDeviceFlags;
+ virDrvDomainUpdateDeviceFlags domainUpdateDeviceFlags;
virDrvDomainGetAutostart domainGetAutostart;
virDrvDomainSetAutostart domainSetAutostart;
virDrvDomainGetSchedulerType domainGetSchedulerType;
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 19e9c02..56d721e 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3380,6 +3380,7 @@ static virDriver esxDriver = {
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDevice */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
NULL, /* domainGetAutostart */
NULL, /* domainSetAutostart */
esxDomainGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/libvirt.c b/src/libvirt.c
index 7b74fd9..24ddc67 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5147,6 +5147,10 @@ error:
* Create a virtual device attachment to backend. This function,
* having hotplug semantics, is only allowed on an active domain.
*
+ * For compatability, this method can also be used to change the media
+ * in an existing CDROM/Floppy device, however, applications are
+ * recommended to use the virDomainUpdateDeviceFlag method instead.
+ *
* Returns 0 in case of success, -1 in case of failure.
*/
int
@@ -5201,6 +5205,10 @@ error:
* return failure if LIVE is specified but it only supports modifying the
* persisted device allocation.
*
+ * For compatability, this method can also be used to change the media
+ * in an existing CDROM/Floppy device, however, applications are
+ * recommended to use the virDomainUpdateDeviceFlag method instead.
+ *
* Returns 0 in case of success, -1 in case of failure.
*/
int
@@ -5336,6 +5344,64 @@ error:
}
/**
+ * virDomainUpdateDeviceFlags:
+ * @domain: pointer to domain object
+ * @xml: pointer to XML description of one device
+ * @flags: an OR'ed set of virDomainDeviceModifyFlags
+ *
+ * Change a virtual device on a domain, using the flags parameter
+ * to control how the device is changed. VIR_DOMAIN_DEVICE_MODIFY_CURRENT
+ * specifies that the device change is made based on current domain
+ * state. VIR_DOMAIN_DEVICE_MODIFY_LIVE specifies that the device shall be
+ * changed on the active domain instance only and is not added to the
+ * persisted domain configuration. VIR_DOMAIN_DEVICE_MODIFY_CONFIG
+ * specifies that the device shall be changed on the persisted domain
+ * configuration only. Note that the target hypervisor must return an
+ * error if unable to satisfy flags. E.g. the hypervisor driver will
+ * return failure if LIVE is specified but it only supports modifying the
+ * persisted device allocation.
+ *
+ * This method is used for actions such changing CDROM/Floppy device
+ * media, altering the graphics configuration such as password,
+ * reconfiguring the NIC device backend connectivity, etc.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
+int
+virDomainUpdateDeviceFlags(virDomainPtr domain,
+ const char *xml, unsigned int flags)
+{
+ virConnectPtr conn;
+ DEBUG("domain=%p, xml=%s, flags=%d", domain, xml, flags);
+
+ virResetLastError();
+
+ if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
+ virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+ return (-1);
+ }
+ if (domain->conn->flags & VIR_CONNECT_RO) {
+ virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ goto error;
+ }
+ conn = domain->conn;
+
+ if (conn->driver->domainUpdateDeviceFlags) {
+ int ret;
+ ret = conn->driver->domainUpdateDeviceFlags(domain, xml, flags);
+ if (ret < 0)
+ goto error;
+ return ret;
+ }
+
+ virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+ virDispatchError(domain->conn);
+ return -1;
+}
+
+/**
* virNodeGetCellsFreeMemory:
* @conn: pointer to the hypervisor connection
* @freeMems: pointer to the array of unsigned long long
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 623e53c..7247125 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -362,6 +362,7 @@ LIBVIRT_0.7.8 {
global:
virStorageVolWipe;
virDomainMigrateSetMaxDowntime;
+ virDomainUpdateDeviceFlags;
} LIBVIRT_0.7.7;
# .... define new API here using predicted next version number ....
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index ba13065..88c3c55 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2428,6 +2428,7 @@ static virDriver lxcDriver = {
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDevice */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
lxcDomainGetAutostart, /* domainGetAutostart */
lxcDomainSetAutostart, /* domainSetAutostart */
lxcGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c
index e1d1efc..23f4c09 100644
--- a/src/opennebula/one_driver.c
+++ b/src/opennebula/one_driver.c
@@ -757,6 +757,7 @@ static virDriver oneDriver = {
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDevice */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
oneGetAutostart, /* domainGetAutostart */
NULL, /* domainSetAutostart */
NULL, /* domainGetSchedulerType */
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 50aadfc..ea71d3b 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1509,6 +1509,7 @@ static virDriver openvzDriver = {
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDevice */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
openvzDomainGetAutostart, /* domainGetAutostart */
openvzDomainSetAutostart, /* domainSetAutostart */
NULL, /* domainGetSchedulerType */
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index e4d67dc..909de77 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1616,6 +1616,7 @@ virDriver phypDriver = {
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDevice */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
NULL, /* domainGetAutostart */
NULL, /* domainSetAutostart */
NULL, /* domainGetSchedulerType */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 257f914..caa6442 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9644,6 +9644,7 @@ static virDriver qemuDriver = {
qemudDomainAttachDeviceFlags, /* domainAttachDeviceFlags */
qemudDomainDetachDevice, /* domainDetachDevice */
qemudDomainDetachDeviceFlags, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
qemudDomainGetAutostart, /* domainGetAutostart */
qemudDomainSetAutostart, /* domainSetAutostart */
qemuGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index b7e1c03..3b0b372 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -9151,6 +9151,7 @@ static virDriver remote_driver = {
remoteDomainAttachDeviceFlags, /* domainAttachDeviceFlags */
remoteDomainDetachDevice, /* domainDetachDevice */
remoteDomainDetachDeviceFlags, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
remoteDomainGetAutostart, /* domainGetAutostart */
remoteDomainSetAutostart, /* domainSetAutostart */
remoteDomainGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index f54ebae..5254851 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5214,6 +5214,7 @@ static virDriver testDriver = {
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDevice */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
testDomainGetAutostart, /* domainGetAutostart */
testDomainSetAutostart, /* domainSetAutostart */
testDomainGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index bf06787..5031d39 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1901,6 +1901,7 @@ static virDriver umlDriver = {
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDevice */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
umlDomainGetAutostart, /* domainGetAutostart */
umlDomainSetAutostart, /* domainSetAutostart */
NULL, /* domainGetSchedulerType */
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 1765d63..f398041 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -7030,6 +7030,7 @@ virDriver NAME(Driver) = {
vboxDomainAttachDeviceFlags, /* domainAttachDeviceFlags */
vboxDomainDetachDevice, /* domainDetachDevice */
vboxDomainDetachDeviceFlags, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
NULL, /* domainGetAutostart */
NULL, /* domainSetAutostart */
NULL, /* domainGetSchedulerType */
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 204ed91..1ff3e44 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1875,6 +1875,7 @@ static virDriver xenUnifiedDriver = {
xenUnifiedDomainAttachDeviceFlags, /* domainAttachDeviceFlags */
xenUnifiedDomainDetachDevice, /* domainDetachDevice */
xenUnifiedDomainDetachDeviceFlags, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
xenUnifiedDomainGetAutostart, /* domainGetAutostart */
xenUnifiedDomainSetAutostart, /* domainSetAutostart */
xenUnifiedDomainGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index ac00424..a06781a 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -1716,6 +1716,7 @@ static virDriver xenapiDriver = {
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDevice */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
xenapiDomainGetAutostart, /* domainGetAutostart */
xenapiDomainSetAutostart, /* domainSetAutostart */
xenapiDomainGetSchedulerType, /* domainGetSchedulerType */
--
1.6.6.1
3:11 p.m.
New subject: [libvirt] [PATCH 1/6] Introduce a new virDomainUpdateDeviceFlags public API
On 03/22/2010 01:05 PM, Daniel P. Berrange wrote:
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5147,6 +5147,10 @@ error:
* Create a virtual device attachment to backend. This function,
* having hotplug semantics, is only allowed on an active domain.
*
+ * For compatability, this method can also be used to change the media
s/compatability/compatibility/ (twice in your patch)
+++ b/src/libvirt_public.syms
@@ -362,6 +362,7 @@ LIBVIRT_0.7.8 {
global:
virStorageVolWipe;
virDomainMigrateSetMaxDowntime;
+ virDomainUpdateDeviceFlags;
} LIBVIRT_0.7.7;
Weird mix of space vs. tabs in this file. How come 'make syntax-check'
doesn't warn?
ACK.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:05 p.m.
New subject: [libvirt] [PATCH 2/6] Remote protocol impl for virDomainUpdateDeviceFlags
This defines the wire format for the new virDomainUpdateDeviceFlags()
API, and implements the server & client side of the marshalling code.
* daemon/remote.c: Server side dispatch for virDomainUpdateDeviceFlags
* src/remote/remote_driver.c: Client side serialization for
virDomainUpdateDeviceFlags
* src/remote/remote_protocol.x: Define wire format for
virDomainUpdateDeviceFlags
* daemon/remote_dispatch_args.h, daemon/remote_dispatch_prototypes.h,
daemon/remote_dispatch_table.h, src/remote/remote_protocol.c,
src/remote/remote_protocol.h: Re-generate code
---
daemon/remote.c | 26 ++++++++++++++++++++++++++
daemon/remote_dispatch_args.h | 1 +
daemon/remote_dispatch_prototypes.h | 8 ++++++++
daemon/remote_dispatch_table.h | 5 +++++
src/remote/remote_driver.c | 28 +++++++++++++++++++++++++++-
src/remote/remote_protocol.c | 13 +++++++++++++
src/remote/remote_protocol.h | 10 ++++++++++
src/remote/remote_protocol.x | 9 ++++++++-
8 files changed, 98 insertions(+), 2 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 1b02143..0673351 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -916,6 +916,32 @@ remoteDispatchDomainAttachDeviceFlags (struct qemud_server *server
ATTRIBUTE_UNU
}
static int
+remoteDispatchDomainUpdateDeviceFlags (struct qemud_server *server ATTRIBUTE_UNUSED,
+ struct qemud_client *client ATTRIBUTE_UNUSED,
+ virConnectPtr conn,
+ remote_message_header *hdr ATTRIBUTE_UNUSED,
+ remote_error *rerr,
+ remote_domain_update_device_flags_args *args,
+ void *ret ATTRIBUTE_UNUSED)
+{
+ virDomainPtr dom;
+
+ dom = get_nonnull_domain (conn, args->dom);
+ if (dom == NULL) {
+ remoteDispatchConnError(rerr, conn);
+ return -1;
+ }
+
+ if (virDomainUpdateDeviceFlags (dom, args->xml, args->flags) == -1) {
+ virDomainFree(dom);
+ remoteDispatchConnError(rerr, conn);
+ return -1;
+ }
+ virDomainFree(dom);
+ return 0;
+}
+
+static int
remoteDispatchDomainCreate (struct qemud_server *server ATTRIBUTE_UNUSED,
struct qemud_client *client ATTRIBUTE_UNUSED,
virConnectPtr conn,
diff --git a/daemon/remote_dispatch_args.h b/daemon/remote_dispatch_args.h
index 68016aa..21217f3 100644
--- a/daemon/remote_dispatch_args.h
+++ b/daemon/remote_dispatch_args.h
@@ -142,3 +142,4 @@
remote_domain_abort_job_args val_remote_domain_abort_job_args;
remote_storage_vol_wipe_args val_remote_storage_vol_wipe_args;
remote_domain_migrate_set_max_downtime_args
val_remote_domain_migrate_set_max_downtime_args;
+ remote_domain_update_device_flags_args val_remote_domain_update_device_flags_args;
diff --git a/daemon/remote_dispatch_prototypes.h b/daemon/remote_dispatch_prototypes.h
index 13e78ce..2c9cc83 100644
--- a/daemon/remote_dispatch_prototypes.h
+++ b/daemon/remote_dispatch_prototypes.h
@@ -490,6 +490,14 @@ static int remoteDispatchDomainUndefine(
remote_error *err,
remote_domain_undefine_args *args,
void *ret);
+static int remoteDispatchDomainUpdateDeviceFlags(
+ struct qemud_server *server,
+ struct qemud_client *client,
+ virConnectPtr conn,
+ remote_message_header *hdr,
+ remote_error *err,
+ remote_domain_update_device_flags_args *args,
+ void *ret);
static int remoteDispatchDomainXmlFromNative(
struct qemud_server *server,
struct qemud_client *client,
diff --git a/daemon/remote_dispatch_table.h b/daemon/remote_dispatch_table.h
index b0d0d1e..ebd9b8b 100644
--- a/daemon/remote_dispatch_table.h
+++ b/daemon/remote_dispatch_table.h
@@ -837,3 +837,8 @@
.args_filter = (xdrproc_t) xdr_remote_domain_migrate_set_max_downtime_args,
.ret_filter = (xdrproc_t) xdr_void,
},
+{ /* DomainUpdateDeviceFlags => 167 */
+ .fn = (dispatch_fn) remoteDispatchDomainUpdateDeviceFlags,
+ .args_filter = (xdrproc_t) xdr_remote_domain_update_device_flags_args,
+ .ret_filter = (xdrproc_t) xdr_void,
+},
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 3b0b372..7ba2a7b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3158,6 +3158,32 @@ done:
}
static int
+remoteDomainUpdateDeviceFlags (virDomainPtr domain, const char *xml,
+ unsigned int flags)
+{
+ int rv = -1;
+ remote_domain_update_device_flags_args args;
+ struct private_data *priv = domain->conn->privateData;
+
+ remoteDriverLock(priv);
+
+ make_nonnull_domain (&args.dom, domain);
+ args.xml = (char *) xml;
+ args.flags = flags;
+
+ if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_UPDATE_DEVICE_FLAGS,
+ (xdrproc_t) xdr_remote_domain_update_device_flags_args, (char *)
&args,
+ (xdrproc_t) xdr_void, (char *) NULL) == -1)
+ goto done;
+
+ rv = 0;
+
+done:
+ remoteDriverUnlock(priv);
+ return rv;
+}
+
+static int
remoteDomainGetAutostart (virDomainPtr domain, int *autostart)
{
int rv = -1;
@@ -9151,7 +9177,7 @@ static virDriver remote_driver = {
remoteDomainAttachDeviceFlags, /* domainAttachDeviceFlags */
remoteDomainDetachDevice, /* domainDetachDevice */
remoteDomainDetachDeviceFlags, /* domainDetachDeviceFlags */
- NULL, /* domainUpdateDeviceFlags */
+ remoteDomainUpdateDeviceFlags, /* domainUpdateDeviceFlags */
remoteDomainGetAutostart, /* domainGetAutostart */
remoteDomainSetAutostart, /* domainSetAutostart */
remoteDomainGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c
index 3e5c1c7..3950049 100644
--- a/src/remote/remote_protocol.c
+++ b/src/remote/remote_protocol.c
@@ -1319,6 +1319,19 @@ xdr_remote_domain_detach_device_flags_args (XDR *xdrs,
remote_domain_detach_devi
}
bool_t
+xdr_remote_domain_update_device_flags_args (XDR *xdrs,
remote_domain_update_device_flags_args *objp)
+{
+
+ if (!xdr_remote_nonnull_domain (xdrs, &objp->dom))
+ return FALSE;
+ if (!xdr_remote_nonnull_string (xdrs, &objp->xml))
+ return FALSE;
+ if (!xdr_u_int (xdrs, &objp->flags))
+ return FALSE;
+ return TRUE;
+}
+
+bool_t
xdr_remote_domain_get_autostart_args (XDR *xdrs, remote_domain_get_autostart_args *objp)
{
diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h
index 43a3dc3..cd58c47 100644
--- a/src/remote/remote_protocol.h
+++ b/src/remote/remote_protocol.h
@@ -739,6 +739,13 @@ struct remote_domain_detach_device_flags_args {
};
typedef struct remote_domain_detach_device_flags_args
remote_domain_detach_device_flags_args;
+struct remote_domain_update_device_flags_args {
+ remote_nonnull_domain dom;
+ remote_nonnull_string xml;
+ u_int flags;
+};
+typedef struct remote_domain_update_device_flags_args
remote_domain_update_device_flags_args;
+
struct remote_domain_get_autostart_args {
remote_nonnull_domain dom;
};
@@ -1887,6 +1894,7 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_ABORT_JOB = 164,
REMOTE_PROC_STORAGE_VOL_WIPE = 165,
REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_DOWNTIME = 166,
+ REMOTE_PROC_DOMAIN_UPDATE_DEVICE_FLAGS = 167,
};
typedef enum remote_procedure remote_procedure;
@@ -2028,6 +2036,7 @@ extern bool_t xdr_remote_domain_attach_device_args (XDR *,
remote_domain_attach
extern bool_t xdr_remote_domain_attach_device_flags_args (XDR *,
remote_domain_attach_device_flags_args*);
extern bool_t xdr_remote_domain_detach_device_args (XDR *,
remote_domain_detach_device_args*);
extern bool_t xdr_remote_domain_detach_device_flags_args (XDR *,
remote_domain_detach_device_flags_args*);
+extern bool_t xdr_remote_domain_update_device_flags_args (XDR *,
remote_domain_update_device_flags_args*);
extern bool_t xdr_remote_domain_get_autostart_args (XDR *,
remote_domain_get_autostart_args*);
extern bool_t xdr_remote_domain_get_autostart_ret (XDR *,
remote_domain_get_autostart_ret*);
extern bool_t xdr_remote_domain_set_autostart_args (XDR *,
remote_domain_set_autostart_args*);
@@ -2313,6 +2322,7 @@ extern bool_t xdr_remote_domain_attach_device_args ();
extern bool_t xdr_remote_domain_attach_device_flags_args ();
extern bool_t xdr_remote_domain_detach_device_args ();
extern bool_t xdr_remote_domain_detach_device_flags_args ();
+extern bool_t xdr_remote_domain_update_device_flags_args ();
extern bool_t xdr_remote_domain_get_autostart_args ();
extern bool_t xdr_remote_domain_get_autostart_ret ();
extern bool_t xdr_remote_domain_set_autostart_args ();
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 1633908..def6c9e 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -730,6 +730,12 @@ struct remote_domain_detach_device_flags_args {
unsigned int flags;
};
+struct remote_domain_update_device_flags_args {
+ remote_nonnull_domain dom;
+ remote_nonnull_string xml;
+ unsigned int flags;
+};
+
struct remote_domain_get_autostart_args {
remote_nonnull_domain dom;
};
@@ -1717,7 +1723,8 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_GET_JOB_INFO = 163,
REMOTE_PROC_DOMAIN_ABORT_JOB = 164,
REMOTE_PROC_STORAGE_VOL_WIPE = 165,
- REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_DOWNTIME = 166
+ REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_DOWNTIME = 166,
+ REMOTE_PROC_DOMAIN_UPDATE_DEVICE_FLAGS = 167
/*
* Notice how the entries are grouped in sets of 10 ?
--
1.6.6.1
3:28 p.m.
New subject: [libvirt] [PATCH 2/6] Remote protocol impl for virDomainUpdateDeviceFlags
On 03/22/2010 01:05 PM, Daniel P. Berrange wrote:
+ if (virDomainUpdateDeviceFlags (dom, args->xml,
args->flags) == -1) {
+ virDomainFree(dom);
Inconsistency in whether there is a space before ( in function calls.
Which style is intended, or are both permitted?
Otherwise, ACK.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:19 a.m.
New subject: [libvirt] [PATCH 2/6] Remote protocol impl for virDomainUpdateDeviceFlags
On Mon, Mar 22, 2010 at 02:28:51PM -0600, Eric Blake wrote:
On 03/22/2010 01:05 PM, Daniel P. Berrange wrote:
> + if (virDomainUpdateDeviceFlags (dom, args->xml, args->flags) == -1) {
> + virDomainFree(dom);
Inconsistency in whether there is a space before ( in function calls.
Which style is intended, or are both permitted?
This was a copy & paste mistake. Functions should not have any spaces
between their name & (, though plenty of code violates this
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2:05 p.m.
New subject: [libvirt] [PATCH 3/6] Implement virDomainUpdateDeviceFlags API in all drivers with media change
To allow the new virDomainUpdateDeviceFlags() API to be universally
used with all drivers, this patch adds an impl to all the current
drivers which support CDROM or Floppy disk media change via the
current virDomainAttachDeviceFlags API
* src/qemu/qemu_driver.c, src/vbox/vbox_tmpl.c,
src/xen/proxy_internal.c, src/xen/xen_driver.c,
src/xen/xend_internal.c: Implement media change via the
virDomainUpdateDeviceFlags API
* src/xen/xen_driver.h, src/xen/xen_hypervisor.c,
src/xen/xen_inotify.c, src/xen/xm_internal.c,
src/xen/xs_internal.c: Stubs for Xen driver entry points
---
src/qemu/qemu_driver.c | 117 +++++++++++++++++++++++++++++++++++++++++++++-
src/vbox/vbox_tmpl.c | 21 +++++++-
src/xen/proxy_internal.c | 1 +
src/xen/xen_driver.c | 17 ++++++-
src/xen/xen_driver.h | 1 +
src/xen/xen_hypervisor.c | 1 +
src/xen/xen_inotify.c | 1 +
src/xen/xend_internal.c | 114 ++++++++++++++++++++++++++++++++++++++++++++
src/xen/xm_internal.c | 1 +
src/xen/xs_internal.c | 1 +
10 files changed, 270 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index caa6442..b9cfae0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6819,6 +6819,121 @@ static int qemudDomainAttachDeviceFlags(virDomainPtr dom,
return qemudDomainAttachDevice(dom, xml);
}
+
+static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
+ const char *xml,
+ unsigned int flags)
+{
+ struct qemud_driver *driver = dom->conn->privateData;
+ virDomainObjPtr vm;
+ virDomainDeviceDefPtr dev = NULL;
+ unsigned long long qemuCmdFlags;
+ virCgroupPtr cgroup = NULL;
+ int ret = -1;
+
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("cannot modify the persistent
configuration of a domain"));
+ return -1;
+ }
+
+ qemuDriverLock(driver);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ if (!vm) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(dom->uuid, uuidstr);
+ qemuReportError(VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s'"),
uuidstr);
+ goto cleanup;
+ }
+
+ if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("cannot attach device on inactive
domain"));
+ goto endjob;
+ }
+
+ dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
+ VIR_DOMAIN_XML_INACTIVE);
+ if (dev == NULL)
+ goto endjob;
+
+ if (qemudExtractVersionInfo(vm->def->emulator,
+ NULL,
+ &qemuCmdFlags) < 0)
+ goto endjob;
+
+ switch (dev->type) {
+ case VIR_DOMAIN_DEVICE_DISK:
+ if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
+ if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup,
0) !=0 ) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to find cgroup for %s\n"),
+ vm->def->name);
+ goto endjob;
+ }
+ if (dev->data.disk->src != NULL &&
+ dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK &&
+ virCgroupAllowDevicePath(cgroup,
+ dev->data.disk->src) < 0) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unable to allow device %s"),
+ dev->data.disk->src);
+ goto endjob;
+ }
+ }
+
+ switch (dev->data.disk->device) {
+ case VIR_DOMAIN_DISK_DEVICE_CDROM:
+ case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+ ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk);
+ if (ret == 0)
+ dev->data.disk = NULL;
+ break;
+
+
+ default:
+ qemuReportError(VIR_ERR_NO_SUPPORT,
+ _("disk bus '%s' cannot be updated."),
+ virDomainDiskBusTypeToString(dev->data.disk->bus));
+ break;
+ }
+
+ if (ret != 0 && cgroup) {
+ virCgroupDenyDevicePath(cgroup,
+ dev->data.disk->src);
+ }
+ break;
+
+ default:
+ qemuReportError(VIR_ERR_NO_SUPPORT,
+ _("disk device type '%s' cannot be updated"),
+ virDomainDiskDeviceTypeToString(dev->data.disk->device));
+ break;
+ }
+
+ if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm)
< 0)
+ ret = -1;
+
+endjob:
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
+
+cleanup:
+ if (cgroup)
+ virCgroupFree(&cgroup);
+
+ virDomainDeviceDefFree(dev);
+ if (vm)
+ virDomainObjUnlock(vm);
+ qemuDriverUnlock(driver);
+ return ret;
+}
+
+
static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev,
@@ -9644,7 +9759,7 @@ static virDriver qemuDriver = {
qemudDomainAttachDeviceFlags, /* domainAttachDeviceFlags */
qemudDomainDetachDevice, /* domainDetachDevice */
qemudDomainDetachDeviceFlags, /* domainDetachDeviceFlags */
- NULL, /* domainUpdateDeviceFlags */
+ qemuDomainUpdateDeviceFlags, /* domainUpdateDeviceFlags */
qemudDomainGetAutostart, /* domainGetAutostart */
qemudDomainSetAutostart, /* domainSetAutostart */
qemuGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index f398041..b02a633 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -4638,7 +4638,7 @@ cleanup:
return ret;
}
-static int vboxDomainAttachDevice(virDomainPtr dom, const char *xml) {
+static int vboxDomainAttachDeviceImpl(virDomainPtr dom, const char *xml, int
mediaChangeOnly ATTRIBUTE_UNUSED) {
VBOX_OBJECT_CHECK(dom->conn, int, -1);
IMachine *machine = NULL;
vboxIID *iid = NULL;
@@ -4836,6 +4836,10 @@ cleanup:
return ret;
}
+static int vboxDomainAttachDevice(virDomainPtr dom, const char *xml) {
+ return vboxDomainAttachDeviceImpl(dom, xml, 0);
+}
+
static int vboxDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
unsigned int flags) {
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
@@ -4844,7 +4848,18 @@ static int vboxDomainAttachDeviceFlags(virDomainPtr dom, const char
*xml,
return -1;
}
- return vboxDomainAttachDevice(dom, xml);
+ return vboxDomainAttachDeviceImpl(dom, xml, 0);
+}
+
+static int vboxDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
+ unsigned int flags) {
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
+ vboxError(dom->conn, VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot modify the persistent configuration of a
domain"));
+ return -1;
+ }
+
+ return vboxDomainAttachDeviceImpl(dom, xml, 1);
}
static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml) {
@@ -7030,7 +7045,7 @@ virDriver NAME(Driver) = {
vboxDomainAttachDeviceFlags, /* domainAttachDeviceFlags */
vboxDomainDetachDevice, /* domainDetachDevice */
vboxDomainDetachDeviceFlags, /* domainDetachDeviceFlags */
- NULL, /* domainUpdateDeviceFlags */
+ vboxDomainUpdateDeviceFlags, /* domainUpdateDeviceFlags */
NULL, /* domainGetAutostart */
NULL, /* domainSetAutostart */
NULL, /* domainGetSchedulerType */
diff --git a/src/xen/proxy_internal.c b/src/xen/proxy_internal.c
index 0c00abc..26eec13 100644
--- a/src/xen/proxy_internal.c
+++ b/src/xen/proxy_internal.c
@@ -78,6 +78,7 @@ struct xenUnifiedDriver xenProxyDriver = {
NULL, /* domainUndefine */
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
NULL, /* domainGetAutostart */
NULL, /* domainSetAutostart */
NULL, /* domainGetSchedulerType */
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 1ff3e44..4386693 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1489,6 +1489,21 @@ xenUnifiedDomainDetachDeviceFlags (virDomainPtr dom, const char
*xml,
}
static int
+xenUnifiedDomainUpdateDeviceFlags (virDomainPtr dom, const char *xml,
+ unsigned int flags)
+{
+ GET_PRIVATE(dom->conn);
+ int i;
+
+ for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
+ if (priv->opened[i] && drivers[i]->domainUpdateDeviceFlags
&&
+ drivers[i]->domainUpdateDeviceFlags(dom, xml, flags) == 0)
+ return 0;
+
+ return -1;
+}
+
+static int
xenUnifiedDomainGetAutostart (virDomainPtr dom, int *autostart)
{
GET_PRIVATE(dom->conn);
@@ -1875,7 +1890,7 @@ static virDriver xenUnifiedDriver = {
xenUnifiedDomainAttachDeviceFlags, /* domainAttachDeviceFlags */
xenUnifiedDomainDetachDevice, /* domainDetachDevice */
xenUnifiedDomainDetachDeviceFlags, /* domainDetachDeviceFlags */
- NULL, /* domainUpdateDeviceFlags */
+ xenUnifiedDomainUpdateDeviceFlags, /* domainUpdateDeviceFlags */
xenUnifiedDomainGetAutostart, /* domainGetAutostart */
xenUnifiedDomainSetAutostart, /* domainSetAutostart */
xenUnifiedDomainGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index 590777c..3e7c1d0 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -95,6 +95,7 @@ struct xenUnifiedDriver {
virDrvDomainUndefine domainUndefine;
virDrvDomainAttachDeviceFlags domainAttachDeviceFlags;
virDrvDomainDetachDeviceFlags domainDetachDeviceFlags;
+ virDrvDomainUpdateDeviceFlags domainUpdateDeviceFlags;
virDrvDomainGetAutostart domainGetAutostart;
virDrvDomainSetAutostart domainSetAutostart;
virDrvDomainGetSchedulerType domainGetSchedulerType;
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 4af3dba..552fbf5 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -795,6 +795,7 @@ struct xenUnifiedDriver xenHypervisorDriver = {
NULL, /* domainUndefine */
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
NULL, /* domainGetAutostart */
NULL, /* domainSetAutostart */
xenHypervisorGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c
index ee24bc4..8ff3256 100644
--- a/src/xen/xen_inotify.c
+++ b/src/xen/xen_inotify.c
@@ -82,6 +82,7 @@ struct xenUnifiedDriver xenInotifyDriver = {
NULL, /* domainUndefine */
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
NULL, /* domainGetAutostart */
NULL, /* domainSetAutostart */
NULL, /* domainGetSchedulerType */
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 85ae2a1..116523c 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -4230,6 +4230,119 @@ cleanup:
}
/**
+ * xenDaemonUpdateDeviceFlags:
+ * @domain: pointer to domain object
+ * @xml: pointer to XML description of device
+ * @flags: an OR'ed set of virDomainDeviceModifyFlags
+ *
+ * Create a virtual device attachment to backend.
+ * XML description is translated into S-expression.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
+static int
+xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml,
+ unsigned int flags)
+{
+ xenUnifiedPrivatePtr priv;
+ char *sexpr = NULL;
+ int ret = -1;
+ virDomainDeviceDefPtr dev = NULL;
+ virDomainDefPtr def = NULL;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char class[8], ref[80];
+
+ if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
+ virXendError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
+ __FUNCTION__);
+ return -1;
+ }
+
+ priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
+
+ if (domain->id < 0) {
+ /* If xendConfigVersion < 3 only live config can be changed */
+ if (priv->xendConfigVersion < 3) {
+ virXendError(domain->conn, VIR_ERR_OPERATION_INVALID, "%s",
+ _("Xend version does not support modifying "
+ "persisted config"));
+ return -1;
+ }
+ /* Cannot modify live config if domain is inactive */
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
+ virXendError(domain->conn, VIR_ERR_OPERATION_INVALID, "%s",
+ _("Cannot modify live config if domain is
inactive"));
+ return -1;
+ }
+ } else {
+ /* Only live config can be changed if xendConfigVersion < 3 */
+ if (priv->xendConfigVersion < 3 &&
+ (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT ||
+ flags != VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
+ virXendError(domain->conn, VIR_ERR_OPERATION_INVALID, "%s",
+ _("Xend version does not support modifying "
+ "persisted config"));
+ return -1;
+ }
+ /* Xen only supports modifying both live and persisted config if
+ * xendConfigVersion >= 3
+ */
+ if (flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
+ VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
+ virXendError(domain->conn, VIR_ERR_OPERATION_INVALID, "%s",
+ _("Xend only supports modifying both live and "
+ "persisted config"));
+ return -1;
+ }
+ }
+
+ if (!(def = xenDaemonDomainFetch(domain->conn,
+ domain->id,
+ domain->name,
+ NULL)))
+ goto cleanup;
+
+ if (!(dev = virDomainDeviceDefParse(priv->caps,
+ def, xml, VIR_DOMAIN_XML_INACTIVE)))
+ goto cleanup;
+
+
+ switch (dev->type) {
+ case VIR_DOMAIN_DEVICE_DISK:
+ if (xenDaemonFormatSxprDisk(domain->conn,
+ dev->data.disk,
+ &buf,
+ STREQ(def->os.type, "hvm") ? 1 : 0,
+ priv->xendConfigVersion, 1) < 0)
+ goto cleanup;
+ break;
+
+ default:
+ virXendError(domain->conn, VIR_ERR_NO_SUPPORT, "%s",
+ _("unsupported device type"));
+ goto cleanup;
+ }
+
+ sexpr = virBufferContentAndReset(&buf);
+
+ if (virDomainXMLDevID(domain, dev, class, ref, sizeof(ref))) {
+ virXendError(domain->conn, VIR_ERR_OPERATION_INVALID, "%s",
+ _("requested device does not exist"));
+ goto cleanup;
+ } else {
+ /* device exists, attempt to modify it */
+ ret = xend_op(domain->conn, domain->name, "op",
"device_configure",
+ "config", sexpr, "dev", ref, NULL);
+ }
+
+cleanup:
+ VIR_FREE(sexpr);
+ virDomainDefFree(def);
+ virDomainDeviceDefFree(dev);
+ return ret;
+}
+
+/**
* xenDaemonDetachDeviceFlags:
* @domain: pointer to domain object
* @xml: pointer to XML description of device
@@ -5224,6 +5337,7 @@ struct xenUnifiedDriver xenDaemonDriver = {
xenDaemonDomainUndefine, /* domainUndefine */
xenDaemonAttachDeviceFlags, /* domainAttachDeviceFlags */
xenDaemonDetachDeviceFlags, /* domainDetachDeviceFlags */
+ xenDaemonUpdateDeviceFlags, /* domainUpdateDeviceFlags */
xenDaemonDomainGetAutostart, /* domainGetAutostart */
xenDaemonDomainSetAutostart, /* domainSetAutostart */
xenDaemonGetSchedulerType, /* domainGetSchedulerType */
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 74bf0b6..ddbd2fe 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -113,6 +113,7 @@ struct xenUnifiedDriver xenXMDriver = {
xenXMDomainUndefine, /* domainUndefine */
xenXMDomainAttachDeviceFlags, /* domainAttachDeviceFlags */
xenXMDomainDetachDeviceFlags, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
NULL, /* domainGetAutostart */
NULL, /* domainSetAutostart */
NULL, /* domainGetSchedulerType */
diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c
index 3b241c7..613f97a 100644
--- a/src/xen/xs_internal.c
+++ b/src/xen/xs_internal.c
@@ -78,6 +78,7 @@ struct xenUnifiedDriver xenStoreDriver = {
NULL, /* domainUndefine */
NULL, /* domainAttachDeviceFlags */
NULL, /* domainDetachDeviceFlags */
+ NULL, /* domainUpdateDeviceFlags */
NULL, /* domainGetAutostart */
NULL, /* domainSetAutostart */
NULL, /* domainGetSchedulerType */
--
1.6.6.1
3:58 p.m.
New subject: [libvirt] [PATCH 3/6] Implement virDomainUpdateDeviceFlags API in all drivers with media change
On 03/22/2010 01:05 PM, Daniel P. Berrange wrote:
To allow the new virDomainUpdateDeviceFlags() API to be universally
used with all drivers, this patch adds an impl to all the current
drivers which support CDROM or Floppy disk media change via the
current virDomainAttachDeviceFlags API
+ switch (dev->type) {
+ case VIR_DOMAIN_DEVICE_DISK:
...
+
+ switch (dev->data.disk->device) {
+ case VIR_DOMAIN_DISK_DEVICE_CDROM:
+ case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+ ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk);
+ if (ret == 0)
+ dev->data.disk = NULL;
+ break;
+
+
+ default:
+ qemuReportError(VIR_ERR_NO_SUPPORT,
+ _("disk bus '%s' cannot be updated."),
+ virDomainDiskBusTypeToString(dev->data.disk->bus));
+ break;
This break only exits the innermost switch...
+ }
+
+ if (ret != 0 && cgroup) {
+ virCgroupDenyDevicePath(cgroup,
+ dev->data.disk->src);
+ }
+ break;
...is it still safe to call this after reporting an error like that,
before breaking from the outermost switch?
-static int vboxDomainAttachDevice(virDomainPtr dom, const char *xml)
{
+static int vboxDomainAttachDeviceImpl(virDomainPtr dom, const char *xml, int
mediaChangeOnly ATTRIBUTE_UNUSED) {
80 columns?
+ /* Only live config can be changed if xendConfigVersion <
3 */
+ if (priv->xendConfigVersion < 3 &&
+ (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT ||
+ flags != VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
+ virXendError(domain->conn, VIR_ERR_OPERATION_INVALID, "%s",
+ _("Xend version does not support modifying "
+ "persisted config"));
I think 'persistent' sounds better than 'persisted' as an adjective
(multiple instances).
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:18 a.m.
New subject: [libvirt] [PATCH 3/6] Implement virDomainUpdateDeviceFlags API in all drivers with media change
On Mon, Mar 22, 2010 at 02:58:16PM -0600, Eric Blake wrote:
On 03/22/2010 01:05 PM, Daniel P. Berrange wrote:
> To allow the new virDomainUpdateDeviceFlags() API to be universally
> used with all drivers, this patch adds an impl to all the current
> drivers which support CDROM or Floppy disk media change via the
> current virDomainAttachDeviceFlags API
>
> + switch (dev->type) {
> + case VIR_DOMAIN_DEVICE_DISK:
...
> +
> + switch (dev->data.disk->device) {
> + case VIR_DOMAIN_DISK_DEVICE_CDROM:
> + case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
> + ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk);
> + if (ret == 0)
> + dev->data.disk = NULL;
> + break;
> +
> +
> + default:
> + qemuReportError(VIR_ERR_NO_SUPPORT,
> + _("disk bus '%s' cannot be
updated."),
> +
virDomainDiskBusTypeToString(dev->data.disk->bus));
> + break;
This break only exits the innermost switch...
> + }
> +
> + if (ret != 0 && cgroup) {
> + virCgroupDenyDevicePath(cgroup,
> + dev->data.disk->src);
> + }
> + break;
...is it still safe to call this after reporting an error like that,
before breaking from the outermost switch?
Yep, this is correct. THe virCgroupDenyDevicePath() function has to be called
on all error scenarios here, both from the "default:" case, and error in the
previous 'VIR_DOMAIN_DISK_DEVICE_FLOPPY:' case, so that it removes the ACL
that was setup immediately before the switch.
> -static int vboxDomainAttachDevice(virDomainPtr dom, const char
*xml) {
> +static int vboxDomainAttachDeviceImpl(virDomainPtr dom, const char *xml, int
mediaChangeOnly ATTRIBUTE_UNUSED) {
80 columns?
> + /* Only live config can be changed if xendConfigVersion < 3 */
> + if (priv->xendConfigVersion < 3 &&
> + (flags != VIR_DOMAIN_DEVICE_MODIFY_CURRENT ||
> + flags != VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
> + virXendError(domain->conn, VIR_ERR_OPERATION_INVALID,
"%s",
> + _("Xend version does not support modifying "
> + "persisted config"));
I think 'persistent' sounds better than 'persisted' as an adjective
(multiple instances).
This was just a copied diagnostic from code higher up
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2:05 p.m.
New subject: [libvirt] [PATCH 4/6] Introduce a update-device command in virsh
Support the new virDomainUpdateDeviceFlags API in virsh by adding
a new 'update-device' command. In the future this should be augmented
with an explicit 'change-disk' command for media change to make it
end user discoverable, as attach-disk is.
* tools/virsh.c: Add 'update-device' command
---
tools/virsh.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 32895b2..35bdbf1 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -6677,6 +6677,73 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
/*
+ * "update-device" command
+ */
+static const vshCmdInfo info_update_device[] = {
+ {"help", N_("update device from an XML file")},
+ {"desc", N_("Update device from an XML <file>.")},
+ {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_update_device[] = {
+ {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
+ {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("XML file")},
+ {"persistent", VSH_OT_BOOL, 0, N_("persist device update")},
+ {NULL, 0, 0, NULL}
+};
+
+static int
+cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
+{
+ virDomainPtr dom;
+ char *from;
+ char *buffer;
+ int ret = TRUE;
+ int found;
+ unsigned int flags;
+
+ if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ return FALSE;
+
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+ return FALSE;
+
+ from = vshCommandOptString(cmd, "file", &found);
+ if (!found) {
+ vshError(ctl, "%s", _("update-device: Missing <file>
option"));
+ virDomainFree(dom);
+ return FALSE;
+ }
+
+ if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
+ virDomainFree(dom);
+ return FALSE;
+ }
+
+ if (vshCommandOptBool(cmd, "persistent")) {
+ flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
+ if (virDomainIsActive(dom) == 1)
+ flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
+ } else {
+ flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE;
+ }
+ ret = virDomainUpdateDeviceFlags(dom, buffer, flags);
+ VIR_FREE(buffer);
+
+ if (ret < 0) {
+ vshError(ctl, _("Failed to update device from %s"), from);
+ virDomainFree(dom);
+ return FALSE;
+ } else {
+ vshPrint(ctl, "%s", _("Device updated successfully\n"));
+ }
+
+ virDomainFree(dom);
+ return TRUE;
+}
+
+
+/*
* "attach-interface" command
*/
static const vshCmdInfo info_attach_interface[] = {
@@ -7891,6 +7958,7 @@ static const vshCmdDef commands[] = {
{"suspend", cmdSuspend, opts_suspend, info_suspend},
{"ttyconsole", cmdTTYConsole, opts_ttyconsole, info_ttyconsole},
{"undefine", cmdUndefine, opts_undefine, info_undefine},
+ {"update-device", cmdUpdateDevice, opts_update_device,
info_update_device},
{"uri", cmdURI, NULL, info_uri},
{"vol-create", cmdVolCreate, opts_vol_create, info_vol_create},
--
1.6.6.1
2:05 p.m.
New subject: [libvirt] [PATCH 5/6] Allow parsing <graphics> in device XML
Expand the parser for the standalone <device> XML format to
allow inclusion of the <graphics> device type
* src/conf/domain_conf.h: Add virDomainGraphicsDef to
the virDomainDeviceDef struct
* src/conf/domain_conf.c: Wire up parser for virDomainGraphicsDef
to virDomainDeviceDefParse method
---
src/conf/domain_conf.c | 10 +++++++++-
src/conf/domain_conf.h | 2 ++
2 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 56c5239..96568a0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -88,7 +88,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
"video",
"hostdev",
"watchdog",
- "controller")
+ "controller",
+ "graphics")
VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
"none",
@@ -575,6 +576,9 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
case VIR_DOMAIN_DEVICE_CONTROLLER:
virDomainControllerDefFree(def->data.controller);
break;
+ case VIR_DOMAIN_DEVICE_GRAPHICS:
+ virDomainGraphicsDefFree(def->data.graphics);
+ break;
}
VIR_FREE(def);
@@ -3295,6 +3299,10 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps,
dev->type = VIR_DOMAIN_DEVICE_CONTROLLER;
if (!(dev->data.controller = virDomainControllerDefParseXML(node, flags)))
goto error;
+ } else if (xmlStrEqual(node->name, BAD_CAST "graphics")) {
+ dev->type = VIR_DOMAIN_DEVICE_GRAPHICS;
+ if (!(dev->data.graphics = virDomainGraphicsDefParseXML(node, flags)))
+ goto error;
} else {
virDomainReportError(VIR_ERR_XML_ERROR,
"%s", _("unknown device type"));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0540a77..1ecadfc 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -544,6 +544,7 @@ enum virDomainDeviceType {
VIR_DOMAIN_DEVICE_HOSTDEV,
VIR_DOMAIN_DEVICE_WATCHDOG,
VIR_DOMAIN_DEVICE_CONTROLLER,
+ VIR_DOMAIN_DEVICE_GRAPHICS,
VIR_DOMAIN_DEVICE_LAST,
};
@@ -562,6 +563,7 @@ struct _virDomainDeviceDef {
virDomainVideoDefPtr video;
virDomainHostdevDefPtr hostdev;
virDomainWatchdogDefPtr watchdog;
+ virDomainGraphicsDefPtr graphics;
} data;
};
--
1.6.6.1
2:05 p.m.
New subject: [libvirt] [PATCH 6/6] Implement VNC password change in QEMU
Use the new virDomainUpdateDeviceFlags API to allow the VNC password
to be changed on the fly
* src/internal.h: Define STREQ_NULLABLE() which is like STREQ()
but does not crash if either argument is NULL, and treats two
NULLs as equal.
* src/libvirt_private.syms: Export virDomainGraphicsTypeToString
* src/qemu/qemu_driver.c: Support VNC password change on a live
machine
* src/qemu/qemu_monitor.c: Disable crazy debugging info. Treat a
NULL password as "" (empty string), allowing passwords to be
disabled in the monitor
---
src/internal.h | 6 +++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.c | 17 ++++++++-
4 files changed, 103 insertions(+), 2 deletions(-)
diff --git a/src/internal.h b/src/internal.h
index 4ec6edc..a636fe5 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -58,6 +58,12 @@
# define STRCASENEQLEN(a,b,n) (strncasecmp(a,b,n) != 0)
# define STRPREFIX(a,b) (strncmp(a,b,strlen(b)) == 0)
+# define STREQ_NULLABLE(a, b) \
+ ((!a && !b) || (a && b && STREQ(a, b)))
+# define STRNEQ_NULLABLE(a, b) \
+ ((!a && b) || (a && !b) || (a && b && STRNEQ(a, b)))
+
+
# define NUL_TERMINATE(buf) do { (buf)[sizeof(buf)-1] = '\0'; } while (0)
# define ARRAY_CARDINALITY(Array) (sizeof (Array) / sizeof *(Array))
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c5ee23d..989152d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -139,6 +139,7 @@ virDomainFindByName;
virDomainFindByUUID;
virDomainGetRootFilesystem;
virDomainGraphicsTypeFromString;
+virDomainGraphicsTypeToString;
virDomainGraphicsDefFree;
virDomainHostdevDefFree;
virDomainHostdevModeTypeToString;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b9cfae0..8ebbbe2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6820,6 +6820,83 @@ static int qemudDomainAttachDeviceFlags(virDomainPtr dom,
}
+static virDomainGraphicsDefPtr qemuDomainFindGraphics(virDomainObjPtr vm,
+ virDomainGraphicsDefPtr dev)
+{
+ int i;
+
+ for (i = 0 ; i < vm->def->ngraphics ; i++) {
+ if (vm->def->graphics[i]->type == dev->type)
+ return vm->def->graphics[i];
+ }
+
+ return NULL;
+}
+
+
+static int
+qemuDomainChangeGraphics(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainGraphicsDefPtr dev)
+{
+ virDomainGraphicsDefPtr olddev = qemuDomainFindGraphics(vm, dev);
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int ret = -1;
+
+ if (!olddev) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot find existing graphics device to modify"));
+ return -1;
+ }
+
+ switch (dev->type) {
+ case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
+ if ((olddev->data.vnc.autoport != dev->data.vnc.autoport) ||
+ (!dev->data.vnc.autoport && (olddev->data.vnc.port !=
dev->data.vnc.port))) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot change port settings on vnc graphics"));
+ return -1;
+ }
+ if (STRNEQ_NULLABLE(olddev->data.vnc.listenAddr, dev->data.vnc.listenAddr))
{
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot change listen address setting on vnc
graphics"));
+ return -1;
+ }
+ if (STRNEQ_NULLABLE(olddev->data.vnc.keymap, dev->data.vnc.keymap)) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot change keymap setting on vnc
graphics"));
+ return -1;
+ }
+
+ if (STRNEQ_NULLABLE(olddev->data.vnc.passwd, dev->data.vnc.passwd)) {
+ VIR_DEBUG("Updating password on VNC server %p",
dev->data.vnc.passwd, driver->vncPassword);
+ qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ ret = qemuMonitorSetVNCPassword(priv->mon,
+ dev->data.vnc.passwd ?
+ dev->data.vnc.passwd :
+ driver->vncPassword);
+ qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+ /* Steal the new dev's char * reference */
+ VIR_FREE(olddev->data.vnc.passwd);
+ olddev->data.vnc.passwd = dev->data.vnc.passwd;
+ dev->data.vnc.passwd = NULL;
+ } else {
+ ret = 0;
+ }
+ break;
+
+ default:
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unable to change config on '%s' graphics
type"),
+ virDomainGraphicsTypeToString(dev->type));
+ break;
+ }
+
+ return ret;
+}
+
+
static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
const char *xml,
unsigned int flags)
@@ -6908,6 +6985,10 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
}
break;
+ case VIR_DOMAIN_DEVICE_GRAPHICS:
+ ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics);
+ break;
+
default:
qemuReportError(VIR_ERR_NO_SUPPORT,
_("disk device type '%s' cannot be updated"),
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 6b68db8..fe95afd 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -39,7 +39,8 @@
#define VIR_FROM_THIS VIR_FROM_QEMU
-#define QEMU_DEBUG_RAW_IO 0
+#define DEBUG_IO 0
+#define DEBUG_RAW_IO 0
struct _qemuMonitor {
virMutex lock;
@@ -302,7 +303,8 @@ qemuMonitorIOProcess(qemuMonitorPtr mon)
if (mon->msg && mon->msg->txOffset == mon->msg->txLength)
msg = mon->msg;
-#if QEMU_DEBUG_RAW_IO
+#if DEBUG_IO
+#if DEBUG_RAW_IO
char *str1 = qemuMonitorEscapeNonPrintable(msg ? msg->txBuffer : "");
char *str2 = qemuMonitorEscapeNonPrintable(mon->buffer);
VIR_ERROR("Process %d %p %p [[[[%s]]][[[%s]]]", (int)mon->bufferOffset,
mon->msg, msg, str1, str2);
@@ -311,6 +313,8 @@ qemuMonitorIOProcess(qemuMonitorPtr mon)
#else
VIR_DEBUG("Process %d", (int)mon->bufferOffset);
#endif
+#endif
+
if (mon->json)
len = qemuMonitorJSONIOProcess(mon,
mon->buffer, mon->bufferOffset,
@@ -332,7 +336,9 @@ qemuMonitorIOProcess(qemuMonitorPtr mon)
VIR_FREE(mon->buffer);
mon->bufferOffset = mon->bufferLength = 0;
}
+#if DEBUG_IO
VIR_DEBUG("Process done %d used %d", (int)mon->bufferOffset, len);
+#endif
if (msg && msg->finished)
virCondBroadcast(&mon->notify);
return len;
@@ -455,7 +461,9 @@ qemuMonitorIORead(qemuMonitorPtr mon)
mon->buffer[mon->bufferOffset] = '\0';
}
+#if DEBUG_IO
VIR_DEBUG("Now read %d bytes of data", (int)mon->bufferOffset);
+#endif
return ret;
}
@@ -485,7 +493,9 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
qemuMonitorLock(mon);
qemuMonitorRef(mon);
+#if DEBUG_IO
VIR_DEBUG("Monitor %p I/O on watch %d fd %d events %d", mon, watch, fd,
events);
+#endif
if (mon->fd != fd || mon->watch != watch) {
VIR_ERROR("event from unexpected fd %d!=%d / watch %d!=%d", mon->fd,
fd, mon->watch, watch);
@@ -904,6 +914,9 @@ int qemuMonitorSetVNCPassword(qemuMonitorPtr mon,
int ret;
DEBUG("mon=%p, fd=%d", mon, mon->fd);
+ if (!password)
+ password = "";
+
if (mon->json)
ret = qemuMonitorJSONSetVNCPassword(mon, password);
else
--
1.6.6.1
4:21 p.m.
New subject: [libvirt] [PATCH 6/6] Implement VNC password change in QEMU
On 03/22/2010 01:05 PM, Daniel P. Berrange wrote:
Use the new virDomainUpdateDeviceFlags API to allow the VNC password
to be changed on the fly
* src/internal.h: Define STREQ_NULLABLE() which is like STREQ()
but does not crash if either argument is NULL, and treats two
NULLs as equal.
...
+++ b/src/internal.h
@@ -58,6 +58,12 @@
# define STRCASENEQLEN(a,b,n) (strncasecmp(a,b,n) != 0)
# define STRPREFIX(a,b) (strncmp(a,b,strlen(b)) == 0)
+# define STREQ_NULLABLE(a, b) \
+ ((!a && !b) || (a && b && STREQ(a, b)))
+# define STRNEQ_NULLABLE(a, b) \
+ ((!a && b) || (a && !b) || (a && b && STRNEQ(a, b)))
This seems like an independently useful change, and one worth
documenting in docs/hacking.html.in next to the existing documentation
on STREQ. It also has a bug - it is under-parenthesized. And you can
use ^ for compactness:
# define STREQ_NULLABLE(a, b) \
((!(a) && !(b)) || ((a) && (b) && STREQ(a, b)))
# define STRNEQ_NULLABLE(a, b) \
((!(a) ^ !(b)) || ((a) && (b) && STRNEQ(a, b)))
Hmmm. The existing STREQ and STRNEQ only evaluate arguments once, but
this evaluates arguments multiple times. Then again, so does the
existing STRPREFIX. Are any of these three worth making static inline
functions, rather than macros, to avoid side-effects of multiple evaluation?
I did not closely review the rest of the patch.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:16 a.m.
New subject: [libvirt] [PATCH 6/6] Implement VNC password change in QEMU
On Mon, Mar 22, 2010 at 03:21:06PM -0600, Eric Blake wrote:
On 03/22/2010 01:05 PM, Daniel P. Berrange wrote:
> Use the new virDomainUpdateDeviceFlags API to allow the VNC password
> to be changed on the fly
>
> * src/internal.h: Define STREQ_NULLABLE() which is like STREQ()
> but does not crash if either argument is NULL, and treats two
> NULLs as equal.
...
> +++ b/src/internal.h
> @@ -58,6 +58,12 @@
> # define STRCASENEQLEN(a,b,n) (strncasecmp(a,b,n) != 0)
> # define STRPREFIX(a,b) (strncmp(a,b,strlen(b)) == 0)
>
> +# define STREQ_NULLABLE(a, b) \
> + ((!a && !b) || (a && b && STREQ(a, b)))
> +# define STRNEQ_NULLABLE(a, b) \
> + ((!a && b) || (a && !b) || (a && b && STRNEQ(a,
b)))
This seems like an independently useful change, and one worth
documenting in docs/hacking.html.in next to the existing documentation
on STREQ. It also has a bug - it is under-parenthesized. And you can
use ^ for compactness:
# define STREQ_NULLABLE(a, b) \
((!(a) && !(b)) || ((a) && (b) && STREQ(a, b)))
# define STRNEQ_NULLABLE(a, b) \
((!(a) ^ !(b)) || ((a) && (b) && STRNEQ(a, b)))
Hmmm. The existing STREQ and STRNEQ only evaluate arguments once, but
this evaluates arguments multiple times. Then again, so does the
existing STRPREFIX. Are any of these three worth making static inline
functions, rather than macros, to avoid side-effects of multiple evaluation?
I don't think the multiple evaluation matters - none of the uses of
these macros pass in complex expressions where multiple evaluation
would cause problems. THe args are always just constant strings, or
char * pointers.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
5358
days inactive
5359
days old
13 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Eric Blake