Re: [libvirt] [PATCH] LXC: update comments of lxcDomainCreateXMLWithFiles() and lxcDomainCreateXML()

2014-06-24 Thread Wangrui (K)


> -Original Message-
> From: Michal Privoznik [mailto:mpriv...@redhat.com]
> Sent: Tuesday, June 24, 2014 9:38 PM
> To: Wangrui (K); libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH] LXC: update comments of
> lxcDomainCreateXMLWithFiles() and lxcDomainCreateXML()
> 
 
> 
> One does not simply apply this patch. Have you edited the patch itself
> prior to sending? The best way to send a patch is via 'git send-email'.
> 
> Michal

Yes, I'll send this patch with 'git send-email' again.


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


[libvirt] [PATCH] LXC: update comments of lxcDomainCreateXMLWithFiles() and lxcDomainCreateXML()

2014-06-24 Thread Wangrui (K)
update comments of lxcDomainCreateXMLWithFiles() and lxcDomainCreateXML()

Signed-off-by: Yue wenyuan 
Signed-off-by: Wang Rui 
---
src/lxc/lxc_driver.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 06f3e18..11bfb80 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1122,14 +1122,16 @@ static int lxcDomainCreateWithFlags(virDomainPtr dom,
}

 /**
- * lxcDomainCreateXML:
+ * lxcDomainCreateXMLWithFiles:
  * @conn: pointer to connection
+ * @nfiles: number of file descriptors passed
+ * @files: list of file descriptors passed
  * @xml: XML definition of domain
  * @flags: Must be 0 for now
  *
  * Creates a domain based on xml and starts it
  *
- * Returns 0 on success or -1 in case of error
+ * Returns a new domain object or NULL in case of failure.
  */
static virDomainPtr
lxcDomainCreateXMLWithFiles(virConnectPtr conn,
@@ -1209,7 +1211,16 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn,
 return dom;
}

-
+/**
+ * lxcDomainCreateXML:
+ * @conn: pointer to connection
+ * @xml: XML definition of domain
+ * @flags: Must be 0 for now
+ *
+ * Creates a domain based on xml and starts it
+ *
+ * Returns a new domain object or NULL in case of failure.
+ */
static virDomainPtr
lxcDomainCreateXML(virConnectPtr conn,
const char *xml,
-- 
1.7.12.4

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


Re: [libvirt] [PATCH RFC 0/3] allow setting video ram size for graphics

2014-06-13 Thread Wangrui (K)

> -Original Message-
> From: Gerd Hoffmann [mailto:kra...@redhat.com]
> Sent: Thursday, June 12, 2014 4:16 PM
> To: Wangrui (K)
> Cc: libvir-list@redhat.com; Zengjunliang; ebl...@redhat.com
> Subject: Re: [PATCH RFC 0/3] allow setting video ram size for graphics
> 
> On Do, 2014-06-12 at 07:22 +, Wangrui (K) wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1076098
> >
> > Zeng Junliang (3):
> >   For vga/cirrus/vmvga/qxl device, qemu supports commandline
> >   parameter "vgamem_mb" to specifie the size of the
> >   framebuffer portion of the "ram" region. As the vram attribute
> >   in libvirt is only valid for qxl device in KVM/QEMU to specifie the total
> >   size of the "vram" region, we expect a new attribute in libvirt.
> >   The following patches introduce "vgamem" attribute to make
> >   vgamem_mb configurable in libvirt xml.
> 
> Adding a new vgamem attribute looks good to me.  Things will be more
> consistent then and it will certainly be less confusion than trying to
> use vram instead.

You are right, I will leave out cirrus on PATCH v2.

>
> The vgamem attribute makes sense for vga, vmvga and qxl.  Please leave
> out cirrus.  It is pointless to assign more memory to cirrus, guests
> would not use it anyway.

OK.

> 
> Removing the confusing vram=9216 default which gets added for cirrus and
> stdvga and which is never ever used would be nice too.
> 
> While touching the vga code anyway you might consider adding support for
> the new '-device secondary-vga' (currently in master / will be in qemu
> 2.1).  That is simliar to stdvga (i.e. it has a vgamem_mb property too).
> It doesn't occupy the standard vga ports though, so it can be used as
> secondary display device, like qxl.

Thanks for reminding, I almost missed it. 

> 
> cheers,
>   Gerd
> 


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


[libvirt] [PATCH RFC 3/3] docs: add description for vgamem attribute

2014-06-12 Thread Wangrui (K)
Enhance schema and add description for vgamem attribute.

Signed-off-by: Zeng Junliang 
Signed-off-by: Wang Rui 
---
 docs/formatdomain.html.in | 24 ++--
 docs/schemas/domaincommon.rng |  5 +
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1b6ced8..72796ec 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4263,7 +4263,8 @@ qemu-kvm -net nic,model=? /dev/null
 will add a default video according to the guest type.
 For a guest of type "kvm", the default video for it is:
 type with value "cirrus", vram with value
-"9216", and heads with value "1". By default, the first
+"9216", vgamem (since 1.2.6) 
with
+value "16384", and heads with value "1". By default, the 
first
 video device in domain xml is the primary one, but the optional
 attribute primary (since 1.0.2)
 with value 'yes' can be used to mark the primary in cases of multiple
@@ -4271,8 +4272,8 @@ qemu-kvm -net nic,model=? /dev/null
 attribute ram (since
 1.0.2) is allowed for "qxl" type only and specifies
 the size of the primary bar, while vram specifies the
-secondary bar size.  If "ram" or "vram" are not supplied a default
-value is used.
+secondary bar size.  If "ram", "vram" or "vgamem" are not supplied
+a default value is used.
   
 
   model
@@ -4281,9 +4282,20 @@ qemu-kvm -net nic,model=? /dev/null
 attribute which takes the value "vga", "cirrus", "vmvga", "xen",
 "vbox", or "qxl" (since 0.8.6)
 depending on the hypervisor features available.
-You can also provide the amount of video memory in kibibytes
-(blocks of 1024 bytes) using
-vram and the number of screen with heads.
+
+vram attribute specifies the amount of video memory
+in kibibytes (blocks of 1024 bytes). For a guest type of "kvm",
+it is only valid when type is "qxl".
+
+
+vgamem attribute since 1.2.6,
+QEMU and KVM only specifies the size of the framebuffer
+portion of the "ram" region.
+And it is not valid when type is "xen" and "vbox".
+
+
+heads attribute specifies the number of screen.
+
   
 
   acceleration
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6cc922c..a3f2e60 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2698,6 +2698,11 @@
 
   
   
+
+  
+
+  
+  
 
   
 
-- 
1.7.12.4


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


[libvirt] [PATCH RFC 2/3] tests: modify test case related to vgamem attribute

2014-06-12 Thread Wangrui (K)
This patch modify test case related to vgamem attribute.

Signed-off-by: Zeng Junliang 
Signed-off-by: Wang Rui 
---
 tests/domainschemadata/domain-parallels-ct-simple.xml| 2 +-
 tests/domainschemadata/domain-parallels-vm-simple.xml| 2 +-
 .../qemuhotplug-console-compat-2+console-virtio.xml  | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml  | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network2.xml | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.xml  | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 2 +-
 .../qemuxml2argv-graphics-spice-agent-file-xfer.args | 5 +++--
 .../qemuxml2argv-graphics-spice-agent-file-xfer.xml  | 4 ++--
 .../qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.xml  | 2 +-
 .../qemuxml2argv-graphics-spice-compression.args | 4 +++-
 .../qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml | 4 ++--
 .../qemuxml2argv-graphics-spice-listen-network.xml   | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args  | 3 ++-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml   | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args | 3 ++-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.xml  | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml   | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args  | 5 +++--
 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml   | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-policy.xml  | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.xml| 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml  | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.xml | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-websocket.xml   | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml| 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-addr.xml | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-idx.xml  | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml   | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args   | 3 ++-
 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml| 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-q35.args | 3 ++-
 tests/qemuxml2argvdata/qemuxml2argv-q35.xml  | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args| 3 ++-
 tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.xml | 2 +-
 .../qemuxml2argv-video-device-pciaddr-default.args   | 9 ++---
 .../qemuxml2argv-video-device-pciaddr-default.xml| 6 +++---
 .../qemuxml2xmlout-graphics-listen-network2.xml  | 2 +-
 .../qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml | 2 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autoadd-addr.xml | 2 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autoadd-idx.xml  | 2 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml  | 2 +-
 tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml  | 2 +-
 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml   | 2 +-
 tests/xml2vmxdata/xml2vmx-svga.xml   | 2 +-
 tests/xml2vmxdata/xml2vmx-ws-in-the-wild-1.xml   | 2 +-
 tests/xml2vmxdata/xml2vmx-ws-in-the-wild-2.xml   | 2 +-
 51 files changed, 74 insertions(+), 62 deletions(-)

diff --git a/tests/domainschemadata/domain-parallels-ct-simple.xml 
b/tests/domainschemadata/domain-parallels-ct-simple.xml
index a2b87ce..2a5daf6 100644
--- a/tests/domainschemadata/domain-parallels-ct-simple.xml
+++ b/tests/domainschemadata/domain-parallels-ct-simple.xml
@@ -19,7 +19,7 @@
 
 
 
-  
+  
 
   
 
diff --git a/tests/domainschemadata/domain-parallels-vm-simple.xml 
b/tests/domainschemadata/domain-parallels-vm-simple.xml
index 4e21583..ca93373 100644
--- a/tests/domainschemadata/domain-parallels-vm-simple.xml
+++ b/tests/domainschemadata/domain-parallels-vm-simple.xml
@@ -18,7 +18,7 @@
   
 
 
-  
+  
 
   
 
diff --git 
a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml 
b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
index ec1c6e8..714b466 100644
--- a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
+++ b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml
@@ -91,7 +

[libvirt] [PATCH RFC 1/3] qemu: Introduce vgamem attribute for video model

2014-06-12 Thread Wangrui (K)
This patch introduces vgamem attribute for video model, and sets
its default value as qemu used. Parse it in two ways accroding to
qemu startup parameters supported: -device or -vga.

Signed-off-by: Zeng Junliang 
Signed-off-by: Wang Rui 
---
For KVM, vram attribute seems to be invalid for cirrus/vga/vmvga
device. Should we replace vram with vgamem? Or any other plans
for this vram attribute?

 src/conf/domain_conf.c   | 44 ++-
 src/conf/domain_conf.h   |  3 +-
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  | 79 +---
 4 files changed, 100 insertions(+), 27 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ff2d447..58f6ed6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9358,6 +9358,24 @@ virDomainVideoDefaultRAM(const virDomainDef *def,
 }
 }
 
+int
+virDomainVideoDefaultVgamem(int type)
+{
+switch (type) {
+case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
+/* QEMU use 8M as default value for cirrus device */
+return 8 * 1024;
+
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
+case VIR_DOMAIN_VIDEO_TYPE_QXL:
+/* QEMU use 16M as default value for vga/vmvga/qxl device*/
+return 16 * 1024;
+
+default:
+return 0;
+}
+}
 
 int
 virDomainVideoDefaultType(const virDomainDef *def)
@@ -9443,6 +9461,7 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 char *type = NULL;
 char *heads = NULL;
 char *vram = NULL;
+char *vgamem = NULL;
 char *ram = NULL;
 char *primary = NULL;
 
@@ -9452,11 +9471,12 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 cur = node->children;
 while (cur != NULL) {
 if (cur->type == XML_ELEMENT_NODE) {
-if (!type && !vram && !ram && !heads &&
+if (!type && !vram && !ram && !heads && !vgamem &&
 xmlStrEqual(cur->name, BAD_CAST "model")) {
 type = virXMLPropString(cur, "type");
 ram = virXMLPropString(cur, "ram");
 vram = virXMLPropString(cur, "vram");
+vgamem = virXMLPropString(cur, "vgamem");
 heads = virXMLPropString(cur, "heads");
 
 if ((primary = virXMLPropString(cur, "primary")) != NULL) {
@@ -9510,6 +9530,16 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 def->vram = virDomainVideoDefaultRAM(dom, def->type);
 }
 
+if (vgamem) {
+if (virStrToLong_ui(vgamem, NULL, 10, &def->vgamem) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("cannot parse video vgamem '%s'"), vgamem);
+goto error;
+}
+} else {
+def->vgamem = virDomainVideoDefaultVgamem(def->type);
+}  
+
 if (heads) {
 if (virStrToLong_ui(heads, NULL, 10, &def->heads) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -9526,6 +9556,7 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 VIR_FREE(type);
 VIR_FREE(ram);
 VIR_FREE(vram);
+VIR_FREE(vgamem);
 VIR_FREE(heads);
 
 return def;
@@ -9535,6 +9566,7 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 VIR_FREE(type);
 VIR_FREE(ram);
 VIR_FREE(vram);
+VIR_FREE(vgamem);
 VIR_FREE(heads);
 return NULL;
 }
@@ -12658,6 +12690,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 }
 video->vram = virDomainVideoDefaultRAM(def, video->type);
+video->vgamem = virDomainVideoDefaultVgamem(video->type);
 video->heads = 1;
 if (VIR_ALLOC_N(def->videos, 1) < 0) {
 virDomainVideoDefFree(video);
@@ -13578,6 +13611,13 @@ 
virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src,
 return false;
 }
 
+if (src->vgamem!= dst->vgamem) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target video card vgamem %u does not match source 
%u"),
+   dst->vgamem, src->vgamem);
+return false;
+}
+
 if (src->heads != dst->heads) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target video card heads %u does not match source 
%u"),
@@ -16532,6 +16572,8 @@ virDomainVideoDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, " ram='%u'", def->ram);
 if (def->vram)
 virBufferAsprintf(buf, " vram='%u'", def->vram);
+if (def->vgamem)
+virBufferAsprintf(buf, " vgamem='%u'", def->vgamem);
 if (def->heads)
 virBufferAsprintf(buf, " heads='%u'", def->heads);
 if (def->primary)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a6ac95a..a72eeec 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1216,6 +1216,7 @@ struct _virDomainVideoDef {
 int type;
 unsigned int ram;  /* kibibytes (multiples of 1024) */
 unsigned int vram; /* kibibytes (multiples of 1024) */
+unsigned int vgamem; /* kibibytes (multiples of 1024) */
 uns

[libvirt] [PATCH RFC 0/3] allow setting video ram size for graphics

2014-06-12 Thread Wangrui (K)
https://bugzilla.redhat.com/show_bug.cgi?id=1076098

Zeng Junliang (3):
  For vga/cirrus/vmvga/qxl device, qemu supports commandline
  parameter "vgamem_mb" to specifie the size of the
  framebuffer portion of the "ram" region. As the vram attribute
  in libvirt is only valid for qxl device in KVM/QEMU to specifie the total
  size of the "vram" region, we expect a new attribute in libvirt.
  The following patches introduce "vgamem" attribute to make 
  vgamem_mb configurable in libvirt xml.

  qemu: Introduce vgamem attribute for video model
  tests: modify test case related to vgamem attribute
  docs: add description for vgamem attribute

 docs/formatdomain.html.in  | 24 +--
 docs/schemas/domaincommon.rng  |  5 ++
 src/conf/domain_conf.c | 44 +++-
 src/conf/domain_conf.h |  3 +-
 src/libvirt_private.syms   |  1 +
 src/qemu/qemu_command.c| 79 +++---
 .../domain-parallels-ct-simple.xml |  2 +-
 .../domain-parallels-vm-simple.xml |  2 +-
 ...qemuhotplug-console-compat-2+console-virtio.xml |  2 +-
 .../qemuxml2argv-console-compat-2.xml  |  2 +-
 .../qemuxml2argv-controller-order.xml  |  2 +-
 .../qemuxml2argv-graphics-listen-network.xml   |  2 +-
 .../qemuxml2argv-graphics-listen-network2.xml  |  2 +-
 .../qemuxml2argv-graphics-sdl-fullscreen.xml   |  2 +-
 .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml |  2 +-
 ...emuxml2argv-graphics-spice-agent-file-xfer.args |  5 +-
 ...qemuxml2argv-graphics-spice-agent-file-xfer.xml |  4 +-
 .../qemuxml2argv-graphics-spice-agentmouse.xml |  2 +-
 .../qemuxml2argv-graphics-spice-compression.args   |  4 +-
 .../qemuxml2argv-graphics-spice-compression.xml|  4 +-
 .../qemuxml2argv-graphics-spice-listen-network.xml |  4 +-
 .../qemuxml2argv-graphics-spice-qxl-vga.args   |  3 +-
 .../qemuxml2argv-graphics-spice-qxl-vga.xml|  4 +-
 .../qemuxml2argv-graphics-spice-sasl.args  |  3 +-
 .../qemuxml2argv-graphics-spice-sasl.xml   |  2 +-
 .../qemuxml2argv-graphics-spice-timeout.xml|  2 +-
 .../qemuxml2argv-graphics-spice.args   |  5 +-
 .../qemuxml2argv-graphics-spice.xml|  4 +-
 .../qemuxml2argv-graphics-vnc-policy.xml   |  2 +-
 .../qemuxml2argv-graphics-vnc-sasl.xml |  2 +-
 .../qemuxml2argv-graphics-vnc-socket.xml   |  2 +-
 .../qemuxml2argv-graphics-vnc-tls.xml  |  2 +-
 .../qemuxml2argv-graphics-vnc-websocket.xml|  2 +-
 .../qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml |  2 +-
 .../qemuxml2argv-net-bandwidth.xml |  2 +-
 .../qemuxml2argv-pci-autoadd-addr.xml  |  2 +-
 .../qemuxml2argv-pci-autoadd-idx.xml   |  2 +-
 tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml |  2 +-
 .../qemuxml2argv-pcihole64-q35.args|  3 +-
 .../qemuxml2argv-pcihole64-q35.xml |  2 +-
 .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  2 +-
 tests/qemuxml2argvdata/qemuxml2argv-q35.args   |  3 +-
 tests/qemuxml2argvdata/qemuxml2argv-q35.xml|  2 +-
 .../qemuxml2argv-serial-spiceport.args |  3 +-
 .../qemuxml2argv-serial-spiceport.xml  |  2 +-
 .../qemuxml2argv-video-device-pciaddr-default.args |  9 ++-
 .../qemuxml2argv-video-device-pciaddr-default.xml  |  6 +-
 .../qemuxml2xmlout-graphics-listen-network2.xml|  2 +-
 .../qemuxml2xmlout-graphics-spice-timeout.xml  |  2 +-
 .../qemuxml2xmlout-pci-autoadd-addr.xml|  2 +-
 .../qemuxml2xmlout-pci-autoadd-idx.xml |  2 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml|  2 +-
 tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml|  2 +-
 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml |  2 +-
 tests/xml2vmxdata/xml2vmx-svga.xml |  2 +-
 tests/xml2vmxdata/xml2vmx-ws-in-the-wild-1.xml |  2 +-
 tests/xml2vmxdata/xml2vmx-ws-in-the-wild-2.xml |  2 +-
 57 files changed, 197 insertions(+), 95 deletions(-)

-- 
1.7.12.4


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


[libvirt] [PATCH] docs: fix a typo in hacking.html.in

2014-06-10 Thread Wangrui (K)
Signed-off-by: Wang Rui 
---
 docs/hacking.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index b2ef85a..9c6dd26 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -122,7 +122,7 @@
   Some tests are skipped by default in a development environment,
   based on the time they take in comparison to the likelihood
   that those tests will turn up problems during incremental builds.
-  These tests default to being run when when building from a
+  These tests default to being run when building from a
   tarball or with the configure option --enable-expensive-tests;
   you can also force a one-time toggle of these tests by
   setting VIR_TEST_EXPENSIVE to 0 or 1 at make time, as in:
--
1.7.12.4

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


Re: [libvirt] [PATCH v4] util: new function virTimeLocalOffsetFromUTC

2014-05-28 Thread Wangrui (K)
And don't forget to update comments if localtime_r is removed :)

> > + * This function is threadsafe, but is *not* async signal safe (due to
> > + * localtime_r()).

> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
> On Behalf Of Laine Stump
> Sent: Tuesday, May 27, 2014 7:46 PM
> To: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH v4] util: new function virTimeLocalOffsetFromUTC
> 
> On 05/24/2014 05:21 PM, Eric Blake wrote:
> > From: Laine Stump 
> >
> > Since there isn't a single libc API to get this value, this patch
> > supplies one which gets the value by grabbing current time, then
> > converting that into a struct tm with gmtime_r(), then back to a
> > time_t using mktime.
> >
> > The returned value is the difference between UTC and localtime in
> > seconds. If localtime is ahead of UTC (east) the offset will be a
> > positive number, and if localtime is behind UTC (west) the offset will
> > be negative.
> >
> > This function should be POSIX-compliant, and is threadsafe, but not
> > async signal safe. If it was ever necessary to know this value in a
> > child process, we could cache it with a one-time init function when
> > libvirtd starts, then just supply the cached value, but that
> > complexity isn't needed for current usage; that would also have the
> > problem that it might not be accurate after a local daylight savings
> > boundary.
> >
> > (If it weren't for DST, we could simply replace this entire function
> > with "-timezone"; timezone contains the offset of the current timezone
> > (negated from what we want) but doesn't account for DST. And in spite
> > of being guaranteed by POSIX, it isn't available on older versions of
> > mingw.)
> >
> > Signed-off-by: Eric Blake 
> > ---
> >
> > After some more playing around, I learned we don't need localtime_r
> > at all: time() and mktime() both honor the current timezone, so the
> > original and resulting value were always the same.  I also figured
> > out how to force a timezone with a daylight savings different than
> > an hour; the code still works without having to hardcode any guess
> > based on tm_isdst.
> 
> Actually this patch doesn't work as well as an initial test might
> indicate. When I first reviewed it on Saturday afternoon (UTC+3), it
> passed the tests, but when I went back and ran it again in the morning
> on Monday, it failed the tests that had DST set.
> 
> In the interest of getting as much testing time as possible on the code
> (and with DV's agreement), I disabled the failing tests and pushed the
> patches, then began investigating in more detail.
> 
> I ran some  custom-written tests based on this patch on a Fedora guest
> while modifying the system date and found that at different times of the
> day the tests that involved DST failed, yet at some times they failed
> (the failure times of different tests were different, and as the clock
> ticked forward, TZs with larger offsets would begin to succeed).
> 
> I spent a significant amount of time experimenting and found that the
> function as written results in a *lot* of failures over the course of a
> year (although there are certain times of certain days when it is
> successful for some or all of the tests).
> 
> So I tried setting gmtimeinfo.tm_isdst = -1 prior to calling mktime and
> Yay! It seemed to succeed for all tests on all dates within a year. Then
> I added a few more tests, and changed my test script to change the
> system to a different timezone prior to calling the test - after seeing
> a pattern during a couple tests, to save time I changed the test to only
> run through the clock on Jan 1, May 31, and Dec 31, but to do that both
> with the shell environment set to
> 
>   TZ="MST07:00MDT06:00,0/00:00:00,366/23:59:59"
> 
> and to
> 
>TZ="EET-02:00EEDT-03:00,0/00:00:00,366/23:59:59".
> 
> (in recognition of my timezone and Eric's timezone :-)
> 
> I've found that in the both cases only three of the 4 new tests I'd
> added will fail - these two new tests:
> 
>   TEST_LOCALOFFSET("VIR05:00VID04:00,0/00:00:00,366/23:59:59",  ((-4 *
> 60) +  0) * 60);
>   TEST_LOCALOFFSET("VIR11:00VID10:00,0/00:00:00,366/23:59:59", ((-10 *
> 60) +  0) * 60);
> 
> will fail for:
> 
>   Dec 31 18:00-18:59 for MST
>   Jan 1 from 03:00 - 03:59 for EET
> 
> and this test:
> 
>TEST_LOCALOFFSET("VIR02:45VID00:45,0/00:00:00,366/23:59:59", -45 *
> 60);
> 
> fails for
> 
>   Dec 31 18:00-19:59 for MST
>   Jan 1 from 03:00 - 04:59 for EET
> 
> (Note that the output of the date command *always* showed "MDT" or
> "EEDT", indicating that the trick of forcing DST for every time of every
> day was working properly, at least for the time in the shell).
> 
> The failure in all cases is that the offset returned is the offset that
> would be correct if DST hadn't been in effect. Since the method of
> setting permanent DST appears to work correctly, and it's happening for
> an entire 1-2 hour stretch, I'm guessing that the prob

Re: [libvirt] [PATCH 2/3] qemu: Send migrate_cancel when aborting migration

2014-05-22 Thread Wangrui (K)


> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
> On Behalf Of Jiri Denemark
> Sent: Thursday, May 22, 2014 7:55 PM
> To: libvir-list@redhat.com
> Subject: [libvirt] [PATCH 2/3] qemu: Send migrate_cancel when aborting
> migration
> 
> When QEMU reports failed or cancelled migration, we don't need to send
> it migrate_cancel QMP command. But in all other error paths, such as if
> we detect broken connection to a destination daemon or something else
> happens inside libvirt, we need to explicitly send migrate_cancel
> command instead of relying on the migration to be implicitly cancelled
> when destination QEMU is killed.
> 
> Because we were not doing so, one could end up with a paused domain
> after failed migration.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=807023

Hi, Jiri
I'm interested in the patch. I want to know the specific scene.
But the bug link you posted is about device_del. Is there another link about 
migration? (I think)


> Signed-off-by: Jiri Denemark 
> ---



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


Re: [libvirt] [RFC] require for suggestions on support for ivshmem device

2014-05-21 Thread Wangrui (K)
Thank you for reply

> -Original Message-
> From: Martin Kletzander [mailto:mklet...@redhat.com]
> Sent: Tuesday, May 20, 2014 5:18 PM
> To: Wangrui (K)
> Cc: libvir-list@redhat.com; Zhangbo (Oscar); Yanqiangjun; Zengjunliang;
> Moyuxiang; jdene...@redhat.com
> Subject: Re: [libvirt] [RFC] require for suggestions on support for ivshmem
> device
> 
> On Wed, May 14, 2014 at 08:23:21AM +, Wangrui (K) wrote:
> >Hi,
> >
> >Libvirt does not support ivshmem(Inter-VM Shared Memory) device
> >recently, thus, I would like to know if there's any plan to support it in the
> future?
> >If not, I would like to contribute a serial of patches to do so.
> >
> >On Jan 28, Wangyufei (James) asked about this question, and Daniel replied
> with 2 suggestions:
> >1. Libvirt should be capable of configuring the guest's xml on ivshmem.
> >2.An ivshmem daemon is needed to run on the host to support it, libvirt is
> recommended to provide such a daemon.
> >Please refer to
> https://www.redhat.com/archives/libvir-list/2014-January/msg01335.html for
> details.
> >
> >What I'll do later is the 1st suggestion, the 2nd one is left to be 
> >accomplished
> by someone else.
> >
> >Here is the detailed work I'll do to support configuration of the guest in 
> >libvirt:
> >virDomainDefParseXML: parse ivshmem device xml when
> defining dom.xml
> >virDomainDeviceInfoIterateInternal:   iterate ivshmem devices
> >qemuAssignDevicePCISlots:  assign ivshmem device pci slots
> >virDomainDefFormatInternal: format ivshmem device xml (Eg.
> virsh edit dom)
> >virDomainDefFree: free ivshmem device def
> >
> >qemuBuildCommandLine:   build ivshmem device command
> line when vm starts
> >qemuAssignDeviceAliases:  assign ivshmem device aliases
> when vm starts
> >
> >virDomainDeviceDefParse:   attach and parse ivshmem
> device xml
> >qemuDomainAttachDeviceConfig: attach ivshmem device xml
> persistently
> >qemuDomainAttachDeviceLive:   attach ivshmem device online
> >
> >qemuDomainDetachDeviceConfig: detach ivshmem device xml
> persistently
> >qemuDomainDetachDeviceLive:   detach ivshmem device online
> >
> 
> Don't forget about checking for the qemu capability and error-ing out properly
> in case it's not supported, you probably know you can use
> qemuBuildChrChardevStr() for the '-chardev' part of the commandline, various
> backends are supported and the code is in already.
> 

OK. Thanks for your reminding.

> The idea looks good, it would be nice improvement to have.  About the
> daemon, you mean it would be another daemon we have in the repo like
> virtlockd, I suppose.
> 

Yes, I think the daemon can be libvirtd or others.
The existing ivmshm daemon was just a proof of concept
demo by the original developers (as Dan said). Maybe
libvirt provides the daemon in feature.

> >
> >There are two ways to use ivshmem with qemu (please refer to
> >http://qemu.weilnetz.de/qemu-doc.html#pcsys_005fother_005fdevs ):
> >1.Guests map a POSIX shared memory region into the guest as a PCI
> >device that enables zero-copy communication to the application level of the
> guests, The basic syntax is:
> >
> >  qemu-system-i386-device ivshmem, size =  > -m> [, shm = ]
> >
> >2.If desired, interrupts can be sent between guest VMs accessing the same
> shared memory region.
> >Interrupt support requires using a shared memory server and using a chardev
> socket to connect to it.
> >An example syntax when using the shared memory server is:
> >
> >  qemu-system-i386-device ivshmem, size =  [,
> chardev = ] [, msi = on]
> >   [, ioeventfd = on] [, vectors = n] [,
> > role = peer | master]  qemu-system-i386-chardev socket, path = ,
> > id = 
> >
> >The respective xml configuration for the above 2 qemu command lines are
> shown as below:
> >
> >Example1: automatically attach device with KVM
> >
> >  
> >
> >  
> >
> >  
> >
> >NOTE: "size" means ivshmem size in unit MB, "name" means shm name
> >  "role" is optional, it may be set to "master" or "peer", the default 
> > one
> is "master"
> >
> 
> What do these roles mean, I mean what's the difference between master and
> peer and why is it only used with the chardev?  Does it mean master can only
> send interrupts or...?  Just curious.

Re: [libvirt] [PATCH] virlog: Add stack trace log when libvirt receives fatal signal

2014-05-21 Thread Wangrui (K)

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Tuesday, May 20, 2014 5:05 PM
> To: Wangrui (K)
> Cc: libvir-list@redhat.com; zhouyimin Zhou(Yimin); Yanqiangjun
> Subject: Re: [PATCH] virlog: Add stack trace log when libvirt receives fatal 
> signal
> 
> On Tue, May 20, 2014 at 08:59:37AM +, Wangrui (K) wrote:
> > An earlier commit(c0c8c1) Dan removed global log buffer feature entirely
> > because of duplicate log messages. An improvement is introduced. That is
> > dumping stack trace instead of log buffer upon libvirt crash.
> 
> 
> This is not going to work. virLogStackTraceToFd invokes the
> backtrace/backtrace_symbols_fd functions which are not async
> signal safe. They are also not likely to be reliable to use
> when you have memory corruption triggering the signal.
> 
> The 'abrt' program is commonly used on modern Linux distros to
> generate stack traces when processes crash / terminate abnormally.
> abrt has the added benefit that the stack traces it records include
> all function parameters and local variables.
> 

Oops.
I didn't realize Async-signal-(un)safe functions in virLogStackTraceToFd.
And it seems that only kernel coredump can be used in this case.
My original purpose is to trace stack on crash/obort and etc without
using kernel.core_pattern (sometimes kernel coredump is not turned on ).

Thanks.

> Regards,
> 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


[libvirt] [PATCH] virlog: Add stack trace log when libvirt receives fatal signal

2014-05-20 Thread Wangrui (K)
An earlier commit(c0c8c1) Dan removed global log buffer feature entirely
because of duplicate log messages. An improvement is introduced. That is
dumping stack trace instead of log buffer upon libvirt crash.

Signed-off-by: Zhou Yimin 
Signed-off-by: Wang Rui 
---

When libvirt receives fatal signal, the log output shows as following.

Caught abort signal dumping stack trace:

== start of stack trace =

/usr/lib64/libvirt.so.0(+0x1d4fde)[0x7f001b1b0fde]
/lib64/libpthread.so.0(+0xf800)[0x7f00182ed800]
/lib64/libc.so.6(gsignal+0x35)[0x7f0017b91b55]
/lib64/libc.so.6(abort+0x181)[0x7f0017b93131]
/lib64/libc.so.6(+0x70e0f)[0x7f0017bcfe0f]
/lib64/libc.so.6(+0x76618)[0x7f0017bd5618]
/lib64/libc.so.6(+0x76c3f)[0x7f0017bd5c3f]
/lib64/libc.so.6(+0x78fee)[0x7f0017bd7fee]
/lib64/libc.so.6(__libc_malloc+0x77)[0x7f0017bda747]
/usr/lib64/libvirt.so.0(virReallocN+0x65)[0x7f001b03c06d]
/usr/lib64/libvirt.so.0(+0x7ebbc)[0x7f001b05abbc]
/usr/lib64/libvirt.so.0(virFileReadLimFD+0x50)[0x7f001b05ad4a]
/usr/lib64/libvirt.so.0(virFileReadAll+0xa6)[0x7f001b05c7d1]
/usr/lib64/libvirt.so.0(virProcessGetStartTime+0x9a)[0x7f001b080ee9]
/usr/lib64/libvirt.so.0(virNetSocketGetUNIXIdentity+0xc1)[0x7f001b1bf709]
/usr/lib64/libvirt.so.0(virNetServerClientGetUNIXIdentity+0x68)[0x7f001b1b35b6]
/usr/sbin/libvirtd(+0x20103)[0x7f001bb8f103]
/usr/sbin/libvirtd(+0x20347)[0x7f001bb8f347]
/usr/lib64/libvirt.so.0(+0x1db9bd)[0x7f001b1b79bd]
/usr/lib64/libvirt.so.0(virNetServerProgramDispatch+0x209)[0x7f001b1b7dff]
/usr/lib64/libvirt.so.0(+0x1d5448)[0x7f001b1b1448]
/usr/lib64/libvirt.so.0(+0x1d572a)[0x7f001b1b172a]
/usr/lib64/libvirt.so.0(+0xb3964)[0x7f001b08f964]
/usr/lib64/libvirt.so.0(+0xb2dd0)[0x7f001b08edd0]
/lib64/libpthread.so.0(+0x77f6)[0x7f00182e57f6]
/lib64/libc.so.6(clone+0x6d)[0x7f0017c3a09d]

 == end of stack trace =

 src/libvirt_private.syms |  1 +
 src/rpc/virnetserver.c   | 43 +++
 src/util/virlog.c| 88 
 src/util/virlog.h|  2 ++
 4 files changed, 134 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c3332c9..d781595 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1501,6 +1501,7 @@ virLogGetFilters;
 virLogGetNbFilters;
 virLogGetNbOutputs;
 virLogGetOutputs;
+virLogEmergencyDumpAll;
 virLogLock;
 virLogMessage;
 virLogParseDefaultPriority;
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 7e3fc0a..2845098 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -340,6 +340,32 @@ static int 
virNetServerDispatchNewClient(virNetServerServicePtr svc,
 return 0;
 }
 
+static void
+virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED,
+void *context ATTRIBUTE_UNUSED)
+{
+struct sigaction sig_action;
+int origerrno;
+
+origerrno = errno;
+virLogEmergencyDumpAll(sig);
+
+/*
+ * If the signal is fatal, avoid looping over this handler
+ * by deactivating it
+ */
+#ifdef SIGUSR2
+if (sig != SIGUSR2) {
+#endif
+memset(&sig_action, 0, sizeof(sig_action));
+sig_action.sa_handler = SIG_DFL;
+sigaction(sig, &sig_action, NULL);
+raise(sig);
+#ifdef SIGUSR2
+}
+#endif
+errno = origerrno;
+}
 
 virNetServerPtr virNetServerNew(size_t min_workers,
 size_t max_workers,
@@ -401,6 +427,23 @@ virNetServerPtr virNetServerNew(size_t min_workers,
 sig_action.sa_handler = SIG_IGN;
 sigaction(SIGPIPE, &sig_action, NULL);
 
+/*
+ * catch fatal errors to dump stack trace, also hook to USR2 for dynamic
+ * debugging purposes or testing
+ */
+sig_action.sa_sigaction = virNetServerFatalSignal;
+sig_action.sa_flags = SA_SIGINFO;
+sigaction(SIGFPE, &sig_action, NULL);
+sigaction(SIGSEGV, &sig_action, NULL);
+sigaction(SIGILL, &sig_action, NULL);
+sigaction(SIGABRT, &sig_action, NULL);
+#ifdef SIGBUS
+sigaction(SIGBUS, &sig_action, NULL);
+#endif
+#ifdef SIGUSR2
+sigaction(SIGUSR2, &sig_action, NULL);
+#endif
+
 return srv;
 
  error:
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 056950e..081b194 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #if HAVE_SYSLOG_H
@@ -661,6 +662,93 @@ virLogStackTraceToFd(int fd)
 }
 
 static void
+virLogDumpAllFD(const char *msg, int len)
+{
+size_t i;
+bool found = false;
+
+if (len <= 0)
+len = strlen(msg);
+
+for (i = 0; i < virLogNbOutputs; i++) {
+if (virLogOutputs[i].f == virLogOutputToFd) {
+int fd = (intptr_t) virLogOutputs[i].data;
+
+if (fd >= 0) {
+if (msg)
+ignore_value(safewrite(fd, msg, len));
+else
+virLogStackTraceToFd(fd);
+
+found = true;
+}
+}
+}
+i

[libvirt] [RFC] require for suggestions on support for ivshmem device

2014-05-14 Thread Wangrui (K)
Hi,

Libvirt does not support ivshmem(Inter-VM Shared Memory) device recently, 
thus, I would like to know if there's any plan to support it in the future?
If not, I would like to contribute a serial of patches to do so.

On Jan 28, Wangyufei (James) asked about this question, and Daniel replied with 
2 suggestions:
1. Libvirt should be capable of configuring the guest's xml on ivshmem.
2.An ivshmem daemon is needed to run on the host to support it, libvirt is 
recommended to provide such a daemon.
Please refer to  
https://www.redhat.com/archives/libvir-list/2014-January/msg01335.html for 
details.

What I'll do later is the 1st suggestion, the 2nd one is left to be 
accomplished by someone else.

Here is the detailed work I'll do to support configuration of the guest in 
libvirt:
virDomainDefParseXML: parse ivshmem device xml when defining 
dom.xml
virDomainDeviceInfoIterateInternal:   iterate ivshmem devices
qemuAssignDevicePCISlots:  assign ivshmem device pci slots
virDomainDefFormatInternal: format ivshmem device xml (Eg. virsh 
edit dom)
virDomainDefFree: free ivshmem device def

qemuBuildCommandLine:   build ivshmem device command line when vm 
starts
qemuAssignDeviceAliases:  assign ivshmem device aliases when vm 
starts

virDomainDeviceDefParse:   attach and parse ivshmem device xml
qemuDomainAttachDeviceConfig: attach ivshmem device xml persistently
qemuDomainAttachDeviceLive:   attach ivshmem device online

qemuDomainDetachDeviceConfig: detach ivshmem device xml persistently
qemuDomainDetachDeviceLive:   detach ivshmem device online


There are two ways to use ivshmem with qemu 
(please refer to http://qemu.weilnetz.de/qemu-doc.html#pcsys_005fother_005fdevs 
):
1.Guests map a POSIX shared memory region into the guest as a PCI device
that enables zero-copy communication to the application level of the guests, 
The basic syntax is:

  qemu-system-i386-device ivshmem, size =  [, 
shm = ] 

2.If desired, interrupts can be sent between guest VMs accessing the same 
shared memory region.
Interrupt support requires using a shared memory server and using a chardev 
socket to connect to it. 
An example syntax when using the shared memory server is: 

  qemu-system-i386-device ivshmem, size =  [, 
chardev = ] [, msi = on]
   [, ioeventfd = on] [, vectors = n] [, role = 
peer | master] 
  qemu-system-i386-chardev socket, path = , id = 

The respective xml configuration for the above 2 qemu command lines are shown 
as below:

Example1: automatically attach device with KVM

  

  

  

NOTE: "size" means ivshmem size in unit MB, "name" means shm name
  "role" is optional, it may be set to "master" or "peer", the default one 
is "master"

Example2: manually attach device with static PCI slot 4 requested 

  

  
  

  

Example3: automatically attach device with KVM

  

  
  

  

NOTE: "path" means shared memory socket path which is set by the daemon.
  " source mode " and "type" is similar with vmchannel.


I'm looking forward to your suggestions, thank you very much.

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


Re: [libvirt] [PATCH] network: Defer online of macvtap during qemu migration

2014-05-13 Thread Wangrui (K)


> -Original Message-
> From: sendmail [mailto:justsendmailnothinge...@gmail.com] On Behalf Of
> Laine Stump
> Sent: Tuesday, May 13, 2014 10:11 PM
> To: Matthew Rosato; libvir-list@redhat.com
> Cc: Wangrui (K)
> Subject: Re: [libvirt] [PATCH] network: Defer online of macvtap during qemu
> migration
> 
> On 05/13/2014 04:31 PM, Matthew Rosato wrote:
> > On 05/05/2014 12:33 PM, Matthew Rosato wrote:
> >> On 05/05/2014 12:26 PM, Matthew Rosato wrote:
> >>> When generating macvtaps via
> virNetDevMacVLanCreateWithVPortProfile,
> >>> the macvtap device is unconditionally set to the up state.  However,
> >>> during migration, this results in a case where both the source and
> >>> target system are simultaneously up with the same MAC address.  This
> >>> patch defers bringing the target macvtap up until later in the
> >>> migration to shrink this window.
> >>>
> >>> Signed-off-by: Matthew Rosato 
> >> Forgot to mention that this patch is associated with what Wangrui
> >> reported here:
> >>
> >> http://www.redhat.com/archives/libvir-list/2014-March/msg01054.html
> >>
> >> and follows Viktor's suggested solution mentioned here:
> >>
> >> http://www.redhat.com/archives/libvir-list/2014-March/msg01654.html
> >>
> >>
> > Ping.

Thanks for fix the issue that I reported.
I think it can be considered that all types of netdevs are brought up until 
migration finish, not only macvtap.

> 
> Review coming up. Sorry for the delay. (I'm looking to see if the fix
> can be simplified to bring up the macvtap interfaces *always*, rather
> than just during migration.)
> 
> BTW, there is an open BZ for this:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1081461
> 
> 

I haven't seen the open BZ until Laine mentioned.
Next time I will search the open BZs before report a new issue.

> >
> >>> ---
> >>>  src/qemu/qemu_migration.c   |   18 ++
> >>>  src/util/virnetdevmacvlan.c |   11 ---
> >>>  2 files changed, 26 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> >>> index a9f7fea..aee803a 100644
> >>> --- a/src/qemu/qemu_migration.c
> >>> +++ b/src/qemu/qemu_migration.c
> >>> @@ -56,6 +56,7 @@
> >>>  #include "virhook.h"
> >>>  #include "virstring.h"
> >>>  #include "virtypedparam.h"
> >>> +#include "virnetdev.h"
> >>>
> >>>  #define VIR_FROM_THIS VIR_FROM_QEMU
> >>>
> >>> @@ -4468,6 +4469,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
> >>>  virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> >>>  virCapsPtr caps = NULL;
> >>>  unsigned short port;
> >>> +virDomainNetDefPtr net;
> >>> +size_t i;
> >>>
> >>>  VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s,
> cookieinlen=%d, "
> >>>"cookieout=%p, cookieoutlen=%p, flags=%lx,
> retcode=%d",
> >>> @@ -4574,6 +4577,21 @@ qemuMigrationFinish(virQEMUDriverPtr
> driver,
> >>>  }
> >>>
> >>>  if (!(flags & VIR_MIGRATE_PAUSED) && !(flags &
> VIR_MIGRATE_OFFLINE)) {
> >>> +/* Macvtaps were previously left offline, bring them online
> now */
> >>> +for (i = 0; i < vm->def->nnets; i++) {
> >>> +net = vm->def->nets[i];
> >>> +if (virDomainNetGetActualType(net) ==
> VIR_DOMAIN_NET_TYPE_DIRECT) {
> >>> +if (virNetDevSetOnline(net->ifname, true) < 0) {
> >>> +
> ignore_value(virNetDevVPortProfileDisassociate(net->ifname,
> >>> +
> virDomainNetGetActualVirtPortProfile(net),
> >>> +
> &net->mac,
> >>> +
> virDomainNetGetActualDirectDev(net),
> >>> +
> -1,
> >>> +
> VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH));
> >>> +
> ignore_value(virNetDevMacVLanDelete(net->ifname));
> >>> +}
> >>> +}
> >>> +}
> >>>  /* run 'cont' on the destination, which allows migration on
> qemu
> >>>   * >= 0.10.6 to work properly.  This isn't strictly
> necessary on
> >>>  

Re: [libvirt] [PATCH] util: refactor virNetlinkCommand to fix several bugs / style problems

2014-05-13 Thread Wangrui (K)
Good improvement especially in coding style from which I learned difference 
between rc and ret.
:)

> -Original Message-
> From: Laine Stump [mailto:la...@laine.org]
> Sent: Tuesday, May 13, 2014 7:52 PM
> To: libvir-list@redhat.com; Wangrui (K); Yanqiangjun; Zhangbo (Oscar)
> Subject: [PATCH] util: refactor virNetlinkCommand to fix several bugs / style
> problems
> 
> Inspired by a simpler patch from "Wangrui (K) ".
> 
> A submitted patch pointed out that virNetlinkCommand() was doing an
> improper typecast of the return value from nl_recv() (int to
> unsigned), causing it to miss error returns, and that even after
> remedying that problem, virNetlinkCommand() was calling VIR_FREE() on
> the pointer returned from nl_recv() (*resp) even if nl_recv() had
> returned an error, and that in this case the pointer was verifiably
> invalid, as it was pointing to memory that had been allocated by
> libnl, but then freed prior to returning the error.
> 
> While reviewing this patch, I noticed several other problems with this
> seemingly simple function (at least one of them as serious as the
> problem being reported/fixed by the aforementioned patch), and decided
> they all deserved to be fixed. Here is the list:
> 
> 1) The return value from nl_recv() must be assigned to an int (rather
>than unsigned int) in order to detect failure.
> 
> 2) When nl_recv() returns an error, the contents of *resp is invalid,
>and should be simply set to 0, *not* VIR_FREE()'d.
> 
> 3) The first error return from virNetlinkCommand returns -EINVAL,
>incorrectly implying that the caller can expect the return value to
>be of the "-errno" variety, which is not true in any other case.
> 
> 4) The 2nd error return returns directly with garbage in *resp. While
>the caller should never use *resp in this case, it's still good
>practice to set it to NULL.
> 
> 5) For the next 5 (!!) error conditions, *resp will contain garbage,
>and virNetlinkCommand() will goto it's cleanup code which will
>VIR_FREE(*resp), almost surely leading to a segfault.
> 
> In addition to fixing these 5 problems, this patch also makes the
> following two changes to make the function conform more closely to the
> style of other libvirt code:
> 
> 1) Change the handling of return code from "named rc and defaulted to
> 0, but changed to -1 on error" to the more common "named ret and
> defaulted to -1, but changed to 0 on success".
> 
> 2) Rename the "error" label to "cleanup", since the code that follows
> is executed in success cases as well as failure.
> ---
>  src/util/virnetlink.c | 42 --
>  1 file changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index 0cf18f2..db8d59e 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2010-2013 Red Hat, Inc.
> + * Copyright (C) 2010-2014 Red Hat, Inc.
>   * Copyright (C) 2010-2012 IBM Corporation
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -180,7 +180,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
>uint32_t src_pid, uint32_t dst_pid,
>unsigned int protocol, unsigned int groups)
>  {
> -int rc = 0;
> +int ret = -1;
>  struct sockaddr_nl nladdr = {
>  .nl_family = AF_NETLINK,
>  .nl_pid= dst_pid,
> @@ -192,41 +192,39 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
>  int n;
>  struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg);
>  virNetlinkHandle *nlhandle = NULL;
> +int len = 0;
> 
>  if (protocol >= MAX_LINKS) {
>  virReportSystemError(EINVAL,
>   _("invalid protocol argument: %d"),
> protocol);
> -return -EINVAL;
> +goto cleanup;
>  }
> 
>  nlhandle = virNetlinkAlloc();
>  if (!nlhandle) {
>  virReportSystemError(errno,
>   "%s", _("cannot allocate nlhandle for
> netlink"));
> -return -1;
> +goto cleanup;
>  }
> 
>  if (nl_connect(nlhandle, protocol) < 0) {
>  virReportSystemError(errno,
>  _("cannot connect to netlink socket with
> protocol %d"),
>   protocol);
> -rc = -1;
> -goto error;
> +goto cleanup;
>  }
> 
>  fd = nl_socket_get_fd(nlhandle);
>  if (fd < 0) {
>  virReportSystemError(errno,
>

[libvirt] [Question] (python API) setMemoryParameters doesn't support flags CONFIG | LIVE

2014-05-13 Thread Wangrui (K)
Hi, all

when I use (python API) setMemoryParameters with flags= 
VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE ,
an error occurs as follow: (libvirt 1.2.4)

libvirt:  error : flags 'affect live' and 'affect config' in 
virDomainGetMemoryParameters are mutually exclusive

I check the function libvirt_virDomainSetMemoryParameters in libvirt-override.c 
and find the code below:

Line 1204-1206:
 LIBVIRT_BEGIN_ALLOW_THREADS;
 i_retval = virDomainGetMemoryParameters(domain, NULL, &nparams, flags);
 LIBVIRT_END_ALLOW_THREADS;

Line 1220-1222:
LIBVIRT_BEGIN_ALLOW_THREADS;
i_retval = virDomainGetMemoryParameters(domain, params, &nparams, flags); 
LIBVIRT_END_ALLOW_THREADS;

Function virDomainGetMemoryParameters doesn't support flags 
VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE.
So the error above occurs.

The first call of virDomainGetMemoryParameters is to "cause @nparams on output 
to contain the number of parameters
supported by the hypervisor". And what's the role of the second call?

Maybe, so do other functions such as *libvirt_virDomainSetSchedulerParameters, 
*libvirt_virDomainSetBlkioParameters
In commit 56cec18d761a1f99862c43811de60380c65881e6.

IMHO, an example flag (such as VIR_DOMAIN_AFFECT_CONFIG) can be used in 
virDomainGetMemoryParameters  
just to get something from hypervisor rather than @flags which is a parameter 
of setMemoryParameters .

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


[libvirt] [PATCH 2/2] util: do not set resp to NULL after VIR_FREE

2014-05-12 Thread Wangrui (K)
Signed-off-by: Zhang Bo 
---
src/util/virnetlink.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 4f4dedc..40912f5 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -271,7 +271,6 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
  cleanup:
 if (rc == -1) {
 VIR_FREE(*resp);
-*resp = NULL;
 *respbuflen = 0;
 }
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] util: replace error with cleanup in virNetlinkCommand

2014-05-12 Thread Wangrui (K)
In funtcion virNetlinkCommand, it uses the word "error" as the goto out
keyword, but it's not an error-handling branch, so, use "cleanup" instead.

Signed-off-by: Zhang Bo 
---
src/util/virnetlink.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 1a09567..4f4dedc 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -212,7 +212,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 _("cannot connect to netlink socket with protocol %d"),
  protocol);
 rc = -1;
-goto error;
+goto cleanup;
 }
 fd = nl_socket_get_fd(nlhandle);
@@ -220,14 +220,14 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 virReportSystemError(errno,
  "%s", _("cannot get netlink socket fd"));
 rc = -1;
-goto error;
+goto cleanup ;
 }
 if (groups && nl_socket_add_membership(nlhandle, groups) < 0) {
 virReportSystemError(errno,
  "%s", _("cannot add netlink membership"));
 rc = -1;
-goto error;
+goto cleanup;
 }
 nlmsg_set_dst(nl_msg, &nladdr);
@@ -239,7 +239,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 virReportSystemError(errno,
  "%s", _("cannot send to netlink socket"));
 rc = -1;
-goto error;
+goto cleanup;
 }
 memset(fds, 0, sizeof(fds));
@@ -255,7 +255,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 virReportSystemError(ETIMEDOUT, "%s",
  _("no valid netlink response was received"));
 rc = -1;
-goto error;
+goto cleanup;
 }
 length = nl_recv(nlhandle, &nladdr,
@@ -268,7 +268,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 } else {
 *respbuflen = length;
 }
- error:
+ cleanup:
 if (rc == -1) {
 VIR_FREE(*resp);
 *resp = NULL;
--
1.7.12.4

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

[libvirt] [PATCH 0/2] *** replace error with cleanup in virNetlinkCommand ***

2014-05-12 Thread Wangrui (K)
Zhang Bo (2):
  util: replace error with cleanup in virNetlinkCommand
  util: do not set resp to NULL after VIR_FREE

src/util/virnetlink.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)

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

[libvirt] [PATCH] util: Fix return type mismatch for nl_recv

2014-05-12 Thread Wangrui (K)
1.
As shown in the definition of nl_recv, its return value is of INT type.

int nl_recv(struct nl_handle *handle, struct sockaddr_nl *nla,
 unsigned char **buf, struct ucred **creds)

However, in function virNetlinkCommand, it uses an unsigned int param,
respbuflen, to receive its return value. Thus, the branch "*respbuflen < 0"
is unreachable, negative return values are handled incorrectly.

2.
It's said in nl_recv's description that "The caller is responsible for
freeing the buffer allocated * in \c *buf if a positive value is returned."
which means, we needn't to free it in case it fails.
Additionally, nl_recv has a BUG in handling buf: in the error branch, it frees
the buf, but didn't set it to NULL. Freeing it outside in the caller function
will cause double free.
Thus, we set but(resp) to NULL if nl_recv fails.

Signed-off-by: Zhang Bo 
---
src/util/virnetlink.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0cf18f2..1a09567 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -192,6 +192,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 int n;
 struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg);
 virNetlinkHandle *nlhandle = NULL;
+int length = 0;
 if (protocol >= MAX_LINKS) {
 virReportSystemError(EINVAL,
@@ -257,12 +258,15 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 goto error;
 }
-*respbuflen = nl_recv(nlhandle, &nladdr,
-  (unsigned char **)resp, NULL);
+length = nl_recv(nlhandle, &nladdr,
+ (unsigned char **)resp, NULL);

-if (*respbuflen <= 0) {
+if (length <= 0) {
 virReportSystemError(errno,
  "%s", _("nl_recv failed"));
+   *resp = NULL;
 rc = -1;
+} else {
+*respbuflen = length;
 }
  error:
 if (rc == -1) {
--
1.7.12.4

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

Re: [libvirt] [patch v3 1/2] Add support for migration URI configuration

2014-05-07 Thread Wangrui (K)
I think *cfg* should be unref by virObjectUnref(cfg) .
And so does patch 2/2.

> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
> On Behalf Of Chen Fan
> Sent: Wednesday, May 07, 2014 6:12 PM
> To: libvir-list@redhat.com
> Subject: [libvirt] [patch v3 1/2] Add support for migration URI configuration
> 
> Signed-off-by: Chen Fan 
> ---
>  src/qemu/qemu.conf| 5 -
>  src/qemu/qemu_conf.c  | 2 ++
>  src/qemu/qemu_conf.h  | 2 ++
>  src/qemu/qemu_migration.c | 5 +
>  4 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index f0e802f..2973631 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -449,7 +449,10 @@
>  #
>  #seccomp_sandbox = 1
> 
> -
> +# Override the URI used for any specific migration URI to be sent.
> +# Defaults to NULL, will be set to as "tcp://hostIP[:port]".
> +#
> +#migrate_uri = "tcp://hostIP:port"
> 
>  # Override the listen address for all incoming migrations. Defaults to
>  # 0.0.0.0, or :: if both host and qemu are capable of IPv6.
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 198ee2f..0bd943d 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -576,6 +576,8 @@ int
> virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> 
>  GET_VALUE_STR("migration_address", cfg->migrationAddress);
> 
> +GET_VALUE_STR("migrate_uri", cfg->migrateUri);
> +
>  ret = 0;
> 
>   cleanup:
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index a36ea63..2e45421 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -167,6 +167,8 @@ struct _virQEMUDriverConfig {
>  char *migrationAddress;
>  int migrationPortMin;
>  int migrationPortMax;
> +
> +char *migrateUri;
>  };
> 
>  /* Main driver state */
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index a9f7fea..bcf966a 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2639,6 +2639,7 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr
> driver,
>  int ret = -1;
>  virURIPtr uri = NULL;
>  bool well_formed_uri = true;
> +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> 
>  VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, "
>"cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, "
> @@ -2649,6 +2650,10 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr
> driver,
> 
>  *uri_out = NULL;
> 
> +if (uri_in == NULL && cfg->migrateUri) {
> +uri_in = cfg->migrateUri;
> +}
> +
>  /* The URI passed in may be NULL or a string
> "tcp://somehostname:port".
>   *
>   * If the URI passed in is NULL then we allocate a port number
> --
> 1.8.1.4
> 
> --
> 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


[libvirt] [question]qemu: migrate and destory vm immediately on destination causes libvirt deadlock or segment fault

2014-05-06 Thread Wangrui (K)
Hi everyone,

I do ram migration operation in KVM environment(libvirt1.2.4 qemu1.5.1).
I encountered libvirtd deadlock or segmentfault when I destroy the
migration VM on destination.
I got the problem by flowing steps:
step 1: migrate VM.
step 2: execute "virsh destroy [VMName]" to destroy the migration VM on
  destination immediately.
step 3: the destination libvirtd will be probably deadlock or segmentfault.

Deadlock stack as followed:
#0  0x7fb5c18132d4 in __lll_lock_wait () from /lib64/libpthread.so.0
#1  0x7fb5c180e659 in _L_lock_1008 () from /lib64/libpthread.so.0
#2  0x7fb5c180e46e in pthread_mutex_lock () from /lib64/libpthread.so.0
#3  0x7fb5c45d175f in virMutexLock (m=0x7fb5b0066ed0) at util/virthread.c:88
#4  0x7fb5c45b6b04 in virObjectLock (anyobj=0x7fb5b0066ec0) at
util/virobject.c:323
#5  0x7fb5b8f4842a in qemuMonitorEmitEvent (mon=0x7fb5b0066ec0,
event=0x7fb5b00688d0 "SHUTDOWN", seconds=1399374472, micros=509994,
details=0x0) at qemu/qemu_monitor.c:1185
#6  0x7fb5b8f62af2 in qemuMonitorJSONIOProcessEvent (mon=0x7fb5b0066ec0,
obj=0x7fb5b0069080) at qemu/qemu_monitor_json.c:158
#7  0x7fb5b8f62d25 in qemuMonitorJSONIOProcessLine (mon=0x7fb5b0066ec0,
line=0x7fb5b005bbe0 "{\"timestamp\": {\"seconds\": 1399374472,
\"microseconds\": 509994}, \"event\": \"SHUTDOWN\"}",msg=0x7fb5bd873c80)
at qemu/qemu_monitor_json.c:195
#8  0x7fb5b8f62f85 in qemuMonitorJSONIOProcess (mon=0x7fb5b0066ec0,
data=0x7fb5b0060770 "{\"timestamp\": {\"seconds\": 1399374472,
\"microseconds\": 509994},\"event\": \"SHUTDOWN\"}\r\n", len=85,
msg=0x7fb5bd873c80) at qemu/qemu_monitor_json.c:237
#9  0x7fb5b8f49aa0 in qemuMonitorIOProcess (mon=0x7fb5b0066ec0)
at qemu/qemu_monitor.c:402
#10 0x7fb5b8f4a09b in qemuMonitorIO (watch=20, fd=24, events=0,
opaque=0x7fb5b0066ec0) at qemu/qemu_monitor.c:651
#11 0x7fb5c458c4d9 in virEventPollDispatchHandles (nfds=17, 
fds=0x7fb5b0068a60)
at util/vireventpoll.c:510
#12 0x7fb5c458decf in virEventPollRunOnce () at util/vireventpoll.c:659
#13 0x7fb5c458bfcc in virEventRunDefaultImpl () at util/virevent.c:308
#14 0x7fb5c51a17a9 in virNetServerRun (srv=0x7fb5c5411d70)
at rpc/virnetserver.c:1139
#15 0x7fb5c5157f63 in main (argc=3, argv=0x7fff7fc04f48) at libvirtd.c:150


After analysis, I found it may be caused by multithreaded simultaneously
access to the global variables "vm->privateData->mon".
When problems occur,there are three libvirtd threads at work on destination
host,suppose:
ThreadA: migration thread,do qemuProcessStart.
ThreadB: destroy thread,do qemuDoaminDestroy -> qemuProcessStop.
ThreadC:Monitor Thread,do IOWrite、IORead and some other operations according to
the mon->msg when mon->fd change. When threadB destroy happpens, this thread 
would
handle the SUHTDOWN event.

In threadA, when it sends QMP command to Qemu, it will operate the 
vm->privateData->mon
lock. Such as the operation "qemuDomainObjEnterMonitor -> qemuMonitorSetBalloon 
->
qemuDomainObjExitMonitor", but it's not an atomic operation. If "virsh destroy 
[VMName]"
happens during this operation, threadB will set the lock vm->privateData->mon 
to NULL in
qemuProcessStop. And then in threadA, the function qemuDomainObjExitMonitor 
will fail to
unlock vm->privateData->mon as it's NULL. So, threadC will never acquire the
vm->privateData->mon lock and the deadlock problem happened.

what was worse, if qemuMonitorSetBalloon perform succeed in threadA.
ThreadA will coutinue to execute till it enter the function 
qemuMonitorSetDomainLog,
it would cause segmentfault at VIR_FORCE_CLOSE(mon->logfd) due to mon is NULL.

I could not find a good way to sovle this problem. Does anyone have good ideas?
Thanks.

Ps:
I find an easy way to reproduce this problem  more probably by following steps:

step 1: Fault Injection, fit into this patch and update the libvirtd on 
destination host:
--- src/qemu/qemu_process.c 2014-05-06 19:06:00.0 +0800
+++ src/qemu/qemu_process.c 2014-05-06 19:07:12.0 +0800
@@ -4131,6 +4131,8 @@
vm->def->mem.cur_balloon);
 goto cleanup;
 }
+VIR_DEBUG("Fault Injection, sleep 3 seconds.");
+sleep(3);
 qemuDomainObjEnterMonitor(driver, vm);
 if (vm->def->memballoon && vm->def->memballoon->period)
 qemuMonitorSetMemoryStatsPeriod(priv->mon, 
vm->def->memballoon->period);

step 2: migrate VM.
step 3: execute "virsh destroy [VMName]" to destroy the migration VM on 
destination
when log prints "Fault Injection, sleep 3 seconds."
step 4: the libvirtd deadlock stack occurs as above mentioned.

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

Re: [libvirt] lxc: shutdown $domain broken in 1.2.2

2014-05-06 Thread Wangrui (K)
Does this problem has any progress now? I have the same problem.

# virsh shutdown vm1
error: Failed to shutdown domain vm1
error: Mount namespaces are not available on this platform:
 Function not implemented from the logfile:

 [2014-05-06 15:26:57 CST]: libvirtd : 5194: error : 
virProcessRunInMountNamespace:960 :
 Mount namespaces are not available on this platform: Function not implemented

My glibc version is glibc-2.11.3-17.54.1, and virProcessRunInMountNamespace 
takes
the "!HAVE_SETNS" branch.

Thanks.

> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
> On Behalf Of Stephan Sachse
> Sent: Monday, March 03, 2014 8:49 PM
> To: libvir-list@redhat.com
> Subject: [libvirt] lxc: shutdown $domain broken in 1.2.2
> 
> host: centos-6.5
> kernel: 3.13.2-4.el6.x86_64
> guest: fedora20
> 
> since libvirt-1.2.2 i can not shutdown this domain. works fine with
> libvirt-1.2.1
> 
> # virsh shutdown fedora2
> error: Failed to shutdown domain fedora2
> error: Mount namespaces are not available on this platform: Function not
> implemented
> 
> from the logfile: 14478: error : virProcessRunInMountNamespace:982 :
> Mount namespaces are not available on this platform: Function not
> implemented
> 
> the buildhost for the rpm is a centos-6.5 system with kernel 3.1.8 (dont ask! 
> its
> a vserver system) and the test for "setns" failed
> 
> configure:9592: checking for setns
> configure:9592: gcc -std=gnu99 -o conftest -g -O2   conftest.c  >&5
> /tmp/cchlir6v.o: In function `main':
> /builddir/build/BUILD/libvirt-1.2.2/conftest.c:184: undefined reference to
> `setns'
> collect2: ld returned 1 exit status
> 
> setns() was added in glibc-2.14 centos6 has glibc-2.12
> 
> attached is a fix for this problem.
> 
> stolen from:
> http://cgit.freedesktop.org/systemd/systemd/commit/src/shared/missing.h?i
> d=3b794314149e40afaf3c456285e1e529747b6560
> 
> /stephan
> 
> --
> Software is like sex, it's better when it's free!

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


[libvirt] [PATCH] virnetdev: Fix Memory Leak in virNetDevReplaceMacAddress() and virNetDevRestoreMacAddress()

2014-04-07 Thread Wangrui (K)
function virNetDevRestoreMacAddress() and virNetDevRestoreMacAddress() alloc 
memory for
variable 'path' using virAsprintf(), but they havn't free that memory before 
returning out.

Signed-off-by: Zhang bo 
---
 src/util/virnetdev.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index f26ea80..853f6ef 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -310,6 +310,7 @@ virNetDevReplaceMacAddress(const char *linkdev,
 virMacAddr oldmac;
 char *path = NULL;
 char macstr[VIR_MAC_STRING_BUFLEN];
+int ret = -1;
 
 if (virNetDevGetMAC(linkdev, &oldmac) < 0)
 return -1;
@@ -323,13 +324,17 @@ virNetDevReplaceMacAddress(const char *linkdev,
 if (virFileWriteStr(path, macstr, O_CREAT|O_TRUNC|O_WRONLY) < 0) {
 virReportSystemError(errno, _("Unable to preserve mac for %s"),
  linkdev);
-return -1;
+   goto cleanup;
 }
 
 if (virNetDevSetMAC(linkdev, macaddress) < 0)
-return -1;
+goto cleanup;
 
-return 0;
+ret = 0;
+
+cleanup:
+VIR_FREE(path);
+return ret;
 }
 
 /**
@@ -344,7 +349,7 @@ int
 virNetDevRestoreMacAddress(const char *linkdev,
const char *stateDir)
 {
-int rc;
+int rc = -1;
 char *oldmacname = NULL;
 char *macstr = NULL;
 char *path = NULL;
@@ -356,21 +361,22 @@ virNetDevRestoreMacAddress(const char *linkdev,
 return -1;
 
 if (virFileReadAll(path, VIR_MAC_STRING_BUFLEN, &macstr) < 0)
-return -1;
+goto cleanup;
 
 if (virMacAddrParse(macstr, &oldmac) != 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse MAC address from '%s'"),
oldmacname);
-VIR_FREE(macstr);
-return -1;
+goto cleanup;
 }
 
 /*reset mac and remove file-ignore results*/
 rc = virNetDevSetMAC(linkdev, &oldmac);
 ignore_value(unlink(path));
-VIR_FREE(macstr);
 
+cleanup:
+VIR_FREE(macstr);
+VIR_FREE(path);
 return rc;
 }
 
-- 
1.8.3.4

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


[libvirt] question for qemu migrate fail by virDomainMigrate API

2014-03-27 Thread Wangrui (K)
Hi everyone,

when I use the API virDomainMigrate to do migration operation in KVM 
environment(libvirt1.1.0 qemu1.5.1), I encountered with some problems.
I found that If the connection to source side is disconnected in the BEGIN 
phase of a migration, the migration job would fail.
Further more, the following-up migration of the same VM would not be successful 
until restart libvirtd.

The error log:
libvirtd : 8406: error : virQEMUCloseCallbacksSet:781 : internal error Close 
callback for domain myvm1 already registered with another connection 
0x7f023801a900
libvirt: Domain Config error : Requested operation is not valid: domain is 
already active as 'myvm1'

I got the above error by following steps:
step 1 :  migrate VM.
step 2 :  disconnect the client connection to source libvirtd at once ,such as 
ctrl+c.
step 3 :  the migrate API returns fail.
step 4 :  migrate this VM again , fails and reports above error.

The reason causing this problem may be:

In the BEGIN phase of a migraion, it registers a close callback through 
virQEMUCloseCallbacksSet function.
In the normal flow of migration, the registered callback is cleanup in the 
PERFORM and CONFIRM phase. 
But if the connection to source side is disconnected before PERFORM phase or 
between PERFORM phase and CONFIRM phase, 
the close callback will not be cleanup, and the problem mentioned above occurs.

I have test it on libvirt1.2.2, it also happens. 

I tried, but could not find a good way to solve this problem. Does anyone have 
any good ideas? Thanks!

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

[libvirt] question for in ethernet type and multiple queues in KVM (start VM failed)

2014-03-26 Thread Wangrui (K)
Hi,
  
I use ethernet vif for VM (libvirt 1.1.0  qemu 1.5.1). xml as such









tap_mq is a tap device with multi_queue property which was created on host 
by ip command, such as follows
#ip tuntap add tap_mq mode tap multi_queue

I want to use multiple queue feature for this vif so I set queues='2' in 
xml.

When start VM , an error occurs 
"error: unsupported configuration: Multiqueue network is not supported for: 
ethernet".
This error is reported from function qemuBuildInterfaceCommandLine

/* Currently nothing besides TAP devices supports multiqueue. */
if (net->driver.virtio.queues > 0 &&
!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
  actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
   _("Multiqueue network is not supported for: %s"),
   virDomainNetTypeToString(actualType));
return -1;
}

The comment specifies that only TAP device supports multiqueue. But 
VIR_DOMAIN_NET_TYPE_ETHERNET is also TAP device I think.

IMHO, libvirt can build qemu commandline as 
"-device virtio-net,netdev=net1,mq=on,vectors=5,mac=52:54:00:19:9e:4e"
"-netdev type=tap,ifname=tap0,id=net1,vhost=on,vhostforce=on,queues=2".
Qemu can open tapfds and vhostfds by itself other than libvirt opens and 
transfers them to qemu in this case.

Or there's other way to use multiqueue feature in ethernet tpye ?

Regards,
Rui

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


Re: [libvirt] [RFC] VM which uses macvtap will not respond ping request when being migrated

2014-03-24 Thread Wangrui (K)
Thanks.

libvirt 1.1.0  qemu 1.5.1  host kernel 3.0.58

guest hasn't ipv6 enabled.

macvtap0  Link encap:Ethernet  HWaddr 52:54:00:37:CD:68
  inet6 addr: fe80::5054:ff:fe37:cd68/64 Scope:Link
  UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
  RX packets:0 errors:0 dropped:0 overruns:0 frame:0
  TX packets:6 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:500
  RX bytes:0 (0.0 b)  TX bytes:468 (468.0 b)

macvtap0 is belong to vm at the dest side. When migrate begins macvtap0 will 
send the packet like this:
  
07:57:59.233270 IP6 :: > ff02::16: HBH ICMP6, multicast listener report v2, 1 
group record(s), lengt 28
07:58:00.096088 IP6 :: > ff02::1:ff6d:4665: ICMP6, neighbor solicitation, who 
has fe80::5054:ff:fe6d4665, length 24
07:58:00.123900 IP6 fe80::2a6e:d4ff:fe50:8d7 > ipv6-allrouters: ICMP6, router 
solicitation, length 1
07:58:00.175891 IP6 fe80::2a6e:d4ff:fe50:8d7 > ff02::16: HBH ICMP6, multicast 
listener report v2, 1 roup record(s), length 28
07:58:01.096089 IP6 fe80::5054:ff:fe6d:4665 > ipv6-allrouters: ICMP6, router 
solicitation, length 16

the virNetDevSetOnline in the libvirt makes macvtap0 up on the dest side, so 
macvtap0 sends the packets.

I do nothing when VM is migrating. The IPv6 packets are sent automatically from 
the dest host when it is up.
I guess this makes route table updated in switch.

the configure of macvtap in xml like this 

  
  
 

-Original Message-
From: sendmail [mailto:justsendmailnothinge...@gmail.com] On Behalf Of Laine 
Stump
Sent: Saturday, March 22, 2014 2:39 AM
To: libvir-list@redhat.com
Cc: Wangrui (K); chenliang (T); Zhaoyanbin (A); Michael S. Tsirkin
Subject: Re: [libvirt] [RFC] VM which uses macvtap will not respond ping 
request when being migrated

On 03/18/2014 02:12 AM, Wangrui (K) wrote:
> A vm which uses macvtap will not respond ping request, when the vm is being 
> migrated. 
>
> I found that on the destination side the macvtap will send a IPv6 packet at 
> the begin of migration to update the route table in switch, however VM is 
> still on the src.
>
> In this case , what can I do to avoid VM's network disconnection
>

Can you provide more details on this? What are the versions of libvirt,
qemu, and kernel on your host system? Can you provide the 
section of the guest's config?

Does this only occur when the guest has IPv6 enabled, and only for an
IPv6 ping? What is the packet that is sent? At the beginning of the
migration the guest is not running on the destination, and the host has
no idea what the guest's IPv6 (or v4) address might be, so it can't send
a IPv6 packet on behalf of the guest. The only thing I can think of
could be something to do with the link-local address (the fe80:...
address that is used for IPv6's discovery and autoconf protocols).

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


Re: [libvirt] [PATCH] virlog: Modify virLogParseDefaultPriority's comment of return value

2014-03-20 Thread Wangrui (K)
>From 918f814aacb258ea9641ed1bb9e48baf2d93082a Mon Sep 17 00:00:00 2001
From: Zhou Yimin 
Date: Fri, 21 Mar 2014 14:52:40 +0800
Subject: [PATCH] virlog: Modify virLogParseDefaultPriority's comment of return 
value

virLogParseDefaultPriority's successful return value is the same as
virLogSetDefaultPriority's successful return value. So it should be 0
rather than the parsed log level.

Signed-off-by: Zhou Yimin 
---
 src/util/virlog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 5d4d3c7..9ae30b6 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1365,7 +1365,7 @@ virLogGetNbOutputs(void)
  *3: WARNING
  *4: ERROR
  *
- * Returns the parsed log level or -1 on error.
+ * Returns 0 if successful, -1 in case of error.
  */
 int
 virLogParseDefaultPriority(const char *priority)
-- 
1.7.12.4

-Original Message-
From: Michal Privoznik [mailto:mpriv...@redhat.com] 
Sent: Thursday, March 20, 2014 10:56 PM
To: Wangrui (K); libvir-list@redhat.com
Cc: zhouyimin Zhou(Yimin); Yanqiangjun; Zhaoyanbin (A)
Subject: Re: [libvirt] [PATCH] virlog: Modify virLogParseDefaultPriority's 
comment of return value

On 17.03.2014 13:31, Wangrui (K) wrote:
> virLogParseDefaultPriority's successful return value is the same as
>
> virLogSetDefaultPriority's successful return value. So it should be 0
>
> rather than the parsed log level.
>
> Signed-off-by: Zhou Yimin 
>
> ---
>
> src/util/virlog.c | 2 +-
>
> 1 file changed, 1 insertion(+), 1 deletion(-)
>

I was just about to review this patch, but it does not apply anymore. 
Can you rebase and resend?

Michal



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


[libvirt] [RFC] VM which uses macvtap will not respond ping request when being migrated

2014-03-18 Thread Wangrui (K)
A vm which uses macvtap will not respond ping request, when the vm is being 
migrated. 

I found that on the destination side the macvtap will send a IPv6 packet at the 
begin of migration to update the route table in switch, however VM is still on 
the src.

In this case , what can I do to avoid VM's network disconnection


Regards,
Wangrui

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


[libvirt] [PATCH] virlog: Modify virLogParseDefaultPriority's comment of return value

2014-03-17 Thread Wangrui (K)
virLogParseDefaultPriority's successful return value is the same as
virLogSetDefaultPriority's successful return value. So it should be 0
rather than the parsed log level.

Signed-off-by: Zhou Yimin 
---
src/util/virlog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 68af0f3..3d4edb8 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1638,7 +1638,7 @@ virLogGetNbOutputs(void)
  *3: WARNING
  *4: ERROR
  *
- * Returns the parsed log level or -1 on error.
+ * Returns 0 if successful, -1 in case of error.
  */
int
virLogParseDefaultPriority(const char *priority)
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virlog: add log lock in virLogSetDefaultPriority

2014-03-17 Thread Wangrui (K)
When the global variable virLogDefaultPriority is modified, it will
be better to have a lock.

Signed-off-by: Zhou Yimin 
---
src/util/virlog.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 68af0f3..ba370e2 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -498,7 +498,10 @@ virLogSetDefaultPriority(virLogPriority priority)
 if (virLogInitialize() < 0)
 return -1;
+virLogLock();
 virLogDefaultPriority = priority;
+virLogUnlock();
+
 return 0;
}
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] hotplug:Fix log mistake in qemuMonitorAddNetdev

2014-03-13 Thread Wangrui (K)
>From 81de0ce710bad91a2df18247e681b5a83872b8d5 Mon Sep 17 00:00:00 2001
From: Wang Rui 
Date: Thu, 13 Mar 2014 17:05:03 +
Subject: [PATCH] hotplug:Fix log mistake in qemuMonitorAddNetdev

VIR_DEBUG  in qemuMonitorAddNetdev should print vhostfdSize

Signed-off-by: Wang Rui 
---
 src/qemu/qemu_monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index e4707b7..b2af0ae 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2781,7 +2781,7 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon,
 VIR_DEBUG("mon=%p netdevstr=%s tapfd=%p tapfdName=%p tapfdSize=%d"
   "vhostfd=%p vhostfdName=%p vhostfdSize=%d",
   mon, netdevstr, tapfd, tapfdName, tapfdSize,
-  vhostfd, vhostfdName, tapfdSize);
+  vhostfd, vhostfdName, vhostfdSize);

 if (!mon) {
 virReportError(VIR_ERR_INVALID_ARG, "%s",
--
1.8.3.4

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


[libvirt] libvirt xml can't specify USB physical port when passthrough USB device to VM

2014-03-05 Thread Wangrui (K)
Hello,

I passthrough a host usb device to a guest  with a bus number and device number 
in the xml to identify the usb device.
When I unplug and then plug back into the same USB physical port, the guest 
identifies the usb device failed. Because on the host ,the usb device number 
increases.
So , I want to know that whether Libvirt will provide an interface that 
identifies a host usb device with bus number and physical port .
As far as I know ,Qemu upstream had added a hostport property which allows to 
specify the host usb with bus number and physical port.
the patch is :
https://lists.gnu.org/archive/html/qemu-devel/2011-05/msg02341.html

Best Regards

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