Re: [libvirt] [PATCH 1/4] vcpupin: inroduce a new libvir API (virDomainPinVcpuFlags)

[Wen Congyang]

At 04/08/2011 01:20 PM, Taku Izumi Write:
> 
> This patch introduces a new libvirt API (virDomainPinVcpuFlags)
> 
> Signed-off-by: Taku Izumi 
> ---
>  include/libvirt/libvirt.h.in |6 +++
>  src/driver.h |7 +++
>  src/libvirt.c|   83 
> +++
>  src/libvirt_public.syms  |4 ++
>  src/lxc/lxc_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/vmware/vmware_driver.c   |1 
>  src/xen/xen_driver.c |1 
>  src/xenapi/xenapi_driver.c   |1 
>  15 files changed, 111 insertions(+)
> 
> Index: libvirt/include/libvirt/libvirt.h.in
> ===
> --- libvirt.orig/include/libvirt/libvirt.h.in
> +++ libvirt/include/libvirt/libvirt.h.in
> @@ -1017,6 +1017,7 @@ typedef virVcpuInfo *virVcpuInfoPtr;
>  
>  /* Flags for controlling virtual CPU hot-plugging.  */
>  typedef enum {
> +VIR_DOMAIN_VCPU_CURRENT = 0,/* Affect current domain state */
>  /* Must choose at least one of these two bits; SetVcpus can choose both 
> */
>  VIR_DOMAIN_VCPU_LIVE= (1 << 0), /* Affect active domain */
>  VIR_DOMAIN_VCPU_CONFIG  = (1 << 1), /* Affect next boot */
> @@ -1037,6 +1038,11 @@ int virDomainPinVcpu
>   unsigned int vcpu,
>   unsigned char *cpumap,
>   int maplen);
> +int virDomainPinVcpuFlags   (virDomainPtr domain,
> + unsigned int vcpu,
> + unsigned char *cpumap,
> + int maplen,
> + unsigned int flags);
>  
>  /**
>   * VIR_USE_CPU:
> Index: libvirt/src/driver.h
> ===
> --- libvirt.orig/src/driver.h
> +++ libvirt/src/driver.h
> @@ -220,6 +220,12 @@ typedef int
>   unsigned char *cpumap,
>   int maplen);
>  typedef int
> +(*virDrvDomainPinVcpuFlags) (virDomainPtr domain,

Please use tab between 'virDrvDomainPinVcpuFlags)' and '(virDomainPtr' instead 
of
space as we use tab in all the other places.

> + unsigned int vcpu,
> + unsigned char *cpumap,
> + int maplen,
> + unsigned int flags);
> +typedef int
>  (*virDrvDomainGetVcpus)  (virDomainPtr domain,
>   virVcpuInfoPtr info,
>   int maxinfo,
> @@ -570,6 +576,7 @@ struct _virDriver {
>  virDrvDomainSetVcpusFlagsdomainSetVcpusFlags;
>  virDrvDomainGetVcpusFlagsdomainGetVcpusFlags;
>  virDrvDomainPinVcpu  domainPinVcpu;
> +virDrvDomainPinVcpuFlagsdomainPinVcpuFlags;

Please use tab between virDrvDomainPinVcpuFlags and domainPinVcpuFlags.

>  virDrvDomainGetVcpus domainGetVcpus;
>  virDrvDomainGetMaxVcpus  domainGetMaxVcpus;
>  virDrvDomainGetSecurityLabel domainGetSecurityLabel;
> Index: libvirt/src/libvirt.c
> ===
> --- libvirt.orig/src/libvirt.c
> +++ libvirt/src/libvirt.c
> @@ -5461,6 +5461,89 @@ error:
>  }
>  
>  /**
> + * virDomainPinVcpuFlags:
> + * @domain: pointer to domain object, or NULL for Domain0
> + * @vcpu: virtual CPU number
> + * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN)
> + *  Each bit set to 1 means that corresponding CPU is usable.
> + *  Bytes are stored in little-endian order: CPU0-7, 8-15...
> + *  In each byte, lowest CPU number is least significant bit.
> + * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in
> + *  underlying virtualization system (Xen...).
> + *  If maplen < size, missing bytes are set to zero.
> + *  If maplen > size, failure code is returned.
> + * @flags: an OR'ed subset of virDomainVcpuFlags
> + *
> + * Dynamically change the real CPUs which can be allocated to a virtual CPU.
> + * This function requires privileged access to the hypervisor.
> + *
> + * @flags may include VIR_DOMAIN_VCPU_LIVE or VIR_DOMAIN_VCPU_CONFIG.
> + * Both flags may be set, but VIR_DOMAIN_VCPU_MAXIMUM cannot be set.
> + * If VIR_DOMAIN_VCPU_LIVE is set, the change affects a running domain
> + * and may fail if domain is not

[libvirt] [PATCH 4/4] vcpupin: add the new options to "virsh vcpupin" command

[Taku Izumi]

This patch adds the new option (--live, --config and --current) to 
"virsh vcpupin" command. The behavior of above aption is the same as 
that of "virsh setmem", "virsh setvcpus", and whatnot. 
When the --config option is specified, the command affects a persistent domain, 
while --live option is specified, it affects a running (live) domain.
The --current option cannot be used with --config or --live at the same time,
and when --current is specified, it affects a "current" domain.


Signed-off-by: Taku Izumi 
---
 tools/virsh.c   |   33 +++--
 tools/virsh.pod |8 +++-
 2 files changed, 38 insertions(+), 3 deletions(-)

Index: libvirt/tools/virsh.c
===
--- libvirt.orig/tools/virsh.c
+++ libvirt/tools/virsh.c
@@ -2721,6 +2721,9 @@ static const vshCmdOptDef opts_vcpupin[]
 {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
 {"vcpu", VSH_OT_INT, VSH_OFLAG_REQ, N_("vcpu number")},
 {"cpulist", VSH_OT_DATA, VSH_OFLAG_REQ, N_("host cpu number(s) (comma 
separated)")},
+{"config", VSH_OT_BOOL, 0, N_("affect next boot")},
+{"live", VSH_OT_BOOL, 0, N_("affect running domain")},
+{"current", VSH_OT_BOOL, 0, N_("affect current domain")},
 {NULL, 0, 0, NULL}
 };
 
@@ -2737,6 +2740,26 @@ cmdVcpupin(vshControl *ctl, const vshCmd
 int cpumaplen;
 int i;
 enum { expect_num, expect_num_or_comma } state;
+int config = vshCommandOptBool(cmd, "config");
+int live = vshCommandOptBool(cmd, "live");
+int current = vshCommandOptBool(cmd, "current");
+int flags = 0;
+
+if (current) {
+if (live || config) {
+vshError(ctl, "%s", _("--current must be specified exclusively"));
+return FALSE;
+}
+flags = VIR_DOMAIN_VCPU_CURRENT;
+} else {
+if (config)
+flags |= VIR_DOMAIN_VCPU_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_VCPU_LIVE;
+/* neither option is specified */
+if (!live && !config)
+flags = -1;
+}
 
 if (!vshConnectionUsability(ctl, ctl->conn))
 return FALSE;
@@ -2833,8 +2856,14 @@ cmdVcpupin(vshControl *ctl, const vshCmd
 cpulist++;
 } while (cpulist);
 
-if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) != 0) {
-ret = FALSE;
+if (flags == -1) {
+if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) != 0) {
+ret = FALSE;
+}
+} else {
+if (virDomainPinVcpuFlags(dom, vcpu, cpumap, cpumaplen, flags) != 0) {
+ret = FALSE;
+}
 }
 
 VIR_FREE(cpumap);
Index: libvirt/tools/virsh.pod
===
--- libvirt.orig/tools/virsh.pod
+++ libvirt/tools/virsh.pod
@@ -749,10 +749,16 @@ values; these two flags cannot both be s
 Returns basic information about the domain virtual CPUs, like the number of
 vCPUs, the running time, the affinity to physical processors.
 
-=item B I I I
+=item B I I I optional I<--live> I<--config>
+I<--current>
 
 Pin domain VCPUs to host physical CPUs. The I number must be provided
 and I is a comma separated list of physical CPU numbers.
+If I<--live> is specified, affect a running guest.
+If I<--config> is specified, affect the next boot of a persistent guest.
+If I<--current> is specified, affect the current guest state.
+Both I<--live> and I<--config> flags may be given, but I<--current> is 
exclusive.
+If no flag is specified, behavior is different depending on hypervisor.
 
 =item B I
 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/4] vcpupin: implement the remote protocol to address the new API

[Taku Izumi]

This patch implements the remote protocol to address the new API
(virDomainPinVcpuFlags).

Signed-off-by: Taku Izumi 
---
 daemon/remote.c |   37 +
 daemon/remote_dispatch_args.h   |1 
 daemon/remote_dispatch_prototypes.h |8 +++
 daemon/remote_dispatch_table.h  |5 
 src/remote/remote_driver.c  |   40 +++-
 src/remote/remote_protocol.c|   16 ++
 src/remote/remote_protocol.h|   14 
 src/remote/remote_protocol.x|   10 -
 src/remote_protocol-structs |9 
 9 files changed, 138 insertions(+), 2 deletions(-)

Index: libvirt/daemon/remote.c
===
--- libvirt.orig/daemon/remote.c
+++ libvirt/daemon/remote.c
@@ -2196,6 +2196,43 @@ remoteDispatchDomainPinVcpu (struct qemu
 }
 
 static int
+remoteDispatchDomainPinVcpuFlags (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_pin_vcpu_flags_args *args,
+  void *ret ATTRIBUTE_UNUSED)
+{
+virDomainPtr dom;
+int rv;
+
+dom = get_nonnull_domain (conn, args->dom);
+if (dom == NULL) {
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
+
+if (args->cpumap.cpumap_len > REMOTE_CPUMAP_MAX) {
+virDomainFree(dom);
+remoteDispatchFormatError (rerr, "%s", _("cpumap_len > 
REMOTE_CPUMAP_MAX"));
+return -1;
+}
+
+rv = virDomainPinVcpuFlags (dom, args->vcpu,
+(unsigned char *) args->cpumap.cpumap_val,
+args->cpumap.cpumap_len,
+args->flags);
+if (rv == -1) {
+remoteDispatchConnError(rerr, conn);
+virDomainFree(dom);
+return -1;
+}
+virDomainFree(dom);
+return 0;
+}
+
+static int
 remoteDispatchDomainReboot (struct qemud_server *server ATTRIBUTE_UNUSED,
 struct qemud_client *client ATTRIBUTE_UNUSED,
 virConnectPtr conn,
Index: libvirt/daemon/remote_dispatch_args.h
===
--- libvirt.orig/daemon/remote_dispatch_args.h
+++ libvirt/daemon/remote_dispatch_args.h
@@ -178,3 +178,4 @@
 remote_domain_migrate_set_max_speed_args 
val_remote_domain_migrate_set_max_speed_args;
 remote_storage_vol_upload_args val_remote_storage_vol_upload_args;
 remote_storage_vol_download_args val_remote_storage_vol_download_args;
+remote_domain_pin_vcpu_flags_args val_remote_domain_pin_vcpu_flags_args;
Index: libvirt/daemon/remote_dispatch_prototypes.h
===
--- libvirt.orig/daemon/remote_dispatch_prototypes.h
+++ libvirt/daemon/remote_dispatch_prototypes.h
@@ -506,6 +506,14 @@ static int remoteDispatchDomainPinVcpu(
 remote_error *err,
 remote_domain_pin_vcpu_args *args,
 void *ret);
+static int remoteDispatchDomainPinVcpuFlags(
+struct qemud_server *server,
+struct qemud_client *client,
+virConnectPtr conn,
+remote_message_header *hdr,
+remote_error *err,
+remote_domain_pin_vcpu_flags_args *args,
+void *ret);
 static int remoteDispatchDomainReboot(
 struct qemud_server *server,
 struct qemud_client *client,
Index: libvirt/daemon/remote_dispatch_table.h
===
--- libvirt.orig/daemon/remote_dispatch_table.h
+++ libvirt/daemon/remote_dispatch_table.h
@@ -1052,3 +1052,8 @@
 .args_filter = (xdrproc_t) xdr_remote_storage_vol_download_args,
 .ret_filter = (xdrproc_t) xdr_void,
 },
+{   /* DomainPinVcpuFlags => 210 */
+.fn = (dispatch_fn) remoteDispatchDomainPinVcpuFlags,
+.args_filter = (xdrproc_t) xdr_remote_domain_pin_vcpu_flags_args,
+.ret_filter = (xdrproc_t) xdr_void,
+},
Index: libvirt/src/remote/remote_driver.c
===
--- libvirt.orig/src/remote/remote_driver.c
+++ libvirt/src/remote/remote_driver.c
@@ -3046,6 +3046,44 @@ done:
 }
 
 static int
+remoteDomainPinVcpuFlags (virDomainPtr domain,
+  unsigned int vcpu,
+  unsigned char *cpumap,
+  int maplen,
+  unsigned int flags)
+{
+int rv = -1;
+remote_domain_pin_vcpu_flags_args args;
+struct private_data *priv = domain->conn->privateData;
+
+remoteDriverLock(priv);
+
+if (maplen > REMOTE_CPUMAP_MAX) {
+remoteError(VIR_ERR_RPC,
+_("map length gre

[libvirt] [PATCH 2/4] vcpupin: implement the code to address the new API in the qemu driver

[Taku Izumi]

This patch implements the code to address the new API (virDomainPinVcpuFlags)
in the qemu driver.

Signed-off-by: Taku Izumi 
---
 src/qemu/qemu_driver.c |   94 +
 1 file changed, 72 insertions(+), 22 deletions(-)

Index: libvirt/src/qemu/qemu_driver.c
===
--- libvirt.orig/src/qemu/qemu_driver.c
+++ libvirt/src/qemu/qemu_driver.c
@@ -2630,17 +2630,24 @@ qemudDomainSetVcpus(virDomainPtr dom, un
 
 
 static int
-qemudDomainPinVcpu(virDomainPtr dom,
-   unsigned int vcpu,
-   unsigned char *cpumap,
-   int maplen) {
+qemudDomainPinVcpuFlags(virDomainPtr dom,
+unsigned int vcpu,
+unsigned char *cpumap,
+int maplen,
+unsigned int flags) {
+
 struct qemud_driver *driver = dom->conn->privateData;
 virDomainObjPtr vm;
+virDomainDefPtr persistentDef = NULL;
 int maxcpu, hostcpus;
 virNodeInfo nodeinfo;
 int ret = -1;
+bool isActive;
 qemuDomainObjPrivatePtr priv;
 
+virCheckFlags(VIR_DOMAIN_VCPU_LIVE |
+  VIR_DOMAIN_VCPU_CONFIG, -1);
+
 qemuDriverLock(driver);
 vm = virDomainFindByUUID(&driver->domains, dom->uuid);
 qemuDriverUnlock(driver);
@@ -2653,7 +2660,15 @@ qemudDomainPinVcpu(virDomainPtr dom,
 goto cleanup;
 }
 
-if (!virDomainObjIsActive(vm)) {
+isActive = virDomainObjIsActive(vm);
+if (flags == VIR_DOMAIN_VCPU_CURRENT) {
+if (isActive)
+flags = VIR_DOMAIN_VCPU_LIVE;
+else
+flags = VIR_DOMAIN_VCPU_CONFIG;
+}
+
+if (!isActive && (flags & VIR_DOMAIN_VCPU_LIVE)) {
 qemuReportError(VIR_ERR_OPERATION_INVALID,
 "%s",_("cannot pin vcpus on an inactive domain"));
 goto cleanup;
@@ -2668,27 +2683,54 @@ qemudDomainPinVcpu(virDomainPtr dom,
 goto cleanup;
 }
 
-if (nodeGetInfo(dom->conn, &nodeinfo) < 0)
-goto cleanup;
+if (flags & VIR_DOMAIN_VCPU_CONFIG) {
+if (!vm->persistent) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("cannot change persistent config of a transient domain"));
+goto cleanup;
+}
+if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm)))
+goto cleanup;
+}
 
-hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo);
-maxcpu = maplen * 8;
-if (maxcpu > hostcpus)
-maxcpu = hostcpus;
+if (flags & VIR_DOMAIN_VCPU_LIVE) {
 
-if (priv->vcpupids != NULL) {
-if (virProcessInfoSetAffinity(priv->vcpupids[vcpu],
-  cpumap, maplen, maxcpu) < 0)
+if (nodeGetInfo(dom->conn, &nodeinfo) < 0)
 goto cleanup;
-} else {
-qemuReportError(VIR_ERR_NO_SUPPORT,
-"%s", _("cpu affinity is not supported"));
-goto cleanup;
+
+hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo);
+maxcpu = maplen * 8;
+if (maxcpu > hostcpus)
+maxcpu = hostcpus;
+
+if (priv->vcpupids != NULL) {
+if (virProcessInfoSetAffinity(priv->vcpupids[vcpu],
+  cpumap, maplen, maxcpu) < 0)
+goto cleanup;
+} else {
+qemuReportError(VIR_ERR_NO_SUPPORT,
+"%s", _("cpu affinity is not supported"));
+goto cleanup;
+}
+
+if (virDomainVcpupinAdd(vm->def, cpumap, maplen, vcpu) < 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+"%s",
+_("failed to update or add vcpupin xml of a 
running domain"));
+goto cleanup;
+}
+
 }
 
-if (virDomainVcpupinAdd(vm->def, cpumap, maplen, vcpu) < 0) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR,
-"%s", _("failed to update or add vcpupin xml"));
+if (flags & VIR_DOMAIN_VCPU_CONFIG) {
+
+if (virDomainVcpupinAdd(persistentDef, cpumap, maplen, vcpu) < 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+"%s",
+_("failed to update or add vcpupin xml of a 
persistent domain"));
+goto cleanup;
+}
+ret = virDomainSaveConfig(driver->configDir, persistentDef);
 goto cleanup;
 }
 
@@ -2701,6 +2743,14 @@ cleanup:
 }
 
 static int
+qemudDomainPinVcpu(virDomainPtr dom,
+   unsigned int vcpu,
+   unsigned char *cpumap,
+   int maplen) {
+return qemudDomainPinVcpuFlags(dom, vcpu, cpumap, maplen, 
VIR_DOMAIN_VCPU_LIVE);
+}
+
+static int
 qemudDomainGetVcpus(virDomainPtr dom,
 virVcpuInfoPtr info,
 int maxinfo,
@@ -6864,7 +6914,7 @@ static virDriver qemuDriver = {
 qemudDomainSetV

[libvirt] [PATCH 1/4] vcpupin: inroduce a new libvir API (virDomainPinVcpuFlags)

[Taku Izumi]

This patch introduces a new libvirt API (virDomainPinVcpuFlags)

Signed-off-by: Taku Izumi 
---
 include/libvirt/libvirt.h.in |6 +++
 src/driver.h |7 +++
 src/libvirt.c|   83 +++
 src/libvirt_public.syms  |4 ++
 src/lxc/lxc_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/vmware/vmware_driver.c   |1 
 src/xen/xen_driver.c |1 
 src/xenapi/xenapi_driver.c   |1 
 15 files changed, 111 insertions(+)

Index: libvirt/include/libvirt/libvirt.h.in
===
--- libvirt.orig/include/libvirt/libvirt.h.in
+++ libvirt/include/libvirt/libvirt.h.in
@@ -1017,6 +1017,7 @@ typedef virVcpuInfo *virVcpuInfoPtr;
 
 /* Flags for controlling virtual CPU hot-plugging.  */
 typedef enum {
+VIR_DOMAIN_VCPU_CURRENT = 0,/* Affect current domain state */
 /* Must choose at least one of these two bits; SetVcpus can choose both */
 VIR_DOMAIN_VCPU_LIVE= (1 << 0), /* Affect active domain */
 VIR_DOMAIN_VCPU_CONFIG  = (1 << 1), /* Affect next boot */
@@ -1037,6 +1038,11 @@ int virDomainPinVcpu
  unsigned int vcpu,
  unsigned char *cpumap,
  int maplen);
+int virDomainPinVcpuFlags   (virDomainPtr domain,
+ unsigned int vcpu,
+ unsigned char *cpumap,
+ int maplen,
+ unsigned int flags);
 
 /**
  * VIR_USE_CPU:
Index: libvirt/src/driver.h
===
--- libvirt.orig/src/driver.h
+++ libvirt/src/driver.h
@@ -220,6 +220,12 @@ typedef int
  unsigned char *cpumap,
  int maplen);
 typedef int
+(*virDrvDomainPinVcpuFlags) (virDomainPtr domain,
+ unsigned int vcpu,
+ unsigned char *cpumap,
+ int maplen,
+ unsigned int flags);
+typedef int
 (*virDrvDomainGetVcpus)(virDomainPtr domain,
  virVcpuInfoPtr info,
  int maxinfo,
@@ -570,6 +576,7 @@ struct _virDriver {
 virDrvDomainSetVcpusFlags  domainSetVcpusFlags;
 virDrvDomainGetVcpusFlags  domainGetVcpusFlags;
 virDrvDomainPinVcpudomainPinVcpu;
+virDrvDomainPinVcpuFlagsdomainPinVcpuFlags;
 virDrvDomainGetVcpus   domainGetVcpus;
 virDrvDomainGetMaxVcpusdomainGetMaxVcpus;
 virDrvDomainGetSecurityLabel domainGetSecurityLabel;
Index: libvirt/src/libvirt.c
===
--- libvirt.orig/src/libvirt.c
+++ libvirt/src/libvirt.c
@@ -5461,6 +5461,89 @@ error:
 }
 
 /**
+ * virDomainPinVcpuFlags:
+ * @domain: pointer to domain object, or NULL for Domain0
+ * @vcpu: virtual CPU number
+ * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN)
+ *  Each bit set to 1 means that corresponding CPU is usable.
+ *  Bytes are stored in little-endian order: CPU0-7, 8-15...
+ *  In each byte, lowest CPU number is least significant bit.
+ * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in
+ *  underlying virtualization system (Xen...).
+ *  If maplen < size, missing bytes are set to zero.
+ *  If maplen > size, failure code is returned.
+ * @flags: an OR'ed subset of virDomainVcpuFlags
+ *
+ * Dynamically change the real CPUs which can be allocated to a virtual CPU.
+ * This function requires privileged access to the hypervisor.
+ *
+ * @flags may include VIR_DOMAIN_VCPU_LIVE or VIR_DOMAIN_VCPU_CONFIG.
+ * Both flags may be set, but VIR_DOMAIN_VCPU_MAXIMUM cannot be set.
+ * If VIR_DOMAIN_VCPU_LIVE is set, the change affects a running domain
+ * and may fail if domain is not alive.
+ * If VIR_DOMAIN_VCPU_CONFIG is set, the change affects persistent state,
+ * and will fail for transient domains.
+ * If neither flag is specified (tha is, @flags is VIR_DOMAIN_VCPU_CURRENT),
+ * then an inactive domain modifies persistent setup, while an active domain
+ * is hypervisor-dependent on whether just live or both live and persistent
+ * state is changed.
+ * Not all hypervisors can support all flag combinations.
+ *
+ * Returns 

[libvirt] [PATCH v2 0/4] RFC: vcpupin: configure inactive domains' CPU affinity setting

[Taku Izumi]

Hi all,

This patchset enables us to configure inactive domains' CPU affinity setting.

The basic technique is the same as that of "virsh setmem" command. 
 => http://www.redhat.com/archives/libvir-list/2011-March/msg00013.html
 
v1 -> v2:
 - recreate based on 0.9.0
 - add "CURRENT" mode 

 *[PATCH 1/4] vcpupin: inroduce a new libvir API (virDomainPinVcpuFlags)
 *[PATCH 2/4] vcpupin: implement the code to address the new API in the qemu 
driver
 *[PATCH 3/4] vcpupin: implement the remote protocol to address the new API
 *[PATCH 4/4] vcpupin: add the new options to "virsh vcpupin" command

Best regards,
Taku Izumi

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/3] setmaxmem: add the new options to "virsh setmaxmem" command

[Taku Izumi]

This patch adds the new options (--live, --config, and --current) to "virsh 
setmaxmem"
command. The behavior of above options is the same as that of "virsh setmem".
When the --config option is specified, a modofication is effective for the
persistent domain, while the --live option is specified, a modification is 
effective
for an active domain. The --current option is specified, it affects a current 
domain.


Signed-off-by: Taku Izumi 
---
 tools/virsh.c   |   35 ---
 tools/virsh.pod |   16 ++--
 2 files changed, 42 insertions(+), 9 deletions(-)

Index: libvirt/tools/virsh.c
===
--- libvirt.orig/tools/virsh.c
+++ libvirt/tools/virsh.c
@@ -3020,6 +3020,9 @@ static const vshCmdInfo info_setmaxmem[]
 static const vshCmdOptDef opts_setmaxmem[] = {
 {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
 {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("maximum memory limit in 
kilobytes")},
+{"config", VSH_OT_BOOL, 0, N_("affect next boot")},
+{"live", VSH_OT_BOOL, 0, N_("affect running domain")},
+{"current", VSH_OT_BOOL, 0, N_("affect current domain")},
 {NULL, 0, 0, NULL}
 };
 
@@ -3030,6 +3033,25 @@ cmdSetmaxmem(vshControl *ctl, const vshC
 virDomainInfo info;
 int kilobytes = 0;
 int ret = TRUE;
+int config = vshCommandOptBool(cmd, "config");
+int live = vshCommandOptBool(cmd, "live");
+int current = vshCommandOptBool(cmd, "current");
+int flags = VIR_DOMAIN_MEM_MAXIMUM;
+
+if (current) {
+if(live || config) {
+vshError(ctl, "%s", _("--current must be specified exclusively"));
+return FALSE;
+}
+} else {
+if (config)
+flags |= VIR_DOMAIN_MEM_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_MEM_LIVE;
+/* neither option is specified */
+if (!live && !config)
+flags = -1;
+}
 
 if (!vshConnectionUsability(ctl, ctl->conn))
 return FALSE;
@@ -3054,9 +3076,16 @@ cmdSetmaxmem(vshControl *ctl, const vshC
 return FALSE;
 }
 
-if (virDomainSetMaxMemory(dom, kilobytes) != 0) {
-vshError(ctl, "%s", _("Unable to change MaxMemorySize"));
-ret = FALSE;
+if (flags == -1) {
+if (virDomainSetMaxMemory(dom, kilobytes) != 0) {
+vshError(ctl, "%s", _("Unable to change MaxMemorySize"));
+ret = FALSE;
+}
+} else {
+if (virDomainSetMemoryFlags(dom, kilobytes, flags) < 0) {
+vshError(ctl, "%s", _("Unable to change MaxMemorySize"));
+ret = FALSE;
+}
 }
 
 virDomainFree(dom);
Index: libvirt/tools/virsh.pod
===
--- libvirt.orig/tools/virsh.pod
+++ libvirt/tools/virsh.pod
@@ -601,12 +601,18 @@ rounds the parameter up unless the kB ar
 For Xen, you can only adjust the memory of a running domain if the domain is
 paravirtualized or running the PV balloon driver.
 
-=item B I B
+=item B I B optional I<--config> I<--live>
+I<--current>
 
-Change the maximum memory allocation limit for an inactive guest domain.
+Change the maximum memory allocation limit for a guest domain.
+If I<--live> is specified, affect a running guest.
+If I<--config> is specified, affect the next boot of a persistent guest.
+If I<--current> is specified, affect the current guest state.
+Both I<--live> and I<--current> flags may be given, but I<--current> is
+exclusive. If no flag is specified, behavior is different depending
+on hypervisor.
 
-This command works for at least the Xen and vSphere/ESX hypervisors,
-but not for QEMU/KVM.
+This command works for at least the Xen, QEMU/KVM and vSphere/ESX hypervisors.
 
 Some hypervisors require a larger granularity than kilobytes, rounding up
 requests that are not an even multiple of the desired amount.  vSphere/ESX
@@ -614,8 +620,6 @@ is one of these, requiring the parameter
 vSphere/ESX, 263168 (257MB) would be rounded up because it's not a multiple
 of 4MB, while 266240 (260MB) is valid without rounding.
 
-Note, to change the maximum memory allocation for a QEMU/KVM guest domain,
-use the virsh B command instead to update its XML  element.
 
 =item B I optional I<--hard-limit> B
 optional I<--soft-limit> B optional I<--swap-hard-limit>


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/3] maxmem: implement virDomainSetMaxMemory API of the qemu driver

[Taku Izumi]

This patch implements the code to support virDomainSetMaxMemory API,
and to support VIR_DOMAIN_MEM_MAXIMUM flag in qemudDomainSetMemoryFlags 
function.
As a result, we can change the maximum memory size of inactive QEMU guests.

Signed-off-by: Taku Izumi 
---
 src/qemu/qemu_driver.c |   81 ++---
 1 file changed, 57 insertions(+), 24 deletions(-)

Index: libvirt/src/qemu/qemu_driver.c
===
--- libvirt.orig/src/qemu/qemu_driver.c
+++ libvirt/src/qemu/qemu_driver.c
@@ -1580,7 +1580,8 @@ static int qemudDomainSetMemoryFlags(vir
 bool isActive;
 
 virCheckFlags(VIR_DOMAIN_MEM_LIVE |
-  VIR_DOMAIN_MEM_CONFIG, -1);
+  VIR_DOMAIN_MEM_CONFIG |
+  VIR_DOMAIN_MEM_MAXIMUM, -1);
 
 qemuDriverLock(driver);
 vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -1593,12 +1594,6 @@ static int qemudDomainSetMemoryFlags(vir
 goto cleanup;
 }
 
-if (newmem > vm->def->mem.max_balloon) {
-qemuReportError(VIR_ERR_INVALID_ARG,
-"%s", _("cannot set memory higher than max memory"));
-goto cleanup;
-}
-
 if (qemuDomainObjBeginJob(vm) < 0)
 goto cleanup;
 
@@ -1610,6 +1605,12 @@ static int qemudDomainSetMemoryFlags(vir
 else
 flags = VIR_DOMAIN_MEM_CONFIG;
 }
+if (flags == VIR_DOMAIN_MEM_MAXIMUM) {
+if (isActive)
+flags = VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_MAXIMUM;
+else
+flags = VIR_DOMAIN_MEM_CONFIG | VIR_DOMAIN_MEM_MAXIMUM;
+}
 
 if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) {
 qemuReportError(VIR_ERR_OPERATION_INVALID,
@@ -1627,27 +1628,54 @@ static int qemudDomainSetMemoryFlags(vir
 goto endjob;
 }
 
-if (flags & VIR_DOMAIN_MEM_LIVE) {
-priv = vm->privateData;
-qemuDomainObjEnterMonitor(vm);
-r = qemuMonitorSetBalloon(priv->mon, newmem);
-qemuDomainObjExitMonitor(vm);
-qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r == 
1);
-if (r < 0)
-goto endjob;
+if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
+/* resize the maximum memory */
 
-/* Lack of balloon support is a fatal error */
-if (r == 0) {
+if (flags & VIR_DOMAIN_MEM_LIVE) {
 qemuReportError(VIR_ERR_OPERATION_INVALID,
-"%s", _("cannot set memory of an active domain"));
+_("cannot resize the maximum memory on an active 
domain"));
 goto endjob;
 }
-}
 
-if (flags& VIR_DOMAIN_MEM_CONFIG) {
-persistentDef->mem.cur_balloon = newmem;
-ret = virDomainSaveConfig(driver->configDir, persistentDef);
-goto endjob;
+if (flags & VIR_DOMAIN_MEM_CONFIG) {
+persistentDef->mem.max_balloon = newmem;
+if (persistentDef->mem.cur_balloon > newmem)
+persistentDef->mem.cur_balloon = newmem;
+ret = virDomainSaveConfig(driver->configDir, persistentDef);
+goto endjob;
+}
+
+} else {
+/* resize the current memory */
+
+if (newmem > vm->def->mem.max_balloon) {
+qemuReportError(VIR_ERR_INVALID_ARG,
+"%s", _("cannot set memory higher than max 
memory"));
+goto endjob;
+}
+
+if (flags & VIR_DOMAIN_MEM_LIVE) {
+priv = vm->privateData;
+qemuDomainObjEnterMonitor(vm);
+r = qemuMonitorSetBalloon(priv->mon, newmem);
+qemuDomainObjExitMonitor(vm);
+qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r 
== 1);
+if (r < 0)
+goto endjob;
+
+/* Lack of balloon support is a fatal error */
+if (r == 0) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+"%s", _("cannot set memory of an active 
domain"));
+goto endjob;
+}
+}
+
+if (flags & VIR_DOMAIN_MEM_CONFIG) {
+persistentDef->mem.cur_balloon = newmem;
+ret = virDomainSaveConfig(driver->configDir, persistentDef);
+goto endjob;
+}
 }
 
 ret = 0;
@@ -1665,6 +1693,11 @@ static int qemudDomainSetMemory(virDomai
 return qemudDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_MEM_LIVE);
 }
 
+static int qemudDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) {
+return qemudDomainSetMemoryFlags(dom, memory,
+ VIR_DOMAIN_MEM_MAXIMUM | 
VIR_DOMAIN_MEM_LIVE);
+}
+
 static int qemudDomainGetInfo(virDomainPtr dom,
   virDomainInfoPtr info) {
 struct qemud_driver *driver = dom->conn->privateData;
@@ -6849,7 +6882,7 @@ static virDriver qemuDriver = {
 qemudDomainDestroy, /* domainDestroy */
 qemudDomai

[libvirt] [PATCH 1/3] maxmem: introduces VIR_DOMAIN_MEM_MAXIMUM flag

[Taku Izumi]

This patch introduces VIR_DOMAIN_MEM_MAXIMUM flag.

Signed-off-by: Taku Izumi 
---
 include/libvirt/libvirt.h.in |1 +
 src/libvirt.c|2 ++
 2 files changed, 3 insertions(+)

Index: libvirt/include/libvirt/libvirt.h.in
===
--- libvirt.orig/include/libvirt/libvirt.h.in
+++ libvirt/include/libvirt/libvirt.h.in
@@ -857,6 +857,7 @@ typedef enum {
 VIR_DOMAIN_MEM_CURRENT = 0,/* affect current domain state */
 VIR_DOMAIN_MEM_LIVE= (1 << 0), /* affect active domain */
 VIR_DOMAIN_MEM_CONFIG  = (1 << 1), /* affect next boot */
+VIR_DOMAIN_MEM_MAXIMUM = (1 << 2), /* affect Max rather than current */
 } virDomainMemoryModFlags;
 
 
Index: libvirt/src/libvirt.c
===
--- libvirt.orig/src/libvirt.c
+++ libvirt/src/libvirt.c
@@ -2832,6 +2832,8 @@ error:
  * (that is, @flags is VIR_DOMAIN_MEM_CURRENT), then an inactive domain
  * modifies persistent setup, while an active domain is hypervisor-dependent
  * on whether just live or both live and persistent state is changed.
+ * If VIR_DOMAIN_MEM_MAXIMUM is set, the change affects domain's maximum memory
+ * size rather than current memory size.
  * Not all hypervisors can support all flag combinations.
  *
  * Returns 0 in case of success, -1 in case of failure.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 0/3] configure inactive domains' maximum memory size

[Taku Izumi]

Hi all,

This patchset enables us to configure inactive domain's maximum memory
size.  

v1 -> v2:
 - recreate based on 0.9.0
 - fix some typo

 *[PATCH 1/3] maxmem: introduces VIR_DOMAIN_MEM_MAXIMUM flag
 *[PATCH 2/3] maxmem: implement virDomainSetMaxMemory API of the qemu driver
 *[PATCH 3/3] setmaxmem: add the new options to "virsh setmaxmem" command
 
Best regards,
Taku Izumi

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/6] qemu: use virObject to manage reference-counting for qemu monitor

[Hu Tao]

On Thu, Apr 07, 2011 at 10:38:34AM +0100, Daniel P. Berrange wrote:
> On Wed, Apr 06, 2011 at 03:19:55PM +0800, Hu Tao wrote:
> > ---
> >  src/qemu/qemu_domain.c|   30 ++---
> >  src/qemu/qemu_migration.c |2 -
> >  src/qemu/qemu_monitor.c   |  109 
> > 
> >  src/qemu/qemu_monitor.h   |4 +-
> >  src/qemu/qemu_process.c   |   32 +++---
> >  5 files changed, 81 insertions(+), 96 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 3a3c953..d11dc5f 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -560,9 +560,8 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj)
> >  {
> >  qemuDomainObjPrivatePtr priv = obj->privateData;
> >  
> > -qemuMonitorLock(priv->mon);
> > -qemuMonitorRef(priv->mon);
> >  virDomainObjUnlock(obj);
> > +qemuMonitorLock(priv->mon);
> >  }
> >  
> >  
> > @@ -573,18 +572,9 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj)
> >  void qemuDomainObjExitMonitor(virDomainObjPtr obj)
> >  {
> >  qemuDomainObjPrivatePtr priv = obj->privateData;
> > -int refs;
> > -
> > -refs = qemuMonitorUnref(priv->mon);
> > -
> > -if (refs > 0)
> > -qemuMonitorUnlock(priv->mon);
> >  
> > +qemuMonitorUnlock(priv->mon);
> >  virDomainObjLock(obj);
> > -
> > -if (refs == 0) {
> > -priv->mon = NULL;
> > -}
> >  }
> >  
> >  
> > @@ -601,10 +591,8 @@ void qemuDomainObjEnterMonitorWithDriver(struct 
> > qemud_driver *driver,
> >  {
> >  qemuDomainObjPrivatePtr priv = obj->privateData;
> >  
> > -qemuMonitorLock(priv->mon);
> > -qemuMonitorRef(priv->mon);
> >  virDomainObjUnlock(obj);
> > -qemuDriverUnlock(driver);
> > +qemuMonitorLock(priv->mon);
> >  }
> >  
> >  
> > @@ -617,19 +605,9 @@ void qemuDomainObjExitMonitorWithDriver(struct 
> > qemud_driver *driver,
> >  virDomainObjPtr obj)
> >  {
> >  qemuDomainObjPrivatePtr priv = obj->privateData;
> > -int refs;
> > -
> > -refs = qemuMonitorUnref(priv->mon);
> > -
> > -if (refs > 0)
> > -qemuMonitorUnlock(priv->mon);
> >  
> > -qemuDriverLock(driver);
> > +qemuMonitorUnlock(priv->mon);
> >  virDomainObjLock(obj);
> > -
> > -if (refs == 0) {
> > -priv->mon = NULL;
> > -}
> >  }
> 
> This means that the 'driver' lock is now whenever any QEMU
> monitor command is runing, which blocks the entire driver.

qemuDomainObjEnterMonitorWithDriver is now called without holding qemu
driver lock.

> 
> >  
> >  void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver,
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 462e6be..6af2e24 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -224,11 +224,9 @@ qemuMigrationWaitForCompletion(struct qemud_driver 
> > *driver, virDomainObjPtr vm)
> >  }
> >  
> >  virDomainObjUnlock(vm);
> > -qemuDriverUnlock(driver);
> >  
> >  nanosleep(&ts, NULL);
> >  
> > -qemuDriverLock(driver);
> >  virDomainObjLock(vm);
> >  }
> 
> Holding the 'driver' lock while sleeping blocks the entire
> QEMU driver.

Now qemuMigrationWaitForCompletion should be called without holding qemu
driver lock.

> 
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 244b22a..4b9087f 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -107,7 +107,6 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon 
> > ATTRIBUTE_UNUSED,
> >  
> >  VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name);
> >  
> > -qemuDriverLock(driver);
> >  virDomainObjLock(vm);
> >  
> >  if (!virDomainObjIsActive(vm)) {
> > @@ -133,15 +132,17 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon 
> > ATTRIBUTE_UNUSED,
> >  qemuProcessStop(driver, vm, 0);
> >  qemuAuditDomainStop(vm, hasError ? "failed" : "shutdown");
> >  
> > -if (!vm->persistent)
> > +if (!vm->persistent) {
> > +qemuDriverLock(driver);
> >  virDomainRemoveInactive(&driver->domains, vm);
> > -else
> > -virDomainObjUnlock(vm);
> > +qemuDriverUnlock(driver);
> > +}
> > +
> > +virDomainObjUnlock(vm);
> >  
> >  if (event) {
> >  qemuDomainEventQueue(driver, event);
> >  }
> > -qemuDriverUnlock(driver);
> >  }
> 
> This violates the lock ordering rules. The 'driver' lock *must* be
> obtained *before* any 'vm' lock is held.

Excepting for introducing virObject for reference-counting, this series
also simplifies the usage of lock: if you want to read/write qemu
driver data, it is enough to first acquire qemu driver lock only; if you
want to read/write virDomainObj data, it is enough to first acquire
virDomainObj lock only; same for others. And we'd better to avoid
acquiring two locks at the same time.

So yes, the code here is problematic, it 

[libvirt] [PATCH] do not build libvirt_iohelper when building without libvirtd

[Wen Congyang]

The libexec program libvirt_iohelper is only for libvirtd. If we build rpm
without libvirtd, we will receive the following messages:

Checking for unpackaged file(s): /usr/lib/rpm/check-files 
/home/wency/rpmbuild/BUILDROOT/libvirt-0.9.0-1.el6.x86_64
error: Installed (but unpackaged) file(s) found:
   /usr/libexec/libvirt_iohelper

---
 src/Makefile.am |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 3649106..dce866e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1183,6 +1183,7 @@ EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
 
 libexec_PROGRAMS =
 
+if WITH_LIBVIRTD
 libexec_PROGRAMS += libvirt_iohelper
 libvirt_iohelper_SOURCES = $(UTIL_IO_HELPER_SOURCES)
 libvirt_iohelper_LDFLAGS = $(WARN_LDFLAGS) $(AM_LDFLAGS)
@@ -1191,6 +1192,7 @@ libvirt_iohelper_LDADD =  \
../gnulib/lib/libgnu.la
 
 libvirt_iohelper_CFLAGS = $(AM_CFLAGS)
+endif
 
 if WITH_STORAGE_DISK
 if WITH_LIBVIRTD
-- 
1.7.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/6] use virObject to manage reference-count of virDomainObj

[Hu Tao]

On Thu, Apr 07, 2011 at 10:33:03AM +0100, Daniel P. Berrange wrote:
> On Wed, Apr 06, 2011 at 03:19:51PM +0800, Hu Tao wrote:
> > This patch also eliminates a dead-lock bug in
> > qemuDomainObjBeginJobWithDriver: if virCondWaitUntil() timeouts, the
> > thread tries to acquire qemu driver lock while holding virDomainObj
> > lock.
> > ---
> >  src/conf/domain_conf.c|   56 
> >  src/conf/domain_conf.h|6 +-
> >  src/openvz/openvz_conf.c  |8 +-
> >  src/qemu/qemu_domain.c|   32 ++---
> >  src/qemu/qemu_domain.h|2 +-
> >  src/qemu/qemu_driver.c|  304 
> > -
> >  src/qemu/qemu_migration.c |   45 +++
> >  src/qemu/qemu_process.c   |   33 ++---
> >  src/vmware/vmware_conf.c  |2 +-
> >  9 files changed, 215 insertions(+), 273 deletions(-)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 90a1317..fc76a00 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -47,6 +47,7 @@
> >  #include "storage_file.h"
> >  #include "files.h"
> >  #include "bitmap.h"
> > +#include "virobject.h"
> >  
> >  #define VIR_FROM_THIS VIR_FROM_DOMAIN
> >  
> > @@ -395,9 +396,7 @@ static void
> >  virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
> >  {
> >  virDomainObjPtr obj = payload;
> > -virDomainObjLock(obj);
> > -if (virDomainObjUnref(obj) > 0)
> > -virDomainObjUnlock(obj);
> > +virDomainObjUnref(obj);
> >  }
> >  
> >  int virDomainObjListInit(virDomainObjListPtr doms)
> > @@ -437,7 +436,7 @@ virDomainObjPtr virDomainFindByID(const 
> > virDomainObjListPtr doms,
> >  virDomainObjPtr obj;
> >  obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id);
> >  if (obj)
> > -virDomainObjLock(obj);
> > +virDomainObjRef(obj);
> >  return obj;
> >  }
> >  
> > @@ -452,7 +451,7 @@ virDomainObjPtr virDomainFindByUUID(const 
> > virDomainObjListPtr doms,
> >  
> >  obj = virHashLookup(doms->objs, uuidstr);
> >  if (obj)
> > -virDomainObjLock(obj);
> > +virDomainObjRef(obj);
> >  return obj;
> >  }
> >  
> > @@ -476,7 +475,7 @@ virDomainObjPtr virDomainFindByName(const 
> > virDomainObjListPtr doms,
> >  virDomainObjPtr obj;
> >  obj = virHashSearch(doms->objs, virDomainObjListSearchName, name);
> >  if (obj)
> > -virDomainObjLock(obj);
> > +virDomainObjRef(obj);
> >  return obj;
> >  }
> 
> This is a major change in semantics, which makes pretty much
> every single caller non-thread safe, unless the callers are
> all changed todo virDomainObjLock immediately after calling
> this. So I don't really see the point in this - it just means
> more code duplication.

If we do this:

obj = virHashSearch(doms->objs, virDomainObjListSearchName, name);
if (obj)
  virDomainObjLock(obj);
return obj;

And at the meantime another thread removes the same obj from doms->objs
and frees it, than we are accessing a freed obj.

lock doesn't help prevent object from being freed.

> 
> >  
> > @@ -967,6 +966,12 @@ static void virDomainObjFree(virDomainObjPtr dom)
> >  {
> >  if (!dom)
> >  return;
> > +virDomainObjUnref(dom);
> > +}
> > +
> > +static void doDomainObjFree(virObjectPtr obj)
> > +{
> > +virDomainObjPtr dom = (virDomainObjPtr)obj;
> >  
> >  VIR_DEBUG("obj=%p", dom);
> >  virDomainDefFree(dom->def);
> > @@ -984,21 +989,13 @@ static void virDomainObjFree(virDomainObjPtr dom)
> >  
> >  void virDomainObjRef(virDomainObjPtr dom)
> >  {
> > -dom->refs++;
> > -VIR_DEBUG("obj=%p refs=%d", dom, dom->refs);
> > +virObjectRef(&dom->obj);
> >  }
> >  
> >  
> > -int virDomainObjUnref(virDomainObjPtr dom)
> > +void virDomainObjUnref(virDomainObjPtr dom)
> >  {
> > -dom->refs--;
> > -VIR_DEBUG("obj=%p refs=%d", dom, dom->refs);
> > -if (dom->refs == 0) {
> > -virDomainObjUnlock(dom);
> > -virDomainObjFree(dom);
> > -return 0;
> > -}
> > -return dom->refs;
> > +virObjectUnref(&dom->obj);
> >  }
> >  
> >  static virDomainObjPtr virDomainObjNew(virCapsPtr caps)
> > @@ -1010,6 +1007,11 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr 
> > caps)
> >  return NULL;
> >  }
> >  
> > +if (virObjectInit(&domain->obj, doDomainObjFree)) {
> > +VIR_FREE(domain);
> > +return NULL;
> > +}
> > +
> >  if (caps->privateDataAllocFunc &&
> >  !(domain->privateData = (caps->privateDataAllocFunc)())) {
> >  virReportOOMError();
> > @@ -1027,9 +1029,7 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr 
> > caps)
> >  return NULL;
> >  }
> >  
> > -virDomainObjLock(domain);
> >  domain->state = VIR_DOMAIN_SHUTOFF;
> > -domain->refs = 1;
> >  
> >  virDomainSnapshotObjListInit(&domain->snapshots);
> >  
> > @@ -1075,8 +1075,10 @@ virDomainObjPtr virDomainAssignDef(virCapsPtr caps,
> >  domain->def = def;
> >  
> >  vir

Re: [libvirt] [PATCH v5] qemu: Remove the managed state file only if restoring succeeded

[Osier Yang]

于 2011年04月07日 22:56, Eric Blake 写道:

On 04/06/2011 08:31 PM, Osier Yang wrote:

  managed_save = qemuDomainManagedSavePath(driver, vm);
+
+if (!managed_save)
+goto cleanup;
+
  if ((managed_save)&&  (virFileExists(managed_save))) {


This second check for non-NULL managed_save is now redundant.


Argh, it my fault.




+++ b/tools/virsh.pod
@@ -546,7 +546,11 @@ I  parameter in the domain's XML definition.

  =item B  I

-Restores a domain from an B  state file.  See I  for more 
info.
+Restores a domain from an B  state file. See I  for more 
info.


Sorry for not noticing sooner, but we could do some grammar cleanups.

Does this look like a reasonable followup?

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index a84780b..0734a76 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -3430,7 +3430,7 @@ static int qemudDomainObjStart(virConnectPtr conn,
  if (!managed_save)
  goto cleanup;

-if ((managed_save)&&  (virFileExists(managed_save))) {
+if (virFileExists(managed_save)) {
  ret = qemuDomainObjRestore(conn, driver, vm, managed_save);

  if ((ret == 0)&&  (unlink(managed_save)<  0))
diff --git i/tools/virsh.pod w/tools/virsh.pod
index 6319373..9ce6905 100644
--- i/tools/virsh.pod
+++ w/tools/virsh.pod
@@ -546,11 +546,12 @@ I  parameter in the domain's XML definition.

  =item B  I

-Restores a domain from an B  state file. See I  for
more info.
+Restores a domain from a B  state file. See I  for more
info.

  B: To avoid corrupting file system contents within the domain, you
-should not reuse the saved state file to B  unless you are
convinced
-with reverting the domain to the previous state.
+should not reuse the saved state file for a second B  unless
+you have also reverted all storage volumes back to the same contents
+as when the state file was created.


Doesn't reuse implies the "second, third, " B ? :)

But anyway, it looks more clear, good to me.



  =item B  I  I





--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [TCK][PATCH] nwfilter: test support for TCP flags evaluation

[Stefan Berger]

On 04/01/2011 12:24 PM, Daniel P. Berrange wrote:

On Fri, Apr 01, 2011 at 12:17:32PM -0400, Stefan Berger wrote:

This patch extends an existing test with test cases for the TCP flags.

Signed-off-by: Stefan Berger


ACK

Pushed
  Stefan

Daniel


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] nwfilters: support for TCP flags evaluation

[Stefan Berger]

On 04/07/2011 05:47 PM, Eric Blake wrote:

On 04/07/2011 01:47 PM, Stefan Berger wrote:

This patch adds support for the evaluation of TCP flags in nwfilters.

It adds documentation to the web page and extends the tests as well.
Also, the nwfilter schema is extended.

The following are some example for rules using the tcp flags:









Signed-off-by: Stefan Berger

---
  docs/formatnwfilter.html.in   |   10 ++
  docs/schemas/nwfilter.rng |   16 
  src/conf/nwfilter_conf.c  |  115
+++---
  src/conf/nwfilter_conf.h  |9 ++
  src/libvirt_private.syms  |1
  src/nwfilter/nwfilter_ebiptables_driver.c |9 ++
  tests/nwfilterxml2xmlin/tcp-test.xml  |   12 +++
  tests/nwfilterxml2xmlout/tcp-test.xml |   12 +++
  8 files changed, 174 insertions(+), 10 deletions(-)

ACK, looks reasonable to me, with one optimization nit fixed:


+int32_t mask = 0x1;

-for (i = 0; int_map[i].val; i++) {
-if (last_attr != int_map[i].attr&&
-flags&  int_map[i].attr) {
-if (c>= 1)
-virBufferVSprintf(buf, "%s", sep);
-virBufferVSprintf(buf, "%s", int_map[i].val);
-c++;
+while (mask) {
+if ((mask&  flags)) {
+for (i = 0; int_map[i].val; i++) {
+if (mask == int_map[i].attr) {
+if (c>= 1)
+virBufferVSprintf(buf, "%s", sep);
+virBufferVSprintf(buf, "%s", int_map[i].val);
+c++;
+}
+}
+flags ^= mask;
+if (!flags)
+break;

This if condition should be after...


  }

this brace; otherwise, if flags == 0 on entry, then you will
(needlessly) iterate through all 32 bits of mask, rather than breaking
on the first iteration.


Excellent! I'll modify and push.

   Stefan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] setmem: add --current option to virsh setmem command

[Eric Blake]

On 03/22/2011 11:49 PM, Taku Izumi wrote:
> 
> This patch adds the new option (--current) to the "virsh setmem" command.
> When --current option is specified, it affects a "current" domain.
> The word "current" denotes that if a domain is running, it affects
> a running domain only; otherwise it affects a persistent domain.
> 
> Signed-off-by: Taku Izumi 
> ---
>  tools/virsh.c   |   20 +++-
>  tools/virsh.pod |7 +--

All right - you remembered to update the docs at the same time!

> +if (current) {
> +if (live || config) {
> +vshError(ctl, "%s", _("--current must be specified 
> exclusively"));
> +return FALSE;
> +}
> +flags = VIR_DOMAIN_MEM_CURRENT;
> +} else {
> +if (config)
> +flags |= VIR_DOMAIN_MEM_CONFIG;
>  if (live)
>  flags |= VIR_DOMAIN_MEM_LIVE;
> +/* neither option is specified */
> +if (!live && !config)
> +flags = -1;
>  }
> 
>  if (!vshConnectionUsability(ctl, ctl->conn))
> @@ -2972,7 +2982,7 @@ cmdSetmem(vshControl *ctl, const vshCmd
>  return FALSE;
>  }
> 
> -if (!flags) {
> +if (flags == -1) {
>  if (virDomainSetMemory(dom, kilobytes) != 0) {

Looks reasonable.

> +++ libvirt/tools/virsh.pod
> @@ -578,12 +578,15 @@ Therefore, -1 is a useful shorthand for
>  B: The weight and cap parameters are defined only for the
>  XEN_CREDIT scheduler and are now I.
> 
> -=item B I B optional I<--config> I<--live>
> +=item B I B optional I<--config> I<--live> 
> I<--current>

I wrapped this at 80 columns.

> 
>  Change the memory allocation for a guest domain.
>  If I<--live> is specified, perform a memory balloon of a running guest.
>  If I<--config> is specified, affect the next boot of a persistent guest.
> -Both flags may be given.  If neither flag is given, I<--live> is assumed.
> +If I<--current> is specified, affect a current guest.

I tweaked this to 'current guest state (active or persistent)'.

> +Both I<--live> and I<--config> flags may be given, but I<--current> is
> +exclusive. If neither flag is specified, behavior is different depending
> +on hypervisor.

ACK and applied.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] setmem: introduce VIR_DOMAIN_MEM_CURRENT flag

[Eric Blake]

On 03/22/2011 11:47 PM, Taku Izumi wrote:
> 
> This patch introduces VIR_DOMAIN_MEM_CURRENT flag and
> modifies virDomainSetMemoryFlags function to support it.
> 
> 
>  /* Memory size modification flags. */
>  typedef enum {
> +VIR_DOMAIN_MEM_CURRENT= 0,/* affect current domain */

I tweaked the spacing here.

>  VIR_DOMAIN_MEM_LIVE   = (1 << 0), /* affect active domain */
>  VIR_DOMAIN_MEM_CONFIG = (1 << 1), /* affect next boot */
>  } virDomainMemoryModFlags;
> Index: libvirt/src/libvirt.c
> ===
> --- libvirt.orig/src/libvirt.c
> +++ libvirt/src/libvirt.c
> @@ -2862,10 +2862,12 @@ error:
>   * to Domain0 i.e. the domain where the application runs.
>   * This funcation may requires privileged access to the hypervisor.

Oops, let's fix that typo in "funcation" while we're here.

>   *
> - * @flags must include VIR_DOMAIN_MEM_LIVE to affect a running
> - * domain (which may fail if domain is not active), or
> - * VIR_DOMAIN_MEM_CONFIG to affect the next boot via the XML
> - * description of the domain. Both flags may be set.
> + * @flags may include VIR_DOMAIN_MEM_LIVE or VIR_DOMAIN_MEM_CONFIG.
> + * Both flags may be set. If VIR_DOMAIN_MEM_LIVE is set, the change affects
> + * a running domain and may fail if domain is not active.
> + * If VIR_DOMAIN_MEM_CONFIG is set, the change affects the next boot via
> + * the XML description on the domain. If neither flag is specified
> + * (=VIR_DOMAIN_MEM_CURRENT), behavior is different depending on hypervisor.

I tweaked this wording a bit.

>   *
>   * Returns 0 in case of success, -1 in case of failure.
>   */
> @@ -2891,8 +2893,7 @@ virDomainSetMemoryFlags(virDomainPtr dom
>  goto error;
>  }
> 
> -if (memory < 4096 ||
> -(flags & (VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_CONFIG)) == 0) {
> +if (memory < 4096) {
>  virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
>  goto error;
>  }

Here's what I squashed in before pushing:

diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
index c9129bc..d765412 100644
--- i/include/libvirt/libvirt.h.in
+++ w/include/libvirt/libvirt.h.in
@@ -854,9 +854,9 @@ int virDomainGetMemoryParameters(virDomainPtr
domain,

 /* Memory size modification flags. */
 typedef enum {
-VIR_DOMAIN_MEM_CURRENT= 0,/* affect current domain */
-VIR_DOMAIN_MEM_LIVE   = (1 << 0), /* affect active domain */
-VIR_DOMAIN_MEM_CONFIG = (1 << 1), /* affect next boot */
+VIR_DOMAIN_MEM_CURRENT = 0,/* affect current domain state */
+VIR_DOMAIN_MEM_LIVE= (1 << 0), /* affect active domain */
+VIR_DOMAIN_MEM_CONFIG  = (1 << 1), /* affect next boot */
 } virDomainMemoryModFlags;


diff --git i/src/libvirt.c w/src/libvirt.c
index ee11643..dde4bd4 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -2822,14 +2822,17 @@ error:
  * Dynamically change the target amount of physical memory allocated to a
  * domain. If domain is NULL, then this change the amount of memory
reserved
  * to Domain0 i.e. the domain where the application runs.
- * This funcation may requires privileged access to the hypervisor.
+ * This function may requires privileged access to the hypervisor.
  *
  * @flags may include VIR_DOMAIN_MEM_LIVE or VIR_DOMAIN_MEM_CONFIG.
  * Both flags may be set. If VIR_DOMAIN_MEM_LIVE is set, the change affects
- * a running domain and may fail if domain is not active.
- * If VIR_DOMAIN_MEM_CONFIG is set, the change affects the next boot via
- * the XML description on the domain. If neither flag is specified
- * (=VIR_DOMAIN_MEM_CURRENT), behavior is different depending on
hypervisor.
+ * a running domain and will fail if domain is not active.
+ * If VIR_DOMAIN_MEM_CONFIG is set, the change affects persistent state,
+ * and will fail for transient domains. If neither flag is specified
+ * (that is, @flags is VIR_DOMAIN_MEM_CURRENT), then an inactive domain
+ * modifies persistent setup, while an active domain is
hypervisor-dependent
+ * on whether just live or both live and persistent state is changed.
+ * Not all hypervisors can support all flag combinations.
  *
  * Returns 0 in case of success, -1 in case of failure.
  */


-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] setmem: add virDomainSetMemroyFlags(, , VIR_DOMAIN_MEM_CURRENT) support to qemu driver

[Eric Blake]

Long subject line.  'git shortlog -20' should give you a feel for how to
shorten this; I went with:

setmem: add VIR_DOMAIN_MEM_CURRENT support to qemu

On 03/22/2011 11:48 PM, Taku Izumi wrote:
> 
> This patch adds virDomainSetMemroyFlags(,,VIR_DOMAIN_MEM_CURRENT) support

s/Memroy/Memory/

> code to qemu driver.
> 
> Signed-off-by: Taku Izumi 
> ---
>  src/qemu/qemu_driver.c |   18 +++---
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> Index: libvirt/src/qemu/qemu_driver.c
> ===
> --- libvirt.orig/src/qemu/qemu_driver.c
> +++ libvirt/src/qemu/qemu_driver.c
> @@ -1575,16 +1575,11 @@ static int qemudDomainSetMemoryFlags(vir
>  qemuDomainObjPrivatePtr priv;
>  virDomainObjPtr vm;
>  virDomainDefPtr persistentDef = NULL;
> -int ret = -1, r;
> +int ret = -1, r, isActive;

Hmm, based on the name, I would have guessed isActive is better as a bool.

> 
>  virCheckFlags(VIR_DOMAIN_MEM_LIVE |
>VIR_DOMAIN_MEM_CONFIG, -1);
> 
> -if ((flags & (VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_CONFIG)) == 0) {
> -qemuReportError(VIR_ERR_INVALID_ARG,
> -_("invalid flag combination: (0x%x)"), flags);
> -}
> -
>  qemuDriverLock(driver);
>  vm = virDomainFindByUUID(&driver->domains, dom->uuid);
>  qemuDriverUnlock(driver);
> @@ -1605,7 +1600,16 @@ static int qemudDomainSetMemoryFlags(vir
>  if (qemuDomainObjBeginJob(vm) < 0)
>  goto cleanup;
> 
> -if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_MEM_LIVE)) {
> +isActive = virDomainObjIsActive(vm);

Surprisingly, virDomainObjIsActive returns int instead of bool.  Well,
no longer :)

> +
> +if (flags == VIR_DOMAIN_MEM_CURRENT) {
> +if (isActive)
> +flags = VIR_DOMAIN_MEM_LIVE;
> +else
> +flags = VIR_DOMAIN_MEM_CONFIG;
> +}
> +
> +if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) {
>  qemuReportError(VIR_ERR_OPERATION_INVALID,
>  "%s", _("domain is not running"));
>  goto endjob;

I squashed this in, then pushed:

diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h
index 10e73cb..95bd11e 100644
--- i/src/conf/domain_conf.h
+++ w/src/conf/domain_conf.h
@@ -1172,7 +1172,7 @@ struct _virDomainObjList {
 virHashTable *objs;
 };

-static inline int
+static inline bool
 virDomainObjIsActive(virDomainObjPtr dom)
 {
 return dom->def->id != -1;
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index c0e706c..6a0bf24 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -1576,7 +1576,8 @@ static int qemudDomainSetMemoryFlags(virDomainPtr
dom, unsigned long newmem,
 qemuDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
 virDomainDefPtr persistentDef = NULL;
-int ret = -1, r, isActive;
+int ret = -1, r;
+bool isActive;

 virCheckFlags(VIR_DOMAIN_MEM_LIVE |
   VIR_DOMAIN_MEM_CONFIG, -1);


-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: Install libxenlight log dir

[Jim Fehlig]

Eric Blake wrote:
> On 04/07/2011 04:13 PM, Jim Fehlig wrote:
>   
>> Add $localstatedir/log/libvirt/libxl when building libxenlight driver
>> ---
>>  src/Makefile.am |2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 9b54679..3649106 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -1275,6 +1275,7 @@ endif
>>  if WITH_LIBXL
>>  $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/libxl"
>>  $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/libxl"
>> +$(MKDIR_P) "$(DESTDIR)$(localstatedir)/log/libvirt/libxl"
>> 
>
> ACK.  But does this also need any specfile changes as a followup?
>   

It's already included in the spec file

%if %{with_libxl}
%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/libxl/
%endif

I'll push this one shortly.

Regards,
Jim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/4] maxmem: introduces VIR_DOMAIN_MEM_MAXIMUM flag

[Eric Blake]

On 03/30/2011 11:30 PM, Taku Izumi wrote:
> 
> This patch introduces VIR_DOMAIN_MEM_MAXIMUM flag.
> 
> Signed-off-by: Taku Izumi 
> ---
>  include/libvirt/libvirt.h.in |1 +
>  src/libvirt.c|2 ++
>  2 files changed, 3 insertions(+)
> 
> Index: libvirt/include/libvirt/libvirt.h.in
> ===
> --- libvirt.orig/include/libvirt/libvirt.h.in
> +++ libvirt/include/libvirt/libvirt.h.in
> @@ -857,6 +857,7 @@ typedef enum {
>  VIR_DOMAIN_MEM_CURRENT= 0,/* affect current domain */

This part of the patch doesn't exist.  Am I missing some other patch
series that should have been applied first?

[goes back and re-reads - yep, you said so in mail 0/4].  OK, I'll
review that first.  But I've now applied 1/4 from this series from when
it was first acked.

>  VIR_DOMAIN_MEM_LIVE   = (1 << 0), /* affect active domain */
>  VIR_DOMAIN_MEM_CONFIG = (1 << 1), /* affect next boot */
> +VIR_DOMAIN_MEM_MAXIMUM= (1 << 2), /* affect Max rather than current */

I'll probably reindent things to put a space before = and line up the
other lines, but that's trivial.

> Index: libvirt/src/libvirt.c
> ===
> --- libvirt.orig/src/libvirt.c
> +++ libvirt/src/libvirt.c
> @@ -2869,6 +2869,8 @@ error:
>   * the XML description on the domain. If neither flag is specified
>   * (=VIR_DOMAIN_MEM_CURRENT), behavior is different depending on hypervisor.
>   *
> + * If @flags incluces VIR_DOMAIN_MEM_MAXIMUM,
> + *

Incomplete sentence.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: Install libxenlight log dir

[Eric Blake]

On 04/07/2011 04:13 PM, Jim Fehlig wrote:
> Add $localstatedir/log/libvirt/libxl when building libxenlight driver
> ---
>  src/Makefile.am |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 9b54679..3649106 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1275,6 +1275,7 @@ endif
>  if WITH_LIBXL
>   $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/libxl"
>   $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/libxl"
> + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/log/libvirt/libxl"

ACK.  But does this also need any specfile changes as a followup?

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] setmaxmem: remove the code to invoke virDomainSetMemory in cmdSetmaxmem

[Eric Blake]

On 03/17/2011 04:56 AM, Daniel P. Berrange wrote:
> On Wed, Mar 16, 2011 at 05:58:57PM +0900, Taku Izumi wrote:
>>
>> When the new maximum memory size becomes less than the current memory size,
>> I think it is not the libvirt client but the each driver that decides the 
>> behavior
>> (reject the operation or shrink the current memory size).
>>
> 
> ACK

Pushed.

Oh dear - we missed this one for 0.9.0, even though it was acked in
plenty of time.  I'm starting to agree with Laine's proposal that having
a mailing-list based helper engine that parses out known pending patch
series would be helpful to avoid losing these in the flurry of a busy inbox.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] build: Install libxenlight log dir

[Jim Fehlig]

Add $localstatedir/log/libvirt/libxl when building libxenlight driver
---
 src/Makefile.am |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 9b54679..3649106 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1275,6 +1275,7 @@ endif
 if WITH_LIBXL
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/libxl"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/libxl"
+   $(MKDIR_P) "$(DESTDIR)$(localstatedir)/log/libvirt/libxl"
 endif
 if WITH_UML
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/uml"
@@ -1316,6 +1317,7 @@ endif
 if WITH_LIBXL
rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/libxl" ||:
rmdir "$(DESTDIR)$(localstatedir)/run/libvirt/libxl" ||:
+   rmdir "$(DESTDIR)$(localstatedir)/log/libvirt/libxl" ||:
 endif
 if WITH_UML
rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/uml" ||:
-- 
1.7.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [BUG] Managed save qemu state gets deleted after failed resume

[Eric Blake]

On 04/01/2011 10:28 AM, Eric Blake wrote:
> On 03/30/2011 02:24 AM, Philipp Hahn wrote:
>> Hello,
>>
>> I haven't had time to provide a fix, but still want you to inform you about 
>> a 
>> bug: If resuming a saved VM fails with Qemu-0.14, the managed save state 
>> file /var/lib/libvirt/qemu/save/$VM.save is still deleted. I think it would 
>> be better to only delete the state after an successful resume.
> 
>>
>> This issue is tracked in our (German) bug-tracker at 
>> 
> 
> Thanks for the link.

Should be fixed now (modulo anyone approving my followup patch to tweak
the 'man virsh' wording):
https://www.redhat.com/archives/libvir-list/2011-April/msg00405.html

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 6/8] virFDStream: Add option for delete file after it's opening

[Eric Blake]

On 04/06/2011 02:59 AM, Daniel P. Berrange wrote:
> On Tue, Apr 05, 2011 at 10:55:40AM -0600, Eric Blake wrote:
>> On 04/05/2011 10:12 AM, Michal Privoznik wrote:
>>> This is needed if we want to transfer a temporary file. If the
>>> transfer is done with iohelper, we might run into a race condition,
>>> where we unlink() file before iohelper is executed.
>>>
>>> * src/fdstream.c, src/fdstream.h,
>>>   src/util/iohelper.c: Add new option
>>> * src/lxc/lxc_driver.c, src/qemu/qemu_driver.c,
>>>   src/storage/storage_driver.c, src/uml/uml_driver.c,
>>>   src/xen/xen_driver.c: Expand existing function calls
>>
>> I'm not convinced about this commit.  We should instead be fixing
>> iohelper to receive its file by fd inheritance, so that it doesn't have
>> to open() in the first place.  Then you don't need iohelper to worry
>> about unlink().
> 
> My intention was that iohelper could also do uid/gid changes
> in the future to cope with opening files on root squashing
> NFS. For that we'd want it to be doing the open, not libvirtd

Even if the parent process does virFileOpenAs?  I think we've already
got all the uid/gid changes covered in the parent process, without
having to offload that into the child and coordinate a second open();
and even if we don't, fchmod/fchown are better than a second
open()/chmod()/chown().

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] nwfilters: support for TCP flags evaluation

[Eric Blake]

On 04/07/2011 01:47 PM, Stefan Berger wrote:
> This patch adds support for the evaluation of TCP flags in nwfilters.
> 
> It adds documentation to the web page and extends the tests as well.
> Also, the nwfilter schema is extended.
> 
> The following are some example for rules using the tcp flags:
> 
> 
> 
> 
> 
> 
> 
> 
> 
> Signed-off-by: Stefan Berger 
> 
> ---
>  docs/formatnwfilter.html.in   |   10 ++
>  docs/schemas/nwfilter.rng |   16 
>  src/conf/nwfilter_conf.c  |  115
> +++---
>  src/conf/nwfilter_conf.h  |9 ++
>  src/libvirt_private.syms  |1
>  src/nwfilter/nwfilter_ebiptables_driver.c |9 ++
>  tests/nwfilterxml2xmlin/tcp-test.xml  |   12 +++
>  tests/nwfilterxml2xmlout/tcp-test.xml |   12 +++
>  8 files changed, 174 insertions(+), 10 deletions(-)

ACK, looks reasonable to me, with one optimization nit fixed:

> +int32_t mask = 0x1;
> 
> -for (i = 0; int_map[i].val; i++) {
> -if (last_attr != int_map[i].attr &&
> -flags & int_map[i].attr) {
> -if (c >= 1)
> -virBufferVSprintf(buf, "%s", sep);
> -virBufferVSprintf(buf, "%s", int_map[i].val);
> -c++;
> +while (mask) {
> +if ((mask & flags)) {
> +for (i = 0; int_map[i].val; i++) {
> +if (mask == int_map[i].attr) {
> +if (c >= 1)
> +virBufferVSprintf(buf, "%s", sep);
> +virBufferVSprintf(buf, "%s", int_map[i].val);
> +c++;
> +}
> +}
> +flags ^= mask;
> +if (!flags)
> +break;

This if condition should be after...

>  }

this brace; otherwise, if flags == 0 on entry, then you will
(needlessly) iterate through all 32 bits of mask, rather than breaking
on the first iteration.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/6] Implement disk streaming in the qemu driver

[Adam Litke]

Add support for: starting/stopping full device streaming, streaming a single
sector, and getting the status of streaming.  These operations are done by
using the 'stream' and 'info stream' qemu monitor commands.

* src/qemu/qemu_driver.c src/qemu/qemu_monitor_text.[ch]: implement disk
  streaming by using the stream and info stream text monitor commands
* src/qemu/qemu_monitor_json.[ch]: implement commands using the qmp monitor

Signed-off-by: Adam Litke 
---
 src/qemu/qemu_driver.c   |   77 +++-
 src/qemu/qemu_monitor.c  |   42 +++
 src/qemu/qemu_monitor.h  |6 ++
 src/qemu/qemu_monitor_json.c |  108 
 src/qemu/qemu_monitor_json.h |7 ++
 src/qemu/qemu_monitor_text.c |  162 ++
 src/qemu/qemu_monitor_text.h |8 ++
 7 files changed, 408 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5e2d725..fee9e1e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6687,6 +6687,79 @@ cleanup:
 return ret;
 }
 
+static unsigned long long
+qemudDomainStreamDisk (virDomainPtr dom, const char *path,
+   unsigned long long offset, unsigned int flags)
+{
+struct qemud_driver *driver = dom->conn->privateData;
+virDomainObjPtr vm;
+unsigned long long ret = -1;
+
+qemuDriverLock(driver);
+vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+qemuDriverUnlock(driver);
+
+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 (virDomainObjIsActive(vm)) {
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuDomainObjEnterMonitor(vm);
+ret = qemuMonitorStreamDisk(priv->mon, path, offset, flags);
+qemuDomainObjExitMonitor(vm);
+} else {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+"%s", _("domain is not running"));
+}
+
+cleanup:
+if (vm)
+virDomainObjUnlock(vm);
+return ret;
+}
+
+static int
+qemudDomainStreamDiskInfo (virDomainPtr dom, virStreamDiskStatePtr states,
+   unsigned int nr_states,
+   unsigned int flags ATTRIBUTE_UNUSED)
+{
+struct qemud_driver *driver = dom->conn->privateData;
+virDomainObjPtr vm;
+unsigned int ret = -1;
+
+qemuDriverLock(driver);
+vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+qemuDriverUnlock(driver);
+
+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 (virDomainObjIsActive(vm)) {
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuDomainObjEnterMonitor(vm);
+ret = qemuMonitorStreamDiskInfo(priv->mon, states, nr_states);
+qemuDomainObjExitMonitor(vm);
+} else {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+"%s", _("domain is not running"));
+}
+
+cleanup:
+if (vm)
+virDomainObjUnlock(vm);
+return ret;
+}
+
 static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
 char **result, unsigned int flags)
 {
@@ -6928,8 +7001,8 @@ static virDriver qemuDriver = {
 qemuDomainSnapshotDelete, /* domainSnapshotDelete */
 qemuDomainMonitorCommand, /* qemuDomainMonitorCommand */
 qemuDomainOpenConsole, /* domainOpenConsole */
-NULL, /* domainStreamDisk */
-NULL, /* domainStreamDiskInfo */
+qemudDomainStreamDisk, /* domainStreamDisk */
+qemudDomainStreamDiskInfo, /* domainStreamDiskInfo */
 };
 
 
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2d28f8d..fcb8561 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2213,6 +2213,48 @@ int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const 
char *name)
 return ret;
 }
 
+unsigned long long
+qemuMonitorStreamDisk(qemuMonitorPtr mon, const char *path,
+  unsigned long long offset, unsigned int flags)
+{
+unsigned long long ret;
+
+VIR_DEBUG("mon=%p, path=%p, offset=%llu, flags=%u", mon, path, offset,
+  flags);
+
+if (!mon) {
+qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+_("monitor must not be NULL"));
+return -1;
+}
+
+if (mon->json)
+ret = qemuMonitorJSONStreamDisk(mon, path, offset, flags);
+else
+ret = qemuMonitorTextStreamDisk(mon, path, offset, flags);
+return ret;
+}
+
+int qemuMonitorStreamDiskInfo(qemuMonitorPtr mon, virStreamDiskStatePtr states,
+  unsigned int nr_states)
+{
+int ret;
+
+VIR_DEB

[libvirt] [PATCH 4/6] Add disk streaming support to the remote driver

[Adam Litke]

* src/remote/remote_protocol.x: provide defines for the new entry points
* src/remote/remote_driver.c daemon/remote.c: implement the client and
  server side
* daemon/remote_dispatch_args.h daemon/remote_dispatch_prototypes.h
  daemon/remote_dispatch_ret.h daemon/remote_dispatch_table.h
  src/remote/remote_protocol.c src/remote/remote_protocol.h: generated
  stubs

Signed-off-by: Adam Litke 
---
 daemon/remote.c |   96 +++
 daemon/remote_dispatch_args.h   |2 +
 daemon/remote_dispatch_prototypes.h |   16 ++
 daemon/remote_dispatch_ret.h|2 +
 daemon/remote_dispatch_table.h  |   10 
 src/remote/remote_driver.c  |   87 +++-
 src/remote/remote_protocol.c|   63 +++
 src/remote/remote_protocol.h|   51 ++
 src/remote/remote_protocol.x|   37 +-
 9 files changed, 361 insertions(+), 3 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index dd85ef1..87bb11b 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -7024,6 +7024,102 @@ cleanup:
 return rc;
 }
 
+static int
+remoteDispatchDomainStreamDisk (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_stream_disk_args *args,
+remote_domain_stream_disk_ret *ret)
+{
+virDomainPtr dom;
+const char *path;
+unsigned long long offset;
+unsigned int flags;
+int rc;
+
+dom = get_nonnull_domain (conn, args->dom);
+if (dom == NULL) {
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
+
+path = args->path;
+offset = args->offset;
+flags = args->flags;
+
+rc = virDomainStreamDisk(dom, path, offset, flags);
+if (rc == (unsigned long long) -1) {
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
+ret->offset = rc;
+return 0;
+}
+
+static int
+remoteDispatchDomainStreamDiskInfo (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_stream_disk_info_args *args,
+remote_domain_stream_disk_info_ret *ret)
+{
+virDomainPtr dom;
+struct _virStreamDiskState *states;
+unsigned int nr_states, flags, i;
+int nr_returned;
+
+if (args->nr_results > REMOTE_DOMAIN_STREAM_DISK_STATES_MAX) {
+remoteDispatchFormatError (rerr, "%s",
+_("nr_results > 
REMOTE_DOMAIN_STREAM_DISK_STATES_MAX"));
+return -1;
+}
+
+dom = get_nonnull_domain (conn, args->dom);
+if (dom == NULL) {
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
+
+nr_states = args->nr_results;
+flags = args->flags;
+
+/* Allocate array of stats structs for making dispatch call */
+if (VIR_ALLOC_N(states, nr_states) < 0) {
+virDomainFree (dom);
+remoteDispatchOOMError(rerr);
+return -1;
+}
+
+nr_returned = virDomainStreamDiskInfo (dom, states, nr_states, flags);
+virDomainFree (dom);
+if (nr_returned < 0) {
+VIR_FREE(states);
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
+
+/* Allocate return buffer */
+if (VIR_ALLOC_N(ret->states.states_val, nr_returned) < 0) {
+VIR_FREE(states);
+remoteDispatchOOMError(rerr);
+return -1;
+}
+
+/* Copy the stats into the return structure */
+ret->states.states_len = nr_returned;
+for (i = 0; i < nr_returned; i++) {
+ret->states.states_val[i].path.path_val = strdup(states[i].path);
+ret->states.states_val[i].path.path_len = strlen(states[i].path);
+ret->states.states_val[i].offset = states[i].offset;
+ret->states.states_val[i].size = states[i].size;
+}
+VIR_FREE(states);
+return 0;
+}
 
 static int
 remoteDispatchDomainEventsRegisterAny (struct qemud_server *server 
ATTRIBUTE_UNUSED,
diff --git a/daemon/remote_dispatch_args.h b/daemon/remote_dispatch_args.h
index f9537d7..c36a836 100644
--- a/daemon/remote_dispatch_args.h
+++ b/daemon/remote_dispatch_args.h
@@ -178,3 +178,5 @@
 remote_domain_migrate_set_max_speed_args 
val_remote_domain_migrate_set_max_speed_args;
 remote_storage_vol_upload_args val_remote_storage_vol_upload_args;
 remote_storage_vol_download_args val_remote_storage_vol_download_args;
+remote_domain_stream_disk_args val

[libvirt] [PATCH 1/6] Add new API virDomainStreamDisk[Info] to header and drivers

[Adam Litke]

Set up the types for the disk streaming functions and insert it into the
virDriver structure definition.  Because of static initializers, update every
driver and set the new field to NULL.

* include/libvirt/libvirt.h.in: new API
* src/driver.h src/*/*_driver.c src/vbox/vbox_tmpl.c: add the new
  entry to the driver structure
* python/generator.py: fix compiler errors, the actual python bindings are
  implemented later

Signed-off-by: Adam Litke 
---
 include/libvirt/libvirt.h.in |   34 ++
 python/generator.py  |3 +++
 src/driver.h |   11 +++
 src/esx/esx_driver.c |2 ++
 src/lxc/lxc_driver.c |2 ++
 src/openvz/openvz_driver.c   |2 ++
 src/phyp/phyp_driver.c   |2 ++
 src/qemu/qemu_driver.c   |2 ++
 src/remote/remote_driver.c   |2 ++
 src/test/test_driver.c   |2 ++
 src/uml/uml_driver.c |2 ++
 src/vbox/vbox_tmpl.c |2 ++
 src/vmware/vmware_driver.c   |2 ++
 src/xen/xen_driver.c |2 ++
 14 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index bd36015..7c7686d 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1143,6 +1143,40 @@ int virDomainUpdateDeviceFlags(virDomainPtr domain,
const char *xml, unsigned int flags);
 
 /*
+ * Disk Streaming
+ */
+typedef enum {
+VIR_STREAM_DISK_ONE   = 1,  /* Stream a single disk unit */
+VIR_STREAM_DISK_START = 2,  /* Stream the entire disk */
+VIR_STREAM_DISK_STOP  = 4,  /* Stop streaming a disk */
+} virDomainStreamDiskFlags;
+
+#define VIR_STREAM_PATH_BUFLEN 1024
+#define VIR_STREAM_DISK_MAX_STREAMS 10
+
+typedef struct _virStreamDiskState virStreamDiskState;
+struct _virStreamDiskState {
+char path[VIR_STREAM_PATH_BUFLEN];
+/*
+ * The unit of measure for size and offset is unspecified.  These fields
+ * are meant to indicate the progress of a continuous streaming operation.
+ */
+unsigned long long offset; /* Current offset of active streaming */
+unsigned long long size;   /* Disk size */
+};
+typedef virStreamDiskState *virStreamDiskStatePtr;
+
+unsigned long long   virDomainStreamDisk(virDomainPtr dom,
+ const char *path,
+ unsigned long long offset,
+ unsigned int flags);
+
+int  virDomainStreamDiskInfo(virDomainPtr dom,
+ virStreamDiskStatePtr states,
+ unsigned int nr_states,
+ unsigned int flags);
+
+/*
  * NUMA support
  */
 
diff --git a/python/generator.py b/python/generator.py
index 4fa4f65..69ffcad 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -166,6 +166,8 @@ def enum(type, name, value):
 functions_failed = []
 functions_skipped = [
 "virConnectListDomains",
+"virDomainStreamDisk",
+"virDomainStreamDiskInfo",
 ]
 
 skipped_modules = {
@@ -180,6 +182,7 @@ skipped_types = {
  'virConnectDomainEventIOErrorCallback': "No function types in python",
  'virConnectDomainEventGraphicsCallback': "No function types in python",
  'virEventAddHandleFunc': "No function types in python",
+ 'virStreamDiskStatePtr': "Not implemented yet",
 }
 
 ###
diff --git a/src/driver.h b/src/driver.h
index e5f91ca..b333075 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -505,6 +505,15 @@ typedef int
 (*virDrvDomainSnapshotDelete)(virDomainSnapshotPtr snapshot,
   unsigned int flags);
 
+typedef unsigned long long
+(*virDrvDomainStreamDisk)(virDomainPtr dom, const char *path,
+  unsigned long long offset, unsigned int flags);
+
+typedef int
+(*virDrvDomainStreamDiskInfo)(virDomainPtr dom,
+  virStreamDiskStatePtr states,
+  unsigned int nr_states, unsigned int flags);
+
 typedef int
 (*virDrvQemuDomainMonitorCommand)(virDomainPtr domain, const char *cmd,
   char **result, unsigned int flags);
@@ -639,6 +648,8 @@ struct _virDriver {
 virDrvDomainSnapshotDelete domainSnapshotDelete;
 virDrvQemuDomainMonitorCommand qemuDomainMonitorCommand;
 virDrvDomainOpenConsole domainOpenConsole;
+virDrvDomainStreamDisk domainStreamDisk;
+virDrvDomainStreamDiskInfo domainStreamDiskInfo;
 };
 
 typedef int
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index deda372..22a8cb7 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4675,6 +4675,8 @@ static virDriver esxDriver = {
 esxDomainSnapshotDelete, /* domainSnapshotDelete */
 NULL,   

[libvirt] [PATCH 0/6] Add disk streaming API to libvirt

[Adam Litke]

I've been working with Anthony Liguori and Stefan Hajnoczi to enable data
streaming to copy-on-read disk images in qemu.  This work is working its way
through review and I expect it to be upstream soon as part of the support for
the new QED disk image format.

Disk streaming is extremely useful when provisioning domains from a central
repository of template images.  Currently the domain must be provisioned by
either: 1) copying the template image to local storage before the VM can be
started or, 2) creating a qcow2 image that backs to a base image in the remote
repository.  Option 1 can introduce a significant delay when provisioning large
disks.  Option 2 introduces a permanent dependency on a remote service and
increased network load to satisfy disk reads.

Device streaming provides the "instant-on" benefits of option 2 without
introducing a permanent dependency to the image repository.  Once the VM is
started, the contents of the disk can be streamed to the local image in
parallel.  Once streaming is finished, the domain has a complete and coherent
copy of the image and no longer depends on the central image repository.

Qemu will support two streaming modes: full device and single sector.  Full
device streaming is the easiest to use because one command will cause the whole
device to be streamed as fast as possible.  Single sector mode can be used if
one wants to throttle streaming to reduce I/O pressure.  In this mode, a
management tool issues individual commands to stream single sectors.

To enable this support in libvirt, I propose the following API...

virDomainStreamDisk() will start or stop a full device stream or stream a
single sector of a device.  The behavior is controlled by setting
virDomainStreamDiskFlags.  When either starting or stopping a full device
stream, the return value is either 0 or -1 to indicate whether the operation
succeeded.  For a single sector stream, a device offset is returned (or -1 on
failure).  This value can be used to continue streaming with a subsequent call
to virDomainStreamDisk().

virDomainStreamDiskInfo() returns information about active full device streams
(the device alias, current streaming position, and total size).

Adam Litke (6):
  Add new API virDomainStreamDisk[Info] to header and drivers
  virDomainStreamDisk: Add public symbols to libvirt API
  Implement disk streaming in the qemu driver
  Add disk streaming support to the remote driver
  Add new disk streaming commands to virsh
  python: Add python bindings for virDomainStreamDisk[Info]

 b/daemon/remote.c |   96 
 b/daemon/remote_dispatch_args.h   |2 
 b/daemon/remote_dispatch_prototypes.h |   16 +++
 b/daemon/remote_dispatch_ret.h|2 
 b/daemon/remote_dispatch_table.h  |   10 ++
 b/include/libvirt/libvirt.h.in|   34 +++
 b/python/generator.py |4 
 b/python/libvirt-override-api.xml |5 +
 b/python/libvirt-override.c   |   46 +
 b/src/driver.h|   11 ++
 b/src/esx/esx_driver.c|2 
 b/src/libvirt.c   |  115 
 b/src/libvirt_public.syms |5 +
 b/src/lxc/lxc_driver.c|2 
 b/src/openvz/openvz_driver.c  |2 
 b/src/phyp/phyp_driver.c  |2 
 b/src/qemu/qemu_driver.c  |   77 +++-
 b/src/qemu/qemu_monitor.c |   42 
 b/src/qemu/qemu_monitor.h |6 +
 b/src/qemu/qemu_monitor_json.c|  108 ++
 b/src/qemu/qemu_monitor_json.h|7 +
 b/src/qemu/qemu_monitor_text.c|  162 ++
 b/src/qemu/qemu_monitor_text.h|8 +
 b/src/remote/remote_driver.c  |   87 +-
 b/src/remote/remote_protocol.c|   63 +
 b/src/remote/remote_protocol.h|   51 ++
 b/src/remote/remote_protocol.x|   37 +++
 b/src/test/test_driver.c  |2 
 b/src/uml/uml_driver.c|2 
 b/src/vbox/vbox_tmpl.c|2 
 b/src/vmware/vmware_driver.c  |2 
 b/src/xen/xen_driver.c|2 
 b/tools/virsh.c   |  134 +++-
 python/generator.py   |3 
 src/qemu/qemu_driver.c|2 
 src/remote/remote_driver.c|2 
 36 files changed, 1144 insertions(+), 9 deletions(-)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 6/6] python: Add python bindings for virDomainStreamDisk[Info]

[Adam Litke]

Enable virDomainStreamDiskInfo in the python API.  dom.StreamDiskInfo() will
return a list containing a dictionary for each active stream.  Each dictionary
contains items to report: the disk alias, the current stream offset, and the
total disk size.

virDomainStreamDisk() works with the automatic wrappers.

* python/generator.py: reenable bindings for this entry point
* python/libvirt-override-api.xml python/libvirt-override.c: the
  generator can't handle this new function, add the new binding,
  and the XML description

Signed-off-by: Adam Litke 
---
 python/generator.py |4 +--
 python/libvirt-override-api.xml |5 
 python/libvirt-override.c   |   46 +++
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/python/generator.py b/python/generator.py
index 69ffcad..2c5f6b2 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -166,8 +166,6 @@ def enum(type, name, value):
 functions_failed = []
 functions_skipped = [
 "virConnectListDomains",
-"virDomainStreamDisk",
-"virDomainStreamDiskInfo",
 ]
 
 skipped_modules = {
@@ -182,7 +180,6 @@ skipped_types = {
  'virConnectDomainEventIOErrorCallback': "No function types in python",
  'virConnectDomainEventGraphicsCallback': "No function types in python",
  'virEventAddHandleFunc': "No function types in python",
- 'virStreamDiskStatePtr': "Not implemented yet",
 }
 
 ###
@@ -344,6 +341,7 @@ skip_impl = (
 'virNodeDeviceListCaps',
 'virConnectBaselineCPU',
 'virDomainRevertToSnapshot',
+'virDomainStreamDiskInfo',
 )
 
 
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index 54deeb5..9a74551 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -308,5 +308,10 @@
   
   
 
+
+  collect information about active disk streams
+  
+  
+ 
   
 
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 4a9b432..984d3ef 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -3508,6 +3508,51 @@ 
libvirt_virConnectDomainEventDeregisterAny(ATTRIBUTE_UNUSED PyObject * self,
 return (py_retval);
 }
 
+static PyObject *
+libvirt_virDomainStreamDiskInfo(PyObject *self ATTRIBUTE_UNUSED,
+PyObject *args) {
+virDomainPtr domain;
+PyObject *pyobj_domain;
+unsigned int nr_streams, i;
+struct _virStreamDiskState streams[VIR_STREAM_DISK_MAX_STREAMS];
+PyObject *ret;
+
+if (!PyArg_ParseTuple(args, (char *)"O:virDomainStreamDiskInfo",
+  &pyobj_domain))
+return(NULL);
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+nr_streams = virDomainStreamDiskInfo(domain, streams,
+ VIR_STREAM_DISK_MAX_STREAMS, 0);
+if (nr_streams == -1)
+return VIR_PY_NONE;
+
+if ((ret = PyList_New(nr_streams)) == NULL)
+return VIR_PY_NONE;
+
+for (i = 0; i < nr_streams; i++) {
+PyObject *dict = PyDict_New();
+if (dict == NULL)
+goto error;
+PyDict_SetItem(dict, libvirt_constcharPtrWrap("path"),
+   libvirt_constcharPtrWrap(streams[i].path));
+PyDict_SetItem(dict, libvirt_constcharPtrWrap("offset"),
+   libvirt_ulonglongWrap(streams[i].offset));
+PyDict_SetItem(dict, libvirt_constcharPtrWrap("size"),
+   libvirt_ulonglongWrap(streams[i].size));
+PyList_SetItem(ret, i, dict);
+}
+return ret;
+
+error:
+for (i = 0; i < PyList_Size(ret); i++) {
+PyObject *item = PyList_GET_ITEM(ret, i);
+Py_XDECREF(item);
+}
+Py_DECREF(ret);
+return VIR_PY_NONE;
+}
+
 
 /
  * *
@@ -3585,6 +3630,7 @@ static PyMethodDef libvirtMethods[] = {
 {(char *) "virDomainGetJobInfo", libvirt_virDomainGetJobInfo, 
METH_VARARGS, NULL},
 {(char *) "virDomainSnapshotListNames", 
libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL},
 {(char *) "virDomainRevertToSnapshot", libvirt_virDomainRevertToSnapshot, 
METH_VARARGS, NULL},
+{(char *) "virDomainStreamDiskInfo", libvirt_virDomainStreamDiskInfo, 
METH_VARARGS, NULL},
 {NULL, NULL, 0, NULL}
 };
 
-- 
1.7.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/6] virDomainStreamDisk: Add public symbols to libvirt API

[Adam Litke]

* src/libvirt.c: implement the main entry points
* src/libvirt_public.syms: add them to the exported symbols

Signed-off-by: Adam Litke 
---
 src/libvirt.c   |  115 +++
 src/libvirt_public.syms |5 ++
 2 files changed, 120 insertions(+), 0 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 85dfc58..15c5ddc 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5928,6 +5928,121 @@ error:
 }
 
 /**
+ * virDomainStreamDisk:
+ * @dom: pointer to the domain object
+ * @path: path to the block device
+ * @offset: when streaming a single disk unit, the offset of the unit to stream
+ * @flags: flags to control disk streaming behavior
+ *
+ * Returns: Next offset or 0 on success, -1 on failure.
+ */
+unsigned long long
+virDomainStreamDisk(virDomainPtr dom,
+const char *path,
+unsigned long long offset,
+unsigned int flags)
+{
+virConnectPtr conn;
+unsigned long long ret = -1;
+
+VIR_DOMAIN_DEBUG(dom, "path=%p, offset=%llu, flags=%u",
+ path, offset, flags);
+
+if (path == NULL) {
+virLibDomainError (VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+
+if (flags == VIR_STREAM_DISK_START || flags == VIR_STREAM_DISK_STOP) {
+if (offset != 0) {
+virLibDomainError (VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+} else if (flags != VIR_STREAM_DISK_ONE) {
+virLibDomainError (VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+
+virResetLastError();
+if (!VIR_IS_CONNECTED_DOMAIN (dom)) {
+virLibDomainError (VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+return -1;
+}
+
+conn = dom->conn;
+if (conn->driver->domainStreamDisk) {
+ret = conn->driver->domainStreamDisk (dom, path, offset, flags);
+if (ret == -1)
+goto error;
+return ret;
+}
+
+virLibDomainError (VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(dom->conn);
+return -1;
+}
+
+/**
+ * virDomainStreamDiskInfo:
+ * @dom: pointer to the domain object
+ * @states: An array of virStreamDiskState structures to store stream info
+ * @nr_states: The maximimum number of stream states to report
+ * @flags: future flags, use 0 for now
+ *
+ * Returns: The number of streams reported or -1 on failure.
+ */
+int
+virDomainStreamDiskInfo(virDomainPtr dom,
+virStreamDiskStatePtr states,
+unsigned int nr_states,
+unsigned int flags)
+{
+virConnectPtr conn;
+int ret = -1;
+
+VIR_DOMAIN_DEBUG(dom, "states=%p, nr_states=%u, flags=%u",
+ states, nr_states, flags);
+
+if (states == NULL) {
+virLibDomainError (VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+
+if (nr_states == 0) {
+virLibDomainError (VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+
+if (flags != 0) {
+virLibDomainError (VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+
+virResetLastError();
+if (!VIR_IS_CONNECTED_DOMAIN (dom)) {
+virLibDomainError (VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+return -1;
+}
+
+conn = dom->conn;
+if (conn->driver->domainStreamDiskInfo) {
+ret = conn->driver->domainStreamDiskInfo (dom, states, nr_states,
+  flags);
+if (ret == -1)
+goto error;
+return ret;
+}
+
+virLibDomainError (VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(dom->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 b4aed41..185186a 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -437,3 +437,8 @@ LIBVIRT_0.9.0 {
 } LIBVIRT_0.8.8;
 
 #  define new API here using predicted next version number 
+LIBVIRT_0.9.1 {
+global:
+virDomainStreamDisk;
+virDomainStreamDiskInfo;
+} LIBVIRT_0.9.0;
-- 
1.7.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 5/6] Add new disk streaming commands to virsh

[Adam Litke]

Define two new virsh commands: one to control disk streaming and one to print
the status of active disk streams.

* tools/virsh.c: implement the new commands

Signed-off-by: Adam Litke 
---
 tools/virsh.c |  134 -
 1 files changed, 133 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index f2d2c9d..edffc61 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -3999,6 +3999,137 @@ done:
 }
 
 /*
+ * "domstreamdisk" command
+ */
+static const vshCmdInfo info_domstreamdisk[] = {
+{"help", gettext_noop("Stream data to a disk")},
+{"desc", gettext_noop("Stream data to a disk connected to a running 
domain")},
+{ NULL, NULL },
+};
+
+static const vshCmdOptDef opts_domstreamdisk[] = {
+{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or 
uuid")},
+{"start", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("Start streaming a disk") },
+{"stop", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("Stop streaming a disk") },
+{"incremental", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("Perform an incremental 
stream") },
+{"path", VSH_OT_DATA, VSH_OFLAG_REQ, N_("block device")},
+{"offset", VSH_OT_DATA, VSH_OFLAG_NONE, N_("Device offset for incremental 
stream")},
+{ NULL, 0, 0, NULL },
+};
+
+static int
+cmdDomStreamDisk(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+const char *name, *path;
+unsigned long long offset, next;
+unsigned int flags, start, stop, incr;
+
+if (!vshConnectionUsability(ctl, ctl->conn))
+return FALSE;
+
+flags = start = stop = incr = 0;
+if (vshCommandOptBool(cmd, "start")) {
+start = 1;
+flags = VIR_STREAM_DISK_START;
+}
+if (vshCommandOptBool(cmd, "stop")) {
+stop = 1;
+flags = VIR_STREAM_DISK_STOP;
+}
+if (vshCommandOptBool(cmd, "incremental")) {
+incr = 1;
+flags = VIR_STREAM_DISK_ONE;
+}
+if (start + stop + incr != 1) {
+vshError(ctl, _("Exactly one mode: --start, --stop, --incremental, "
+"is required"));
+return FALSE;
+}
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
+return FALSE;
+
+if (vshCommandOptString(cmd, "path", &path) < 0)
+return FALSE;
+
+if (flags == VIR_STREAM_DISK_ONE) {
+if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) {
+vshError(ctl, _("An offset is required for incremental 
streaming"));
+virDomainFree(dom);
+return FALSE;
+}
+} else {
+offset = 0;
+}
+
+next = virDomainStreamDisk(dom, path, offset, flags);
+if (next == (unsigned long long) -1) {
+virDomainFree(dom);
+return FALSE;
+}
+
+if (flags == VIR_STREAM_DISK_START)
+vshPrint (ctl, "Stream successfully started\n");
+else if (flags == VIR_STREAM_DISK_STOP)
+vshPrint (ctl, "Stream successfully stopped\n");
+else
+vshPrint (ctl, "Strem successful.  Continue at offset %llu\n", next);
+
+virDomainFree(dom);
+return TRUE;
+}
+
+/*
+ * "domstreamdiskinfo" command
+ */
+static const vshCmdInfo info_domstreamdiskinfo[] = {
+{"help", gettext_noop("Get disk streaming status for a domain")},
+{"desc", gettext_noop("Get disk streaming status for a running domain")},
+{ NULL, NULL },
+};
+
+static const vshCmdOptDef opts_domstreamdiskinfo[] = {
+{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or 
uuid")},
+{ NULL, 0, 0, NULL },
+};
+
+static int
+cmdDomStreamDiskInfo(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+const char *name;
+struct _virStreamDiskState streams[VIR_STREAM_DISK_MAX_STREAMS];
+int nr_streams, i;
+
+if (!vshConnectionUsability(ctl, ctl->conn))
+return FALSE;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
+return FALSE;
+
+nr_streams = virDomainStreamDiskInfo(dom, streams,
+VIR_STREAM_DISK_MAX_STREAMS, 0);
+if (nr_streams < 0) {
+vshError(ctl, _("Failed to get disk stream information for domain %s"),
+name);
+virDomainFree(dom);
+return FALSE;
+}
+
+vshPrint (ctl, "%-30s %-10s %-10s\n", _("Device"), _("Offset"),
+  _("Size"));
+vshPrint (ctl, "\n");
+for (i = 0; i < nr_streams; i++) {
+vshPrint (ctl, "%-30s %-10llu %-10llu\n", streams[i].path,
+  streams[i].offset, streams[i].size);
+}
+
+virDomainFree(dom);
+return TRUE;
+}
+
+/*
  * "net-autostart" command
  */
 static const vshCmdInfo info_network_autostart[] = {
@@ -10682,6 +10813,8 @@ static const vshCmdDef domManagementCmds[] = {
 {"domjobabort", cmdDomjobabort, opts_domjobabort, info_domjobabort},
 {"domjobinfo", cmdDomjobinfo, opts_domjobinfo, info_domjobinfo},
 {"domname", cmdDomname, opts_domname, info_domname},

Re: [libvirt] [PATCH] virsh: fix mingw startup

[Eric Blake]

On 04/07/2011 09:02 AM, Laine Stump wrote:
> On 04/06/2011 03:47 PM, Eric Blake wrote:
>> * .gnulib: Update to latest, for pipe2.
>> * bootstrap.conf (gnulib_modules): Add pipe2.
>> * src/util/event_poll.c (virEventPollInit): Use it, to avoid
>> problematic virSetCloseExec on mingw.
>> ---
>>
>> The gnulib changes to pipe2 are now in.
>>
>> * .gnulib f796520...2255b86 (4):
>>>  verify: use _Static_assert if available
>>>  Remove leftover generated .h files after config.status changed.
>>>  Ensure to rebuild generated .h files when config.status has
>> changed.
>>>  pipe2: Relicense under LGPLv2+.
>>
>> This assumes that my earlier .gnulib modification patch is also in:
>> https://www.redhat.com/archives/libvir-list/2011-April/msg00297.html
>> although I can rebase as needed.
>>
>>   .gnulib   |2 +-
>>   bootstrap.conf|3 ++-
>>   src/util/event_poll.c |7 ++-
>>   3 files changed, 5 insertions(+), 7 deletions(-)
> 
> ACK.

Thanks; pushed.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: fix gitignore sorting

[Eric Blake]

On 04/07/2011 01:26 PM, Laine Stump wrote:
> On 04/05/2011 06:51 PM, Eric Blake wrote:
>> Make it so we don't have to 'git add -f' particular files like
>> po/POTFILES.in all the time (tested by fixing one of our
>> special-case files as part of the patch).
>>
>> * .gnulib: Update to latest.
>> * bootstrap: Resync from coreutils.
>> * .gitignore: Sort whitelist entries correctly, including ignoring
>> files rather than directories.
>> * m4/virt-compile-warnings.m4: Convert tabs to space.
>> ---

> 
> ACK (once Eric explained the sed construct I'd never seen before :-)

Thanks; pushed.

> 
>> +# Ensure that lines starting with ! sort last, per gitignore conventions
>> +# for whitelisting exceptions after a more generic blacklist pattern.
>> +sort_patterns() {
>> +  sort -u "$@" | sed

Sed has two buffers - the pattern space (which gets printed for every
line of input unless it is empty) and the hold space (which is used for
scratch work).

 '/^!/ {
>> +H
>> +d
>> +  }

For all lines that start with !, append a newline and that line into the
hold space, then delete the line from the pattern space (so skip
printing ! lines the first time through).

>> +  $ {
>> +P
>> +x
>> +s/^\n//
>> +  }'

On the last line, print the line like normal, then exchange pattern and
buffer spaces, remove the extra leading newline that got put in the hold
space on the first ! line, then print all the deferred ! lines.

> Yeah, that's the one...

I don't blame you for asking - sed's got some pretty arcane (but cool)
features.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/4] Add domainSave/Restore to libxl driver

[Jim Fehlig]

Markus Groß wrote:
> ---
>  src/libxl/libxl_conf.h   |   10 +++
>  src/libxl/libxl_driver.c |  184 
> +-
>  2 files changed, 191 insertions(+), 3 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index f2f0d8a..e28615b 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -1,5 +1,6 @@
>  
> /*---*/
>  /*  Copyright (c) 2011 SUSE LINUX Products GmbH, Nuernberg, Germany.
> + *  Copyright (C) 2011 Univention GmbH.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -17,6 +18,7 @@
>   *
>   * Authors:
>   * Jim Fehlig 
> + * Markus Groß 
>   */
>  
> /*---*/
>  
> @@ -85,6 +87,14 @@ struct _libxlDomainObjPrivate {
>  int eventHdl;
>  };
>  
> +static const char libxlSavefileMagic[16]= "libvirt-xml\n \0 \r";
> +typedef struct _libxlSavefileHeader libxlSavefileHeader;
> +typedef libxlSavefileHeader *libxlSavefileHeaderPtr;
> +struct _libxlSavefileHeader {
> +char magic[16]; /* magic id */
> +uint32_t xmlLen; /* length of saved xml */
> +};
>   

I think we should pad the header, similar to qemu driver, and provide a
version field.

> +
>  
>  # define libxlError(code, ...) \
>  virReportErrorHelper(NULL, VIR_FROM_LIBXL, code, __FILE__, \
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 1539385..b02d4b7 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "internal.h"
>  #include "logging.h"
> @@ -60,7 +61,6 @@
>  
>  static libxlDriverPrivatePtr libxl_driver = NULL;
>  
> -
>  /* Function declarations */
>  static int
>  libxlVmStart(libxlDriverPrivatePtr driver,
> @@ -1555,6 +1555,184 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr 
> info)
>  }
>  
>  static int
> +libxlDomainSave(virDomainPtr dom, const char * to)
> +{
> +libxlDriverPrivatePtr driver = dom->conn->privateData;
> +virDomainObjPtr vm;
> +libxlDomainObjPrivatePtr priv;
> +virDomainEventPtr event = NULL;
> +libxlSavefileHeader hdr;
> +libxl_domain_suspend_info s_info;
> +char * xml;
> +uint32_t xml_len;
> +int fd;
> +int ret = -1;
> +
> +libxlDriverLock(driver);
> +vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> +
> +if (!vm) {
> +char uuidstr[VIR_UUID_STRING_BUFLEN];
> +virUUIDFormat(dom->uuid, uuidstr);
> +libxlError(VIR_ERR_NO_DOMAIN,
> +   _("No domain with matching uuid '%s'"), uuidstr);
> +goto cleanup;
> +}
> +
> +if (!virDomainObjIsActive(vm)) {
> +libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not 
> running"));
> +goto cleanup;
> +}
> +
> +priv = vm->privateData;
> +
> +if (vm->state != VIR_DOMAIN_PAUSED) {
> +memset(&s_info, 0, sizeof(s_info));
> +
> +if ((fd = virFileOpenAs(to, O_CREAT|O_TRUNC|O_WRONLY, 
> S_IRUSR|S_IWUSR,
> +getuid(), getgid(), 0)) < 0) {
> +virReportSystemError(-fd,
> + _("Failed to create domain save file '%s'"),
> + to);
> +goto cleanup;
> +}
> +
> +if ((xml = virDomainDefFormat(vm->def, 0)) == NULL)
> +goto cleanup;
> +xml_len = strlen(xml) + 1;
> +
> +memset(&hdr, 0, sizeof(hdr));
> +memcpy(hdr.magic, libxlSavefileMagic, sizeof(hdr.magic));
> +hdr.xmlLen = xml_len;
> +
> +if (safewrite(fd, &hdr, sizeof(hdr)) != sizeof(hdr)) {
> +libxlError(VIR_ERR_OPERATION_FAILED,
> +   _("Failed to write save file header"));
> +goto cleanup;
> +}
> +
> +if (safewrite(fd, xml, xml_len) != xml_len) {
> +libxlError(VIR_ERR_OPERATION_FAILED,
> +   _("Failed to write xml description"));
> +goto cleanup;
> +}
> +
> +if (libxl_domain_suspend(&priv->ctx, &s_info, dom->id, fd) != 0) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed to save domain '%d' with libxenlight"),
> +   dom->id);
> +goto cleanup;
> +}
> +
> +event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
> + VIR_DOMAIN_EVENT_STOPPED_SAVED);
> +
> +if (libxlVmReap(driver, vm, 1) != 0) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed to destroy domain '%d'"), dom->id);
> +goto cleanup;
> +}
> +
> +if (!vm->persistent) {
> +virDomainRemoveInactive(&driver->domai

[libvirt] [PATCH v2] nwfilters: support for TCP flags evaluation

[Stefan Berger]

This patch adds support for the evaluation of TCP flags in nwfilters.

It adds documentation to the web page and extends the tests as well.
Also, the nwfilter schema is extended.

The following are some example for rules using the tcp flags:









Signed-off-by: Stefan Berger 

---
 docs/formatnwfilter.html.in   |   10 ++
 docs/schemas/nwfilter.rng |   16 
 src/conf/nwfilter_conf.c  |  115 
+++---

 src/conf/nwfilter_conf.h  |9 ++
 src/libvirt_private.syms  |1
 src/nwfilter/nwfilter_ebiptables_driver.c |9 ++
 tests/nwfilterxml2xmlin/tcp-test.xml  |   12 +++
 tests/nwfilterxml2xmlout/tcp-test.xml |   12 +++
 8 files changed, 174 insertions(+), 10 deletions(-)

Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -5,7 +5,8 @@
  * Copyright (C) 2006-2011 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
- * Copyright (C) 2010 IBM Corporation
+ * Copyright (C) 2010-2011 IBM Corporation
+ * Copyright (C) 2010-2011 Stefan Berger
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -726,17 +727,23 @@ printStringItems(virBufferPtr buf, const
  int32_t flags, const char *sep)
 {
 unsigned int i, c = 0;
-int32_t last_attr = 0;
+int32_t mask = 0x1;

-for (i = 0; int_map[i].val; i++) {
-if (last_attr != int_map[i].attr &&
-flags & int_map[i].attr) {
-if (c >= 1)
-virBufferVSprintf(buf, "%s", sep);
-virBufferVSprintf(buf, "%s", int_map[i].val);
-c++;
+while (mask) {
+if ((mask & flags)) {
+for (i = 0; int_map[i].val; i++) {
+if (mask == int_map[i].attr) {
+if (c >= 1)
+virBufferVSprintf(buf, "%s", sep);
+virBufferVSprintf(buf, "%s", int_map[i].val);
+c++;
+}
+}
+flags ^= mask;
+if (!flags)
+break;
 }
-last_attr = int_map[i].attr;
+mask <<= 1;
 }

 return 0;
@@ -799,6 +806,87 @@ stateFormatter(virBufferPtr buf,
 }


+
+static const struct int_map tcpFlags[] = {
+INTMAP_ENTRY(0x1 , "FIN"),
+INTMAP_ENTRY(0x2 , "SYN"),
+INTMAP_ENTRY(0x4 , "RST"),
+INTMAP_ENTRY(0x8 , "PSH"),
+INTMAP_ENTRY(0x10, "ACK"),
+INTMAP_ENTRY(0x20, "URG"),
+INTMAP_ENTRY(0x3F, "ALL"),
+INTMAP_ENTRY(0x0 , "NONE"),
+INTMAP_ENTRY_LAST
+};
+
+
+static bool
+tcpFlagsValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED, union 
data *val,

+  virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
+  nwItemDesc *item)
+{
+bool rc = false;
+char *s_mask = val->c;
+char *sep = strchr(val->c, '/');
+char *s_flags;
+int32_t mask = 0, flags = 0;
+
+if (!sep)
+return false;
+
+s_flags = sep + 1;
+
+*sep = '\0';
+
+if (!parseStringItems(tcpFlags, s_mask , &mask , ',') &&
+!parseStringItems(tcpFlags, s_flags, &flags, ',')) {
+item->u.tcpFlags.mask  = mask & 0x3f;
+item->u.tcpFlags.flags = flags & 0x3f;
+rc = true;
+}
+
+*sep = '/';
+
+return rc;
+}
+
+
+static void
+printTCPFlags(virBufferPtr buf, uint8_t flags)
+{
+if (flags == 0)
+virBufferAddLit(buf, "NONE");
+else if (flags == 0x3f)
+virBufferAddLit(buf, "ALL");
+else
+printStringItems(buf, tcpFlags, flags, ",");
+}
+
+
+void
+virNWFilterPrintTCPFlags(virBufferPtr buf,
+ uint8_t mask, char sep, uint8_t flags)
+{
+printTCPFlags(buf, mask);
+virBufferAddChar(buf, sep);
+printTCPFlags(buf, flags);
+}
+
+
+static bool
+tcpFlagsFormatter(virBufferPtr buf,
+  virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
+  nwItemDesc *item)
+{
+virNWFilterPrintTCPFlags(buf,
+ item->u.tcpFlags.mask,
+ '/',
+ item->u.tcpFlags.flags);
+
+return true;
+}
+
+
 #define COMMON_MAC_PROPS(STRUCT) \
 {\
 .name = SRCMACADDR,\
@@ -1104,6 +1192,13 @@ static const virXMLAttr2Struct tcpAttrib
 .datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
 .dataIdx = offsetof(virNWFilterRuleDef, 
p.tcpHdrFilter.dataTCPOption),

 },
+{
+.name = "flags",
+.datatype = DATATYPE_STRING,
+.dataIdx = offsetof(virNWFilterRuleDef, 
p.tcpHdrFilter.dataTCPFlags),

+.validator = tcpFlagsValidator,
+.formatter = tcpFlagsFormatter,
+},
 COMMENT_PROP_IPHDR(tcpHdrFilter),
 {
 .name = NULL,
Index: libvirt-acl/src/conf/nwfilter_conf.h
=

Re: [libvirt] [PATCH] build: fix gitignore sorting

[Laine Stump]

On 04/05/2011 06:51 PM, Eric Blake wrote:

Make it so we don't have to 'git add -f' particular files like
po/POTFILES.in all the time (tested by fixing one of our
special-case files as part of the patch).

* .gnulib: Update to latest.
* bootstrap: Resync from coreutils.
* .gitignore: Sort whitelist entries correctly, including ignoring
files rather than directories.
* m4/virt-compile-warnings.m4: Convert tabs to space.
---

* .gnulib dec3475...f796520 (7):
   >  bootstrap: compute gnulib_extra_files after updating build_aux
   >  bootstrap: preserve git whitelist item sorting
   >  maint.mk: Don't trigger sc_space_tab check.
   >  areadlink, areadlinkat: rewrite in terms of careadlinkat
   >  autoupdate
   >  wmemchr, wcschr, wcsrchr, wcspbrk, wcsstr: Avoid errors in C++ mode.
   >  wcpcpy, wcpncpy: Ensure declaration on glibc>= 2.13 systems.

I'm also trying to patch 'man gitignore' with things I've learned
from this exercise: http://marc.info/?t=13020324813&r=1&w=2

  .gitignore  |   12 +-
  .gnulib |2 +-
  bootstrap   |   46 +--
  m4/virt-compile-warnings.m4 |8 +++---
  4 files changed, 42 insertions(+), 26 deletions(-)


ACK (once Eric explained the sed construct I'd never seen before :-)


+# Ensure that lines starting with ! sort last, per gitignore conventions
+# for whitelisting exceptions after a more generic blacklist pattern.
+sort_patterns() {
+  sort -u "$@" | sed '/^!/ {
+H
+d
+  }
+  $ {
+P
+x
+s/^\n//
+  }'
+}


Yeah, that's the one...

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/6] use virObject to manage reference-count of virDomainObj

[Eric Blake]

On 04/07/2011 03:41 AM, Hu Tao wrote:
>>> @@ -1010,6 +1007,11 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr 
>>> caps)
>>>  return NULL;
>>>  }
>>>  
>>> +if (virObjectInit(&domain->obj, doDomainObjFree)) {
>>> +VIR_FREE(domain);
>>> +return NULL;
>>
>> Hmm.  virObjectInit used VIR_ERROR, which logs, but doesn't call into
>> virtError.  By returning NULL here, a user will get the dreaded "Unknown
>> error" message since we didn't hook into the virterror machinery.
>> Should virObjectInit instead be using virReportErrorHelper?  Or is it
> 
> How to use virReportErrorHelper in this case?
> 
>> considered a coding bug to ever have virObjectInit fail in the first
>> place, so we don't really have to worry about that.
> 
> Sorry for my bad English, I don't understand this sentence.

Basically, I was envisioning:

# define virObjectReportError(code, ...)  \
virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__, \
 __FUNCTION__, __LINE__, __VA_ARGS__)

and calling virObjectReportError() instead of VIR_ERROR() for any
failure inside of virobject.c.  VIR_ERROR only hooks up to the log file,
but virReportErrorHelper _also_ hooks up to the remote procedure call
(which means that if the error happens due to someone making an API
call, they get the message back rather than just a cryptic "Unknown error").

But if we can guarantee that the only possible errors are due to coding
mistakes (and in reality, that's true), then I'd almost go so far as to
do make virObjectInit return void instead of int (callers need not check
for failure), and inside virObjectInit do:

if (obj->free) {
VIR_ERROR ("Coding bug encountered, aborting");
abort ();
}

Gnulib already has uses of abort() in places that can only be reached by
pure coding bugs, even though libvirt.git does not currently have
anything like that.

On the other hand, there's a difference between errors in
virObjectInit() (which are only possible due to severe coding bugs and
easy to audit that they will never hit without ever risking an abort()
escaping into released code) vs. errors in virObjectUnref() (if we've
unreferenced an object too many times, it's hard to tell which place in
the code was the culprit, so the bug may escape in to a release and a
library should avoid abort() at all costs in released code).  At any
rate, any time we hit a reference counting bug, it is in our best
interest to do something loud and obvious that a bug occurred, to make
it easier to fix the bug, rather than trying to proceed with invalid data.

>> The only potential drawback to that is that if you use the wrong
>> function, the referencing doesn't happen.  Or maybe even make
>> virHashCreateFull take a bool parameter of whether the data must be a
>> virObjectPtr, so you don't have to wrap virHashAddObjEntry (and since
>> virHashRemoveEntry doesn't really have any way to create a
>> virHashRemoveObjEntry wrapper, but it would need to be in on the game of
>> automatic reference count manipulations any time we know the table
>> hashes only virObjects as data).
> 
> Would it be better to have two types of hashtable, one hashes only
> virObjects, the other hashes data except virObjects? this can minimize
> the impact brought by the change of hashtable to existing code.

Proving something is a virObject can be done via type-safe wrapper
functions, but only for functions that take a data argument in the first
place (virHashRemoveEntry does not).  But in the opposite direction, how
can you prove something is not a virObject?  I don't think you can
forbid that (nor do you necessarily want to; it might make sense to hash
void* which happens to be virObject but where the container does not
plan on owning the object).  I think the best you can do is adding
helper methods that operate only if you request and promise to only use
virObject as the data, and adding a bool flag to virHashCreateFull seems
the least invasive way to do it.

> If a dom is added into a hashtable, then it is not safe to modify its
> uuid since it is the key of dom in hashtable. And we have to make some
> rules about when the uuid can be modified(say, hold the driver lock
> first when dom is in hashtable).

Yes, for a given object, especially if that object is hashed by a subset
of itself, then the data being used as a hash key must not be modified.
 And we don't modify domain uuid's after creation (if we need a new
uuid, we are creating a new domain rather than modifying an existing
one).  But without documenting which fields are constant and can be
accessed without obtaining a lock and which must not be modified after
the constructor, compared to the remaining fields that must only be read
or written while holding the lock, is on a case-by-case basis per struct
that inherits from virObject.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description:

Re: [libvirt] [PATCH 3/4] Add cputune support to libxl driver

[Jim Fehlig]

Markus Groß wrote:
> The nodeGetInfo code had to be moved into a helper
> function to reuse it without a virConnectPtr.
>   

Sounds good.

> ---
>  src/libxl/libxl_driver.c |  143 
> ++
>  1 files changed, 107 insertions(+), 36 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index c760a23..1539385 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -199,6 +199,46 @@ libxlAutostartDomain(void *payload, const void *name 
> ATTRIBUTE_UNUSED,
>  virDomainObjUnlock(vm);
>  }
>  
> +static int
> +libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info)
> +{
> +libxl_physinfo phy_info;
> +const libxl_version_info* ver_info;
> +struct utsname utsname;
> +
> +if (libxl_get_physinfo(&driver->ctx, &phy_info)) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("libxl_get_physinfo_info failed"));
> +return -1;
> +}
> +
> +if ((ver_info = libxl_get_version_info(&driver->ctx)) == NULL) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("libxl_get_version_info failed"));
> +return -1;
> +}
> +
> +uname(&utsname);
> +if (virStrncpy(info->model,
> +   utsname.machine,
> +   strlen(utsname.machine),
> +   sizeof(info->model)) == NULL) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("machine type %s too big for destination"),
> +   utsname.machine);
> +return -1;
> +}
> +
> +info->memory = phy_info.total_pages * (ver_info->pagesize / 1024);
> +info->cpus = phy_info.nr_cpus;
> +info->nodes = phy_info.nr_nodes;
> +info->cores = phy_info.cores_per_socket;
> +info->threads = phy_info.threads_per_core;
> +info->sockets = 1;
> +info->mhz = phy_info.cpu_khz / 1000;
> +return 0;
> +}
> +
>  /*
>   * Cleanup function for domain that has reached shutoff state.
>   *
> @@ -391,6 +431,62 @@ error:
>  return -1;
>  }
>  
> +static int
> +libxlDomainSetVcpuAffinites(libxlDriverPrivatePtr driver, virDomainObjPtr vm)
> +{
> +libxlDomainObjPrivatePtr priv = vm->privateData;
> +virDomainDefPtr def = vm->def;
> +libxl_cpumap map;
> +uint8_t *cpumask = NULL;
> +uint8_t *cpumap = NULL;
> +virNodeInfo nodeinfo;
> +size_t cpumaplen;
> +unsigned int pos;
> +int vcpu, i;
> +int ret = -1;
> +
> +if (libxlDoNodeGetInfo(driver, &nodeinfo) < 0)
> +goto cleanup;
> +
> +cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo));
> +
> +for (vcpu = 0; vcpu < def->cputune.nvcpupin; ++vcpu) {
> +if (vcpu != def->cputune.vcpupin[vcpu]->vcpuid)
> +continue;
> +
> +if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) {
> +virReportOOMError();
> +goto cleanup;
> +}
> +
> +cpumask = (uint8_t*) def->cputune.vcpupin[vcpu]->cpumask;
> +
> +for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; ++i) {
> +if (cpumask[i]) {
> +pos = i / 8;
> +cpumap[pos] |= 1 << (i % 8);
> +}
> +}
> +
> +map.size = cpumaplen;
> +map.map = cpumap;
> +
> +if (libxl_set_vcpuaffinity(&priv->ctx, def->id, vcpu, &map) != 0) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed to pin vcpu '%d' with libxenlight"), vcpu);
> +goto cleanup;
> +}
> +
> +VIR_FREE(cpumap);
> +}
> +
> +ret = 0;
> +
> +cleanup:
> +VIR_FREE(cpumap);
> +return ret;
> +}
> +
>  /*
>   * Start a domain through libxenlight.
>   *
> @@ -440,6 +536,9 @@ libxlVmStart(libxlDriverPrivatePtr driver,
>  if (libxlCreateDomEvents(vm) < 0)
>  goto error;
>  
> +if (libxlDomainSetVcpuAffinites(driver, vm) < 0)
> +goto error;
> +
>  if (!start_paused) {
>  libxl_domain_unpause(&priv->ctx, domid);
>  vm->state = VIR_DOMAIN_RUNNING;
> @@ -869,42 +968,7 @@ libxlGetMaxVcpus(virConnectPtr conn, const char *type 
> ATTRIBUTE_UNUSED)
>  static int
>  libxlNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
>  {
> -libxl_physinfo phy_info;
> -const libxl_version_info* ver_info;
> -libxlDriverPrivatePtr driver = conn->privateData;
> -struct utsname utsname;
> -
> -if (libxl_get_physinfo(&driver->ctx, &phy_info)) {
> -libxlError(VIR_ERR_INTERNAL_ERROR,
> -   _("libxl_get_physinfo_info failed"));
> -return -1;
> -}
> -
> -if ((ver_info = libxl_get_version_info(&driver->ctx)) == NULL) {
> -libxlError(VIR_ERR_INTERNAL_ERROR,
> -   _("libxl_get_version_info failed"));
> -return -1;
> -}
> -
> -uname(&utsname);
> -if (virStrncpy(info->model,
> -   utsname.machine,
> -   strlen(utsname.machine),
> -   sizeof(info->model)) == NULL) {
> - 

Re: [libvirt] [PATCH 1/6] Enhance the streams helper to support plain file I/O

[Daniel P. Berrange]

On Thu, Apr 07, 2011 at 10:41:34AM -0600, Jim Fehlig wrote:
> Wen Congyang wrote:
> > At 03/24/2011 01:36 AM, Daniel P. Berrange Write:
> >   
> >> The O_NONBLOCK flag doesn't work as desired on plain files
> >> or block devices. Introduce an I/O helper program that does
> >> the blocking I/O operations, communicating over a pipe that
> >> can support O_NONBLOCK
> >>
> >> * src/fdstream.c, src/fdstream.h: Add non-blocking I/O
> >>   on plain files/block devices
> >> * src/Makefile.am, src/util/iohelper.c: I/O helper program
> >> * src/qemu/qemu_driver.c, src/lxc/lxc_driver.c,
> >>   src/uml/uml_driver.c, src/xen/xen_driver.c: Update for
> >>   streams API change
> >> ---
> >>  po/POTFILES.in |1 +
> >>  src/Makefile.am|   12 +++
> >>  src/fdstream.c |  233 
> >> 
> >>  src/fdstream.h |5 +
> >>  src/lxc/lxc_driver.c   |2 +-
> >>  src/qemu/qemu_driver.c |2 +-
> >>  src/uml/uml_driver.c   |2 +-
> >>  src/util/iohelper.c|  203 +
> >>  src/xen/xen_driver.c   |2 +-
> >>  9 files changed, 402 insertions(+), 60 deletions(-)
> >>  create mode 100644 src/util/iohelper.c
> >>
> >> diff --git a/po/POTFILES.in b/po/POTFILES.in
> >> index 805e5ca..12adb3e 100644
> >> --- a/po/POTFILES.in
> >> +++ b/po/POTFILES.in
> >> @@ -94,6 +94,7 @@ src/util/event_poll.c
> >>  src/util/hash.c
> >>  src/util/hooks.c
> >>  src/util/hostusb.c
> >> +src/util/iohelper.c
> >>  src/util/interface.c
> >>  src/util/iptables.c
> >>  src/util/json.c
> >> diff --git a/src/Makefile.am b/src/Makefile.am
> >> index c3729a6..1d8115b 100644
> >> --- a/src/Makefile.am
> >> +++ b/src/Makefile.am
> >> @@ -380,6 +380,9 @@ STORAGE_DRIVER_DISK_SOURCES =  
> >> \
> >>  STORAGE_HELPER_DISK_SOURCES = \
> >>storage/parthelper.c
> >>  
> >> +UTIL_IO_HELPER_SOURCES =  \
> >> +  util/iohelper.c
> >> +
> >>  # Network filters
> >>  NWFILTER_DRIVER_SOURCES = \
> >>nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c   \
> >> @@ -1203,6 +1206,15 @@ EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
> >>  
> >>  libexec_PROGRAMS =
> >>  
> >> +libexec_PROGRAMS += libvirt_iohelper
> >> +libvirt_iohelper_SOURCES = $(UTIL_IO_HELPER_SOURCES)
> >> +libvirt_iohelper_LDFLAGS = $(WARN_LDFLAGS) $(AM_LDFLAGS)
> >> +libvirt_iohelper_LDADD =  \
> >> +  libvirt_util.la \
> >> +  ../gnulib/lib/libgnu.la
> >> +
> >> +libvirt_iohelper_CFLAGS = $(AM_CFLAGS)
> >> +
> >> 
> >
> > Is libvirt_iohelper for libvirtd?
> >
> > libvirt_iohelper is provided by libvirt-.rpm, but we still install 
> > it
> > when we build withoud libvirtd. We will meet the following problems:
> >
> > Checking for unpackaged file(s): /usr/lib/rpm/check-files 
> > /home/wency/rpmbuild/BUILDROOT/libvirt-0.9.0-1.el6.x86_64
> > error: Installed (but unpackaged) file(s) found:
> >/usr/libexec/libvirt_iohelper
> >   
> 
> I met the same problem and added libvirt-iohelper to our client
> package.  Is it used in client-only configuration?

It is only currently used in the QEMU or storage drivers, which are
both daemon based.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] patch review tool for libvirt patches?

[Daniel P. Berrange]

On Thu, Apr 07, 2011 at 11:24:04AM -0400, Laine Stump wrote:
> Now that 0.9.0 is out, I'd like to ask everyone's opinions about
> patch review tools.
> 
> I've been noticing lately that the volume of libvirt patches has
> increased significantly, and it's getting more and more difficult
> for me to keep up with reading the traffic, much less doing my part
> to review the patches. It would be very helpful if I could just see
> a view of pending patches, each properly colorcoded (or, even
> better, hidden) based on a patch being ACKed, deprecated, someone
> else in process of reviewing, etc. I know there are a few tools
> available, but I've never used any of them and wonder if anyone else
> has, and if they might be able to make a recommendation on something
> the libvirt project could use (or maybe something I could just use
> myself by sending a list feed into an application).
> 
> I'm sending this message 1) to see if others are feeling the
> pressure of the extra traffic too (or is my brain just processing
> more slowly :-/), and 2) to learn what your opinions are of setting
> up such a system for libvirt (for *optional* use only), and any
> opinions you have on what's available (or maybe you could provide a
> recipe for how you already manage all the patches without going
> crazy).
> 
> Here's my list of requirements; feel free to add/shoot down:
> 
> 1) usage must be optional, so it must be able to update itself via
> monitoring messages on the list (a minimal amount of change/addition
> to the current message flow might be acceptable, but nothing major,
> and encountering PATCH/review/ACK messages as currently sent
> shouldn't make it blow up).
> 
> 2) keep track of patches that are posted, NACKed, ACKed, re-posted
> (deprecating the original), and pushed (this should be done by
> monitoring git, not by looking at emails, as I've noticed patches
> are often pushed without a corresponding message to the list).
> 
> 3) would also be nice if there was a place in the UI that those
> wanting to could "take" a patch for review so that two people didn't
> spend a lot of time on something not requiring that much attention,
> at the expense of other ignored patches.
> 
> 4) maintain patchsets according to the "n/n" notation in the header
> (I mention this because one comment I saw about Gerrit said that it
> didn't do this).
> 
> 5) provide some sort of interface for viewing the patch, annotating
> it with comments, and sending that back to the list as an ACK/NACK
> or simply a comment.
> 
> 6) provide a simple way to save a patchset to files that can be "git
> am"ed (or maybe even do it for you, automatically creating a branch
> first, etc. This could possibly even be extended into a command to
> push a given patchset)
> 
> 7) (of course it must be open source. Do I even need to say that? :-))
> 
> Ideally, a person wanting to use this system should be able to setup
> their email to filter all libvir-list traffic containing "PATCH" in
> the subject line, then create an account on the patch review system
> and handle all patch review via the tool's interface
> 
> Here's a list of tool that Rich Jones came up with in another
> discussion on the same topic, along with a few others that were
> mentioned in the ensuing discussion. To the right, I've included
> comments from others that seemed interesting to me. Please point out
> any that you disagree with!
> 
> Gerrit (https://code.google.com/p/gerrit) "The assumption that one issue == 
> one patch is too deeply embedded"
> 
> Kiln (http://www.fogcreek.com/kiln/) (proprietary, so probably shouldn't be 
> considered)
> 
> Patchwork (http://ozlabs.org/~jk/projects/patchwork/) (used by Linux kernel&  
> kvm maintainers)
> 
> 
> http://www.reviewboard.org/  "didn't work well with git"
> 
> 
> Also someone pointed out that gitorious has code review aids:
> 
> http://blog.gitorious.org/2009/11/06/awesome-code-review/
> 
> 
> If I were going to investigate one of these and try setting it up,
> which do you think would have the greatest likelyhood of success
> (if any)?

For me, any tool which requires visiting a web UI to submit or view
patch code review comments/feedback is a non-starter. All code review
feedback must be on the mailing list, and correctly threaded.
In other words, it would be a tool which serves a 'reporting' or
'tracking' patch series, not a code review management system.
AFAICT, patchwork is the only one expressly designed in this
manner. Their website sums it up nicely:

  "patchwork should supplement mailing lists, not replace them

   Patchwork isn't intended to replace a community mailing list;
   that's why you can't comment on a patch in patchwork. If this
   were the case, then there would be two forums of discussion
   on patches, which fragments the patch review process. Developers
   who don't use patchwork would get left out of the discussion."

To also add to that, public mailing lists are a very good archival
system for code review / di

Re: [libvirt] [PATCH] docs: add an IPv6 address to network XML examples

[Laine Stump]

On 04/07/2011 11:41 AM, Eric Blake wrote:

On 04/07/2011 08:54 AM, Laine Stump wrote:

It was just pointed out that, although I added documentation for the
IPv6 additions to the network XML, I neglected to use those additions
in the xamples. This patch adds an IPv6 address to each of the

s/xamples/examples/


examples except for the "default" network, since that is a faithful
reproduction of the default network config that's automatically
installed, which doesn't include any IPv6 address (for good reason -
because there is no such thing as IPv6 NAT, there is no one IPv6
address that would work for all installations).
---
  docs/formatnetwork.html.in |4 
  1 files changed, 4 insertions(+), 0 deletions(-)

ACK.


Thanks. Fixed and pushed.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/6] Enhance the streams helper to support plain file I/O

[Jim Fehlig]

Wen Congyang wrote:
> At 03/24/2011 01:36 AM, Daniel P. Berrange Write:
>   
>> The O_NONBLOCK flag doesn't work as desired on plain files
>> or block devices. Introduce an I/O helper program that does
>> the blocking I/O operations, communicating over a pipe that
>> can support O_NONBLOCK
>>
>> * src/fdstream.c, src/fdstream.h: Add non-blocking I/O
>>   on plain files/block devices
>> * src/Makefile.am, src/util/iohelper.c: I/O helper program
>> * src/qemu/qemu_driver.c, src/lxc/lxc_driver.c,
>>   src/uml/uml_driver.c, src/xen/xen_driver.c: Update for
>>   streams API change
>> ---
>>  po/POTFILES.in |1 +
>>  src/Makefile.am|   12 +++
>>  src/fdstream.c |  233 
>> 
>>  src/fdstream.h |5 +
>>  src/lxc/lxc_driver.c   |2 +-
>>  src/qemu/qemu_driver.c |2 +-
>>  src/uml/uml_driver.c   |2 +-
>>  src/util/iohelper.c|  203 +
>>  src/xen/xen_driver.c   |2 +-
>>  9 files changed, 402 insertions(+), 60 deletions(-)
>>  create mode 100644 src/util/iohelper.c
>>
>> diff --git a/po/POTFILES.in b/po/POTFILES.in
>> index 805e5ca..12adb3e 100644
>> --- a/po/POTFILES.in
>> +++ b/po/POTFILES.in
>> @@ -94,6 +94,7 @@ src/util/event_poll.c
>>  src/util/hash.c
>>  src/util/hooks.c
>>  src/util/hostusb.c
>> +src/util/iohelper.c
>>  src/util/interface.c
>>  src/util/iptables.c
>>  src/util/json.c
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index c3729a6..1d8115b 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -380,6 +380,9 @@ STORAGE_DRIVER_DISK_SOURCES =
>> \
>>  STORAGE_HELPER_DISK_SOURCES =   \
>>  storage/parthelper.c
>>  
>> +UTIL_IO_HELPER_SOURCES =\
>> +util/iohelper.c
>> +
>>  # Network filters
>>  NWFILTER_DRIVER_SOURCES =   \
>>  nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c   \
>> @@ -1203,6 +1206,15 @@ EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
>>  
>>  libexec_PROGRAMS =
>>  
>> +libexec_PROGRAMS += libvirt_iohelper
>> +libvirt_iohelper_SOURCES = $(UTIL_IO_HELPER_SOURCES)
>> +libvirt_iohelper_LDFLAGS = $(WARN_LDFLAGS) $(AM_LDFLAGS)
>> +libvirt_iohelper_LDADD =\
>> +libvirt_util.la \
>> +../gnulib/lib/libgnu.la
>> +
>> +libvirt_iohelper_CFLAGS = $(AM_CFLAGS)
>> +
>> 
>
> Is libvirt_iohelper for libvirtd?
>
> libvirt_iohelper is provided by libvirt-.rpm, but we still install it
> when we build withoud libvirtd. We will meet the following problems:
>
> Checking for unpackaged file(s): /usr/lib/rpm/check-files 
> /home/wency/rpmbuild/BUILDROOT/libvirt-0.9.0-1.el6.x86_64
> error: Installed (but unpackaged) file(s) found:
>/usr/libexec/libvirt_iohelper
>   

I met the same problem and added libvirt-iohelper to our client
package.  Is it used in client-only configuration?

Regards,
Jim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/4] Add domainSet/GetSchedulerParameters to libxl driver

[Jim Fehlig]

Markus Groß wrote:
> Libxenlight currently only supports the credit scheduler.
> Therefore setting or getting a parameter of other
> schedulers raise an error (for now).
> ---
>  src/libxl/libxl_driver.c |  166 
> +-
>  1 files changed, 164 insertions(+), 2 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index e95c403..c760a23 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -2286,6 +2286,168 @@ cleanup:
>  }
>  
>  static int
> +libxlDomainGetSchedulerParameters(virDomainPtr dom, virSchedParameterPtr 
> params,
> +  int * nparams)
>   

In most libvirt code that would be 'int *nparams'.

> +{
> +libxlDriverPrivatePtr driver = dom->conn->privateData;
> +libxlDomainObjPrivatePtr priv;
> +virDomainObjPtr vm;
> +libxl_sched_credit sc_info;
> +int sched_id;
> +int ret = -1;
> +
> +libxlDriverLock(driver);
> +vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> +libxlDriverUnlock(driver);
> +
> +if (!vm) {
> +libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching 
> uuid"));
> +goto cleanup;
> +}
> +
> +if (!virDomainObjIsActive(vm)) {
> +libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not 
> running"));
> +goto cleanup;
> +}
>   

Is this API restricted to active domains only?  I suppose schedule
parameters are classified as runtime tunables, and not defined in
persistent config.

> +
> +priv = vm->privateData;
> +
> +if ((sched_id = libxl_get_sched_id(&priv->ctx)) < 0) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed to get scheduler id for domain '%d'"
> + " with libxenlight"), dom->id);
> +goto cleanup;
> +}
> +
> +if (sched_id != XEN_SCHEDULER_CREDIT) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("Only 'credit' scheduler is supported"));
> +goto cleanup;
> +}
> +
> +if (*nparams != XEN_SCHED_CREDIT_NPARAM) {
> +libxlError(VIR_ERR_INVALID_ARG, "%s", _("Invalid parameter count"));
> +goto cleanup;
> +}
> +
> +if (libxl_sched_credit_domain_get(&priv->ctx, dom->id, &sc_info) != 0) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed to get scheduler parameters for domain '%d'"
> + " with libxenlight"), dom->id);
> +goto cleanup;
> +}
> +
> +params[0].value.ui = sc_info.weight;
> +params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT;
>   

The libxl_sched_credit fields are int, but treated as unsigned
internally.  Using UINT plays nicely with "legacy" weight and cap
handling in virsh and is the correct type to use here IMO.

ACK.  I fixed the whitespace issue noted above and pushed.  Thanks Markus.

Regards,
Jim


> +if (virStrcpyStatic(params[0].field, "weight") == NULL) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   "%s", _("Field weight too long for destination"));
> +goto cleanup;
> +}
> +
> +params[1].value.ui = sc_info.cap;
> +params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT;
> +if (virStrcpyStatic(params[1].field, "cap") == NULL) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   "%s", _("Field cap too long for destination"));
> +goto cleanup;
> +}
> +
> +ret = 0;
> +
> +cleanup:
> +if (vm)
> +virDomainObjUnlock(vm);
> +return ret;
> +}
> +
> +static int
> +libxlDomainSetSchedulerParameters(virDomainPtr dom, virSchedParameterPtr 
> params,
> +  int nparams)
> +{
> +libxlDriverPrivatePtr driver = dom->conn->privateData;
> +libxlDomainObjPrivatePtr priv;
> +virDomainObjPtr vm;
> +libxl_sched_credit sc_info;
> +int sched_id;
> +int i;
> +int ret = -1;
> +
> +libxlDriverLock(driver);
> +vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> +libxlDriverUnlock(driver);
> +
> +if (!vm) {
> +libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching 
> uuid"));
> +goto cleanup;
> +}
> +
> +if (!virDomainObjIsActive(vm)) {
> +libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not 
> running"));
> +goto cleanup;
> +}
> +
> +priv = vm->privateData;
> +
> +if ((sched_id = libxl_get_sched_id(&priv->ctx)) < 0) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed to get scheduler id for domain '%d'"
> + " with libxenlight"), dom->id);
> +goto cleanup;
> +}
> +
> +if (sched_id != XEN_SCHEDULER_CREDIT) {
> +libxlError(VIR_ERR_INTERNAL_ERROR,
> +   _("Only 'credit' scheduler is supported"));
> +goto cleanup;
> +}
> +
> +if (nparams != XEN_SCHED_CREDIT_NPARAM) {
> +libxlError(VIR_ERR_INVALID_ARG, "%s", _("Invalid parameter count"));
> +  

Re: [libvirt] [PATCH] docs: add an IPv6 address to network XML examples

[Eric Blake]

On 04/07/2011 08:54 AM, Laine Stump wrote:
> It was just pointed out that, although I added documentation for the
> IPv6 additions to the network XML, I neglected to use those additions
> in the xamples. This patch adds an IPv6 address to each of the

s/xamples/examples/

> examples except for the "default" network, since that is a faithful
> reproduction of the default network config that's automatically
> installed, which doesn't include any IPv6 address (for good reason -
> because there is no such thing as IPv6 NAT, there is no one IPv6
> address that would work for all installations).
> ---
>  docs/formatnetwork.html.in |4 
>  1 files changed, 4 insertions(+), 0 deletions(-)

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] patch review tool for libvirt patches?

[Laine Stump]

Now that 0.9.0 is out, I'd like to ask everyone's opinions about patch 
review tools.


I've been noticing lately that the volume of libvirt patches has 
increased significantly, and it's getting more and more difficult for me 
to keep up with reading the traffic, much less doing my part to review 
the patches. It would be very helpful if I could just see a view of 
pending patches, each properly colorcoded (or, even better, hidden) 
based on a patch being ACKed, deprecated, someone else in process of 
reviewing, etc. I know there are a few tools available, but I've never 
used any of them and wonder if anyone else has, and if they might be 
able to make a recommendation on something the libvirt project could use 
(or maybe something I could just use myself by sending a list feed into 
an application).


I'm sending this message 1) to see if others are feeling the pressure of 
the extra traffic too (or is my brain just processing more slowly :-/), 
and 2) to learn what your opinions are of setting up such a system for 
libvirt (for *optional* use only), and any opinions you have on what's 
available (or maybe you could provide a recipe for how you already 
manage all the patches without going crazy).


Here's my list of requirements; feel free to add/shoot down:

1) usage must be optional, so it must be able to update itself via 
monitoring messages on the list (a minimal amount of change/addition to 
the current message flow might be acceptable, but nothing major, and 
encountering PATCH/review/ACK messages as currently sent shouldn't make 
it blow up).


2) keep track of patches that are posted, NACKed, ACKed, re-posted 
(deprecating the original), and pushed (this should be done by 
monitoring git, not by looking at emails, as I've noticed patches are 
often pushed without a corresponding message to the list).


3) would also be nice if there was a place in the UI that those wanting 
to could "take" a patch for review so that two people didn't spend a lot 
of time on something not requiring that much attention, at the expense 
of other ignored patches.


4) maintain patchsets according to the "n/n" notation in the header (I 
mention this because one comment I saw about Gerrit said that it didn't 
do this).


5) provide some sort of interface for viewing the patch, annotating it 
with comments, and sending that back to the list as an ACK/NACK or 
simply a comment.


6) provide a simple way to save a patchset to files that can be "git 
am"ed (or maybe even do it for you, automatically creating a branch 
first, etc. This could possibly even be extended into a command to push 
a given patchset)


7) (of course it must be open source. Do I even need to say that? :-))

Ideally, a person wanting to use this system should be able to setup 
their email to filter all libvir-list traffic containing "PATCH" in the 
subject line, then create an account on the patch review system and 
handle all patch review via the tool's interface


Here's a list of tool that Rich Jones came up with in another discussion 
on the same topic, along with a few others that were mentioned in the 
ensuing discussion. To the right, I've included comments from others 
that seemed interesting to me. Please point out any that you disagree with!


Gerrit (https://code.google.com/p/gerrit) "The assumption that one issue == one 
patch is too deeply embedded"

Kiln (http://www.fogcreek.com/kiln/) (proprietary, so probably shouldn't be 
considered)

Patchwork (http://ozlabs.org/~jk/projects/patchwork/) (used by Linux kernel&  
kvm maintainers)


http://www.reviewboard.org/  "didn't work well with git"


Also someone pointed out that gitorious has code review aids:

http://blog.gitorious.org/2009/11/06/awesome-code-review/


If I were going to investigate one of these and try setting it up, which do you 
think would have the greatest likelyhood of success (if any)?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: fix mingw startup

[Laine Stump]

On 04/06/2011 03:47 PM, Eric Blake wrote:

* .gnulib: Update to latest, for pipe2.
* bootstrap.conf (gnulib_modules): Add pipe2.
* src/util/event_poll.c (virEventPollInit): Use it, to avoid
problematic virSetCloseExec on mingw.
---

The gnulib changes to pipe2 are now in.

* .gnulib f796520...2255b86 (4):
   >  verify: use _Static_assert if available
   >  Remove leftover generated .h files after config.status changed.
   >  Ensure to rebuild generated .h files when config.status has changed.
   >  pipe2: Relicense under LGPLv2+.

This assumes that my earlier .gnulib modification patch is also in:
https://www.redhat.com/archives/libvir-list/2011-April/msg00297.html
although I can rebase as needed.

  .gnulib   |2 +-
  bootstrap.conf|3 ++-
  src/util/event_poll.c |7 ++-
  3 files changed, 5 insertions(+), 7 deletions(-)


ACK.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v5] qemu: Remove the managed state file only if restoring succeeded

[Eric Blake]

On 04/06/2011 08:31 PM, Osier Yang wrote:
>  managed_save = qemuDomainManagedSavePath(driver, vm);
> +
> +if (!managed_save)
> +goto cleanup;
> +
>  if ((managed_save) && (virFileExists(managed_save))) {

This second check for non-NULL managed_save is now redundant.

> +++ b/tools/virsh.pod
> @@ -546,7 +546,11 @@ I parameter in the domain's XML definition.
> 
>  =item B I
> 
> -Restores a domain from an B state file.  See I for more 
> info.
> +Restores a domain from an B state file. See I for more 
> info.

Sorry for not noticing sooner, but we could do some grammar cleanups.

Does this look like a reasonable followup?

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index a84780b..0734a76 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -3430,7 +3430,7 @@ static int qemudDomainObjStart(virConnectPtr conn,
 if (!managed_save)
 goto cleanup;

-if ((managed_save) && (virFileExists(managed_save))) {
+if (virFileExists(managed_save)) {
 ret = qemuDomainObjRestore(conn, driver, vm, managed_save);

 if ((ret == 0) && (unlink(managed_save) < 0))
diff --git i/tools/virsh.pod w/tools/virsh.pod
index 6319373..9ce6905 100644
--- i/tools/virsh.pod
+++ w/tools/virsh.pod
@@ -546,11 +546,12 @@ I parameter in the domain's XML definition.

 =item B I

-Restores a domain from an B state file. See I for
more info.
+Restores a domain from a B state file. See I for more
info.

 B: To avoid corrupting file system contents within the domain, you
-should not reuse the saved state file to B unless you are
convinced
-with reverting the domain to the previous state.
+should not reuse the saved state file for a second B unless
+you have also reverted all storage volumes back to the same contents
+as when the state file was created.

 =item B I I



-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] docs: add an IPv6 address to network XML examples

[Laine Stump]

It was just pointed out that, although I added documentation for the
IPv6 additions to the network XML, I neglected to use those additions
in the xamples. This patch adds an IPv6 address to each of the
examples except for the "default" network, since that is a faithful
reproduction of the default network config that's automatically
installed, which doesn't include any IPv6 address (for good reason -
because there is no such thing as IPv6 NAT, there is no one IPv6
address that would work for all installations).
---
 docs/formatnetwork.html.in |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index c6969eb..589aaff 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -121,6 +121,7 @@
 
   
 
+
   
 
 
@@ -226,6 +227,7 @@
 
   
 
+
   
 
 Routed network config
@@ -249,6 +251,7 @@
 
   
 
+
   
 
 Isolated network config
@@ -270,6 +273,7 @@
 
   
 
+
   
 
   
-- 
1.7.3.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] [RFC] PPC Linux: Fix some problems

[Stefan Berger]

 The following patch fixes some problems on powerpc Linux machines when 
trying to start an x86-based VM using qemu.


In the order as the hunks appear the fix the following problems:

- Qemu on (my) powerpc machine for some reason takes a long time for it 
to create its UnixIO socket. It's like 9 seconds or so and probably more 
once on a busy system. So increase the timeout for waiting for that 
socket to appear.
- VM saved images written on an x86 host have a header in little endian 
format. So, when checking the 32 bit int for the header version and if 
the version is out-of-range swap the endianess of the header and try 
again and only then fail. This also then works when the big endian host 
has written the header in big endian format and now the VM is supposed 
to resume on a little endina host.
- Setting the CPU affinity doesn't seem to work at all on (my) PPC host. 
So rather than failing the start of a VM when nodeGetInfo/virNodeGetInfo 
fail, make the setting of the CPU affinity pass on powerpc machines so 
the VM at least starts.


Signed-off-by: Stefan Berger 

---
 src/qemu/qemu_driver.c  |   15 +++
 src/qemu/qemu_monitor.c |4 
 src/qemu/qemu_process.c |   10 +-
 3 files changed, 28 insertions(+), 1 deletion(-)

Index: libvirt/src/qemu/qemu_monitor.c
===
--- libvirt.orig/src/qemu/qemu_monitor.c
+++ libvirt/src/qemu/qemu_monitor.c
@@ -249,7 +249,11 @@ qemuMonitorOpenUnix(const char *monitor)
 {
 struct sockaddr_un addr;
 int monfd;
+#if defined(__powerpc64__) || defined(__powerpc__)
+int timeout = 30; /* In seconds */
+#else
 int timeout = 3; /* In seconds */
+#endif
 int ret, i = 0;

 if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
Index: libvirt/src/qemu/qemu_driver.c
===
--- libvirt.orig/src/qemu/qemu_driver.c
+++ libvirt/src/qemu/qemu_driver.c
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 


 #include "qemu_driver.h"
@@ -1882,6 +1883,15 @@ struct qemud_save_header {
 int unused[15];
 };

+static inline void
+bswap_header(struct qemud_save_header *hdr) {
+hdr->version = bswap_32(hdr->version);
+hdr->xml_len = bswap_32(hdr->xml_len);
+hdr->was_running = bswap_32(hdr->was_running);
+hdr->compressed = bswap_32(hdr->compressed);
+}
+
+
 /* return -errno on failure, or 0 on success */
 static int
 qemuDomainSaveHeader(int fd, const char *path, char *xml,
@@ -3091,6 +3101,11 @@ qemuDomainSaveImageOpen(struct qemud_dri
 }

 if (header.version > QEMUD_SAVE_VERSION) {
+/* try to convert endianess */
+bswap_header(&header);
+}
+
+if (header.version > QEMUD_SAVE_VERSION) {
 qemuReportError(VIR_ERR_OPERATION_FAILED,
 _("image version is not supported (%d > %d)"),
 header.version, QEMUD_SAVE_VERSION);
Index: libvirt/src/qemu/qemu_process.c
===
--- libvirt.orig/src/qemu/qemu_process.c
+++ libvirt/src/qemu/qemu_process.c
@@ -1239,7 +1239,11 @@ qemuProcessInitCpuAffinity(virDomainObjP
 VIR_DEBUG0("Setting CPU affinity");

 if (nodeGetInfo(NULL, &nodeinfo) < 0)
+#if defined (__powerpc64__) || defined (__powerpc__)
+return 0;
+#else
 return -1;
+#endif

 /* setaffinity fails if you set bits for CPUs which
  * aren't present, so we have to limit ourselves */
@@ -1295,7 +1299,11 @@ qemuProcessSetVcpuAffinites(virConnectPt
 int vcpu, cpumaplen, hostcpus, maxcpu;

 if (virNodeGetInfo(conn, &nodeinfo) != 0) {
-return  -1;
+#if defined (__powerpc64__) || defined (__powerpc__)
+return 0;
+#else
+return -1;
+#endif
 }

 if (!def->cputune.nvcpupin)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] qemu: Add timeout for monitor to avoid virsh getting stuck when monitor gets die.

[Osier Yang]

于 2011年04月07日 18:17, Jiri Denemark 写道:

On Thu, Apr 07, 2011 at 18:04:19 +0800, Osier Yang wrote:

于 2011年04月06日 16:51, Jiri Denemark 写道:

This is not the right approach. Introducing a timeout into all monitor command
send to qemu is a bad thing. I think the right approach is to have a simple
API which would just return domain's state without talking to its monitor or
doing other complicated stuff. The new API could then be used by virsh list to
list all domains even though they are blocked on qemu monitor. After all,
virsh is not really interested in current memory consumption of domains when
listing them.



IMHO this won't work, if you restart libvirtd after the monitor gets
dead, you even can't get connection to libvirtd anymore, see:

http://www.redhat.com/archives/libvir-list/2011-March/msg01422.html

How to reproduce:
1. setup a virt guest with qemu-kvm, and start it
2. stop qemu process with following:
 # kill -STOP  `ps aux | grep qemu | grep -v grep | awk '{print $2}'`
3. run the following command:
4. # service libvirtd restart
5. # virsh (hangs here)


Yes, but that's a separate issue, IMHO. virsh hangs at this point since
libvirtd doesn't even get to the point when it is able to accept connections
from client. So introducing a timeout to some specific operations to help
solve this issue might be the right approach but doing all commands with
timeout is certainly not what we should do. Daniel described all the issues
very nicely in his detailed email.


Oh, I didn't mean your proposal won't work for "virsh list", that will
work well I guess. :)

I meant if we consider "virsh list can't work if the monitor dies" as
a problem, then we need to solve the even more bad problem ---
libvirtd can't accept connection anymore if it's restarted.

Regards
Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] qemu: Add timeout for monitor to avoid virsh getting stuck when monitor gets die.

[Jiri Denemark]

On Thu, Apr 07, 2011 at 18:04:19 +0800, Osier Yang wrote:
> 于 2011年04月06日 16:51, Jiri Denemark 写道:
> > This is not the right approach. Introducing a timeout into all monitor 
> > command
> > send to qemu is a bad thing. I think the right approach is to have a simple
> > API which would just return domain's state without talking to its monitor or
> > doing other complicated stuff. The new API could then be used by virsh list 
> > to
> > list all domains even though they are blocked on qemu monitor. After all,
> > virsh is not really interested in current memory consumption of domains when
> > listing them.
> >
> 
> IMHO this won't work, if you restart libvirtd after the monitor gets
> dead, you even can't get connection to libvirtd anymore, see:
> 
> http://www.redhat.com/archives/libvir-list/2011-March/msg01422.html
> 
> How to reproduce:
> 1. setup a virt guest with qemu-kvm, and start it
> 2. stop qemu process with following:
> # kill -STOP  `ps aux | grep qemu | grep -v grep | awk '{print $2}'`
> 3. run the following command:
> 4. # service libvirtd restart
> 5. # virsh (hangs here)

Yes, but that's a separate issue, IMHO. virsh hangs at this point since
libvirtd doesn't even get to the point when it is able to accept connections
from client. So introducing a timeout to some specific operations to help
solve this issue might be the right approach but doing all commands with
timeout is certainly not what we should do. Daniel described all the issues
very nicely in his detailed email.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] qemu: Add timeout for monitor to avoid virsh getting stuck when monitor gets die.

[Osier Yang]

于 2011年04月06日 16:51, Jiri Denemark 写道:

On Wed, Apr 06, 2011 at 12:13:10 +0800, Mark Wu wrote:

Hello Guys,

When the qemu process becomes hung,  virsh will get stuck on the hung
guest and can't move forward.  It can be reproduced by the following steps:

1. setup a virt guest with qemu-kvm, and start it
2. stop qemu process with following:
   kill -STOP  `ps aux | grep qemu | grep -v grep | awk '{print $2}'`
3. run the following command:
virsh list

I think we can add a timeout for qemu monitor to resolve this problem:
using virCondWaitUntil instead of virCondWait in qemuMonitorSend. What's
your opinions?


This is not the right approach. Introducing a timeout into all monitor command
send to qemu is a bad thing. I think the right approach is to have a simple
API which would just return domain's state without talking to its monitor or
doing other complicated stuff. The new API could then be used by virsh list to
list all domains even though they are blocked on qemu monitor. After all,
virsh is not really interested in current memory consumption of domains when
listing them.



IMHO this won't work, if you restart libvirtd after the monitor gets
dead, you even can't get connection to libvirtd anymore, see:

http://www.redhat.com/archives/libvir-list/2011-March/msg01422.html

How to reproduce:
1. setup a virt guest with qemu-kvm, and start it
2. stop qemu process with following:
   # kill -STOP  `ps aux | grep qemu | grep -v grep | awk '{print $2}'`
3. run the following command:
4. # service libvirtd restart
5. # virsh (hangs here)


The new API could also provide a reason which lead to current state so that
one can see the reason even without watching for libvirt events.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/6] use virObject to manage reference-count of virDomainObj

[Hu Tao]

On Wed, Apr 06, 2011 at 04:33:56PM -0600, Eric Blake wrote:
> On 04/06/2011 01:19 AM, Hu Tao wrote:
> > This patch also eliminates a dead-lock bug in
> > qemuDomainObjBeginJobWithDriver: if virCondWaitUntil() timeouts, the
> > thread tries to acquire qemu driver lock while holding virDomainObj
> > lock.
> 
> Let's please separate that bug fix into a separate patch which gets
> applied as soon as possible.
> 
> > ---
> >  src/conf/domain_conf.c|   56 
> >  src/conf/domain_conf.h|6 +-
> >  src/openvz/openvz_conf.c  |8 +-
> >  src/qemu/qemu_domain.c|   32 ++---
> >  src/qemu/qemu_domain.h|2 +-
> >  src/qemu/qemu_driver.c|  304 
> > -
> >  src/qemu/qemu_migration.c |   45 +++
> >  src/qemu/qemu_process.c   |   33 ++---
> >  src/vmware/vmware_conf.c  |2 +-
> >  9 files changed, 215 insertions(+), 273 deletions(-)
> 
> >  int virDomainObjListInit(virDomainObjListPtr doms)
> > @@ -437,7 +436,7 @@ virDomainObjPtr virDomainFindByID(const 
> > virDomainObjListPtr doms,
> >  virDomainObjPtr obj;
> >  obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id);
> >  if (obj)
> > -virDomainObjLock(obj);
> > +virDomainObjRef(obj);
> 
> Wow - changing the semantics so the object is not locked by default,
> just referenced.
> 
> > @@ -1010,6 +1007,11 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr 
> > caps)
> >  return NULL;
> >  }
> >  
> > +if (virObjectInit(&domain->obj, doDomainObjFree)) {
> > +VIR_FREE(domain);
> > +return NULL;
> 
> Hmm.  virObjectInit used VIR_ERROR, which logs, but doesn't call into
> virtError.  By returning NULL here, a user will get the dreaded "Unknown
> error" message since we didn't hook into the virterror machinery.
> Should virObjectInit instead be using virReportErrorHelper?  Or is it

How to use virReportErrorHelper in this case?

> considered a coding bug to ever have virObjectInit fail in the first
> place, so we don't really have to worry about that.

Sorry for my bad English, I don't understand this sentence.

> 
> > @@ -1075,8 +1075,10 @@ virDomainObjPtr virDomainAssignDef(virCapsPtr caps,
> >  domain->def = def;
> >  
> >  virUUIDFormat(def->uuid, uuidstr);
> > +virDomainObjRef(domain);
> >  if (virHashAddEntry(doms->objs, uuidstr, domain) < 0) {
> 
> It seems like incrementing ref count on container addition and
> decrementing it on removal are common actions.  Would it simplify code
> any by making a wrapper for virHashAddEntry:
> 
> int virHashAddObjEntry(virHashTablePtr, const void *name, virObjectPtr
> *data)
> 
> which does the referencing as part of adding/removing a virObject from a
> hash table, rather than making all callers track it?

Agreed. But this needs many changes I think it is better to do it in
a seperate patch.

> 
> The only potential drawback to that is that if you use the wrong
> function, the referencing doesn't happen.  Or maybe even make
> virHashCreateFull take a bool parameter of whether the data must be a
> virObjectPtr, so you don't have to wrap virHashAddObjEntry (and since
> virHashRemoveEntry doesn't really have any way to create a
> virHashRemoveObjEntry wrapper, but it would need to be in on the game of
> automatic reference count manipulations any time we know the table
> hashes only virObjects as data).

Would it be better to have two types of hashtable, one hashes only
virObjects, the other hashes data except virObjects? this can minimize
the impact brought by the change of hashtable to existing code.

> 
> > @@ -1149,9 +1151,7 @@ virDomainObjGetPersistentDef(virCapsPtr caps,
> >  }
> >  
> >  /*
> > - * The caller must hold a lock  on the driver owning 'doms',
> > - * and must also have locked 'dom', to ensure no one else
> > - * is either waiting for 'dom' or still usingn it
> > + * The caller must hold a lock  on the driver owning 'doms'.
> 
> While touching that line, fix the spacing:
> s/lock  on/lock on/

OK.

> 
> > @@ -1159,9 +1159,8 @@ void virDomainRemoveInactive(virDomainObjListPtr doms,
> >  char uuidstr[VIR_UUID_STRING_BUFLEN];
> >  virUUIDFormat(dom->def->uuid, uuidstr);
> >  
> > -virDomainObjUnlock(dom);
> > -
> >  virHashRemoveEntry(doms->objs, uuidstr);
> > +virDomainObjUnref(dom);
> 
> Hmm, this means you are dereferencing dom->def->uuid while holding the
> driver lock but without holding the domain lock.  If there is another
> place in code that holds the domain lock but not the driver lock,
> couldn't that cause a bad read of dom->def?  I don't think you can
> blindly get rid of holding the lock on dom unless we make additional
> rules about which members of dom are safe to modify (for example,
> stating that dom->def cannot be modified unless you hold the driver
> lock), but that's hard to audit.  :(

Don't understand you very well here. Let me explain what I understand:
If a dom is added into a hashtable, then it is not safe to 

Re: [libvirt] [PATCH 4/6] qemu: use virObject to manage reference-counting for qemu monitor

[Daniel P. Berrange]

On Wed, Apr 06, 2011 at 03:19:55PM +0800, Hu Tao wrote:
> ---
>  src/qemu/qemu_domain.c|   30 ++---
>  src/qemu/qemu_migration.c |2 -
>  src/qemu/qemu_monitor.c   |  109 
>  src/qemu/qemu_monitor.h   |4 +-
>  src/qemu/qemu_process.c   |   32 +++---
>  5 files changed, 81 insertions(+), 96 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 3a3c953..d11dc5f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -560,9 +560,8 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj)
>  {
>  qemuDomainObjPrivatePtr priv = obj->privateData;
>  
> -qemuMonitorLock(priv->mon);
> -qemuMonitorRef(priv->mon);
>  virDomainObjUnlock(obj);
> +qemuMonitorLock(priv->mon);
>  }
>  
>  
> @@ -573,18 +572,9 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj)
>  void qemuDomainObjExitMonitor(virDomainObjPtr obj)
>  {
>  qemuDomainObjPrivatePtr priv = obj->privateData;
> -int refs;
> -
> -refs = qemuMonitorUnref(priv->mon);
> -
> -if (refs > 0)
> -qemuMonitorUnlock(priv->mon);
>  
> +qemuMonitorUnlock(priv->mon);
>  virDomainObjLock(obj);
> -
> -if (refs == 0) {
> -priv->mon = NULL;
> -}
>  }
>  
>  
> @@ -601,10 +591,8 @@ void qemuDomainObjEnterMonitorWithDriver(struct 
> qemud_driver *driver,
>  {
>  qemuDomainObjPrivatePtr priv = obj->privateData;
>  
> -qemuMonitorLock(priv->mon);
> -qemuMonitorRef(priv->mon);
>  virDomainObjUnlock(obj);
> -qemuDriverUnlock(driver);
> +qemuMonitorLock(priv->mon);
>  }
>  
>  
> @@ -617,19 +605,9 @@ void qemuDomainObjExitMonitorWithDriver(struct 
> qemud_driver *driver,
>  virDomainObjPtr obj)
>  {
>  qemuDomainObjPrivatePtr priv = obj->privateData;
> -int refs;
> -
> -refs = qemuMonitorUnref(priv->mon);
> -
> -if (refs > 0)
> -qemuMonitorUnlock(priv->mon);
>  
> -qemuDriverLock(driver);
> +qemuMonitorUnlock(priv->mon);
>  virDomainObjLock(obj);
> -
> -if (refs == 0) {
> -priv->mon = NULL;
> -}
>  }

This means that the 'driver' lock is now whenever any QEMU
monitor command is runing, which blocks the entire driver.

>  
>  void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver,
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 462e6be..6af2e24 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -224,11 +224,9 @@ qemuMigrationWaitForCompletion(struct qemud_driver 
> *driver, virDomainObjPtr vm)
>  }
>  
>  virDomainObjUnlock(vm);
> -qemuDriverUnlock(driver);
>  
>  nanosleep(&ts, NULL);
>  
> -qemuDriverLock(driver);
>  virDomainObjLock(vm);
>  }

Holding the 'driver' lock while sleeping blocks the entire
QEMU driver.

> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 244b22a..4b9087f 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -107,7 +107,6 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon 
> ATTRIBUTE_UNUSED,
>  
>  VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name);
>  
> -qemuDriverLock(driver);
>  virDomainObjLock(vm);
>  
>  if (!virDomainObjIsActive(vm)) {
> @@ -133,15 +132,17 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon 
> ATTRIBUTE_UNUSED,
>  qemuProcessStop(driver, vm, 0);
>  qemuAuditDomainStop(vm, hasError ? "failed" : "shutdown");
>  
> -if (!vm->persistent)
> +if (!vm->persistent) {
> +qemuDriverLock(driver);
>  virDomainRemoveInactive(&driver->domains, vm);
> -else
> -virDomainObjUnlock(vm);
> +qemuDriverUnlock(driver);
> +}
> +
> +virDomainObjUnlock(vm);
>  
>  if (event) {
>  qemuDomainEventQueue(driver, event);
>  }
> -qemuDriverUnlock(driver);
>  }

This violates the lock ordering rules. The 'driver' lock *must* be
obtained *before* any 'vm' lock is held.

Now we have some places in the code which do

  lock(vm)
  lock(driver)

and other places which do

  lock(driver)
  lock(vm)

so 2 threads can trivially deadlock waiting for each other

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/6] use virObject to manage reference-count of virDomainObj

[Daniel P. Berrange]

On Wed, Apr 06, 2011 at 03:19:51PM +0800, Hu Tao wrote:
> This patch also eliminates a dead-lock bug in
> qemuDomainObjBeginJobWithDriver: if virCondWaitUntil() timeouts, the
> thread tries to acquire qemu driver lock while holding virDomainObj
> lock.
> ---
>  src/conf/domain_conf.c|   56 
>  src/conf/domain_conf.h|6 +-
>  src/openvz/openvz_conf.c  |8 +-
>  src/qemu/qemu_domain.c|   32 ++---
>  src/qemu/qemu_domain.h|2 +-
>  src/qemu/qemu_driver.c|  304 
> -
>  src/qemu/qemu_migration.c |   45 +++
>  src/qemu/qemu_process.c   |   33 ++---
>  src/vmware/vmware_conf.c  |2 +-
>  9 files changed, 215 insertions(+), 273 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 90a1317..fc76a00 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -47,6 +47,7 @@
>  #include "storage_file.h"
>  #include "files.h"
>  #include "bitmap.h"
> +#include "virobject.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_DOMAIN
>  
> @@ -395,9 +396,7 @@ static void
>  virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
>  {
>  virDomainObjPtr obj = payload;
> -virDomainObjLock(obj);
> -if (virDomainObjUnref(obj) > 0)
> -virDomainObjUnlock(obj);
> +virDomainObjUnref(obj);
>  }
>  
>  int virDomainObjListInit(virDomainObjListPtr doms)
> @@ -437,7 +436,7 @@ virDomainObjPtr virDomainFindByID(const 
> virDomainObjListPtr doms,
>  virDomainObjPtr obj;
>  obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id);
>  if (obj)
> -virDomainObjLock(obj);
> +virDomainObjRef(obj);
>  return obj;
>  }
>  
> @@ -452,7 +451,7 @@ virDomainObjPtr virDomainFindByUUID(const 
> virDomainObjListPtr doms,
>  
>  obj = virHashLookup(doms->objs, uuidstr);
>  if (obj)
> -virDomainObjLock(obj);
> +virDomainObjRef(obj);
>  return obj;
>  }
>  
> @@ -476,7 +475,7 @@ virDomainObjPtr virDomainFindByName(const 
> virDomainObjListPtr doms,
>  virDomainObjPtr obj;
>  obj = virHashSearch(doms->objs, virDomainObjListSearchName, name);
>  if (obj)
> -virDomainObjLock(obj);
> +virDomainObjRef(obj);
>  return obj;
>  }

This is a major change in semantics, which makes pretty much
every single caller non-thread safe, unless the callers are
all changed todo virDomainObjLock immediately after calling
this. So I don't really see the point in this - it just means
more code duplication.

>  
> @@ -967,6 +966,12 @@ static void virDomainObjFree(virDomainObjPtr dom)
>  {
>  if (!dom)
>  return;
> +virDomainObjUnref(dom);
> +}
> +
> +static void doDomainObjFree(virObjectPtr obj)
> +{
> +virDomainObjPtr dom = (virDomainObjPtr)obj;
>  
>  VIR_DEBUG("obj=%p", dom);
>  virDomainDefFree(dom->def);
> @@ -984,21 +989,13 @@ static void virDomainObjFree(virDomainObjPtr dom)
>  
>  void virDomainObjRef(virDomainObjPtr dom)
>  {
> -dom->refs++;
> -VIR_DEBUG("obj=%p refs=%d", dom, dom->refs);
> +virObjectRef(&dom->obj);
>  }
>  
>  
> -int virDomainObjUnref(virDomainObjPtr dom)
> +void virDomainObjUnref(virDomainObjPtr dom)
>  {
> -dom->refs--;
> -VIR_DEBUG("obj=%p refs=%d", dom, dom->refs);
> -if (dom->refs == 0) {
> -virDomainObjUnlock(dom);
> -virDomainObjFree(dom);
> -return 0;
> -}
> -return dom->refs;
> +virObjectUnref(&dom->obj);
>  }
>  
>  static virDomainObjPtr virDomainObjNew(virCapsPtr caps)
> @@ -1010,6 +1007,11 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr caps)
>  return NULL;
>  }
>  
> +if (virObjectInit(&domain->obj, doDomainObjFree)) {
> +VIR_FREE(domain);
> +return NULL;
> +}
> +
>  if (caps->privateDataAllocFunc &&
>  !(domain->privateData = (caps->privateDataAllocFunc)())) {
>  virReportOOMError();
> @@ -1027,9 +1029,7 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr caps)
>  return NULL;
>  }
>  
> -virDomainObjLock(domain);
>  domain->state = VIR_DOMAIN_SHUTOFF;
> -domain->refs = 1;
>  
>  virDomainSnapshotObjListInit(&domain->snapshots);
>  
> @@ -1075,8 +1075,10 @@ virDomainObjPtr virDomainAssignDef(virCapsPtr caps,
>  domain->def = def;
>  
>  virUUIDFormat(def->uuid, uuidstr);
> +virDomainObjRef(domain);
>  if (virHashAddEntry(doms->objs, uuidstr, domain) < 0) {
> -VIR_FREE(domain);
> +virDomainObjUnref(domain);
> +virDomainObjFree(domain);
>  return NULL;
>  }

Simiarly here, you're now requiring all callers to manually obtain
a lock.

> @@ -1239,39 +1243,57 @@ static virDomainPtr qemudDomainCreate(virConnectPtr 
> conn, const char *xml,
>  
>  qemuDriverLock(driver);
>  if (!(def = virDomainDefParseString(driver->caps, xml,
> -VIR_DOMAIN_XML_INACTIVE)))
> +VIR_DOMAIN_X

Re: [libvirt] [PATCH v5] qemu: Remove the managed state file only if restoring succeeded

[Osier Yang]

于 2011年04月07日 15:21, Daniel Veillard 写道:

On Thu, Apr 07, 2011 at 10:31:52AM +0800, Osier Yang wrote:

1) Both "qemuDomainStartWithFlags" and "qemuAutostartDomain" try to
restore the domain from managedsave'ed image if it exists (by
invoking "qemuDomainObjRestore"), but it unlinks the image even
if restoring fails, which causes data loss. (This problem exists
for "virsh managedsave dom; virsh start dom").

The fix for is to unlink the managed state file only if restoring
succeeded.

2) For "virsh save dom; virsh restore dom;", it can cause data
corruption if one reuse the saved state file for restoring. Add
doc to tell user about it.

3) In "qemuDomainObjStart", if "managed_save" is NULL, we shouldn't
fallback to start the domain, skipping it to cleanup as a incidental
fix. Discovered by Eric.

---
  src/qemu/qemu_driver.c |   12 +++-
  tools/virsh.pod|6 +-
  2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 48fe266..a84780b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3423,18 +3423,20 @@ static int qemudDomainObjStart(virConnectPtr conn,

  /*
   * If there is a managed saved state restore it instead of starting
- * from scratch. In any case the old state is removed.
+ * from scratch. The old state is removed once the restoring succeeded.
   */
  managed_save = qemuDomainManagedSavePath(driver, vm);
+
+if (!managed_save)
+goto cleanup;
+
  if ((managed_save)&&  (virFileExists(managed_save))) {
  ret = qemuDomainObjRestore(conn, driver, vm, managed_save);

-if (unlink(managed_save)<  0) {
+if ((ret == 0)&&  (unlink(managed_save)<  0))
  VIR_WARN("Failed to remove the managed state %s", managed_save);
-}

-if (ret == 0)
-goto cleanup;
+goto cleanup;
  }

  ret = qemuProcessStart(conn, driver, vm, NULL, start_paused, -1, NULL,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index f4bd294..6319373 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -546,7 +546,11 @@ I  parameter in the domain's XML definition.

  =item B  I

-Restores a domain from an B  state file.  See I  for more 
info.
+Restores a domain from an B  state file. See I  for more 
info.
+
+B: To avoid corrupting file system contents within the domain, you
+should not reuse the saved state file to B  unless you are convinced
+with reverting the domain to the previous state.

  =item B  I  I


   ACK, thanks !

Daniel



Thanks, applied.

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] docs: remove "returns" word from beginning of lines

[Jean-Baptiste Rouault]

Move "returns" keyword from beginning of API doc lines
when it does not describe return values.
Maybe the API doc extractor could be changed to look for
"returns: " to avoid such confusion.
---
 src/libvirt.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 85dfc58..344e921 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -10097,8 +10097,8 @@ error:
  *
  * The virDomainPtr object handle passed into the callback upon delivery
  * of an event is only valid for the duration of execution of the callback.
- * If the callback wishes to keep the domain object after the callback
- * returns, it shall take a reference to it, by calling virDomainRef.
+ * If the callback wishes to keep the domain object after the callback returns,
+ * it shall take a reference to it, by calling virDomainRef.
  * The reference can be released once the object is no longer required
  * by calling virDomainFree.
  *
@@ -12727,8 +12727,8 @@ error:
  *
  * The virDomainPtr object handle passed into the callback upon delivery
  * of an event is only valid for the duration of execution of the callback.
- * If the callback wishes to keep the domain object after the callback
- * returns, it shall take a reference to it, by calling virDomainRef.
+ * If the callback wishes to keep the domain object after the callback returns,
+ * it shall take a reference to it, by calling virDomainRef.
  * The reference can be released once the object is no longer required
  * by calling virDomainFree.
  *
-- 
1.7.0.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/6] Enhance the streams helper to support plain file I/O

[Wen Congyang]

At 03/24/2011 01:36 AM, Daniel P. Berrange Write:
> The O_NONBLOCK flag doesn't work as desired on plain files
> or block devices. Introduce an I/O helper program that does
> the blocking I/O operations, communicating over a pipe that
> can support O_NONBLOCK
> 
> * src/fdstream.c, src/fdstream.h: Add non-blocking I/O
>   on plain files/block devices
> * src/Makefile.am, src/util/iohelper.c: I/O helper program
> * src/qemu/qemu_driver.c, src/lxc/lxc_driver.c,
>   src/uml/uml_driver.c, src/xen/xen_driver.c: Update for
>   streams API change
> ---
>  po/POTFILES.in |1 +
>  src/Makefile.am|   12 +++
>  src/fdstream.c |  233 
> 
>  src/fdstream.h |5 +
>  src/lxc/lxc_driver.c   |2 +-
>  src/qemu/qemu_driver.c |2 +-
>  src/uml/uml_driver.c   |2 +-
>  src/util/iohelper.c|  203 +
>  src/xen/xen_driver.c   |2 +-
>  9 files changed, 402 insertions(+), 60 deletions(-)
>  create mode 100644 src/util/iohelper.c
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 805e5ca..12adb3e 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -94,6 +94,7 @@ src/util/event_poll.c
>  src/util/hash.c
>  src/util/hooks.c
>  src/util/hostusb.c
> +src/util/iohelper.c
>  src/util/interface.c
>  src/util/iptables.c
>  src/util/json.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index c3729a6..1d8115b 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -380,6 +380,9 @@ STORAGE_DRIVER_DISK_SOURCES = 
> \
>  STORAGE_HELPER_DISK_SOURCES =\
>   storage/parthelper.c
>  
> +UTIL_IO_HELPER_SOURCES = \
> + util/iohelper.c
> +
>  # Network filters
>  NWFILTER_DRIVER_SOURCES =\
>   nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c   \
> @@ -1203,6 +1206,15 @@ EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
>  
>  libexec_PROGRAMS =
>  
> +libexec_PROGRAMS += libvirt_iohelper
> +libvirt_iohelper_SOURCES = $(UTIL_IO_HELPER_SOURCES)
> +libvirt_iohelper_LDFLAGS = $(WARN_LDFLAGS) $(AM_LDFLAGS)
> +libvirt_iohelper_LDADD = \
> + libvirt_util.la \
> + ../gnulib/lib/libgnu.la
> +
> +libvirt_iohelper_CFLAGS = $(AM_CFLAGS)
> +

Is libvirt_iohelper for libvirtd?

libvirt_iohelper is provided by libvirt-.rpm, but we still install it
when we build withoud libvirtd. We will meet the following problems:

Checking for unpackaged file(s): /usr/lib/rpm/check-files 
/home/wency/rpmbuild/BUILDROOT/libvirt-0.9.0-1.el6.x86_64
error: Installed (but unpackaged) file(s) found:
   /usr/libexec/libvirt_iohelper


>  if WITH_STORAGE_DISK
>  if WITH_LIBVIRTD
>  libexec_PROGRAMS += libvirt_parthelper

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/6] virObject for reference-counting

[Hu Tao]

On Wed, Apr 06, 2011 at 03:30:50PM -0600, Eric Blake wrote:
> On 04/06/2011 01:18 AM, Hu Tao wrote:
> > This series adds a virObject structure that manages reference-counting.
> > 
> > Some notes about referece-counting introduced by this series:
> > 
> > A thread owns a virObject by incrementing its reference-count by 1.
> > If a thread owns a virObject, the virObject is guarenteed not be
> > freed until the thread releases ownership by decrementing its
> > reference-count by 1. A thread can't access a virObject after it
> > releases the ownership of virObject because it can be freed at
> > anytime.
> > 
> > A thread can own a virObject legally in these ways:
> > 
> > - a thread owns a virObject that it creates.
> > - a thread owns a virObject if another thread passes the ownership
> >   to it. Example: qemuMonitorOpen
> > - a thread gets a virObject from a container.
> >   Example: virDomainFindByUUID
> > - a container owns a virObject by incrementing its reference-count
> >   by 1 before adding it to the container
> > - if a virObject is removed from a container its reference-count
> >   must be decremented by 1
> > 
> > By following these rules, there is no need to protect operations on
> > an object's reference-count by an external lock. (like in old ways
> > virDomainObj lock protects qemu monitor's ref-count.)
> 
> These rules _need_ to be part of the patch series, preferably added at
> the same time as patch 1/6 (see for example how src/qemu/THREADS.txt
> documents threading issues related to qemu).  I don't know whether
> virobject.h or a standalone document is better (but if you do a
> standalone document, then virobject.h should at least mention it).

OK. Will put it in virobject.h in next version.

> 
> -- 
> Eric Blake   ebl...@redhat.com+1-801-349-2682
> Libvirt virtualization library http://libvirt.org
> 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/6] Add virObject and virAtomic.

[Hu Tao]

On Wed, Apr 06, 2011 at 02:19:57PM -0600, Eric Blake wrote:
> On 04/06/2011 01:19 AM, Hu Tao wrote:
> > virObject is the base struct that manages reference-counting
> > for all structs that need the ability of reference-counting.
> > 
> > virAtomic provides atomic operations which are thread-safe.
> > ---
> >  src/Makefile.am  |2 +
> >  src/libvirt_private.syms |5 
> >  src/util/viratomic.c |   46 
> >  src/util/viratomic.h |   30 ++
> >  src/util/virobject.c |   52 
> > ++
> >  src/util/virobject.h |   39 ++
> >  6 files changed, 174 insertions(+), 0 deletions(-)
> >  create mode 100644 src/util/viratomic.c
> >  create mode 100644 src/util/viratomic.h
> >  create mode 100644 src/util/virobject.c
> >  create mode 100644 src/util/virobject.h
> 
> > +++ b/src/libvirt_private.syms
> > @@ -993,3 +993,8 @@ virXPathUInt;
> >  virXPathULong;
> >  virXPathULongHex;
> >  virXPathULongLong;
> > +
> > +# object.h
> 
> virobject.h, and float this section to appear before virtaudit.h (hmm,

Oops, there are always things escape from under your nose:(

> maybe we should do a preliminary patch to rename that to viraudit.h, so
> that we aren't mixing vir vs. virt in quite so many places).
> 
> > +virObjectInit;
> > +virObjectRef;
> > +virObjectUnref;
> 
> Missing exports from viratomic.h.

Why the linker doesn't complain about this this time?

> 
> > diff --git a/src/util/viratomic.c b/src/util/viratomic.c
> > new file mode 100644
> > index 000..629f189
> > --- /dev/null
> > +++ b/src/util/viratomic.c
> > @@ -0,0 +1,46 @@
> 
> > +
> > +#ifdef WIN32
> > +long virAtomicInc(long *value)
> > +{
> > +return InterlockedIncrement(value);
> > +}
> > +
> > +long virAtomicDec(long *value)
> > +{
> > +return InterlockedDecrement(value);
> 
> This is an OS-specific replacement.
> 
> > +}
> > +#else /* WIN32 */
> > +long virAtomicInc(long *value)
> > +{
> > +return __sync_add_and_fetch(value, 1);
> 
> This is a gcc builtin, and will fail to compile with other C99

Yes.

> compilers.  Meanwhile, won't the gcc builtin just work for mingw (that

Not sure about this. I don't have a mingw environment to test, but I
trust gcc and guess it does.

> is, no need to use the OS-specific InterlockedIncrement if you have the
> compiler builtin instead).
> 
> I think this file needs three implementations:
> 
> #if defined __GNUC__ || 
>   use compiler builtins of __sync_add_and_fetch
> #elif defined WIN32
>   use OS primitives, like InterlockedIncrement
> #else
>   we're hosed when it comes to lightweight versions, but we can still
> implement a heavyweight replacement that uses virMutex
> #endif

Agreed, this is a better way.

> 
> > +++ b/src/util/viratomic.h
> > @@ -0,0 +1,30 @@
> 
> > +#ifndef __VIR_ATOMIC_H
> > +#define __VIR_ATOMIC_H
> > +
> > +long virAtomicInc(long *value);
> > +long virAtomicDec(long *value);
> 
> Mark both of these ATTRIBUTE_NONNULL(1)

OK.

> 
> I'm debating whether they should also be marked ATTRIBUTE_RETURN_CHECK

No, there are cases you just want to inc/dec a value but do not check
the modified value.

> 
> > +++ b/src/util/virobject.c
> > @@ -0,0 +1,52 @@
> > +
> > +#include "viratomic.h"
> > +#include "virobject.h"
> > +#include "logging.h"
> > +
> > +int virObjectInit(virObjectPtr obj, void (*free)(virObjectPtr obj))
> 
> You should declare a typedef:
> 
> typedef void (*virObjectFreer)(virObjectPtr);
> 
> then it becomes simpler to read:
> 
> int virObjectInit(virObjectPtr obj, virObjectFreer f)
> 
> Use a different name (I used freer/f above) to avoid -Wshadow warnings
> with free().

OK.

> 
> > +{
> > +if (!free) {
> 
> Especially since shadowing means it's impossible to tell if this
> statement is always true (the address of free() exists) or conditional
> (there is a local variable named free shadowing the global function),
> and context-sensitive code reviews are tougher :)
> 
> > +VIR_ERROR0("method free is required.");
> > +return -1;
> > +}
> 
> Should this function also check and return -1 if obj->free was not NULL
> on entry (that is, guarantee that you can't initialize an object twice)?

Agreed. And it is dangerous to initialize an object more than once
because the ref count may already changed and a second initializaton
just do the wrong thing.

> 
> > +
> > +obj->ref = 1;
> > +obj->free = free;
> > +
> > +return 0;
> > +}
> > +
> > +void virObjectRef(virObjectPtr obj)
> > +{
> > +sa_assert(obj->ref > 0);
> 
> This is useless.  It only helps static analyzers (like clang),
> 
> > +virAtomicInc(&obj->ref);
> 
> but there's nothing to analyze that depends on knowing the value was
> positive.
> 
> I'm debating whether to do checking.  Maybe we should do:
> 
> if (virAtomicInc(&obj->ref) < 2)

This means bad things happened, and it is not enough to just give a
warning. Think 

Re: [libvirt] [PATCH v5] qemu: Remove the managed state file only if restoring succeeded

[Daniel Veillard]

On Thu, Apr 07, 2011 at 10:31:52AM +0800, Osier Yang wrote:
> 1) Both "qemuDomainStartWithFlags" and "qemuAutostartDomain" try to
> restore the domain from managedsave'ed image if it exists (by
> invoking "qemuDomainObjRestore"), but it unlinks the image even
> if restoring fails, which causes data loss. (This problem exists
> for "virsh managedsave dom; virsh start dom").
> 
> The fix for is to unlink the managed state file only if restoring
> succeeded.
> 
> 2) For "virsh save dom; virsh restore dom;", it can cause data
> corruption if one reuse the saved state file for restoring. Add
> doc to tell user about it.
> 
> 3) In "qemuDomainObjStart", if "managed_save" is NULL, we shouldn't
> fallback to start the domain, skipping it to cleanup as a incidental
> fix. Discovered by Eric.
> 
> ---
>  src/qemu/qemu_driver.c |   12 +++-
>  tools/virsh.pod|6 +-
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 48fe266..a84780b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3423,18 +3423,20 @@ static int qemudDomainObjStart(virConnectPtr conn,
> 
>  /*
>   * If there is a managed saved state restore it instead of starting
> - * from scratch. In any case the old state is removed.
> + * from scratch. The old state is removed once the restoring succeeded.
>   */
>  managed_save = qemuDomainManagedSavePath(driver, vm);
> +
> +if (!managed_save)
> +goto cleanup;
> +
>  if ((managed_save) && (virFileExists(managed_save))) {
>  ret = qemuDomainObjRestore(conn, driver, vm, managed_save);
> 
> -if (unlink(managed_save) < 0) {
> +if ((ret == 0) && (unlink(managed_save) < 0))
>  VIR_WARN("Failed to remove the managed state %s", managed_save);
> -}
> 
> -if (ret == 0)
> -goto cleanup;
> +goto cleanup;
>  }
> 
>  ret = qemuProcessStart(conn, driver, vm, NULL, start_paused, -1, NULL,
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index f4bd294..6319373 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -546,7 +546,11 @@ I parameter in the domain's XML definition.
> 
>  =item B I
> 
> -Restores a domain from an B state file.  See I for more 
> info.
> +Restores a domain from an B state file. See I for more 
> info.
> +
> +B: To avoid corrupting file system contents within the domain, you
> +should not reuse the saved state file to B unless you are convinced
> +with reverting the domain to the previous state.
> 
>  =item B I I

  ACK, thanks !

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] ESX network functionality

[Matthias Bolte]

2011/4/6 James Barkley :
> Yes, I used the managed object called Network. It does get a little dicey
> with VMware due to the addition of port groups and virtual switches
> (admittedly, not sure how this works in most other hypervisors). You
> certainly can't have transient network objects, and any creation/destruction
> of a network object seems like it must necessarily involve either
> creation/destruction of a port group or creation/destruction of a vswitch. I
> believe using the Network managed object is the right approach, but am not
> yet 100% confident. My next task is to facilitate the creation of a new
> network through virNetworkDefineXML or virNetworkCreateXML. If you'd prefer,
> I can wait until I've completed this functionality to submit the patch, as
> it may drive me to adopt a different approach.

The docs about the Network object say that they are created and
destroyed automatically as needed. Did you try if a port group without
virtual machines attached to it has a corresponding Network object?

You could send your current code for an initial review, as the
implementation for virNetworkDefineXML should go into a second patch.

I think you cannot implement virNetworkCreateXML as that's supposed to
create a transient network and that's not how network work.

On the other hand, what would be the semantic of virNetworkCreate and
virNetworkDestroy? Those require that there is some kind of power
switch on the networks, like the power state of the virtual machines.
So the life cycle of a network is not entirely clear to me yet.

>>I assume you made esxNumOfDefinedNetworks return 0 to get virsh
>>net-list working as there network in the VMware context are always
>>active.
> Correct. I was debating on whether or not to have it loop through the
> network list and check the "active/inactive" just for thoroughness, but if
> you're fine with just returning 0 and commenting why, then I'll leave it.

Is there actually something like an active/inactive state for a network?

>>The recommended way for sending patches is git send-email.
> Barring any other recommendations I'll figure out git send email and submit
> the patch to the list shortly.

You can also use git format-patch and attach the patch file as
plaintext to an email, but sending patches inline in email (like git
send email does) is the preferred way.

Matthias

> thanks,
> -jb
>
> On Wed, Apr 6, 2011 at 2:37 AM, Matthias Bolte
>  wrote:
>>
>> Sorry, I forgot to reply to your follow up question on the users list.
>>
>> 2011/4/6 James Barkley :
>> > Greetings:
>> > I've added code to the ESX driver to support some basic network
>> > functionality. I'm pretty new to this list, so please tell me how to
>> > proceed
>> > with code review and patch submission (yes I've read the contributor
>> > guidelines on the wiki). It seems like people are emailing a patch file
>> > for
>> > every file they've changed, each in separate emails with [1 of N] in the
>> > subject, is that right? Or is it better to paste in code, get some
>> > feedback,
>> > and eventually attach the patch to the bug tracker item?
>>
>> The normal approach is to have one commit/patch per logical
>> self-contained change. After each commit/patch the codebase has to be
>> in a compilable stage. For example you cannot add the code using the
>> generated SOAP bindings, before you actually added them to the
>> esx_vi_generator.input file.
>>
>> The [1 of N] style patch series are typically used for large changes
>> that are split in several logical, self-contained parts. Splitting
>> like this simplifies reviewing and later on figuring out bugs using
>> git bisect.
>>
>> You typically don't split on a per file basis.
>>
>> In your case I'd suggest to create a single commit/patch for your
>> addition, as it is one logical piece of work. You _could_ (but I don't
>> recommend to) split it in multiple patches. For example one for the
>> esx_vi_generator.input addition, one for the VI helper function
>> additions and one for the actual driver functions, but I'd consider
>> this to be too fine grained.
>>
>> > I've updated the code for the ESX driver to be able to handle the
>> > following
>> > functions :
>> > - virNetworkLookupByName
>> > - virConnectNumOfDefinedNetworks
>> > - virConnectNumOfNetworks
>> > - virConnectListNetworks
>>
>> Is this sufficient to make virsh net-list work?
>>
>> > I basically mapped the VMware Managed Object Reference for networks into
>> > a
>> > few data structures and added the following functions to the internal
>> > driver
>>
>> You mean the managed object called Network? They _seem_ to be the
>> natural fit. But I'm not sure if that's the correct approach, as you
>> cannot directly create/destroy such objects and they are bound in some
>> way to the port groups on a virtual switch. Also I'm not sure about
>> the exact semantics of networks and port groups.
>>
>> That's what I meant as I said about the mapping between libvirt and
>> VMware. We need to be sure 

Re: [libvirt] virsh: Can't create .vmdk virtual disk over vCenter while over ESX it works well

[Matthias Bolte]

2011/4/6 computern...@rambler.ru :
> Hello,
>
> I am trying to create the virtual disk over "virsh". My target is the ESX
> server that is a part of the vCenter. That is why I connect to it using the
> vpx://-like URL. The connection itself is being established correctly.
> Commands like "list --all", "dumpxml " "vol-dumpxml " work
> perfectly well. What doesn't work for me at the moment is "vol-create --file
>  --pool ". I would suspect that I am passing some
> parameters incorrectly and would be digging more into a documentation but I
> am getting the following response from the server:
>
> virsh # vol-create --file vadp4.xml --pool "iscsi-esx4-1"
> error: Failed to create vol from vadp4.xml
> error: internal error Could not create volume: NotImplemented - Der
> angeforderte
>  Vorgang wird vom Server nicht implementiert.
>
> I have to admit that the very same command worked if I connect to ESX server
> directly (using esx://-like URL).
>
> My question is if it is planned to implement the virtual disk creation in
> case of vCentere is involved ?

The error message is a bit misleading, but expected. The
NotImplemented error actually comes from the vCenter Server, that
doesn't seem to support forwarding CreateVirtualDisk_Task calls to the
ESX servers. I haven't figured out yet how the vSphere Client
connected to a vCenter Server manages to do disk creation.

So you could consider this a missing workaround in libvirt, or the
lack of proper knowledge about how to do disk creation via a vCenter
Server.

Matthias

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list