[libvirt] [PATCH v8 resend 1/4] add new virDomainCoreDumpWithFormat API

2014-03-22 Thread Wen Congyang
From: Qiao Nuohan 

--memory-only option is introduced without compression supported. Now qemu has
support dumping domain's memory in kdump-compressed format. This patch is adding
new virDomainCoreDumpWithFormat API, so that the format in which qemu dumps
domain's memory can be specified.

Signed-off-by: Qiao Nuohan 
---
 include/libvirt/libvirt.h.in | 31 ++
 src/driver.h |  7 
 src/libvirt.c| 98 
 src/libvirt_public.syms  |  5 +++
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 15 ++-
 src/remote_protocol-structs  |  7 
 src/test/test_driver.c   | 22 --
 8 files changed, 182 insertions(+), 4 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 82c602b..91e3e3b 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1180,6 +1180,29 @@ typedef enum {
 VIR_DUMP_MEMORY_ONLY  = (1 << 4), /* use dump-guest-memory */
 } virDomainCoreDumpFlags;
 
+/**
+ * virDomainCoreDumpFormat:
+ *
+ * Values for specifying different formats of domain core dumps.
+ */
+typedef enum {
+VIR_DOMAIN_CORE_DUMP_FORMAT_RAW,  /* dump guest memory in raw 
format */
+VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_ZLIB,   /* kdump-compressed format, with
+   * zlib compression */
+VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_LZO,/* kdump-compressed format, with
+   * lzo compression */
+VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_SNAPPY, /* kdump-compressed format, with
+   * snappy compression */
+#ifdef VIR_ENUM_SENTINELS
+VIR_DOMAIN_CORE_DUMP_FORMAT_LAST
+/*
+ * NB: this enum value will increase over time as new events are
+ * added to the libvirt API. It reflects the last state supported
+ * by this version of the libvirt API.
+ */
+#endif
+} virDomainCoreDumpFormat;
+
 /* Domain migration flags. */
 typedef enum {
 VIR_MIGRATE_LIVE  = (1 << 0), /* live migration */
@@ -1732,6 +1755,14 @@ int virDomainCoreDump   
(virDomainPtr domain,
  unsigned int flags);
 
 /*
+ * Domain core dump with format specified
+ */
+int virDomainCoreDumpWithFormat (virDomainPtr domain,
+ const char *to,
+ unsigned int dumpformat,
+ unsigned int flags);
+
+/*
  * Screenshot of current domain console
  */
 char *  virDomainScreenshot (virDomainPtr domain,
diff --git a/src/driver.h b/src/driver.h
index d728705..e66fc7a 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -306,6 +306,12 @@ typedef int
 const char *to,
 unsigned int flags);
 
+typedef int
+(*virDrvDomainCoreDumpWithFormat)(virDomainPtr domain,
+  const char *to,
+  unsigned int dumpformat,
+  unsigned int flags);
+
 typedef char *
 (*virDrvDomainScreenshot)(virDomainPtr domain,
   virStreamPtr stream,
@@ -1213,6 +1219,7 @@ struct _virDriver {
 virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc;
 virDrvDomainSaveImageDefineXML domainSaveImageDefineXML;
 virDrvDomainCoreDump domainCoreDump;
+virDrvDomainCoreDumpWithFormat domainCoreDumpWithFormat;
 virDrvDomainScreenshot domainScreenshot;
 virDrvDomainSetVcpus domainSetVcpus;
 virDrvDomainSetVcpusFlags domainSetVcpusFlags;
diff --git a/src/libvirt.c b/src/libvirt.c
index c5c3136..551b82f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2999,6 +2999,104 @@ error:
 return -1;
 }
 
+/**
+ * virDomainCoreDumpWithFormat:
+ * @domain: a domain object
+ * @to: path for the core file
+ * @dumpformat: format of domain memory's dump
+ * @flags: bitwise-OR of virDomainCoreDumpFlags
+ *
+ * This method will dump the core of a domain on a given file for analysis.
+ * Note that for remote Xen Daemon the file path will be interpreted in
+ * the remote host. Hypervisors may require  the user to manually ensure
+ * proper permissions on the file named by @to.
+ *
+ * If @flags includes VIR_DUMP_MEMORY_ONLY and dumpformat is set, libvirt
+ * will ask qemu dump domain's memory in kdump-compressed format.
+ *
+ * If @flags includes VIR_DUMP_CRASH, then leave the guest shut off with
+ * a crashed state after the dump completes.  If @flags includes
+ * VIR_DUMP_LIVE, then make the core dump while continuing to allow
+ * the guest to run; otherwise, the guest is suspended during the dump.
+ * VIR_DUMP_RESET flag forces reset of the quest after dump.
+ * The above three flags are mutually exclusive.
+ *
+ * Additionally, if @flags includes VIR_DUMP_BYPASS_CA

[libvirt] [PATCH v8 resend 3/4] qemu: add support for virDomainCoreDumpWithFormat API

2014-03-22 Thread Wen Congyang
From: Qiao Nuohan 

This patch makes qemu driver support virDomainCoreDumpWithFormat API.

Signed-off-by: Qiao Nuohan 
---
 src/qemu/qemu_driver.c   | 75 +---
 src/qemu/qemu_monitor.c  |  6 ++--
 src/qemu/qemu_monitor.h  |  3 +-
 src/qemu/qemu_monitor_json.c | 20 +---
 src/qemu/qemu_monitor_json.h |  3 +-
 tests/qemumonitorjsontest.c  |  2 +-
 6 files changed, 87 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a9d7615..9c6dd49 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2659,6 +2659,13 @@ VIR_ENUM_IMPL(qemuSaveCompression, QEMU_SAVE_FORMAT_LAST,
   "xz",
   "lzop")
 
+VIR_ENUM_DECL(qemuDumpFormat)
+VIR_ENUM_IMPL(qemuDumpFormat, VIR_DOMAIN_CORE_DUMP_FORMAT_LAST,
+  "elf",
+  "kdump-zlib",
+  "kdump-lzo",
+  "kdump-snappy")
+
 typedef struct _virQEMUSaveHeader virQEMUSaveHeader;
 typedef virQEMUSaveHeader *virQEMUSaveHeaderPtr;
 struct _virQEMUSaveHeader {
@@ -3373,7 +3380,8 @@ cleanup:
 }
 
 static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm,
-int fd, enum qemuDomainAsyncJob asyncJob)
+int fd, enum qemuDomainAsyncJob asyncJob,
+const char *dumpformat)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 int ret = -1;
@@ -3393,7 +3401,20 @@ static int qemuDumpToFd(virQEMUDriverPtr driver, 
virDomainObjPtr vm,
 if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
 return -1;
 
-ret = qemuMonitorDumpToFd(priv->mon, fd);
+if (dumpformat) {
+ret = qemuMonitorGetDumpGuestMemoryCapability(priv->mon, dumpformat);
+
+if (ret <= 0) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("unsupported dumpformat '%s'"), dumpformat);
+ret = -1;
+goto cleanup;
+}
+}
+
+ret = qemuMonitorDumpToFd(priv->mon, fd, dumpformat);
+
+cleanup:
 qemuDomainObjExitMonitor(driver, vm);
 
 return ret;
@@ -3404,13 +3425,15 @@ doCoreDump(virQEMUDriverPtr driver,
virDomainObjPtr vm,
const char *path,
virQEMUSaveFormat compress,
-   unsigned int dump_flags)
+   unsigned int dump_flags,
+   unsigned int dumpformat)
 {
 int fd = -1;
 int ret = -1;
 virFileWrapperFdPtr wrapperFd = NULL;
 int directFlag = 0;
 unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING;
+const char *memory_dump_format = NULL;
 
 /* Create an empty file with appropriate ownership.  */
 if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
@@ -3434,8 +3457,25 @@ doCoreDump(virQEMUDriverPtr driver,
 goto cleanup;
 
 if (dump_flags & VIR_DUMP_MEMORY_ONLY) {
-ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP);
+if (!(memory_dump_format = qemuDumpFormatTypeToString(dumpformat))) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("unknown dumpformat '%d'"), dumpformat);
+goto cleanup;
+}
+
+/* qemu dumps in "elf" without dumpformat set */
+if (STREQ(memory_dump_format, "elf"))
+memory_dump_format = NULL;
+
+ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP,
+   memory_dump_format);
 } else {
+if (dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_RAW) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("kdump-compressed format is only supported with "
+ "memory-only dump"));
+goto cleanup;
+}
 ret = qemuMigrationToFile(driver, vm, fd, 0, path,
   qemuCompressProgramName(compress), false,
   QEMU_ASYNC_JOB_DUMP);
@@ -3497,9 +3537,10 @@ cleanup:
 return ret;
 }
 
-static int qemuDomainCoreDump(virDomainPtr dom,
-  const char *path,
-  unsigned int flags)
+static int qemuDomainCoreDumpWithFormat(virDomainPtr dom,
+const char *path,
+unsigned int dumpformat,
+unsigned int flags)
 {
 virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainObjPtr vm;
@@ -3515,7 +3556,7 @@ static int qemuDomainCoreDump(virDomainPtr dom,
 if (!(vm = qemuDomObjFromDomain(dom)))
 return -1;
 
-if (virDomainCoreDumpEnsureACL(dom->conn, vm->def) < 0)
+if (virDomainCoreDumpWithFormatEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
 if (qemuDomainObjBeginAsyncJob(driver, vm,
@@ -3547,7 +3588,8 @@ static int qemuDomainCoreDump(virDomainPtr dom,
 }
 }
 
-ret = doCoreDump(driver, vm, path, getCompressionType(driver), flags);
+ret = doCoreDump(driver, vm, path, getC

[libvirt] [PATCH v8 resend 2/4] qemu: add qemuMonitorGetDumpGuestMemoryCapability

2014-03-22 Thread Wen Congyang
From: Qiao Nuohan 

This patch adds qemuMonitorGetDumpGuestMemoryCapability, which is used to check
whether the specified dump-guest-memory format is supported by qemu.

Signed-off-by: Qiao Nuohan 
---
 src/qemu/qemu_monitor.c  | 21 ++
 src/qemu/qemu_monitor.h  |  3 ++
 src/qemu/qemu_monitor_json.c | 65 
 src/qemu/qemu_monitor_json.h |  3 ++
 tests/qemumonitorjsontest.c  | 43 +
 5 files changed, 135 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 357c970..976a954 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2368,6 +2368,27 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+/**
+ * Returns 1 if @capability is supported, 0 if it's not, or -1 on error.
+ */
+int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon,
+const char *capability)
+{
+VIR_DEBUG("mon=%p capability=%s", mon, capability);
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("monitor must not be NULL"));
+return -1;
+}
+
+/* No capability is supported without JSON monitor */
+if (!mon->json)
+return 0;
+
+return qemuMonitorJSONGetDumpGuestMemoryCapability(mon, capability);
+}
+
 int
 qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd)
 {
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d8f1d10..e896911 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -506,6 +506,9 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
 
 int qemuMonitorMigrateCancel(qemuMonitorPtr mon);
 
+int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon,
+const char *capability);
+
 int qemuMonitorDumpToFd(qemuMonitorPtr mon,
 int fd);
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7a6aac0..ab5890b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2658,6 +2658,71 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon)
 }
 
 int
+qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon,
+const char *capability)
+{
+int ret;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr caps;
+virJSONValuePtr formats;
+size_t i;
+
+if (!(cmd = 
qemuMonitorJSONMakeCommand("query-dump-guest-memory-capability",
+   NULL)))
+return -1;
+
+ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+if (ret == 0) {
+if (qemuMonitorJSONHasError(reply, "CommandNotFound"))
+goto cleanup;
+ret = qemuMonitorJSONCheckError(cmd, reply);
+}
+
+if (ret < 0)
+goto cleanup;
+
+ret = -1;
+
+caps = virJSONValueObjectGet(reply, "return");
+if (!caps || caps->type != VIR_JSON_TYPE_OBJECT) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("missing dump guest memory capabilities"));
+goto cleanup;
+}
+
+formats = virJSONValueObjectGet(caps, "formats");
+if (!formats || formats->type != VIR_JSON_TYPE_ARRAY) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("missing supported dump formats"));
+goto cleanup;
+}
+
+for (i = 0; i < virJSONValueArraySize(formats); i++) {
+virJSONValuePtr dumpformat = virJSONValueArrayGet(formats, i);
+
+if (!dumpformat || dumpformat->type != VIR_JSON_TYPE_STRING) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("missing entry in supported dump formats"));
+goto cleanup;
+}
+
+if (STREQ(virJSONValueGetString(dumpformat), capability)) {
+ret = 1;
+goto cleanup;
+}
+
+ret = 0;
+}
+
+cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
+
+int
 qemuMonitorJSONDump(qemuMonitorPtr mon,
 const char *protocol)
 {
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index ef71588..631b9e4 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -147,6 +147,9 @@ int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorPtr 
mon,
 
 int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon);
 
+int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon,
+const char *capability);
+
 int qemuMonitorJSONDump(qemuMonitorPtr mon,
 const char *protocol);
 
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index d7da5a8..d21e1ce 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1959,6 +1959,48 @@ cleanup:
 return ret;
 }
 
+static int
+testQemuMonitorJSONqemuMonitorJSONGetDumpGuest

[libvirt] [PATCH v8 resend 4/4] allow "virsh dump --memory-only" specify dump format

2014-03-22 Thread Wen Congyang
From: Qiao Nuohan 

This patch adds "[--format] " to "virsh dump --memory-only", which is
changed to use the new virDomainCoreDumpWithFormat API. And "--compress" is
added as an alias for "--format kdump-zlib".

Signed-off-by: Qiao Nuohan 
---
 tools/virsh-domain.c | 47 ---
 tools/virsh.pod  |  6 ++
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0664774..d897c72 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4490,6 +4490,14 @@ static const vshCmdOptDef opts_dump[] = {
  .type = VSH_OT_BOOL,
  .help = N_("dump domain's memory only")
 },
+{.name = "compress",
+ .type = VSH_OT_ALIAS,
+ .help = "format=kdump-zlib",
+},
+{.name = "format",
+ .type = VSH_OT_DATA,
+ .help = N_("specify the format of memory-only dump")
+},
 {.name = NULL}
 };
 
@@ -4505,6 +4513,8 @@ doDump(void *opaque)
 const char *name = NULL;
 const char *to = NULL;
 unsigned int flags = 0;
+const char *format = NULL;
+unsigned int dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_RAW;
 
 sigemptyset(&sigmask);
 sigaddset(&sigmask, SIGINT);
@@ -4528,9 +4538,40 @@ doDump(void *opaque)
 if (vshCommandOptBool(cmd, "memory-only"))
 flags |= VIR_DUMP_MEMORY_ONLY;
 
-if (virDomainCoreDump(dom, to, flags) < 0) {
-vshError(ctl, _("Failed to core dump domain %s to %s"), name, to);
-goto out;
+if (vshCommandOptBool(cmd, "format")) {
+if (!(flags & VIR_DUMP_MEMORY_ONLY)) {
+vshError(ctl, "%s", _("--format only work with --memory-only"));
+goto out;
+}
+
+if (vshCommandOptString(cmd, "format", &format)) {
+if (STREQ(format, "kdump-zlib"))
+dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_ZLIB;
+else if (STREQ(format, "kdump-lzo"))
+dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_LZO;
+else if (STREQ(format, "kdump-snappy"))
+dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_SNAPPY;
+else if (STREQ(format, "elf"))
+dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_RAW;
+else {
+vshError(ctl, _("format '%s' is not supported, expecting "
+"'kdump-zlib', 'kdump-lzo', 'kdump-snappy' "
+"or 'elf'."), format);
+goto out;
+}
+}
+}
+
+if (dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_RAW) {
+if (virDomainCoreDumpWithFormat(dom, to, dumpformat, flags) < 0) {
+vshError(ctl, _("Failed to core dump domain %s to %s"), name, to);
+goto out;
+}
+} else {
+if (virDomainCoreDump(dom, to, flags) < 0) {
+vshError(ctl, _("Failed to core dump domain %s to %s"), name, to);
+goto out;
+}
 }
 
 ret = '0';
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 4645be5..7e3ae5a 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1010,6 +1010,7 @@ I argument may be B or B.
 
 =item B I I [I<--bypass-cache>]
 { [I<--live>] | [I<--crash>] | [I<--reset>] } [I<--verbose>] [I<--memory-only>]
+[I<--format> I]
 
 Dumps the core of a domain to a file for analysis.
 If I<--live> is specified, the domain continues to run until the core
@@ -1023,6 +1024,11 @@ cache, although this may slow down the operation.
 If I<--memory-only> is specified, the file is elf file, and will only
 include domain's memory and cpu common register value. It is very
 useful if the domain uses host devices directly.
+I<--format> I is used to specify the format of 'memory-only'
+dump, and I can be one of them: elf, kdump-zlib(kdump-compressed
+format with zlib-compressed), kdump-lzo(kdump-compressed format with
+lzo-compressed), kdump-snappy(kdump-compressed format with snappy-compressed)
+I<--compress> is an alias for I<--format> I.
 
 The progress may be monitored using B virsh command and canceled
 with B command (sent by another virsh instance). Another option
-- 
1.8.5.3

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


[libvirt] [PATCH v8 resend 0/4] support dumping guest memory in compressed format

2014-03-22 Thread Wen Congyang
From: Qiao Nuohan 

dumping guest's memory is introduced without compression supported, but now
qemu can dump guest's memory in kdump-compressed format. This patchset is used
to add support in libvirt side to let qemu do the dump in compressed format and
please refer the following address to see implementation of the qemu side, the
lastest version of qemu side is v9.

http://lists.nongnu.org/archive/html/qemu-devel/2014-02/msg03016.html

ChangLog:
Changes from v7 to v8:
1. add test for qemuMonitorGetDumpGuestMemoryCapability
2. fix a bug when dumping a elf core with the an older version of qemu

Changes from v6 to v7:
1. revert changing dumpformat of virDomainCoreDumpWithFormat back to an enum

Changes from v5 to v6:
1. add qemuMonitorGetDumpGuestMemoryCapability API to check the available dump
   format

Changes from v4 to v5:
1. modify some restriction check

Changes from v3 to v4:
1. dropping patch "add dump_memory_format in qemu.conf"
2. fix to follow some conventions

Changes from v2 to v3:
1. address Jiri Denemark's comment about adding a new public API instead of
   changing an old one.

Changes from v1 to v2:
1. address Daniel P. Berrange's comment about using a new parameter to replace
   flags like VIR_DUMP_COMPRESS_ZLIB.


Qiao Nuohan (4):
  add new virDomainCoreDumpWithFormat API
  qemu: add qemuMonitorGetDumpGuestMemoryCapability
  qemu: add support for virDomainCoreDumpWithFormat API
  allow "virsh dump --memory-only" specify dump format

 include/libvirt/libvirt.h.in | 31 ++
 src/driver.h |  7 
 src/libvirt.c| 98 
 src/libvirt_public.syms  |  5 +++
 src/qemu/qemu_driver.c   | 75 -
 src/qemu/qemu_monitor.c  | 27 ++--
 src/qemu/qemu_monitor.h  |  6 ++-
 src/qemu/qemu_monitor_json.c | 85 +++---
 src/qemu/qemu_monitor_json.h |  6 ++-
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 15 ++-
 src/remote_protocol-structs  |  7 
 src/test/test_driver.c   | 22 --
 tests/qemumonitorjsontest.c  | 45 +++-
 tools/virsh-domain.c | 47 +++--
 tools/virsh.pod  |  6 +++
 16 files changed, 454 insertions(+), 29 deletions(-)

-- 
1.8.5.3

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


Re: [libvirt] [PATCH v8 1/4] add new virDomainCoreDumpWithFormat API

2014-03-22 Thread Wen Congyang
At 03/22/2014 06:22 AM, Eric Blake Wrote:
> On 03/21/2014 01:03 AM, qiaonuo...@cn.fujitsu.com wrote:
>> --memory-only option is introduced without compression supported. Now qemu 
>> has
>> support dumping domain's memory in kdump-compressed format. This patch is 
>> adding
>> new virDomainCoreDumpWithFormat API, so that the format in which qemu dumps
>> domain's memory can be specified.
>>
>> Signed-off-by: Qiao Nuohan 
> 
> Your From: spelling (qiaonuo...@cn.fujitsu.com) does not match your
> Signed-off-by: spelling (Qiao Nuohan).  If 'git am' were to take your
> email as-is, then 'git shortlog' would show your email instead of your
> name as the author.  You should consider running
>  git config --global user.name "Qiao Nuohan"
> and/or
>  git config --global sendemail.from \
>"Qiao Nuohan 
> so that in the future, 'git send-email' will make your From: match your
> S-o-B.
> 
> On 03/21/2014 08:54 AM, Daniel P. Berrange wrote:
> 
>> ACK
> 
> Unfortunately, I tried to apply your patches, but 'git am' did not like
> them:
> 
> $ git am -3 *API.eml
> Applying: add new virDomainCoreDumpWithFormat API
> Using index info to reconstruct a base tree...
> M include/libvirt/libvirt.h.in
> M src/driver.h
> M src/libvirt.c
> M src/remote/remote_driver.c
> M src/test/test_driver.c
> :20: trailing whitespace.
> /**
> :21: trailing whitespace.
>  * virDomainCoreDumpFormat:
> :22: trailing whitespace.
>  *
> :23: trailing whitespace.
>  * Values for specifying different formats of domain core dumps.
> :24: trailing whitespace.
>  */
> error: patch failed: include/libvirt/libvirt.h.in:1180
> error: include/libvirt/libvirt.h.in: patch does not apply
> error: patch failed: src/driver.h:306
> error: src/driver.h: patch does not apply
> error: patch failed: src/libvirt.c:2997
> error: src/libvirt.c: patch does not apply
> error: patch failed: src/libvirt_public.syms:645
> error: src/libvirt_public.syms: patch does not apply
> error: patch failed: src/remote/remote_driver.c:7516
> error: src/remote/remote_driver.c: patch does not apply
> error: patch failed: src/remote/remote_protocol.x:904
> error: src/remote/remote_protocol.x: patch does not apply
> error: patch failed: src/remote_protocol-structs:557
> error: src/remote_protocol-structs: patch does not apply
> error: patch failed: src/test/test_driver.c:2427
> error: src/test/test_driver.c: patch does not apply
> Did you hand edit your patch?
> It does not apply to blobs recorded in its index.
> Cannot fall back to three-way merge.
> Patch failed at 0001 add new virDomainCoreDumpWithFormat API
> The copy of the patch that failed is found in:
>/home/eblake/libvirt/.git/rebase-apply/patch
> 
> Can you please rebase and repost them in a way that is not whitespace
> munged by your mailer?

I guess it is our new mailserver problem. I will resend them via old mailserver

Thanks
Wen Congyang

> 
> 
> 
> --
> 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] Temporarily shutoff CPUSET cgroup in libvirt.

2012-09-23 Thread Wen Congyang
At 09/22/2012 03:04 AM, Eric Blake Wrote:
> On 09/21/2012 02:20 AM, Tang Chen wrote:
>> When a cpu or memory is offlined and onlined again, cgroup will not
>> recover any corresponding values. This is a problem in kernel.
>> Do not use CPUSET cgroup to limit threads using cpu and memory until
>> the problem is fixed in kernel.
>>
>> Signed-off-by: Tang Chen 
>> ---
>>  src/qemu/qemu.conf   |5 -
>>  src/qemu/qemu_conf.c |7 ++-
>>  src/util/cgroup.c|3 +--
>>  3 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
>> index 18105ca..6bf7290 100644
>> --- a/src/qemu/qemu.conf
>> +++ b/src/qemu/qemu.conf
>> @@ -211,7 +211,10 @@
>>  # can be mounted in different locations. libvirt will detect
>>  # where they are located.
>>  #
>> -#cgroup_controllers = [ "cpu", "devices", "memory", "blkio", "cpuset", 
>> "cpuacct" ]
>> +# When a cpu or memory is offlined and onlined again, cgroup will not
>> +# recover any corresponding values. This is a problem in kernel.
>> +# So DO NOT use cpuset cgroup until this problem is fixed in kernel.
>> +#cgroup_controllers = [ "cpu", "devices", "memory", "blkio", "cpuacct" ]
> 
> We ought to list which versions of the kernel are known to have the bug
> (that is, say something like 'at least through kernel 3.6'); when we
> finally know which kernel version fixes the bugs in the cpuset
> controller, then we can update the text again to give the user better
> advice on whether they want to be using this.
> 
> I'm torn on whether to take this patch prior to 0.10.2 - it makes sense
> to take (that is, the kernel is buggy, and we cause degraded performance
> as a result of the kernel bug any time the system goes through S3), but
> it's rather late, having missed rc2, for such a major change in default
> functionality.  Am I correct, though, that you can manually edit
> /etc/libvirt/qemu.conf to re-enable the use of cpuset even on a buggy
> kernel, for further testing the issues?
> 
>>  
>>  # This is the basic set of devices allowed / required by
>>  # all virtual machines.
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index 91a56f1..80b0787 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -397,12 +397,17 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
>>  driver->cgroupControllers |= (1 << ctl);
>>  }
>>  } else {
>> +/*
>> + * When a cpu or memory is offlined and onlined again, cgroup will 
>> not
>> + * recover any corresponding values. This is a problem in kernel.
>> + * Do not use CPUSET cgroup to limit threads using cpu and memory 
>> until
>> + * the problem is fixed in kernel.
>> + */
>>  driver->cgroupControllers =
>>  (1 << VIR_CGROUP_CONTROLLER_CPU) |
>>  (1 << VIR_CGROUP_CONTROLLER_DEVICES) |
>>  (1 << VIR_CGROUP_CONTROLLER_MEMORY) |
>>  (1 << VIR_CGROUP_CONTROLLER_BLKIO) |
>> -(1 << VIR_CGROUP_CONTROLLER_CPUSET) |
>>  (1 << VIR_CGROUP_CONTROLLER_CPUACCT);
> 
> Rather than hard-coding the non-use of cpuset as our default, can we do
> something like a uname() probe or some other clue of whether we have a
> buggy kernel?
> 
>> +++ b/src/util/cgroup.c
>> @@ -543,8 +543,7 @@ static int virCgroupMakeGroup(virCgroupPtr parent, 
>> virCgroupPtr group,
>>  /* We need to control cpu bandwidth for each vcpu now */
>>  if ((flags & VIR_CGROUP_VCPU) &&
>>  (i != VIR_CGROUP_CONTROLLER_CPU &&
>> - i != VIR_CGROUP_CONTROLLER_CPUACCT &&
>> - i != VIR_CGROUP_CONTROLLER_CPUSET)) {
>> + i != VIR_CGROUP_CONTROLLER_CPUACCT)) {
>>  /* treat it as unmounted and we can use virCgroupAddTask */
>>  VIR_FREE(group->controllers[i].mountPoint);
>>  continue;
> 
> Is this hunk necessary?  In other words, don't we just gracefully skip
> making per-vcpu groups for any controller that was not mounted, or do we
> loudly fail if all of these controllers are not present?

We have checked whether the controller was mounted before this code in this
function.

If all of these controllers are not present, this function don't fails now.
It will fail later, and doesn't cause any problem. So I agree that this
function fails if all of these controllers are not present. It should
be fixed in another patch.

Thanks
Wen Congyang


> 
> 
> 
> 
> --
> 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] [PATCH] fix make syntak error

2012-08-20 Thread Wen Congyang
We move the macro vshStrcasecmp from tools/virsh.c to tools/virsh.h
after commit c2e494cc, so we should not check strcase* in the file
tools/virsh.h.

Some macros are not properly indented in the file tools/virsh.h
and tools/virsh-domain.h. This patch also fixes this problem.

---
 cfg.mk   |2 +-
 tools/virsh-domain.h |2 +-
 tools/virsh.h|2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index e9138a8..d2e54e3 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -727,7 +727,7 @@ $(srcdir)/src/remote/remote_client_bodies.h: 
$(srcdir)/src/remote/remote_protoco
$(MAKE) -C src remote/remote_client_bodies.h
 
 # List all syntax-check exemptions:
-exclude_file_name_regexp--sc_avoid_strcase = ^tools/virsh\.c$$
+exclude_file_name_regexp--sc_avoid_strcase = ^tools/virsh\.h$$
 
 
_src1=libvirt|fdstream|qemu/qemu_monitor|util/(command|util)|xen/xend_internal|rpc/virnetsocket|lxc/lxc_controller
 exclude_file_name_regexp--sc_avoid_write = \
diff --git a/tools/virsh-domain.h b/tools/virsh-domain.h
index 797462f..b1b7930 100644
--- a/tools/virsh-domain.h
+++ b/tools/virsh-domain.h
@@ -24,7 +24,7 @@
  */
 
 #ifndef VIRSH_DOMAIN_H
-#define VIRSH_DOMAIN_H
+# define VIRSH_DOMAIN_H
 
 # include "virsh.h"
 
diff --git a/tools/virsh.h b/tools/virsh.h
index 69f37cc..0b1f123 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -23,7 +23,7 @@
  */
 
 #ifndef VIRSH_H
-#define VIRSH_H
+# define VIRSH_H
 
 # include 
 # include 
-- 
1.7.1

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


Re: [libvirt] [PATCH] building: fix deps error when some drivers are not built

2012-07-24 Thread Wen Congyang

On 07/24/2012 06:12 AM, Eric Blake wrote:

On 07/23/2012 03:58 AM, Wen Congyang wrote:

libvirt-daemon-driver-XXX should be depent on only when with_driver_modules


s/depent on/a dependency/g


is 1.
libvirt-daemon-driver-libxl should be depent on only when with_libxl is 1.
libvirt-daemon-driver-lxc should be depent on only when with_lxc is 1.
libvirt-daemon-driver-qemu should be depent on only when with_qemu is 1.
libvirt-daemon-driver-uml should be depent on only when with_uml is 1.
libvirt-daemon-driver-xen should be depent on only when with_xen is 1.

---
  libvirt.spec.in |   16 
  1 files changed, 16 insertions(+), 0 deletions(-)


ACK once you fix the commit message.


Pushed with the commit message fixed

Thanks
Wen Congyang






--
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] [PATCH] building: fix deps error when some drivers are not built

2012-07-23 Thread Wen Congyang
libvirt-daemon-driver-XXX should be depent on only when with_driver_modules
is 1.
libvirt-daemon-driver-libxl should be depent on only when with_libxl is 1.
libvirt-daemon-driver-lxc should be depent on only when with_lxc is 1.
libvirt-daemon-driver-qemu should be depent on only when with_qemu is 1.
libvirt-daemon-driver-uml should be depent on only when with_uml is 1.
libvirt-daemon-driver-xen should be depent on only when with_xen is 1.

---
 libvirt.spec.in |   16 
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 140a182..cfcfc1c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -326,11 +326,22 @@ Requires: libvirt-daemon-config-network = 
%{version}-%{release}
 %if %{with_nwfilter}
 Requires: libvirt-daemon-config-nwfilter = %{version}-%{release}
 %endif
+%if %{with_driver_modules}
+%if %{with_libxl}
 Requires: libvirt-daemon-driver-libxl = %{version}-%{release}
+%endif
+%if %{with_lxc}
 Requires: libvirt-daemon-driver-lxc = %{version}-%{release}
+%endif
+%if %{with_qemu}
 Requires: libvirt-daemon-driver-qemu = %{version}-%{release}
+%endif
+%if %{with_uml}
 Requires: libvirt-daemon-driver-uml = %{version}-%{release}
+%endif
+%if %{with_xen}
 Requires: libvirt-daemon-driver-xen = %{version}-%{release}
+%endif
 
 Requires: libvirt-daemon-driver-interface = %{version}-%{release}
 Requires: libvirt-daemon-driver-secret = %{version}-%{release}
@@ -339,6 +350,7 @@ Requires: libvirt-daemon-driver-network = 
%{version}-%{release}
 Requires: libvirt-daemon-driver-nodedev = %{version}-%{release}
 Requires: libvirt-daemon-driver-nwfilter = %{version}-%{release}
 %endif
+%endif
 Requires: libvirt-client = %{version}-%{release}
 
 # All build-time requirements. Run-time requirements are
@@ -750,7 +762,9 @@ Summary: Qemu driver plugin for the libvirtd daemon
 Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 # There really is a hard cross-driver dependency here
+%if %{with_driver_modules}
 Requires: libvirt-daemon-driver-network = %{version}-%{release}
+%endif
 
 %description daemon-driver-qemu
 The qemu driver plugin for the libvirtd daemon, providing
@@ -765,7 +779,9 @@ Summary: LXC driver plugin for the libvirtd daemon
 Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 # There really is a hard cross-driver dependency here
+%if %{with_driver_modules}
 Requires: libvirt-daemon-driver-network = %{version}-%{release}
+%endif
 
 %description daemon-driver-lxc
 The LXC driver plugin for the libvirtd daemon, providing
-- 
1.7.1

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


Re: [libvirt] [PATCH] Fix a string format bug in qemu_cgroup.c

2012-07-10 Thread Wen Congyang
At 07/11/2012 09:59 AM, Gao feng Wrote:
> 于 2012年07月10日 23:24, Eric Blake 写道:
>> On 07/05/2012 09:08 PM, Wen Congyang wrote:
>>> At 07/06/2012 09:53 AM, tangchen Wrote:
>>>> Signed-off-by: Tang Chen 
>>>> ---
>>>>  src/qemu/qemu_cgroup.c |4 ++--
>>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
>>>> index f8f375f..662d41d 100644
>>>> --- a/src/qemu/qemu_cgroup.c
>>>> +++ b/src/qemu/qemu_cgroup.c
>>>> @@ -473,8 +473,8 @@ cleanup:
>>>>  rc = virCgroupSetCpuCfsPeriod(cgroup, old_period);
>>>>  if (rc < 0)
>>>>  virReportSystemError(-rc,
>>>> - _("%s"),
>>>> - "Unable to rollback cpu bandwidth 
>>>> period");
>>>> + "%s",
>>>> + _("Unable to rollback cpu bandwidth 
>>>> period"));
>>>>  }
>>>>
>>>>  return -1;
>>>
>>> ACK
>>>
>>> I use make syntax-check , and do not find this bug...
>>
>> That says our syntax-check rule is not strong enough.  I'll work on a
>> patch for that, as there are other violations of this bug.

It seems that this rule does not work for multiple lines...

>>
> 
> We have the same problem in lxcSetVcpuBWLive.
> cleanup:
> if (period) {
> rc = virCgroupSetCpuCfsPeriod(cgroup, old_period);
> if (rc < 0)
> virReportSystemError(-rc,
>  _("%s"),
>  "Unable to rollback cpu bandwidth period");
> }
> 
> it seems copied from qemuSetupCgroupVcpuBW.

I think so. This bug is first introduced by me. :( 

Thanks
Wen Congyang

> 
> --
> 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] Fix a string format bug in qemu_cgroup.c

2012-07-05 Thread Wen Congyang
At 07/06/2012 09:53 AM, tangchen Wrote:
> Signed-off-by: Tang Chen 
> ---
>  src/qemu/qemu_cgroup.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index f8f375f..662d41d 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -473,8 +473,8 @@ cleanup:
>  rc = virCgroupSetCpuCfsPeriod(cgroup, old_period);
>  if (rc < 0)
>  virReportSystemError(-rc,
> - _("%s"),
> - "Unable to rollback cpu bandwidth period");
> + "%s",
> + _("Unable to rollback cpu bandwidth 
> period"));
>  }
> 
>  return -1;

ACK

I use make syntax-check , and do not find this bug...

Thanks
Wen Congyang

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


Re: [libvirt] [PATCH 2/3 v5] qemu: allow the client to choose the vmcore's format

2012-06-15 Thread Wen Congyang

On 06/13/2012 08:57 PM, Martin Kletzander wrote:

On 06/12/2012 05:06 AM, Wen Congyang wrote:

This patch updates qemu driver to allow the client to choose the
vmcore's format: memory only or including device state.
---
  include/libvirt/libvirt.h.in |1 +
  src/qemu/qemu_capabilities.c |5 +++
  src/qemu/qemu_capabilities.h |1 +
  src/qemu/qemu_domain.c   |1 +
  src/qemu/qemu_domain.h   |1 +
  src/qemu/qemu_driver.c   |   60 -
  6 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index fcb6695..120ed14 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -927,6 +927,7 @@ typedef enum {
  VIR_DUMP_LIVE = (1<<  1), /* live dump */
  VIR_DUMP_BYPASS_CACHE = (1<<  2), /* avoid file system cache pollution */
  VIR_DUMP_RESET= (1<<  3), /* reset domain after dump finishes */
+VIR_DUMP_MEMORY_ONLY  = (1<<  4), /* use dump-guest-memory */
  } virDomainCoreDumpFlags;

  /* Domain migration flags. */
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b410648..6eee8cd 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -164,6 +164,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"no-user-config",

"hda-micro", /* 95 */
+  "dump-guest-memory",

  );

@@ -1237,6 +1238,10 @@ qemuCapsComputeCmdFlags(const char *help,

  if (version>= 11000)
  qemuCapsSet(flags, QEMU_CAPS_CPU_HOST);
+
+if (version>= 1001050)
+qemuCapsSet(flags, QEMU_CAPS_DUMP_GUEST_MEMORY);
+
  return 0;
  }



Sorry for bothering you with this, but I tried newest qemu and it shows
the proper list of commands with query-commands in qmp. Instead of this
one hunk, I'd use this:

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 6ca01c5..7d2da21 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -991,6 +991,8 @@ qemuMonitorJSONCheckCommands(qemuMonitorPtr mon,
  qemuCapsSet(qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC);
  else if (STREQ(name, "block-job-cancel"))
  qemuCapsSet(qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC);
+else if (STREQ(name, "dump-guest-memory"))
+qemuCapsSet(qemuCaps, QEMU_CAPS_DUMP_GUEST_MEMORY);
  }

  ret = 0;


diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 64831e2..640f7f5 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -131,6 +131,7 @@ enum qemuCapsFlags {
  QEMU_CAPS_IDE_CD = 93, /* -device ide-cd */
  QEMU_CAPS_NO_USER_CONFIG = 94, /* -no-user-config */
  QEMU_CAPS_HDA_MICRO  = 95, /* -device hda-micro */
+QEMU_CAPS_DUMP_GUEST_MEMORY  = 96, /* dump-guest-memory command */

  QEMU_CAPS_LAST,   /* this must always be the last item */
  };
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3752ddf..91c0645 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -160,6 +160,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
  job->phase = 0;
  job->mask = DEFAULT_JOB_MASK;
  job->start = 0;
+job->dump_memory_only = false;
  memset(&job->info, 0, sizeof(job->info));
  }

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 1256c4e..3a5f1f3 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -104,6 +104,7 @@ struct qemuDomainJobObj {
  int phase;  /* Job phase (mainly for migrations) 
*/
  unsigned long long mask;/* Jobs allowed during async job */
  unsigned long long start;   /* When the async job started */
+bool dump_memory_only;  /* use dump-guest-memory to do dump */
  virDomainJobInfo info;  /* Async job progress data */
  };

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3f74d2..b2da838 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2986,12 +2986,39 @@ cleanup:
  return ret;
  }

+static int qemuDumpToFd(struct qemud_driver *driver, virDomainObjPtr vm,
+int fd, enum qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret = -1;
+
+if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DUMP_GUEST_MEMORY)) {
+qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+_("dump-guest-memory is not supported"));
+return -1;
+}
+
+if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
+  fd)<  0)
+return -1;
+
+priv->job.dump_memory_only = 

Re: [libvirt] [PATCH] qemu: fix potential dead lock

2012-06-15 Thread Wen Congyang

On 05/25/2012 10:14 PM, Martin Kletzander wrote:

On 05/25/2012 12:12 PM, Wen Congyang wrote:

If we lock the qemu_driver, we should call qemuDomainObjBeginJobWithDriver()
not qemuDomainObjBeginJob().

---
  src/qemu/qemu_driver.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 39b27b1..3d62aab 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8905,7 +8905,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
  goto cleanup;
  asyncJob = QEMU_ASYNC_JOB_MIGRATION_OUT;
  } else {
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY)<  0)
+if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY)<  0)
  goto cleanup;
  asyncJob = QEMU_ASYNC_JOB_NONE;
  }


ACK, cite:
"... qemud_driver must NOT be locked ..." for qemuDomainObjBeginJob()
and
"If qemud_driver is passed, it MUST be locked ..." for
qemuDomainObjBeginJobWithDriver() and the driver is being locked about
15 lines before that.

Martin


Thanks, pushed

Wen Congyang



--
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] make dist failed

2012-06-12 Thread Wen Congyang
At 06/13/2012 12:10 PM, Eric Blake Wrote:
> On 06/12/2012 09:07 PM, Wen Congyang wrote:
> 
>>>
>>> We really shouldn't need to compile a file for a virgin 'make dist' to
>>> work, but now I have enough information to repeat it.  It may be a while
>>> before I have a clean solution, though, since every thing I try implies
>>> nuking my tree and starting from a fresh clone module my patch attempt.
> 
> I now have 'make dist' works; if it also passes the longer 'make
> distcheck' on a virgin tree, then I will push my patch under the
> build-breaker rule (patch in separate mail).  The bug was that we had a
> file in the tarball that depended on a generated file, which is a no-no.
>  We really need to ship remote_protocol-structs, and we also want 'make
> check' to ensure that file is up-to-date (which includes a dependency on
> a generated file), but a little bit of refactoring makes it so that
> 'make check' need not interfere with 'make dist', by having the two
> targets depend on different names.

I apply your patch, and the problem does not exist.

Thanks
Wen Congyang

> 
>>>
>>>>
>>>> I revert some commits and test the building. I find that this problem is
>>>> introduced by the commit 7bff56a0d1514cb955eb14adc14281626e80e96c.
>>>
>>> That was fixing real bugs, but I'm not surprised that other latent bugs
>>> were exposed in the process.
>>>
>>
>> Yes, that commit fixes a bug, but introduce a new bug.
> 
> Not a new bug, but a latent one.  The bug has been present since July
> 2011 (commit 62dee6f), but it was the refactoring of commit 7bff56a that
> exposed it better.
> 

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


Re: [libvirt] make dist failed

2012-06-12 Thread Wen Congyang
At 06/13/2012 10:25 AM, Eric Blake Wrote:
> On 06/12/2012 07:21 PM, Wen Congyang wrote:
> 
>>>>>>GENutil/virkeymaps.h
>>>>>>CC libvirt_driver_remote_la-remote_protocol.lo
>>>>>> In file included from ./remote/remote_protocol.h:17,
>>>>>>   from ./remote/remote_protocol.c:7:
>>>>>> ./internal.h:300:31: error: libvirt_probes.h: No such file or directory
>>>
>>> Can you give the steps to reproduce it?  I'm assuming this was a fresh
>>> git clone, in-tree build, ./autogen.sh with no special options, on
>>> GNU/Linux with dtrace available?
>>
>> Steps to reproduce it:
>> 1. use 'git clone' to get a clean tree
>> 2. ./autogen.sh
>> 3. make dist
> 
> As a workaround, does:
> 
> ./autogen.sh && make && make dist
> 
> always work?

Yes, it works.

> 
> We really shouldn't need to compile a file for a virgin 'make dist' to
> work, but now I have enough information to repeat it.  It may be a while
> before I have a clean solution, though, since every thing I try implies
> nuking my tree and starting from a fresh clone module my patch attempt.
> 
>>
>> I revert some commits and test the building. I find that this problem is
>> introduced by the commit 7bff56a0d1514cb955eb14adc14281626e80e96c.
> 
> That was fixing real bugs, but I'm not surprised that other latent bugs
> were exposed in the process.
> 

Yes, that commit fixes a bug, but introduce a new bug.

Thanks
Wen Congyang

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


Re: [libvirt] make dist failed

2012-06-12 Thread Wen Congyang
At 06/13/2012 09:59 AM, Eric Blake Wrote:
> On 06/12/2012 08:01 PM, Wen Congyang wrote:
>> Another building error:
>>
>> error: parse error in expression
>> error: /data1/wency/libvirt/libvirt.spec:138: parseExpressionBoolean returns 
>> -1
> 
> Fixed by commit 48939a4a.
> 

Yes, I forgot to pull the newest code

Thanks
Wen Congyang

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


Re: [libvirt] make dist failed

2012-06-12 Thread Wen Congyang
Another building error:

error: parse error in expression
error: /data1/wency/libvirt/libvirt.spec:138: parseExpressionBoolean returns -1

But I cannot reproduce this problem on another machine.

Steps to reproduce it:
1. ./autogen.sh
2. make dist
3. rpmbuild --define "_sourcedir `pwd`" --define "_without-numad 1" -ba 
libvirt.spec

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


Re: [libvirt] make dist failed

2012-06-12 Thread Wen Congyang
At 06/12/2012 08:55 PM, Eric Blake Wrote:
> On 06/12/2012 12:57 AM, Wen Congyang wrote:
>> At 06/12/2012 02:49 PM, Osier Yang Wrote:
>>> On 2012年06月12日 09:59, Wen Congyang wrote:
>>>> make dist failed with the following error:
>>>>
>>>> make[1]: Entering directory `/home/wency/source/libvirt/src'
>>>>GENrpc/virnetprotocol.h
>>>>GENrpc/virnetprotocol.c
>>>>GENrpc/virkeepaliveprotocol.h
>>>>GENrpc/virkeepaliveprotocol.c
>>>>GENremote/remote_protocol.h
>>>>GENremote/remote_protocol.c
>>>>GENremote/qemu_protocol.h
>>>>GENremote/qemu_protocol.c
>>>>GENremote/qemu_client_bodies.h
>>>>GENutil/virkeymaps.h
>>>>CC libvirt_driver_remote_la-remote_protocol.lo
>>>> In file included from ./remote/remote_protocol.h:17,
>>>>   from ./remote/remote_protocol.c:7:
>>>> ./internal.h:300:31: error: libvirt_probes.h: No such file or directory
> 
> Can you give the steps to reproduce it?  I'm assuming this was a fresh
> git clone, in-tree build, ./autogen.sh with no special options, on
> GNU/Linux with dtrace available?

Steps to reproduce it:
1. use 'git clone' to get a clean tree
2. ./autogen.sh
3. make dist

The OS is RHEL6.2, and the configure summary is:
configure: Configuration summary
configure: =
configure: 
configure: Drivers
configure: 
configure:  Xen: no
configure: QEMU: yes
configure:  UML: yes
configure:   OpenVZ: yes
configure:   VMware: yes
configure: VBox: yes
configure:   XenAPI: no
configure: xenlight: no
configure:  LXC: yes
configure: PHYP: no
configure:  ESX: yes
configure:  Hyper-V: no
configure: Test: yes
configure:   Remote: yes
configure:  Network: yes
configure: Libvirtd: yes
configure:netcf: yes
configure:  macvtap: yes
configure: virtport: yes
configure: 
configure: Storage Drivers
configure: 
configure:  Dir: yes
configure:   FS: yes
configure:NetFS: yes
configure:  LVM: yes
configure:iSCSI: yes
configure: SCSI: yes
configure:mpath: yes
configure: Disk: yes
configure:  RBD: no
configure: 
configure: Security Drivers
configure: 
configure:  SELinux: yes (/selinux)
configure: AppArmor: no
configure: 
configure: Driver Loadable Modules
configure: 
configure:   dlopen: -export-dynamic -ldl
configure: 
configure: Libraries
configure: 
configure:   libxml: -I/usr/include/libxml2   -lxml2  
configure:   dlopen: -ldl
configure:  libcurl: -DCURL_DISABLE_TYPECHECK   -lcurl  
configure: openwsman: no
configure:  libssh2:  
configure:   gnutls:   -lgnutls   -lgcrypt
configure: sasl:   -lsasl2
configure: yajl:   -lyajl
configure:  sanlock:   -lsanlock_client
configure:avahi: -D_REENTRANT   -lavahi-common -lavahi-client  
configure:   polkit: /usr/bin/pkcheck (version 1)
configure:audit:   -laudit
configure:  selinux:  -lselinux
configure: apparmor: no
configure:  numactl:  -lnuma
configure:capng:  -lcap-ng
configure:  xen: no
configure:   xenapi: no
configure: xenlight: no
configure:  hal: -DDBUS_API_SUBJECT_TO_CHANGE -I/usr/include/hal 
-I/usr/include/dbus-1.0 -I/usr/lib64/dbus-1.0/include   -L/lib64 -lhal -ldbus-1 
-lpthread -lrt  
configure: udev:   -ludev -lpciaccess  
configure:netcf:   -lnetcf  
configure: pcap:  -lpcap
configure:   nl:   -lnl  
configure:mscom: no
configure:  xdr: 
configure: 
configure: Test suite
configure: 
configure:Coverage: no
configure:   Alloc OOM: no
configure: 
configure: Miscellaneous
configure: 
configure: Debug: yes
configure:   Use -Werror: yes
configure: Warning Flags:  -Wall -W -Wformat-y2k -Wformat-security -Winit-self 
-Wmissing-include-dirs -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow 
-Wpointer-arith -Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op 
-Waggregate-return -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn 
-Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline 
-Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization 
-Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat -Wsync-nand 
-Wattributes -Wcoverage-mismatch -Wdeprecated-declarations -Wdiv-by-zero 
-Wendif-labels -Wextra -Wformat-contains-nul -Wformat-extra-args 
-Wformat-zero-length -Wformat=2 -Wmultichar -Wnormalized=nfc -Woverflow 
-Wpointer-to-int-cast -Wpragmas -Wno-missing-field-initializers 
-Wno-sign-compare -Wno-format-nonliteral -Wframe-larger-than=4096 
-fstack-protector-all --param=ssp-buffer-size=4 -fexceptions 
-fasynchronous-unwind-ta
bles -fdiagnostics-show-option -funit-at-a-time -fipa-pure-const 
-Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Werro

Re: [libvirt] make dist failed

2012-06-12 Thread Wen Congyang
At 06/12/2012 02:49 PM, Osier Yang Wrote:
> On 2012年06月12日 09:59, Wen Congyang wrote:
>> make dist failed with the following error:
>>
>> make[1]: Entering directory `/home/wency/source/libvirt/src'
>>GENrpc/virnetprotocol.h
>>GENrpc/virnetprotocol.c
>>GENrpc/virkeepaliveprotocol.h
>>GENrpc/virkeepaliveprotocol.c
>>GENremote/remote_protocol.h
>>GENremote/remote_protocol.c
>>GENremote/qemu_protocol.h
>>GENremote/qemu_protocol.c
>>GENremote/qemu_client_bodies.h
>>GENutil/virkeymaps.h
>>CC libvirt_driver_remote_la-remote_protocol.lo
>> In file included from ./remote/remote_protocol.h:17,
>>   from ./remote/remote_protocol.c:7:
>> ./internal.h:300:31: error: libvirt_probes.h: No such file or directory
>>
> 
> Looks like libvirt_probes.h is not generated, is dtrace available
> on your host?

Yes, dtrace is available on my host. I do it in a clean tree. I guess
there is some bug in src/Makefile.am, but I have no time to investigate
recently, so I report this bug here.

Thanks
Wen Congyang

> 
> Regards,
> Osier
> 


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

[libvirt] [PATCH 3/3 v5] virsh: allow the user to specify vmcore's format

2012-06-11 Thread Wen Congyang
Add a new parameter --memory-only for 'virsh dump' command. So
the user can decide the vmcore's format.
---
 tools/virsh.c   |3 +++
 tools/virsh.pod |5 -
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index abcfbff..c875a90 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4006,6 +4006,7 @@ static const vshCmdOptDef opts_dump[] = {
 {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
 {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("where to dump the core")},
 {"verbose", VSH_OT_BOOL, 0, N_("display the progress of dump")},
+{"memory-only", VSH_OT_BOOL, 0, N_("dump domain's memory only")},
 {NULL, 0, 0, NULL}
 };
 
@@ -4044,6 +4045,8 @@ doDump(void *opaque)
 flags |= VIR_DUMP_BYPASS_CACHE;
 if (vshCommandOptBool(cmd, "reset"))
 flags |= VIR_DUMP_RESET;
+if (vshCommandOptBool(cmd, "memory-only"))
+flags |= VIR_DUMP_MEMORY_ONLY;
 
 if (virDomainCoreDump(dom, to, flags) < 0) {
 vshError(ctl, _("Failed to core dump domain %s to %s"), name, to);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index ef71717..e870c03 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -827,7 +827,7 @@ the I argument must be B. For Xen 
hypervisor, the
 I argument may be B or B.
 
 =item B I I [I<--bypass-cache>]
-{ [I<--live>] | [I<--crash>] | [I<--reset>] } [I<--verbose>]
+{ [I<--live>] | [I<--crash>] | [I<--reset>] } [I<--verbose>] [I<--memory-only>]
 
 Dumps the core of a domain to a file for analysis.
 If I<--live> is specified, the domain continues to run until the core
@@ -838,6 +838,9 @@ If I<--reset> is specified, the domain is reset after 
successful dump.
 Note, these three switches are mutually exclusive.
 If I<--bypass-cache> is specified, the save will avoid the file system
 cache, although this may slow down the operation.
+If I<--memory-only> is specified, the file is elf file, and will only
+include domain's memory and cpu common register value. It is very
+useful if the domain uses host devices directly.
 
 The progress may be monitored using B virsh command and canceled
 with B command (sent by another virsh instance). Another option
-- 
1.7.1

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


[libvirt] [PATCH 2/3 v5] qemu: allow the client to choose the vmcore's format

2012-06-11 Thread Wen Congyang
This patch updates qemu driver to allow the client to choose the
vmcore's format: memory only or including device state.
---
 include/libvirt/libvirt.h.in |1 +
 src/qemu/qemu_capabilities.c |5 +++
 src/qemu/qemu_capabilities.h |1 +
 src/qemu/qemu_domain.c   |1 +
 src/qemu/qemu_domain.h   |1 +
 src/qemu/qemu_driver.c   |   60 -
 6 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index fcb6695..120ed14 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -927,6 +927,7 @@ typedef enum {
 VIR_DUMP_LIVE = (1 << 1), /* live dump */
 VIR_DUMP_BYPASS_CACHE = (1 << 2), /* avoid file system cache pollution */
 VIR_DUMP_RESET= (1 << 3), /* reset domain after dump finishes */
+VIR_DUMP_MEMORY_ONLY  = (1 << 4), /* use dump-guest-memory */
 } virDomainCoreDumpFlags;
 
 /* Domain migration flags. */
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b410648..6eee8cd 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -164,6 +164,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
   "no-user-config",
 
   "hda-micro", /* 95 */
+  "dump-guest-memory",
 
 );
 
@@ -1237,6 +1238,10 @@ qemuCapsComputeCmdFlags(const char *help,
 
 if (version >= 11000)
 qemuCapsSet(flags, QEMU_CAPS_CPU_HOST);
+
+if (version >= 1001050)
+qemuCapsSet(flags, QEMU_CAPS_DUMP_GUEST_MEMORY);
+
 return 0;
 }
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 64831e2..640f7f5 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -131,6 +131,7 @@ enum qemuCapsFlags {
 QEMU_CAPS_IDE_CD = 93, /* -device ide-cd */
 QEMU_CAPS_NO_USER_CONFIG = 94, /* -no-user-config */
 QEMU_CAPS_HDA_MICRO  = 95, /* -device hda-micro */
+QEMU_CAPS_DUMP_GUEST_MEMORY  = 96, /* dump-guest-memory command */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3752ddf..91c0645 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -160,6 +160,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
 job->phase = 0;
 job->mask = DEFAULT_JOB_MASK;
 job->start = 0;
+job->dump_memory_only = false;
 memset(&job->info, 0, sizeof(job->info));
 }
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 1256c4e..3a5f1f3 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -104,6 +104,7 @@ struct qemuDomainJobObj {
 int phase;  /* Job phase (mainly for migrations) */
 unsigned long long mask;/* Jobs allowed during async job */
 unsigned long long start;   /* When the async job started */
+bool dump_memory_only;  /* use dump-guest-memory to do dump */
 virDomainJobInfo info;  /* Async job progress data */
 };
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3f74d2..b2da838 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2986,12 +2986,39 @@ cleanup:
 return ret;
 }
 
+static int qemuDumpToFd(struct qemud_driver *driver, virDomainObjPtr vm,
+int fd, enum qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret = -1;
+
+if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DUMP_GUEST_MEMORY)) {
+qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+_("dump-guest-memory is not supported"));
+return -1;
+}
+
+if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
+  fd) < 0)
+return -1;
+
+priv->job.dump_memory_only = true;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+ret = qemuMonitorDumpToFd(priv->mon, 0, fd, 0, 0);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+return ret;
+}
+
 static int
 doCoreDump(struct qemud_driver *driver,
virDomainObjPtr vm,
const char *path,
enum qemud_save_formats compress,
-   bool bypass_cache)
+   unsigned int dump_flags)
 {
 int fd = -1;
 int ret = -1;
@@ -3000,7 +3027,7 @@ doCoreDump(struct qemud_driver *driver,
 unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING;
 
 /* Create an empty file with appropriate ownership.  */
-if (bypass_cache) {
+if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
 flags |= VIR_FILE_WRAPPER_BYPASS_CACHE;
 directFlag = virFileDirectFdFlag();
 if (directFlag < 0) {
@@ -3020,14 +3047,20 @@ doCoreDump(struct qemud_driver *driver,
 if (!(wrapperFd = virFileWrapperFdNew(&fd, path, flags)))
 goto cleanup;
 
-   

[libvirt] [PATCH 1/3 v5] qemu: implement qemu's dump-guest-memory

2012-06-11 Thread Wen Congyang
dump-guest-memory is a new dump mechanism, and it can work when the
guest uses host devices. This patch adds a API to use this new
monitor command.
We will always use json mode if qemu's version is >= 0.15, so I
don't implement the API for text mode.
---
 src/qemu/qemu_monitor.c  |   37 +
 src/qemu/qemu_monitor.h  |   12 
 src/qemu/qemu_monitor_json.c |   34 ++
 src/qemu/qemu_monitor_json.h |6 ++
 4 files changed, 89 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 7084c68..e2a39b8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2022,6 +2022,43 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+int qemuMonitorDumpToFd(qemuMonitorPtr mon,
+unsigned int flags,
+int fd,
+unsigned long long begin,
+unsigned long long length)
+{
+int ret;
+VIR_DEBUG("mon=%p fd=%d flags=%x begin=%llx length=%llx",
+  mon, fd, flags, begin, length);
+
+if (!mon) {
+qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+_("monitor must not be NULL"));
+return -1;
+}
+
+if (!mon->json) {
+/* We don't have qemuMonitorTextDump(), so we should check mon->json
+ * here.
+ */
+qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+_("dump-guest-memory is not supported in text mode"));
+return -1;
+}
+
+if (qemuMonitorSendFileHandle(mon, "dump", fd) < 0)
+return -1;
+
+ret = qemuMonitorJSONDump(mon, flags, "fd:dump", begin, length);
+
+if (ret < 0) {
+if (qemuMonitorCloseFileHandle(mon, "dump") < 0)
+VIR_WARN("failed to close dumping handle");
+}
+
+return ret;
+}
 
 int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int type,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index ffe8fe7..66bec38 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -379,6 +379,18 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
 
 int qemuMonitorMigrateCancel(qemuMonitorPtr mon);
 
+typedef enum {
+  QEMU_MONITOR_DUMP_HAVE_FILTER  = 1 << 0,
+  QEMU_MONITOR_DUMP_PAGING   = 1 << 1,
+  QEMU_MONITOR_DUMP_FLAGS_LAST
+} QEMU_MONITOR_DUMP;
+
+int qemuMonitorDumpToFd(qemuMonitorPtr mon,
+unsigned int flags,
+int fd,
+unsigned long long begin,
+unsigned long long length);
+
 int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int type,
 const char *hostname,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 9030347..6ca01c5 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2465,6 +2465,40 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+int qemuMonitorJSONDump(qemuMonitorPtr mon,
+unsigned int flags,
+const char *protocol,
+unsigned long long begin,
+unsigned long long length)
+{
+int ret;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+
+if (flags & QEMU_MONITOR_DUMP_HAVE_FILTER)
+cmd = qemuMonitorJSONMakeCommand("dump-guest-memory",
+ "b:paging", flags & 
QEMU_MONITOR_DUMP_PAGING ? 1 : 0,
+ "s:protocol", protocol,
+ "U:begin", begin,
+ "U:length", length,
+ NULL);
+else
+cmd = qemuMonitorJSONMakeCommand("dump-guest-memory",
+ "b:paging", flags & 
QEMU_MONITOR_DUMP_PAGING ? 1 : 0,
+ "s:protocol", protocol,
+ NULL);
+if (!cmd)
+return -1;
+
+ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+if (ret == 0)
+ret = qemuMonitorJSONCheckError(cmd, reply);
+
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
 
 int qemuMonitorJSONGraphicsRelocate(qemuMonitorPtr mon,
 int type,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 22a3adf..e8bd9b8 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -136,6 +136,12 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon,
 
 int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon);
 
+int qemuMonitorJSONDump(qemuMonitorPtr mon,
+unsigned int flags,
+const char *protocol,
+unsigned long long begin,
+   

[libvirt] [PATCH 0/3 v5] use qemu's dump-guest-meory when vm uses host device

2012-06-11 Thread Wen Congyang
Currently, we use migrate to dump guest's memory. There is one
restriction in migrate command: the device's status should be
stored in qemu because the device's status should be passed to
target machine.

If we passthrough a host device to guest, the device's status
is stored in the real device. So migrate command will fail.

We usually use dump when guest is panicked. So there is no need
to store device's status in the vmcore.

qemu have a new monitor command dump-guest-memory to dump
guest memory, but it doesn't support async now(it will support
later when the common async API is implemented).

Changes from v4 to v5:
1. address Martin Kletzander's comment

Changes from v3 to v4:
1. allow the user to specify the core file's format

Changes from v2 to v3:
1. qemu supports the fd that is associated with a pipe, socket, or FIFO.
   So pass a pipe fd to qemu and O_DIRECT can work now.

Change from v1 to v2:
1. remove the implemention for text mode.

Wen Congyang (3):
  qemu: implement qemu's dump-guest-memory
  qemu: allow the client to choose the vmcore's format
  virsh: allow the user to specify vmcore's format

 include/libvirt/libvirt.h.in |1 +
 src/qemu/qemu_capabilities.c |5 +++
 src/qemu/qemu_capabilities.h |1 +
 src/qemu/qemu_domain.c   |1 +
 src/qemu/qemu_domain.h   |1 +
 src/qemu/qemu_driver.c   |   60 -
 src/qemu/qemu_monitor.c  |   37 ++
 src/qemu/qemu_monitor.h  |   12 
 src/qemu/qemu_monitor_json.c |   34 +++
 src/qemu/qemu_monitor_json.h |6 
 tools/virsh.c|3 ++
 tools/virsh.pod  |5 +++-
 12 files changed, 152 insertions(+), 14 deletions(-)

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


[libvirt] make dist failed

2012-06-11 Thread Wen Congyang
make dist failed with the following error:

make[1]: Entering directory `/home/wency/source/libvirt/src'
  GENrpc/virnetprotocol.h
  GENrpc/virnetprotocol.c
  GENrpc/virkeepaliveprotocol.h
  GENrpc/virkeepaliveprotocol.c
  GENremote/remote_protocol.h
  GENremote/remote_protocol.c
  GENremote/qemu_protocol.h
  GENremote/qemu_protocol.c
  GENremote/qemu_client_bodies.h
  GENutil/virkeymaps.h
  CC libvirt_driver_remote_la-remote_protocol.lo
In file included from ./remote/remote_protocol.h:17,
 from ./remote/remote_protocol.c:7:
./internal.h:300:31: error: libvirt_probes.h: No such file or directory

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


[libvirt] [PATCH] fix make syntax-check failed

2012-06-03 Thread Wen Congyang
---
 tools/virsh.c |  170 
 1 files changed, 85 insertions(+), 85 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index a934c13..0e41d00 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -3502,16 +3502,16 @@ cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptString(cmd, "file", &file) <= 0)
 return false;
 
-#define EDIT_GET_XML \
-virDomainSaveImageGetXMLDesc(ctl->conn, file, getxml_flags)
-#define EDIT_NOT_CHANGED \
-vshPrint(ctl, _("Saved image %s XML configuration " \
-"not changed.\n"), file);   \
-ret = true; goto edit_cleanup;
-#define EDIT_DEFINE \
-virDomainSaveImageDefineXML(ctl->conn, file, doc_edited, define_flags)
-#define EDIT_FREE /* */
-#include "virsh-edit.c"
+#define EDIT_GET_XML \
+virDomainSaveImageGetXMLDesc(ctl->conn, file, getxml_flags)
+#define EDIT_NOT_CHANGED \
+vshPrint(ctl, _("Saved image %s XML configuration " \
+"not changed.\n"), file);   \
+ret = true; goto edit_cleanup;
+#define EDIT_DEFINE \
+virDomainSaveImageDefineXML(ctl->conn, file, doc_edited, define_flags)
+#define EDIT_FREE /* */
+#include "virsh-edit.c"
 
 vshPrint(ctl, _("State file %s edited.\n"), file);
 ret = true;
@@ -8584,17 +8584,17 @@ cmdInterfaceEdit(vshControl *ctl, const vshCmd *cmd)
 if (iface == NULL)
 goto cleanup;
 
-#define EDIT_GET_XML virInterfaceGetXMLDesc(iface, flags)
-#define EDIT_NOT_CHANGED \
-vshPrint(ctl, _("Interface %s XML configuration not changed.\n"),   \
- virInterfaceGetName(iface));   \
-ret = true; goto edit_cleanup;
-#define EDIT_DEFINE \
-(iface_edited = virInterfaceDefineXML(ctl->conn, doc_edited, 0))
-#define EDIT_FREE \
-if (iface_edited)   \
-virInterfaceFree(iface_edited);
-#include "virsh-edit.c"
+#define EDIT_GET_XML virInterfaceGetXMLDesc(iface, flags)
+#define EDIT_NOT_CHANGED \
+vshPrint(ctl, _("Interface %s XML configuration not changed.\n"),   \
+ virInterfaceGetName(iface));   \
+ret = true; goto edit_cleanup;
+#define EDIT_DEFINE \
+(iface_edited = virInterfaceDefineXML(ctl->conn, doc_edited, 0))
+#define EDIT_FREE \
+if (iface_edited)   \
+virInterfaceFree(iface_edited);
+#include "virsh-edit.c"
 
 vshPrint(ctl, _("Interface %s XML configuration edited.\n"),
  virInterfaceGetName(iface));
@@ -9978,18 +9978,18 @@ cmdNWFilterEdit(vshControl *ctl, const vshCmd *cmd)
 if (nwfilter == NULL)
 goto cleanup;
 
-#define EDIT_GET_XML virNWFilterGetXMLDesc(nwfilter, 0)
-#define EDIT_NOT_CHANGED \
-vshPrint(ctl, _("Network filter %s XML "\
-"configuration not changed.\n"),\
- virNWFilterGetName(nwfilter)); \
-ret = true; goto edit_cleanup;
-#define EDIT_DEFINE \
-(nwfilter_edited = virNWFilterDefineXML(ctl->conn, doc_edited))
-#define EDIT_FREE \
-if (nwfilter_edited)\
-virNWFilterFree(nwfilter);
-#include "virsh-edit.c"
+#define EDIT_GET_XML virNWFilterGetXMLDesc(nwfilter, 0)
+#define EDIT_NOT_CHANGED \
+vshPrint(ctl, _("Network filter %s XML "\
+"configuration not changed.\n"),\
+ virNWFilterGetName(nwfilter)); \
+ret = true; goto edit_cleanup;
+#define EDIT_DEFINE \
+(nwfilter_edited = virNWFilterDefineXML(ctl->conn, doc_edited))
+#define EDIT_FREE \
+if (nwfilter_edited)\
+virNWFilterFree(nwfilter);
+#include "virsh-edit.c"
 
 vshPrint(ctl, _("Network filter %s XML configuration edited.\n"),
  virNWFilterGetName(nwfilter));
@@ -15761,17 +15761,17 @@ cmdEdit(vshControl *ctl, const vshCmd *cmd)
 if (dom == NULL)
 goto cleanup;
 
-#define EDIT_GET_XML virDomainGetXMLDesc(dom, flags)
-#define EDIT_NOT_CHANGED \
-vshPrint(ctl, _("Domain %s XML configuration not changed.\n"),  \
- virDomainGetName (dom));   \
-ret = true; goto edit_cleanup;
-#define EDIT_DEFINE \
-(dom_edited = virDomainDefineXML(ctl->conn, doc_edited))
-#define EDIT_FREE \
-if (dom_edited) \
-virDomainFree(dom_edited);
-#include "virsh-edit.c"
+#define EDIT_GET_XML virDomainGetXMLDesc(dom, flags)
+#define EDIT_NOT_CHANGED \
+vshPrint(ctl, _("Domain %s XML configuration not changed.\n"),  \
+ virDomainGetName (dom));   \
+ret = true; goto edit_cleanup;
+#define EDIT_DEFINE \
+(dom_edited = virDomainDefineXML(ctl->conn, doc_edited))
+#define EDIT_FREE \
+if (dom_edited) \
+virDomainFree(dom_edited);
+#include "virsh-edit.c"
 
 vshPrint(ctl, _("Domain %s XM

[libvirt] [PATCH] build: include viratomic.h into dist file

2012-06-03 Thread Wen Congyang
We add viratomic.h, but we forget to include it into dist file.
This file is used in nwfilter/nwfilter_dhcpsnoop.c now.

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

diff --git a/src/Makefile.am b/src/Makefile.am
index d727889..331ebc8 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -107,7 +107,8 @@ UTIL_SOURCES =  
\
util/virrandom.h util/virrandom.c   \
util/virsocketaddr.h util/virsocketaddr.c \
util/virtime.h util/virtime.c \
-   util/viruri.h util/viruri.c
+   util/viruri.h util/viruri.c \
+   util/viratomic.h
 
 EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
$(srcdir)/util/virkeycode-mapgen.py
-- 
1.7.1

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


[libvirt] [PATCH 3/3] avoid closing fd more than once when running command fails

2012-05-30 Thread Wen Congyang
We should not set *outfd or *errfd if virExecWithHook() failed,
because the caller may close these fds.
---
 src/util/command.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/util/command.c b/src/util/command.c
index 5b94f1e..aef9131 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -493,6 +493,10 @@ virExecWithHook(const char *const*argv,
 }
 
 if (pid) { /* parent */
+if (forkRet < 0) {
+goto cleanup;
+}
+
 VIR_FORCE_CLOSE(null);
 if (outfd && *outfd == -1) {
 VIR_FORCE_CLOSE(pipeout[1]);
@@ -503,10 +507,6 @@ virExecWithHook(const char *const*argv,
 *errfd = pipeerr[0];
 }
 
-if (forkRet < 0) {
-goto cleanup;
-}
-
 *retpid = pid;
 
 if (binary != argv[0])
-- 
1.7.1

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


[libvirt] [PATCH 2/3] qemu: avoid closing fd more than once during migration

2012-05-30 Thread Wen Congyang
If we migrate to fd, spec->fwdType is not MIGRATION_FWD_DIRECT, and
we will close spec->dest.fd.local in qemuMigrationRun(). So we
should set spec->dest.fd.local to -1 in qemuMigrationRun() because
the caller doTunnelMigrate() will close it too.
---
 src/qemu/qemu_migration.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6f42823..b58380b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1910,8 +1910,10 @@ qemuMigrationRun(struct qemud_driver *driver,
 break;
 
 case MIGRATION_DEST_FD:
-if (spec->fwdType != MIGRATION_FWD_DIRECT)
+if (spec->fwdType != MIGRATION_FWD_DIRECT) {
 fd = spec->dest.fd.local;
+spec->dest.fd.local = -1;
+}
 ret = qemuMonitorMigrateToFd(priv->mon, migrate_flags,
  spec->dest.fd.qemu);
 VIR_FORCE_CLOSE(spec->dest.fd.qemu);
-- 
1.7.1

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


[libvirt] [PATCH 1/3] avoid closing fd more than once in virFDStreamOpenFileInternal()

2012-05-30 Thread Wen Congyang
When we assign fd and childfd to elements of fds[], we will
close these fds more than once on error. So we should assign
fds[] to -1 after we assign fd and childfd to elements of
fds[]. We should also close childfd on error.
---
 src/fdstream.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/fdstream.c b/src/fdstream.c
index fca0f41..0de3b04 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -581,6 +581,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
 struct stat sb;
 virCommandPtr cmd = NULL;
 int errfd = -1;
+int childfd = -1;
 
 VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o",
   st, path, oflags, offset, length, mode);
@@ -619,7 +620,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
 if ((st->flags & VIR_STREAM_NONBLOCK) &&
 (!S_ISCHR(sb.st_mode) &&
  !S_ISFIFO(sb.st_mode))) {
-int childfd;
 
 if ((oflags & O_ACCMODE) == O_RDWR) {
 streamsReportError(VIR_ERR_INTERNAL_ERROR,
@@ -650,6 +650,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
 fd = fds[1];
 virCommandSetInputFD(cmd, childfd);
 }
+fds[0] = fds[1] = -1;
 virCommandSetErrorFD(cmd, &errfd);
 
 if (virCommandRunAsync(cmd, NULL) < 0)
@@ -668,6 +669,7 @@ error:
 VIR_FORCE_CLOSE(fds[0]);
 VIR_FORCE_CLOSE(fds[1]);
 VIR_FORCE_CLOSE(fd);
+VIR_FORCE_CLOSE(childfd);
 VIR_FORCE_CLOSE(errfd);
 if (oflags & O_CREAT)
 unlink(path);
-- 
1.7.1

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


Re: [libvirt] [PATCH 2/3] avoid closing fd more than once

2012-05-30 Thread Wen Congyang
At 05/30/2012 09:17 PM, Eric Blake Wrote:
> On 05/30/2012 03:20 AM, Wen Congyang wrote:
>> fdstream:
>> If fd is fds[0] or fds[1], we should set to -1 if we meet
>> some error.
>>
>> childfd is fds[0] or fds[1], so we should close it only when
>> virFDStreamOpenFileInternal() successes.
>>
>> qemu_migration:
>> If we migrate to fd, spec->fwdType is not MIGRATION_FWD_DIRECT,
>> we will close spec->dest.fd.local in qemuMigrationRun(). So we
>> should set spec->dest.fd.local to -1 in qemuMigrationRun(). 
>>
>> command:
>> we should not set *outfd or *errfd if virExecWithHook() failed
>> because the caller may close these fds.
> 
> We should split this into three separate patches, to aid backporting
> each patch across appropriate versions.  Needs a v2 for this reason; as
> I want these bugs fixed sooner rather than later, I'll probably help by
> reposting things myself.
> 
>>
>> ---
>>  src/fdstream.c|   15 ++-
>>  src/qemu/qemu_migration.c |4 +++-
>>  src/util/command.c|8 
>>  3 files changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/fdstream.c b/src/fdstream.c
>> index 32d386d..d0ea0ee 100644
>> --- a/src/fdstream.c
>> +++ b/src/fdstream.c
>> @@ -581,6 +581,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>>  struct stat sb;
>>  virCommandPtr cmd = NULL;
>>  int errfd = -1;
>> +int childfd = -1;
>>  
>>  VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o",
>>st, path, oflags, offset, length, mode);
>> @@ -619,7 +620,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>>  if ((st->flags & VIR_STREAM_NONBLOCK) &&
>>  (!S_ISCHR(sb.st_mode) &&
>>   !S_ISFIFO(sb.st_mode))) {
>> -int childfd;
>>  
>>  if ((oflags & O_ACCMODE) == O_RDWR) {
>>  streamsReportError(VIR_ERR_INTERNAL_ERROR,
>> @@ -652,15 +652,20 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>>  }
>>  virCommandSetErrorFD(cmd, &errfd);
>>  
>> -if (virCommandRunAsync(cmd, NULL) < 0)
>> +if (virCommandRunAsync(cmd, NULL) < 0) {
>> +/* donot close fd twice if we meet some error */
> 
> s/donot/don't/
> 
>> +fd = -1;
>>  goto error;
>> -
>> -VIR_FORCE_CLOSE(childfd);
>> +}
>>  }
>>  
>> -if (virFDStreamOpenInternal(st, fd, cmd, errfd, length) < 0)
>> +if (virFDStreamOpenInternal(st, fd, cmd, errfd, length) < 0) {
>> +/* donot close fd twice if we meet some error */
> 
> and again.
> 
>> +fd = -1;
>>  goto error;
>> +}
>>  
>> +VIR_FORCE_CLOSE(childfd);
>>  return 0;
> 
> Doesn't this leak childfd on error?

No, childfd is fds[0] or fds[1]. We have closed fds[0] and fds[1] on error,
so we should not close childfd on error.

> 
> Maybe a better solution here would be that when we assign fd and childfd
> to elements of fds[], then we also assign fds to -1, as in:

Good idea.

Thanks
Wen Congyang

> 
> diff --git i/src/fdstream.c w/src/fdstream.c
> index 32d386d..b797a05 100644
> --- i/src/fdstream.c
> +++ w/src/fdstream.c
> @@ -581,6 +581,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>  struct stat sb;
>  virCommandPtr cmd = NULL;
>  int errfd = -1;
> +int childfd = -1;
> 
>  VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o",
>st, path, oflags, offset, length, mode);
> @@ -619,7 +620,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>  if ((st->flags & VIR_STREAM_NONBLOCK) &&
>  (!S_ISCHR(sb.st_mode) &&
>   !S_ISFIFO(sb.st_mode))) {
> -int childfd;
> 
>  if ((oflags & O_ACCMODE) == O_RDWR) {
>  streamsReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -650,6 +650,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>  fd = fds[1];
>  virCommandSetInputFD(cmd, childfd);
>  }
> +fds[0] = fds[1] = -1;
>  virCommandSetErrorFD(cmd, &errfd);
> 
>  if (virCommandRunAsync(cmd, NULL) < 0)
> @@ -668,6 +669,7 @@ error:
>  VIR_FORCE_CLOSE(fds[0]);
>  VIR_FORCE_CLOSE(fds[1]);
>  VIR_FORCE_CLOSE(fd);
> +VIR_FORCE_CLOSE(childfd);
>  if (oflags & O_CREAT)
>  unlink(path);
>  return -1;
> 
> 

[libvirt] [PATCH 3/3] avoid fd leak

2012-05-30 Thread Wen Congyang
virCommandRunAsync() will set errfd if it successes. We should
close it if virFDStreamOpenInternal() fails.

---
 src/fdstream.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/fdstream.c b/src/fdstream.c
index d0ea0ee..f068439 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -673,6 +673,7 @@ error:
 VIR_FORCE_CLOSE(fds[0]);
 VIR_FORCE_CLOSE(fds[1]);
 VIR_FORCE_CLOSE(fd);
+VIR_FORCE_CLOSE(errfd);
 if (oflags & O_CREAT)
 unlink(path);
 return -1;
-- 
1.7.1

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


[libvirt] [PATCH 2/3] avoid closing fd more than once

2012-05-30 Thread Wen Congyang
fdstream:
If fd is fds[0] or fds[1], we should set to -1 if we meet
some error.

childfd is fds[0] or fds[1], so we should close it only when
virFDStreamOpenFileInternal() successes.

qemu_migration:
If we migrate to fd, spec->fwdType is not MIGRATION_FWD_DIRECT,
we will close spec->dest.fd.local in qemuMigrationRun(). So we
should set spec->dest.fd.local to -1 in qemuMigrationRun(). 

command:
we should not set *outfd or *errfd if virExecWithHook() failed
because the caller may close these fds.

---
 src/fdstream.c|   15 ++-
 src/qemu/qemu_migration.c |4 +++-
 src/util/command.c|8 
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/fdstream.c b/src/fdstream.c
index 32d386d..d0ea0ee 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -581,6 +581,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
 struct stat sb;
 virCommandPtr cmd = NULL;
 int errfd = -1;
+int childfd = -1;
 
 VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o",
   st, path, oflags, offset, length, mode);
@@ -619,7 +620,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
 if ((st->flags & VIR_STREAM_NONBLOCK) &&
 (!S_ISCHR(sb.st_mode) &&
  !S_ISFIFO(sb.st_mode))) {
-int childfd;
 
 if ((oflags & O_ACCMODE) == O_RDWR) {
 streamsReportError(VIR_ERR_INTERNAL_ERROR,
@@ -652,15 +652,20 @@ virFDStreamOpenFileInternal(virStreamPtr st,
 }
 virCommandSetErrorFD(cmd, &errfd);
 
-if (virCommandRunAsync(cmd, NULL) < 0)
+if (virCommandRunAsync(cmd, NULL) < 0) {
+/* donot close fd twice if we meet some error */
+fd = -1;
 goto error;
-
-VIR_FORCE_CLOSE(childfd);
+}
 }
 
-if (virFDStreamOpenInternal(st, fd, cmd, errfd, length) < 0)
+if (virFDStreamOpenInternal(st, fd, cmd, errfd, length) < 0) {
+/* donot close fd twice if we meet some error */
+fd = -1;
 goto error;
+}
 
+VIR_FORCE_CLOSE(childfd);
 return 0;
 
 error:
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6f42823..b58380b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1910,8 +1910,10 @@ qemuMigrationRun(struct qemud_driver *driver,
 break;
 
 case MIGRATION_DEST_FD:
-if (spec->fwdType != MIGRATION_FWD_DIRECT)
+if (spec->fwdType != MIGRATION_FWD_DIRECT) {
 fd = spec->dest.fd.local;
+spec->dest.fd.local = -1;
+}
 ret = qemuMonitorMigrateToFd(priv->mon, migrate_flags,
  spec->dest.fd.qemu);
 VIR_FORCE_CLOSE(spec->dest.fd.qemu);
diff --git a/src/util/command.c b/src/util/command.c
index eaa9f16..2723fde 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -493,6 +493,10 @@ virExecWithHook(const char *const*argv,
 }
 
 if (pid) { /* parent */
+if (forkRet < 0) {
+goto cleanup;
+}
+
 VIR_FORCE_CLOSE(null);
 if (outfd && *outfd == -1) {
 VIR_FORCE_CLOSE(pipeout[1]);
@@ -503,10 +507,6 @@ virExecWithHook(const char *const*argv,
 *errfd = pipeerr[0];
 }
 
-if (forkRet < 0) {
-goto cleanup;
-}
-
 *retpid = pid;
 
 if (binary != argv[0])
-- 
1.7.1

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


[libvirt] [PATCH 1/3] avoid closing uninitialized fd

2012-05-30 Thread Wen Congyang
If the system does not support bypass cache, we will close fd,
but it is uninitialized.

---
 src/qemu/qemu_driver.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fea9c24..1b3391b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4010,7 +4010,7 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver,
 const char *xmlin, int state, bool edit,
 bool unlink_corrupt)
 {
-int fd;
+int fd = -1;
 struct qemud_save_header header;
 char *xml = NULL;
 virDomainDefPtr def = NULL;
-- 
1.7.1

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


Re: [libvirt] [PATCH] command: avoid double close bugs

2012-05-29 Thread Wen Congyang
At 05/30/2012 09:20 AM, Eric Blake Wrote:
> KAMEZAWA Hiroyuki reported a nasty double-free bug when virCommand
> is used to convert a string into input to a child command.  The
> problem is that the poll() loop of virCommandProcessIO would close()
> the write end of the pipe in order to let the child see EOF, then
> the caller virCommandRun() would also close the same fd number, with
> the second close possibly nuking an fd opened by some other thread
> in the meantime.  This in turn can have all sorts of bad effects.
> 
> This is based on his first attempt at a patch, at
> https://bugzilla.redhat.com/show_bug.cgi?id=823716

close fd more twice is the cause of this bug. But there are some
other codes that have the same problem. I am searching all such
codes recent days.

> 
> * src/util/command.c (_virCommand): Drop inpipe member.
> (virCommandProcessIO): Add argument, to avoid closing caller's fd
> without informing caller.
> (virCommandRun, virCommandNewArgs): Adjust clients.
> ---
>  src/util/command.c |   18 ++
>  1 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/src/util/command.c b/src/util/command.c
> index eaa9f16..bf219e4 100644
> --- a/src/util/command.c
> +++ b/src/util/command.c
> @@ -83,7 +83,6 @@ struct _virCommand {
>  char **errbuf;
> 
>  int infd;
> -int inpipe;
>  int outfd;
>  int errfd;
>  int *outfdptr;
> @@ -788,7 +787,6 @@ virCommandNewArgs(const char *const*args)
>  cmd->handshakeNotify[1] = -1;
> 
>  cmd->infd = cmd->outfd = cmd->errfd = -1;
> -cmd->inpipe = -1;
>  cmd->pid = -1;
> 
>  virCommandAddArgSet(cmd, args);
> @@ -1676,7 +1674,7 @@ virCommandTranslateStatus(int status)
>   * Manage input and output to the child process.
>   */
>  static int
> -virCommandProcessIO(virCommandPtr cmd)
> +virCommandProcessIO(virCommandPtr cmd, int *inpipe)
>  {
>  int infd = -1, outfd = -1, errfd = -1;
>  size_t inlen = 0, outlen = 0, errlen = 0;
> @@ -1687,7 +1685,7 @@ virCommandProcessIO(virCommandPtr cmd)
>   * via pipe */
>  if (cmd->inbuf) {
>  inlen = strlen(cmd->inbuf);
> -infd = cmd->inpipe;
> +infd = *inpipe;
>  }
> 
>  /* With out/err buffer, the outfd/errfd have been filled with an
> @@ -1807,12 +1805,9 @@ virCommandProcessIO(virCommandPtr cmd)
>  }
>  } else {
>  inoff += done;
> -if (inoff == inlen) {
> -int tmpfd ATTRIBUTE_UNUSED;
> -tmpfd = infd;
> -if (VIR_CLOSE(infd) < 0)
> -VIR_DEBUG("ignoring failed close on fd %d", 
> tmpfd);
> -}
> +if (inoff == inlen && VIR_CLOSE(*inpipe) < 0)
> +VIR_DEBUG("ignoring failed close on fd %d", infd);
> +infd = -1;

if inoff != inlen, we should not set infd to -1.

>  }
>  }
>  }
> @@ -1938,7 +1933,6 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
>  return -1;
>  }
>  cmd->infd = infd[0];
> -cmd->inpipe = infd[1];
>  }
> 
>  /* If caller requested the same string for stdout and stderr, then
> @@ -1981,7 +1975,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
>  }
> 
>  if (string_io)
> -ret = virCommandProcessIO(cmd);
> +ret = virCommandProcessIO(cmd, &infd[1]);
> 
>  if (virCommandWait(cmd, exitstatus) < 0)
>  ret = -1;

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


[libvirt] [PATCH] build: include augeas-gentest.pl and libvirt_qemu_probes.h into dist file

2012-05-29 Thread Wen Congyang
We generate *.aug from *.aug.in by augeas-gentest.pl, so this script
should be included in dist file.

We split QEMU dtrace probes into separate file, but we forget include
libvirt_qemu_probes.h into dist file.

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

diff --git a/src/Makefile.am b/src/Makefile.am
index c8efa5b..c418897 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1120,6 +1120,7 @@ check-local: check-augeas
 check-augeas: check-augeas-qemu check-augeas-lxc check-augeas-sanlock
 
 AUG_GENTEST = $(top_srcdir)/build-aux/augeas-gentest.pl
+EXTRA_DIST += $(AUG_GENTEST)
 
 if WITH_QEMU
 qemu/test_libvirtd_qemu.aug: qemu/test_libvirtd_qemu.aug.in qemu/qemu.conf 
$(AUG_GENTEST)
@@ -1289,7 +1290,8 @@ if WITH_REMOTE
 $(REMOTE_DRIVER_GENERATED): libvirt_probes.h
 endif WITH_REMOTE
 
-BUILT_SOURCES += libvirt_probes.h libvirt_probes.stp libvirt_functions.stp
+BUILT_SOURCES += libvirt_probes.h libvirt_probes.stp libvirt_functions.stp \
+libvirt_qemu_probes.h
 
 if WITH_QEMU
 libvirt_driver_qemu_la_LIBADD += libvirt_qemu_probes.lo
-- 
1.7.1

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


[libvirt] [PATCH] qemu: fix potential dead lock

2012-05-25 Thread Wen Congyang
If we lock the qemu_driver, we should call qemuDomainObjBeginJobWithDriver()
not qemuDomainObjBeginJob().

---
 src/qemu/qemu_driver.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 39b27b1..3d62aab 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8905,7 +8905,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
 goto cleanup;
 asyncJob = QEMU_ASYNC_JOB_MIGRATION_OUT;
 } else {
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
 goto cleanup;
 asyncJob = QEMU_ASYNC_JOB_NONE;
 }
-- 
1.7.1

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


Re: [libvirt] [PATCH 10/10] Enable driver modules in libvirt RPM

2012-05-24 Thread Wen Congyang
At 05/22/2012 03:59 AM, Daniel P. Berrange Wrote:
> From: "Daniel P. Berrange" 
> 
> Turn on loadable modules for libvirtd. Add new sub-RPMs
> libvirt-daemon-driver-XXX, one for each loadable .so.
> Modify the libvirt-daemon-YYY RPMs to depend on each of
> the individual drivers they required

I build the libvirt and try to install it, and meet some
dependencies error:
error: Failed dependencies:
libvirt-daemon-network = 0.9.12-1.el6 is needed by 
libvirt-daemon-driver-lxc-0.9.12-1.el6.x86_64
libvirt-daemon-network = 0.9.12-1.el6 is needed by 
libvirt-daemon-driver-qemu-0.9.12-1.el6.x86_64
libvirt-daemon-driver-nwilter = 0.9.12-1.el6 is needed by 
libvirt-daemon-kvm-0.9.12-1.el6.x86_64
libvirt-daemon-driver-nwilter = 0.9.12-1.el6 is needed by 
libvirt-daemon-lxc-0.9.12-1.el6.x86_64

> 
> * libvirt.spec.in: Enable driver modules
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt.spec.in |  296 
> ++-
>  1 files changed, 293 insertions(+), 3 deletions(-)
> 



> +%if %{with_qemu}
> +%package daemon-driver-qemu
> +Summary: Qemu driver plugin for the libvirtd daemon
> +Group: Development/Libraries
> +Requires: libvirt-daemon = %{version}-%{release}
> +# There really is a hard cross-driver dependancy here
> +Requires: libvirt-daemon-network = %{version}-%{release}

I donot find anything about libvirt-daemon-network.

> +
> +%description daemon-driver-qemu
> +The qemu driver plugin for the libvirtd daemon, providing
> +an implementation of the hypervisor driver APIs using
> +QEMU
> +%endif



>  
>  %if %{with_qemu_tcg}
>  %package daemon-qemu
> @@ -626,6 +790,15 @@ Summary: Server side daemon & driver required to run 
> QEMU guests
>  Group: Development/Libraries
>  
>  Requires: libvirt-daemon = %{version}-%{release}
> +%if %{with_driver_modules}
> +Requires: libvirt-daemon-driver-qemu = %{version}-%{release}
> +Requires: libvirt-daemon-driver-interface = %{version}-%{release}
> +Requires: libvirt-daemon-driver-network = %{version}-%{release}
> +Requires: libvirt-daemon-driver-nodedev = %{version}-%{release}
> +Requires: libvirt-daemon-driver-nwilter = %{version}-%{release}

I donot find anything about libvirt-daemon-driver-nwilter in spec file.

Thanks
Wen Congyang

> +Requires: libvirt-daemon-driver-secret = %{version}-%{release}
> +Requires: libvirt-daemon-driver-storage = %{version}-%{release}
> +%endif
>  Requires: qemu
>  

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


Re: [libvirt] [PATCH] fix building error on non fedora system

2012-05-24 Thread Wen Congyang

On 05/22/2012 08:58 PM, Eric Blake wrote:

On 05/22/2012 02:07 AM, Wen Congyang wrote:

We forget to define with_storage_rbd if the system is not fedora,
or the version is less than 16.

---
  libvirt.spec.in |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8c4a2fd..0b83969 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -75,6 +75,8 @@
  %define with_storage_mpath 0%{!?_without_storage_mpath:%{server_drivers}}
  %if 0%{?fedora}>= 16
  %define with_storage_rbd   0%{!?_without_storage_rbd:%{server_drivers}}
+%else
+%define with_storage_rbd   0


This would hard-code things so that even if you backport rbd to Fedora
15 or to RHEL, you cannot use it.  But then again, we have that same
issue for lots of other variables, so it would be a global cleanup to
allow someone to give overrides to force rbd even it is not default.  In
fact, such a global cleanup has been proposed in the past, although I
never cleaned it up to the point of inclusion:
https://www.redhat.com/archives/libvir-list/2011-November/msg00870.html

ACK - I'm fine with you pushing this patch as-is.


Thanks, pushed

Wen Congyang






--
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 1/3] qemu: implement qemu's dump-guest-memory

2012-05-23 Thread Wen Congyang
At 05/23/2012 04:56 PM, Martin Kletzander Wrote:
> On 04/20/2012 09:27 AM, Wen Congyang wrote:
>> dump-guest-memory is a new dump mechanism, and it can work when the
>> guest uses host devices. This patch adds a API to use this new
>> monitor command.
>>
>> ---
>>  src/qemu/qemu_monitor.c  |   38 ++
>>  src/qemu/qemu_monitor.h  |   12 
>>  src/qemu/qemu_monitor_json.c |   34 ++
>>  src/qemu/qemu_monitor_json.h |6 ++
>>  4 files changed, 90 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>> index 2f66c46..a5d3eec 100644
>> --- a/src/qemu/qemu_monitor.c
>> +++ b/src/qemu/qemu_monitor.c
>> @@ -2018,6 +2018,44 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon)
>>  return ret;
>>  }
>>  
>> +int qemuMonitorDumpToFd(qemuMonitorPtr mon,
>> +unsigned int flags,
>> +int fd,
>> +unsigned long long begin,
>> +unsigned long long length)
>> +{
>> +int ret;
>> +VIR_DEBUG("mon=%p fd=%d flags=%x begin=%llx length=%llx",
>> +  mon, fd, flags, begin, length);
>> +
>> +if (!mon) {
>> +qemuReportError(VIR_ERR_INVALID_ARG, "%s",
>> +_("monitor must not be NULL"));
>> +return -1;
>> +}
>> +
>> +if (!mon->json) {
>> +/* dump-guest-memory is supported after qemu-1.0, and we always use 
>> json
>> + * if qemu's version is >= 0.15. So if we use text mode, the qemu is
>> + * old, and it does not support dump-guest-memory.
>> + */
>> +qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
>> +_("dump-guest-memory is not supported in text 
>> mode"));
>> +return -1;
>> +}
> 
> Correct me if I'm wrong, but shouldn't this be rather handled by adding
> it as a qemu capability? Maybe by checking the qemu version in
> qemuCapsComputeCmdFlags()? Or probably even better by checking that
> command in qemuCapsComputeCmdFlags(). That way we can be sure that the
> command is supported.
> 

If mon->json is false, we use the text mode, so the qemu's version is < 0.15.
The dump-guest-memory is supported after qemu-1.0. So we can say that the
qemu does not support dump-guest-memory if mon->json is false.

Thanks
Wen Congyang

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


Re: [libvirt] [PATCH 0/3 v4] use qemu's dump-guest-meory when vm uses host device

2012-05-22 Thread Wen Congyang
Hi, All

The qemu's patch has been applied to QMP tree. I think we
can start this work now.

Thanks
Wen Congyang

At 04/20/2012 03:20 PM, Wen Congyang Wrote:
> Currently, we use migrate to dump guest's memory. There is one
> restriction in migrate command: the device's status should be
> stored in qemu because the device's status should be passed to
> target machine.
> 
> If we passthrough a host device to guest, the device's status
> is stored in the real device. So migrate command will fail.
> 
> We usually use dump when guest is panicked. So there is no need
> to store device's status in the vmcore.
> 
> qemu will have a new monitor command dump-guest-memory to dump
> guest memory, but it doesn't support async now(it will support
> later when the common async API is implemented).
> 
> So I use dump-guest-memory only when the guest uses host device
> in this patchset.
> 
> Note: the patchset for qemu is still queued. Luiz has acked,
> but he said he does not wait an ACK from Jan and/or Anthony.
> 
> Changes from v3 to v4:
> 1. allow the user to specify the core file's format
> 
> Changes from v2 to v3:
> 1. qemu supports the fd that is associated with a pipe, socket, or FIFO.
>So pass a pipe fd to qemu and O_DIRECT can work now.
> 
> Change from v1 to v2:
> 1. remove the implemention for text mode.
> 
> Wen Congyang (3):
>   qemu: implement qemu's dump-guest-memory
>   qemu: allow the client to choose the vmcore's format
>   virsh: allow the user to specify vmcore's format
> 
>  include/libvirt/libvirt.h.in |1 +
>  src/qemu/qemu_domain.c   |1 +
>  src/qemu/qemu_domain.h   |1 +
>  src/qemu/qemu_driver.c   |   54 +++--
>  src/qemu/qemu_monitor.c  |   38 +
>  src/qemu/qemu_monitor.h  |   12 +
>  src/qemu/qemu_monitor_json.c |   35 +++
>  src/qemu/qemu_monitor_json.h |6 
>  tools/virsh.c|3 ++
>  tools/virsh.pod  |5 +++-
>  10 files changed, 142 insertions(+), 14 deletions(-)
> 
> --
> 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] [PATCH] fix building error on non fedora system

2012-05-22 Thread Wen Congyang
We forget to define with_storage_rbd if the system is not fedora,
or the version is less than 16.

---
 libvirt.spec.in |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8c4a2fd..0b83969 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -75,6 +75,8 @@
 %define with_storage_mpath 0%{!?_without_storage_mpath:%{server_drivers}}
 %if 0%{?fedora} >= 16
 %define with_storage_rbd   0%{!?_without_storage_rbd:%{server_drivers}}
+%else
+%define with_storage_rbd   0
 %endif
 %define with_numactl   0%{!?_without_numactl:%{server_drivers}}
 %define with_selinux   0%{!?_without_selinux:%{server_drivers}}
-- 
1.7.1

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


Re: [libvirt] [PATCH 5/6] qemu: Implement hypervisor's period and quota tunable XML configuration and parsing

2012-05-16 Thread Wen Congyang
At 05/17/2012 01:42 PM, Hu Tao Wrote:
> On Thu, May 17, 2012 at 01:25:48PM +0800, Wen Congyang wrote:
>> At 05/17/2012 01:09 PM, KAMEZAWA Hiroyuki Wrote:
>>> (2012/05/17 14:07), Wen Congyang wrote:
>>>
>>>> At 05/17/2012 12:46 PM, KAMEZAWA Hiroyuki Wrote:
>>>>> (2012/05/17 12:54), Wen Congyang wrote:
>>>
>>>>>> So, if we donot limit the hypervisor, the behavior shoule not be
>>>>>> changed. So we should limit the whole vm. If the hypervisor threads
>>>>>> are limited, there is no need to limit the whole vm.
>>>>>>
>>>>>
>>>>>
>>>>> need to tune hypervisor quota to limit vcpu-only quota ?
>>>>> Sounds strange to me.
>>>>
>>>> No, current implemetion is:
>>>> 1. limit vcpu and hypervisor: vm is not limited
>>>> 2. limit vcpu only: vm is limited
>>>> 3. limit hypervisor: vm is not limited
>>>> 4. vcpu and hypervisor are not limited: vm is not limited.
>>>>
>>>
>>>
>>> In 2.
>>>
>>>vm   - quota_is_limited
>>> |- hypervisor   - quota is unlimited
>>> |- vcpu[0...x]  - quota is limited
>>>
>>> Hm...vm is limited. It seems user need to tune vm's quota...can user
>>> makes vm's quota to be unlimited and limit *only* vpcus as Xen ?
>>
>> Without this patchset, the vm will be limited if the vcpu is limited.
>> We cannot know whether the hypervisor is unlimited or the user does not
>> set it. If the user does not set it, we should not change the behavior.
>> So I donot find an easy way to limit only vcpus.
> 
> Can we change the implementation like these:
> 
> 1. 
> 
>   
> period
> quota
>   
> 
> 
> Limit vcpus only, does not limit vm or hypervisor.
> 
> 2.
> 
>   
> period
> quota
>   
> 
> 
> Limit hypervisor only, does not limit vm or vcpus.
> 
> 3.
> 
>   period
>   quota
> 
> 
> Limit the whole vm, in this case vcpus and hypervisor are also limited.
> 
> 4.
> 
>   period
>   quota
>   
> period
> quota
>   
>   
> period
> quota
>   
> 

This is very complex, and the user should know how we implement it
and the period/quota's limit. If the user gives a wrong value, the
vm cannot start.

We implement the limit like this:

  vm
   |
 
 |   ... |   |
vcpu0   vcpun   hypervisor

If all is limited, the vm's quota should be greater than max(vcpu's quota, 
hypervisor's quota)
and vm's period should be less than min(vcpu's period, hypervisor's period).

So I donot like this idea.

Thanks
Wen Congyang
> 
> Limit vm, vcpus and hypervisor. I'm not sure how cpu cgroup behaves
> in this case, but I suppose vcpus' and hypervisor's values will not
> exceed vm's value.
> 
> 

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


Re: [libvirt] [PATCH 5/6] qemu: Implement hypervisor's period and quota tunable XML configuration and parsing

2012-05-16 Thread Wen Congyang
At 05/17/2012 02:06 PM, KAMEZAWA Hiroyuki Wrote:
> (2012/05/17 15:05), Wen Congyang wrote:
> 
>> At 05/17/2012 01:42 PM, Hu Tao Wrote:
>>> On Thu, May 17, 2012 at 01:25:48PM +0800, Wen Congyang wrote:
>>>> At 05/17/2012 01:09 PM, KAMEZAWA Hiroyuki Wrote:
>>>>> (2012/05/17 14:07), Wen Congyang wrote:
>>>>>
>>>>>> At 05/17/2012 12:46 PM, KAMEZAWA Hiroyuki Wrote:
>>>>>>> (2012/05/17 12:54), Wen Congyang wrote:
>>>>>
>>>>>>>> So, if we donot limit the hypervisor, the behavior shoule not be
>>>>>>>> changed. So we should limit the whole vm. If the hypervisor threads
>>>>>>>> are limited, there is no need to limit the whole vm.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> need to tune hypervisor quota to limit vcpu-only quota ?
>>>>>>> Sounds strange to me.
>>>>>>
>>>>>> No, current implemetion is:
>>>>>> 1. limit vcpu and hypervisor: vm is not limited
>>>>>> 2. limit vcpu only: vm is limited
>>>>>> 3. limit hypervisor: vm is not limited
>>>>>> 4. vcpu and hypervisor are not limited: vm is not limited.
>>>>>>
>>>>>
>>>>>
>>>>> In 2.
>>>>>
>>>>>vm   - quota_is_limited
>>>>> |- hypervisor   - quota is unlimited
>>>>> |- vcpu[0...x]  - quota is limited
>>>>>
>>>>> Hm...vm is limited. It seems user need to tune vm's quota...can user
>>>>> makes vm's quota to be unlimited and limit *only* vpcus as Xen ?
>>>>
>>>> Without this patchset, the vm will be limited if the vcpu is limited.
>>>> We cannot know whether the hypervisor is unlimited or the user does not
>>>> set it. If the user does not set it, we should not change the behavior.
>>>> So I donot find an easy way to limit only vcpus.
>>>
>>> Can we change the implementation like these:
>>>
>>> 1. 
>>> 
>>>   
>>> period
>>> quota
>>>   
>>> 
>>>
>>> Limit vcpus only, does not limit vm or hypervisor.
>>>
>>> 2.
>>> 
>>>   
>>> period
>>> quota
>>>   
>>> 
>>>
>>> Limit hypervisor only, does not limit vm or vcpus.
>>>
>>> 3.
>>> 
>>>   period
>>>   quota
>>> 
>>>
>>> Limit the whole vm, in this case vcpus and hypervisor are also limited.
>>>
>>> 4.
>>> 
>>>   period
>>>   quota
>>>   
>>> period
>>> quota
>>>   
>>>   
>>> period
>>> quota
>>>   
>>> 
>>
>> This is very complex, and the user should know how we implement it
>> and the period/quota's limit. If the user gives a wrong value, the
>> vm cannot start.
>>
>> We implement the limit like this:
>>
>>   vm
>>|
>>  
>>  |   ... |   |
>> vcpu0   vcpun   hypervisor
>>
>> If all is limited, the vm's quota should be greater than max(vcpu's quota, 
>> hypervisor's quota)
>> and vm's period should be less than min(vcpu's period, hypervisor's period).
>>
>> So I donot like this idea.
>>
>  
> 
> Hmm. my proposal is not so complicated as you say. And simpler than your way.
> 
>   # all is default and limit the whole 
> vm.
># vcpu is just limit vcpus rather than 
> whole vm.
>   # period is shared in all settings.
> 
> Isn't it ?

Do you mean this:

  
period
quota
  
  
period
quota
  


Thanks
Wen Congyang
> 
> Thanks,
> -Kame
> 
> 
> 

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


Re: [libvirt] [PATCH 5/6] qemu: Implement hypervisor's period and quota tunable XML configuration and parsing

2012-05-16 Thread Wen Congyang
At 05/17/2012 01:09 PM, KAMEZAWA Hiroyuki Wrote:
> (2012/05/17 14:07), Wen Congyang wrote:
> 
>> At 05/17/2012 12:46 PM, KAMEZAWA Hiroyuki Wrote:
>>> (2012/05/17 12:54), Wen Congyang wrote:
> 
>>>> So, if we donot limit the hypervisor, the behavior shoule not be
>>>> changed. So we should limit the whole vm. If the hypervisor threads
>>>> are limited, there is no need to limit the whole vm.
>>>>
>>>
>>>
>>> need to tune hypervisor quota to limit vcpu-only quota ?
>>> Sounds strange to me.
>>
>> No, current implemetion is:
>> 1. limit vcpu and hypervisor: vm is not limited
>> 2. limit vcpu only: vm is limited
>> 3. limit hypervisor: vm is not limited
>> 4. vcpu and hypervisor are not limited: vm is not limited.
>>
> 
> 
> In 2.
> 
>vm   - quota_is_limited
> |- hypervisor   - quota is unlimited
> |- vcpu[0...x]  - quota is limited
> 
> Hm...vm is limited. It seems user need to tune vm's quota...can user
> makes vm's quota to be unlimited and limit *only* vpcus as Xen ?

Without this patchset, the vm will be limited if the vcpu is limited.
We cannot know whether the hypervisor is unlimited or the user does not
set it. If the user does not set it, we should not change the behavior.
So I donot find an easy way to limit only vcpus.

Thanks
Wen Cong

> 
> Thanks,
> -Kame
> 
> 
> 
> 

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


Re: [libvirt] [PATCH 5/6] qemu: Implement hypervisor's period and quota tunable XML configuration and parsing

2012-05-16 Thread Wen Congyang
At 05/17/2012 12:46 PM, KAMEZAWA Hiroyuki Wrote:
> (2012/05/17 12:54), Wen Congyang wrote:
> 
>> At 05/17/2012 11:13 AM, Hu Tao Wrote:
>>> On Wed, Apr 25, 2012 at 05:46:06PM +0800, Wen Congyang wrote:
>>>> set hypervisor's period and quota when the vm starts. It will affect to set
>>>> vm's period and quota: donot set vm's period and quota if we limit 
>>>> hypevisor
>>>> thread's bandwidth(hypervisor_quota > 0).
>>>> ---
>>>>  src/conf/domain_conf.c |   25 ++-
>>>>  src/conf/domain_conf.h |2 +
>>>>  src/qemu/qemu_cgroup.c |   37 ---
>>>>  src/qemu/qemu_driver.c |   75 
>>>> ++--
>>>>  4 files changed, 98 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>>> index 5ab052a..28a6436 100644
>>>> --- a/src/conf/domain_conf.c
>>>> +++ b/src/conf/domain_conf.c
>>>> @@ -7931,6 +7931,14 @@ static virDomainDefPtr 
>>>> virDomainDefParseXML(virCapsPtr caps,
>>>>   &def->cputune.quota) < 0)
>>>>  def->cputune.quota = 0;
>>>>  
>>>> +if (virXPathULongLong("string(./cputune/hypervisor_period[1])", ctxt,
>>>> +  &def->cputune.hypervisor_period) < 0)
>>>> +def->cputune.hypervisor_period = 0;
>>>> +
>>>> +if (virXPathLongLong("string(./cputune/hypervisor_quota[1])", ctxt,
>>>> + &def->cputune.hypervisor_quota) < 0)
>>>> +def->cputune.hypervisor_quota = 0;
>>>> +
>>>>  if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) {
>>>>  goto error;
>>>>  }
>>>> @@ -12406,7 +12414,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>>>  virBufferAsprintf(buf, ">%u\n", def->maxvcpus);
>>>>  
>>>>  if (def->cputune.shares || def->cputune.vcpupin ||
>>>> -def->cputune.period || def->cputune.quota)
>>>> +def->cputune.period || def->cputune.quota ||
>>>> +def->cputune.hypervisor_period || def->cputune.hypervisor_quota)
>>>>  virBufferAddLit(buf, "  \n");
>>>>  
>>>>  if (def->cputune.shares)
>>>> @@ -12418,6 +12427,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>>>  if (def->cputune.quota)
>>>>  virBufferAsprintf(buf, "%lld\n",
>>>>def->cputune.quota);
>>>> +
>>>> +if (def->cputune.hypervisor_period)
>>>> +virBufferAsprintf(buf, "%llu"
>>>> +  "\n",
>>>> +  def->cputune.hypervisor_period);
>>>> +
>>>> +if (def->cputune.hypervisor_period)
>>>> +virBufferAsprintf(buf, "%lld"
>>>> +  "\n",
>>>> +  def->cputune.hypervisor_quota);
>>>> +
>>>>  if (def->cputune.vcpupin) {
>>>>  for (i = 0; i < def->cputune.nvcpupin; i++) {
>>>>  virBufferAsprintf(buf, ">>> @@ -12439,7 +12459,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>>>  }
>>>>  
>>>>  if (def->cputune.shares || def->cputune.vcpupin ||
>>>> -def->cputune.period || def->cputune.quota)
>>>> +def->cputune.period || def->cputune.quota ||
>>>> +def->cputune.hypervisor_period || def->cputune.hypervisor_quota)
>>>>  virBufferAddLit(buf, "  \n");
>>>>  
>>>>  if (def->numatune.memory.nodemask) {
>>>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>>>> index 0eed60e..2a37e26 100644
>>>> --- a/src/conf/domain_conf.h
>>>> +++ b/src/conf/domain_conf.h
>>>> @@ -1558,6 +1558,8 @@ struct _virDomainDef {
>>>>  unsigned long shares;
>>>>  unsigned long long period;
>>>>  long long quota;
>>>> +unsigned long long hypervisor_period;
>>>> +  

Re: [libvirt] [PATCH 5/6] qemu: Implement hypervisor's period and quota tunable XML configuration and parsing

2012-05-16 Thread Wen Congyang
At 05/17/2012 11:13 AM, Hu Tao Wrote:
> On Wed, Apr 25, 2012 at 05:46:06PM +0800, Wen Congyang wrote:
>> set hypervisor's period and quota when the vm starts. It will affect to set
>> vm's period and quota: donot set vm's period and quota if we limit hypevisor
>> thread's bandwidth(hypervisor_quota > 0).
>> ---
>>  src/conf/domain_conf.c |   25 ++-
>>  src/conf/domain_conf.h |2 +
>>  src/qemu/qemu_cgroup.c |   37 ---
>>  src/qemu/qemu_driver.c |   75 
>> ++--
>>  4 files changed, 98 insertions(+), 41 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 5ab052a..28a6436 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -7931,6 +7931,14 @@ static virDomainDefPtr 
>> virDomainDefParseXML(virCapsPtr caps,
>>   &def->cputune.quota) < 0)
>>  def->cputune.quota = 0;
>>  
>> +if (virXPathULongLong("string(./cputune/hypervisor_period[1])", ctxt,
>> +  &def->cputune.hypervisor_period) < 0)
>> +def->cputune.hypervisor_period = 0;
>> +
>> +if (virXPathLongLong("string(./cputune/hypervisor_quota[1])", ctxt,
>> + &def->cputune.hypervisor_quota) < 0)
>> +def->cputune.hypervisor_quota = 0;
>> +
>>  if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) {
>>  goto error;
>>  }
>> @@ -12406,7 +12414,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>  virBufferAsprintf(buf, ">%u\n", def->maxvcpus);
>>  
>>  if (def->cputune.shares || def->cputune.vcpupin ||
>> -def->cputune.period || def->cputune.quota)
>> +def->cputune.period || def->cputune.quota ||
>> +def->cputune.hypervisor_period || def->cputune.hypervisor_quota)
>>  virBufferAddLit(buf, "  \n");
>>  
>>  if (def->cputune.shares)
>> @@ -12418,6 +12427,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>  if (def->cputune.quota)
>>  virBufferAsprintf(buf, "%lld\n",
>>def->cputune.quota);
>> +
>> +if (def->cputune.hypervisor_period)
>> +virBufferAsprintf(buf, "%llu"
>> +  "\n",
>> +  def->cputune.hypervisor_period);
>> +
>> +if (def->cputune.hypervisor_period)
>> +virBufferAsprintf(buf, "%lld"
>> +  "\n",
>> +  def->cputune.hypervisor_quota);
>> +
>>  if (def->cputune.vcpupin) {
>>  for (i = 0; i < def->cputune.nvcpupin; i++) {
>>  virBufferAsprintf(buf, "> @@ -12439,7 +12459,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>  }
>>  
>>  if (def->cputune.shares || def->cputune.vcpupin ||
>> -def->cputune.period || def->cputune.quota)
>> +def->cputune.period || def->cputune.quota ||
>> +def->cputune.hypervisor_period || def->cputune.hypervisor_quota)
>>  virBufferAddLit(buf, "  \n");
>>  
>>  if (def->numatune.memory.nodemask) {
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 0eed60e..2a37e26 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -1558,6 +1558,8 @@ struct _virDomainDef {
>>  unsigned long shares;
>>  unsigned long long period;
>>  long long quota;
>> +unsigned long long hypervisor_period;
>> +long long hypervisor_quota;
>>  int nvcpupin;
>>  virDomainVcpuPinDefPtr *vcpupin;
>>  } cputune;
>> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
>> index 727c0d4..7c6ef33 100644
>> --- a/src/qemu/qemu_cgroup.c
>> +++ b/src/qemu/qemu_cgroup.c
>> @@ -493,17 +493,23 @@ int qemuSetupCgroupForVcpu(struct qemud_driver 
>> *driver, virDomainObjPtr vm)
>>  goto cleanup;
>>  }
> 
> Check if cgroup CPU is active here:
> 
>if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
>virReportSystemError(EINVAL,
>   _("%s"),
>   "Cgroup CPU is not active.");
&

[libvirt] [PATCH 5/6] qemu: Implement hypervisor's period and quota tunable XML configuration and parsing

2012-04-25 Thread Wen Congyang
set hypervisor's period and quota when the vm starts. It will affect to set
vm's period and quota: donot set vm's period and quota if we limit hypevisor
thread's bandwidth(hypervisor_quota > 0).
---
 src/conf/domain_conf.c |   25 ++-
 src/conf/domain_conf.h |2 +
 src/qemu/qemu_cgroup.c |   37 ---
 src/qemu/qemu_driver.c |   75 ++--
 4 files changed, 98 insertions(+), 41 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5ab052a..28a6436 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7931,6 +7931,14 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
  &def->cputune.quota) < 0)
 def->cputune.quota = 0;
 
+if (virXPathULongLong("string(./cputune/hypervisor_period[1])", ctxt,
+  &def->cputune.hypervisor_period) < 0)
+def->cputune.hypervisor_period = 0;
+
+if (virXPathLongLong("string(./cputune/hypervisor_quota[1])", ctxt,
+ &def->cputune.hypervisor_quota) < 0)
+def->cputune.hypervisor_quota = 0;
+
 if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) {
 goto error;
 }
@@ -12406,7 +12414,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 virBufferAsprintf(buf, ">%u\n", def->maxvcpus);
 
 if (def->cputune.shares || def->cputune.vcpupin ||
-def->cputune.period || def->cputune.quota)
+def->cputune.period || def->cputune.quota ||
+def->cputune.hypervisor_period || def->cputune.hypervisor_quota)
 virBufferAddLit(buf, "  \n");
 
 if (def->cputune.shares)
@@ -12418,6 +12427,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 if (def->cputune.quota)
 virBufferAsprintf(buf, "%lld\n",
   def->cputune.quota);
+
+if (def->cputune.hypervisor_period)
+virBufferAsprintf(buf, "%llu"
+  "\n",
+  def->cputune.hypervisor_period);
+
+if (def->cputune.hypervisor_period)
+virBufferAsprintf(buf, "%lld"
+  "\n",
+  def->cputune.hypervisor_quota);
+
 if (def->cputune.vcpupin) {
 for (i = 0; i < def->cputune.nvcpupin; i++) {
 virBufferAsprintf(buf, "cputune.shares || def->cputune.vcpupin ||
-def->cputune.period || def->cputune.quota)
+def->cputune.period || def->cputune.quota ||
+def->cputune.hypervisor_period || def->cputune.hypervisor_quota)
 virBufferAddLit(buf, "  \n");
 
 if (def->numatune.memory.nodemask) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0eed60e..2a37e26 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1558,6 +1558,8 @@ struct _virDomainDef {
 unsigned long shares;
 unsigned long long period;
 long long quota;
+unsigned long long hypervisor_period;
+long long hypervisor_quota;
 int nvcpupin;
 virDomainVcpuPinDefPtr *vcpupin;
 } cputune;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 727c0d4..7c6ef33 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -493,17 +493,23 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, 
virDomainObjPtr vm)
 goto cleanup;
 }
 
-/* Set cpu bandwidth for the vm */
-if (period || quota) {
-if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
-/* Ensure that we can multiply by vcpus without overflowing. */
-if (quota > LLONG_MAX / vm->def->vcpus) {
-virReportSystemError(EINVAL,
- _("%s"),
- "Unable to set cpu bandwidth quota");
-goto cleanup;
-}
+/*
+ * Set cpu bandwidth for the vm if any of the following is true:
+ * 1. we donot know VCPU<->PID mapping or all vcpus run in the same thread
+ * 2. the hypervisor threads are not limited(quota <= 0)
+ */
+if ((period || quota) &&
+qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
+/* Ensure that we can multiply by vcpus without overflowing. */
+if (quota > LLONG_MAX / vm->def->vcpus) {
+virReportSystemError(EINVAL,
+ _("%s"),
+ "Unable to set cpu bandwidth quota");
+goto cleanup;
+}
 
+if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid ||
+vm->def->cputune.hypervisor_quota <= 0) {
 if (quota > 0)
 vm_quota = quota * vm->def->vcpus;
 else
@@ -514,7 +520,7 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, 
virDomainObjPtr vm)
 }
 
 if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) {
-/* If we does not know VCPU<->PID map

[libvirt] [PATCH 1/6] Introduce the function virCgroupForHypervisor

2012-04-25 Thread Wen Congyang
Introduce the function virCgroupForHypervisor() to create sub directory
for hypervisor thread(include I/O thread, vhost-net thread)
---
 src/libvirt_private.syms |1 +
 src/util/cgroup.c|   42 ++
 src/util/cgroup.h|4 
 3 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8d29e75..a11b29c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -66,6 +66,7 @@ virCgroupDenyAllDevices;
 virCgroupDenyDevicePath;
 virCgroupForDomain;
 virCgroupForDriver;
+virCgroupForHypervisor;
 virCgroupForVcpu;
 virCgroupFree;
 virCgroupGetBlkioWeight;
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index ad49bc2..901a841 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -944,6 +944,48 @@ int virCgroupForVcpu(virCgroupPtr driver ATTRIBUTE_UNUSED,
 #endif
 
 /**
+ * virCgroupForHypervisor:
+ *
+ * @driver: group for the domain
+ * @group: Pointer to returned virCgroupPtr
+ *
+ * Returns 0 on success
+ */
+#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
+int virCgroupForHypervisor(virCgroupPtr driver,
+   virCgroupPtr *group,
+   int create)
+{
+int rc;
+char *path;
+
+if (driver == NULL)
+return -EINVAL;
+
+if (virAsprintf(&path, "%s/hypervisor", driver->path) < 0)
+return -ENOMEM;
+
+rc = virCgroupNew(path, group);
+VIR_FREE(path);
+
+if (rc == 0) {
+rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_VCPU);
+if (rc != 0)
+virCgroupFree(group);
+}
+
+return rc;
+}
+#else
+int virCgroupForHypervisor(virCgroupPtr driver ATTRIBUTE_UNUSED,
+   virCgroupPtr *group ATTRIBUTE_UNUSED,
+   int create ATTRIBUTE_UNUSED)
+{
+return -ENXIO;
+}
+
+#endif
+/**
  * virCgroupSetBlkioWeight:
  *
  * @group: The cgroup to change io weight for
diff --git a/src/util/cgroup.h b/src/util/cgroup.h
index 8486c42..34fcbf5 100644
--- a/src/util/cgroup.h
+++ b/src/util/cgroup.h
@@ -45,6 +45,10 @@ int virCgroupForVcpu(virCgroupPtr driver,
  virCgroupPtr *group,
  int create);
 
+int virCgroupForHypervisor(virCgroupPtr driver,
+   virCgroupPtr *group,
+   int create);
+
 int virCgroupPathOfController(virCgroupPtr group,
   int controller,
   const char *key,
-- 
1.7.1

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


[libvirt] [PATCH 3/6] create a new cgroup and move all hypervisor threads to the new cgroup

2012-04-25 Thread Wen Congyang
create a new cgroup and move all hypervisor threads to the new cgroup.
And then we can do the other things:
1. limit only vcpu usage rather than the whole qemu
2. limit for hypervisor threads(include vhost-net threads)
---
 src/qemu/qemu_cgroup.c  |   57 +++
 src/qemu/qemu_cgroup.h  |2 +
 src/qemu/qemu_process.c |4 +++
 3 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index a07b6cd..727c0d4 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -564,6 +564,63 @@ cleanup:
 return -1;
 }
 
+int qemuSetupCgroupForHypervisor(struct qemud_driver *driver,
+ virDomainObjPtr vm)
+{
+virCgroupPtr cgroup = NULL;
+virCgroupPtr cgroup_hypervisor = NULL;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int rc;
+
+if (driver->cgroup == NULL)
+return 0; /* Not supported, so claim success */
+
+rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _("Unable to find cgroup for %s"),
+ vm->def->name);
+goto cleanup;
+}
+
+if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) {
+/* If we does not know VCPU<->PID mapping or all vcpu runs in the same
+ * thread, we cannot control each vcpu.
+ */
+virCgroupFree(&cgroup);
+return 0;
+}
+
+rc = virCgroupForHypervisor(cgroup, &cgroup_hypervisor, 1);
+if (rc < 0) {
+virReportSystemError(-rc,
+ _("Unable to create hypervisor cgroup for %s"),
+ vm->def->name);
+goto cleanup;
+}
+
+rc = virCgroupMoveTask(cgroup, cgroup_hypervisor);
+if (rc < 0) {
+virReportSystemError(-rc,
+ _("Unable to move taks from domain cgroup to "
+   "hypervisor cgroup for %s"),
+ vm->def->name);
+goto cleanup;
+}
+
+virCgroupFree(&cgroup_hypervisor);
+virCgroupFree(&cgroup);
+return 0;
+
+cleanup:
+virCgroupFree(&cgroup_hypervisor);
+if (cgroup) {
+virCgroupRemove(cgroup);
+virCgroupFree(&cgroup);
+}
+
+return -1;
+}
 
 int qemuRemoveCgroup(struct qemud_driver *driver,
  virDomainObjPtr vm,
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 17164d9..92eff68 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -53,6 +53,8 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
   unsigned long long period,
   long long quota);
 int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm);
+int qemuSetupCgroupForHypervisor(struct qemud_driver *driver,
+ virDomainObjPtr vm);
 int qemuRemoveCgroup(struct qemud_driver *driver,
  virDomainObjPtr vm,
  int quiet);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 566a17e..b605eee 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3668,6 +3668,10 @@ int qemuProcessStart(virConnectPtr conn,
 if (qemuSetupCgroupForVcpu(driver, vm) < 0)
 goto cleanup;
 
+VIR_DEBUG("Setting cgroup for hypervisor(if required)");
+if (qemuSetupCgroupForHypervisor(driver, vm) < 0)
+goto cleanup;
+
 VIR_DEBUG("Setting VCPU affinities");
 if (qemuProcessSetVcpuAffinites(conn, vm) < 0)
 goto cleanup;
-- 
1.7.1

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


[libvirt] [PATCH 6/6] qemu: Implement hypervisor_period and hypervisor_quota's modification

2012-04-25 Thread Wen Congyang
allow the user change/get hypervisor's period and quota when the vm is running.
If the hypervisor's bandwidth is changed to unlimited, we should limit the vm's
bandwidth again. If we limit the hypervisor's bandwidth, there is no need to
limit the vm's bandwidth.

---
 include/libvirt/libvirt.h.in |   16 +
 src/qemu/qemu_driver.c   |  147 +-
 2 files changed, 162 insertions(+), 1 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 97ad99d..e015499 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -662,6 +662,22 @@ typedef virTypedParameter *virTypedParameterPtr;
 #define VIR_DOMAIN_SCHEDULER_VCPU_QUOTA "vcpu_quota"
 
 /**
+ * VIR_DOMAIN_SCHEDULER_HYPERVISOR_PERIOD:
+ *
+ * Macro represents the enforcement period for a quota, in microseconds,
+ * when using the posix scheduler, as a ullong.
+ */
+#define VIR_DOMAIN_SCHEDULER_HYPERVISOR_PERIOD "hypervisor_period"
+
+/**
+ * VIR_DOMAIN_SCHEDULER_HYPERVISOR_QUOTA:
+ *
+ * Macro represents the maximum bandwidth to be used within a period,
+ * when using the posix scheduler, as an llong.
+ */
+#define VIR_DOMAIN_SCHEDULER_HYPERVISOR_QUOTA "hypervisor_quota"
+
+/**
  * VIR_DOMAIN_SCHEDULER_WEIGHT:
  *
  * Macro represents the relative weight,  when using the credit
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2e40aee..165d1c0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5973,7 +5973,7 @@ static char *qemuGetSchedulerType(virDomainPtr dom,
 else if (rc == 0)
 *nparams = 1;
 else
-*nparams = 3;
+*nparams = 5;
 }
 
 ret = strdup("posix");
@@ -7124,6 +7124,54 @@ cleanup:
 }
 
 static int
+qemuSetHypervisorBWLive(virDomainObjPtr vm, virCgroupPtr cgroup,
+unsigned long long period, long long quota)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virCgroupPtr cgroup_hypervisor = NULL;
+int rc;
+
+if (period == 0 && quota == 0)
+return 0;
+
+if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) {
+return 0;
+}
+
+rc = virCgroupForHypervisor(cgroup, &cgroup_hypervisor, 0);
+if (rc < 0) {
+virReportSystemError(-rc,
+ _("Unable to find hypervisor cgroup for %s"),
+ vm->def->name);
+goto cleanup;
+}
+
+/* we have limited hypervisor thread, so unlimit vm cgoup */
+if (quota > 0 && qemuSetupCgroupVcpuBW(cgroup, 0, -1) < 0)
+goto cleanup;
+
+if (qemuSetupCgroupVcpuBW(cgroup_hypervisor, period, quota) < 0)
+goto cleanup;
+
+/*
+ * we have unlimited hypervisor thread, so limit vm cgoup again.
+ *
+ * We have ensured that we can multiply by vcpus without overflowing when
+ * setting vcpu's quota. So we donot check it here.
+ */
+if (quota < 0 &&
+qemuSetupCgroupVcpuBW(cgroup, vm->def->cputune.period,
+  vm->def->cputune.quota * vm->def->vcpus) < 0)
+goto cleanup;
+
+return 0;
+
+cleanup:
+virCgroupFree(&cgroup_hypervisor);
+return -1;
+}
+
+static int
 qemuSetSchedulerParametersFlags(virDomainPtr dom,
 virTypedParameterPtr params,
 int nparams,
@@ -7146,6 +7194,10 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom,
VIR_TYPED_PARAM_ULLONG,
VIR_DOMAIN_SCHEDULER_VCPU_QUOTA,
VIR_TYPED_PARAM_LLONG,
+   VIR_DOMAIN_SCHEDULER_HYPERVISOR_PERIOD,
+   VIR_TYPED_PARAM_ULLONG,
+   VIR_DOMAIN_SCHEDULER_HYPERVISOR_QUOTA,
+   VIR_TYPED_PARAM_LLONG,
NULL) < 0)
 return -1;
 
@@ -7228,6 +7280,32 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom,
 if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
 vmdef->cputune.quota = params[i].value.l;
 }
+} else if (STREQ(param->field, 
VIR_DOMAIN_SCHEDULER_HYPERVISOR_PERIOD)) {
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+rc = qemuSetHypervisorBWLive(vm, group, params[i].value.ul, 0);
+if (rc != 0)
+goto cleanup;
+
+if (params[i].value.ul)
+vm->def->cputune.hypervisor_period = params[i].value.ul;
+}
+
+if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+vmdef->cputune.hypervisor_period = params[i].value.ul;
+}
+} else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_HYPERVISOR_QUOTA)) 
{
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+rc = qemuSetHypervisorBWLive(vm, group, 0, params[i].value.l);
+   

[libvirt] [PATCH 2/6] introduce the function virCgroupMoveTask()

2012-04-25 Thread Wen Congyang
introduce a new API to move all tasks from a cgroup to another cgroup

---
 src/util/cgroup.c |   55 +
 src/util/cgroup.h |2 +
 2 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index 901a841..724cc6e 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -789,6 +789,61 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid)
 return rc;
 }
 
+static int virCgrouAddTaskStr(virCgroupPtr group, const char *pidstr)
+{
+unsigned long long value;
+
+if (virStrToLong_ull(pidstr, NULL, 10, &value) < 0)
+return -EINVAL;
+
+return virCgroupAddTask(group, value);
+}
+
+/**
+ * virCgroupMoveTask:
+ *
+ * @src_group: The source cgroup where all tasks are removed from
+ * @dest_group: The destination where all tasks are added to
+ *
+ * Returns: 0 on success
+ */
+int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group)
+{
+int rc = 0;
+int i;
+char *content, *value, *next;
+
+for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) {
+/* Skip over controllers not mounted */
+if (!src_group->controllers[i].mountPoint ||
+!dest_group->controllers[i].mountPoint)
+continue;
+
+rc = virCgroupGetValueStr(src_group, i, "tasks", &content);
+if (rc != 0)
+break;
+
+value = content;
+while((next = strchr(value, '\n')) != NULL) {
+*next = '\0';
+if ((rc = virCgrouAddTaskStr(dest_group, value) < 0))
+goto cleanup;
+value = next + 1;
+}
+if (*value != '\0') {
+if ((rc = virCgrouAddTaskStr(dest_group, value) < 0))
+goto cleanup;
+}
+
+VIR_FREE(content);
+}
+
+return 0;
+
+cleanup:
+virCgroupMoveTask(dest_group, src_group);
+return rc;
+}
 
 /**
  * virCgroupForDriver:
diff --git a/src/util/cgroup.h b/src/util/cgroup.h
index 34fcbf5..2419ef6 100644
--- a/src/util/cgroup.h
+++ b/src/util/cgroup.h
@@ -56,6 +56,8 @@ int virCgroupPathOfController(virCgroupPtr group,
 
 int virCgroupAddTask(virCgroupPtr group, pid_t pid);
 
+int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group);
+
 int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight);
 int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight);
 
-- 
1.7.1

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


[libvirt] [PATCH 4/6] Update XML Schema for new entries

2012-04-25 Thread Wen Congyang
---
 docs/schemas/domaincommon.rng |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f116710..8165c5b 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -544,6 +544,16 @@
   
 
   
+  
+
+  
+
+  
+  
+
+  
+
+  
   
 
   
-- 
1.7.1

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


[libvirt] [PATCH 0/6] support to set cpu bandwidth for hypervisor threads

2012-04-25 Thread Wen Congyang
Currently, we only can set cpu bandwidth for vcpu. If the hypervisor threads
consume too much cpu time, it may affect the vcpu.

This patchset allows the user to control the cpu bandwidth for hypervisor
threads. It does not change the behavior if the cpu bandwidth for hypervisor
is unlimited.

Wen Congyang (6):
  Introduce the function virCgroupForHypervisor
  introduce the function virCgroupMoveTask()
  create a new cgroup and move all hypervisor threads to the new cgroup
  Update XML Schema for new entries
  qemu: Implement hypervisor's period and quota tunable XML
configuration and parsing
  qemu: Implement hypervisor_period and hypervisor_quota's modification

 docs/schemas/domaincommon.rng |   10 ++
 include/libvirt/libvirt.h.in  |   16 +++
 src/conf/domain_conf.c|   25 +-
 src/conf/domain_conf.h|2 +
 src/libvirt_private.syms  |1 +
 src/qemu/qemu_cgroup.c|   94 +++--
 src/qemu/qemu_cgroup.h|2 +
 src/qemu/qemu_driver.c|  220 +++-
 src/qemu/qemu_process.c   |4 +
 src/util/cgroup.c |   97 ++
 src/util/cgroup.h |6 +
 11 files changed, 436 insertions(+), 41 deletions(-)

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


Re: [libvirt] [PATCH] building: remove libvirt_dbus.syms from EXTRA_DIST

2012-04-22 Thread Wen Congyang

On 04/20/2012 11:10 AM, Osier Yang wrote:

On 2012年04月20日 11:05, Wen Congyang wrote:

commit 2223ea98 removes src/libvirt_dbus.syms, but it forgets
to remove it from EXTRA_DIST. It will cause 'make dist' failed.

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

diff --git a/src/Makefile.am b/src/Makefile.am
index b8a19b4..e48dfa5 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1230,7 +1230,6 @@ EXTRA_DIST += \
libvirt_driver_modules.syms \
libvirt_daemon.syms \
libvirt_linux.syms \
- libvirt_dbus.syms \
libvirt_network.syms \
libvirt_nwfilter.syms \
libvirt_sasl.syms \


ACK


Thanks, pushed.

Wen Congyang



Osier

--
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] building error

2012-04-20 Thread Wen Congyang
At 04/20/2012 04:50 PM, Daniel Veillard Wrote:
> On Fri, Apr 20, 2012 at 02:26:31PM +0800, Wen Congyang wrote:
>> When I build libvirt, I meet the following error message
>> sometimes:
>>
>> make[4]: Entering directory `/home/wency/rpmbuild/BUILD/libvirt-0.9.11/docs'
>>   GENlibvirt-api.xml
>>   GENlibvirt-qemu-api.xml
>>   GENhtml/index.html
>> ./libvirt-api.xml:2450: parser error : AttValue: ' expected
>>  program.
> 
> [...]
>> unable to parse ./libvirt-api.xml
>> make[4]: *** [html/index.html] Error 6
>> make[4]: *** Waiting for unfinished jobs
>>
>> If I rebuild it without anything change, the building will success.
> 
>   It may be a problem of running make with a default parallel seting
> and a bug in docs/Makefile.am where ./libvirt-api.xml would not be
> properly serialized with the building of html/index.html.
>   The weird thing is that from your output they seems to be serialized
> but I would definitely investigate in that direction.
> 
>   I just checked from git head here and that seems to work fine 

The following is my build script:

=
./autogen.sh
if [[ $? -ne 0 ]]; then
exit 1
fi

perl -pi -e 's/ *-O2//' configure #remove -O2 from configure
if [[ $? -ne 0 ]]; then
exit 1
fi

./configure --enable-compile-warnings=error
if [[ $? -ne 0 ]]; then
exit 1
fi

make syntax-check
if [[ $? -ne 0 ]]; then
exit 1
fi

perl -pi -e 's/ --enable-compile-warnings=error//' libvirt.spec
perl -pi -e 's/%configure/%configure --enable-compile-warnings=error/' 
libvirt.spec

make dist
if [[ $? -ne 0 ]]; then
exit 1
fi

rpmbuild --define "_sourcedir `pwd`"  --define "_without-numad 1" -ba 
libvirt.spec
=

Thanks
Wen Congyang
> 
> Daniel
> 

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


[libvirt] [PATCH 3/3 v4] virsh: allow the user to specify vmcore's format

2012-04-20 Thread Wen Congyang
Add a new parameter --memory-only for 'virsh dump' command. So
the user can decide the vmcore's format.

---
 tools/virsh.c   |3 +++
 tools/virsh.pod |5 -
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 6168a13..c1e8891 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -3982,6 +3982,7 @@ static const vshCmdOptDef opts_dump[] = {
 {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
 {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("where to dump the core")},
 {"verbose", VSH_OT_BOOL, 0, N_("display the progress of dump")},
+{"memory-only", VSH_OT_BOOL, 0, N_("dump domain's memory only")},
 {NULL, 0, 0, NULL}
 };
 
@@ -4020,6 +4021,8 @@ doDump(void *opaque)
 flags |= VIR_DUMP_BYPASS_CACHE;
 if (vshCommandOptBool(cmd, "reset"))
 flags |= VIR_DUMP_RESET;
+if (vshCommandOptBool(cmd, "memory-only"))
+flags |= VIR_DUMP_MEMORY_ONLY;
 
 if (virDomainCoreDump(dom, to, flags) < 0) {
 vshError(ctl, _("Failed to core dump domain %s to %s"), name, to);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index ca5c853..9378714 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -784,7 +784,7 @@ the I argument must be B. For Xen 
hypervisor, the
 I argument may be B or B.
 
 =item B I I [I<--bypass-cache>]
-{ [I<--live>] | [I<--crash>] | [I<--reset>] } [I<--verbose>]
+{ [I<--live>] | [I<--crash>] | [I<--reset>] } [I<--verbose>] [I<--memory-only>]
 
 Dumps the core of a domain to a file for analysis.
 If I<--live> is specified, the domain continues to run until the core
@@ -795,6 +795,9 @@ If I<--reset> is specified, the domain is reset after 
successful dump.
 Note, these three switches are mutually exclusive.
 If I<--bypass-cache> is specified, the save will avoid the file system
 cache, although this may slow down the operation.
+If I<--memory-only> is specified, the file is elf file, and will only
+include domain's memory and cpu common register value. It is very
+useful if the domain uses host devices directly.
 
 The progress may be monitored using B virsh command and canceled
 with B command (sent by another virsh instance). Another option
-- 
1.7.1

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


[libvirt] [PATCH 1/3] qemu: implement qemu's dump-guest-memory

2012-04-20 Thread Wen Congyang
dump-guest-memory is a new dump mechanism, and it can work when the
guest uses host devices. This patch adds a API to use this new
monitor command.

---
 src/qemu/qemu_monitor.c  |   38 ++
 src/qemu/qemu_monitor.h  |   12 
 src/qemu/qemu_monitor_json.c |   34 ++
 src/qemu/qemu_monitor_json.h |6 ++
 4 files changed, 90 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2f66c46..a5d3eec 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2018,6 +2018,44 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+int qemuMonitorDumpToFd(qemuMonitorPtr mon,
+unsigned int flags,
+int fd,
+unsigned long long begin,
+unsigned long long length)
+{
+int ret;
+VIR_DEBUG("mon=%p fd=%d flags=%x begin=%llx length=%llx",
+  mon, fd, flags, begin, length);
+
+if (!mon) {
+qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+_("monitor must not be NULL"));
+return -1;
+}
+
+if (!mon->json) {
+/* dump-guest-memory is supported after qemu-1.0, and we always use 
json
+ * if qemu's version is >= 0.15. So if we use text mode, the qemu is
+ * old, and it does not support dump-guest-memory.
+ */
+qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+_("dump-guest-memory is not supported in text mode"));
+return -1;
+}
+
+if (qemuMonitorSendFileHandle(mon, "dump", fd) < 0)
+return -1;
+
+ret = qemuMonitorJSONDump(mon, flags, "fd:dump", begin, length);
+
+if (ret < 0) {
+if (qemuMonitorCloseFileHandle(mon, "dump") < 0)
+VIR_WARN("failed to close dumping handle");
+}
+
+return ret;
+}
 
 int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int type,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index f3cdcdd..7df52ad 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -379,6 +379,18 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
 
 int qemuMonitorMigrateCancel(qemuMonitorPtr mon);
 
+typedef enum {
+  QEMU_MONITOR_DUMP_HAVE_FILTER  = 1 << 0,
+  QEMU_MONITOR_DUMP_PAGING   = 1 << 1,
+  QEMU_MONITOR_DUMP_FLAGS_LAST
+} QEMU_MONITOR_DUMP;
+
+int qemuMonitorDumpToFd(qemuMonitorPtr mon,
+unsigned int flags,
+int fd,
+unsigned long long begin,
+unsigned long long length);
+
 int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int type,
 const char *hostname,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index eb58f13..b229a31 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2461,6 +2461,40 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+int qemuMonitorJSONDump(qemuMonitorPtr mon,
+unsigned int flags,
+const char *protocol,
+unsigned long long begin,
+unsigned long long length)
+{
+int ret;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+
+if (flags & QEMU_MONITOR_DUMP_HAVE_FILTER)
+cmd = qemuMonitorJSONMakeCommand("dump-guest-memory",
+ "b:paging", flags & 
QEMU_MONITOR_DUMP_PAGING ? 1 : 0,
+ "s:protocol", protocol,
+ "U:begin", begin,
+ "U:length", length,
+ NULL);
+else
+cmd = qemuMonitorJSONMakeCommand("dump-guest-memory",
+ "b:paging", flags & 
QEMU_MONITOR_DUMP_PAGING ? 1 : 0,
+ "s:protocol", protocol,
+ NULL);
+if (!cmd)
+return -1;
+
+ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+if (ret == 0)
+ret = qemuMonitorJSONCheckError(cmd, reply);
+
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
 
 int qemuMonitorJSONGraphicsRelocate(qemuMonitorPtr mon,
 int type,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index aacbb5f..ccb2624 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -136,6 +136,12 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon,
 
 int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon);
 
+int qemuMonitorJSONDump(qemuMonitorPtr mon,
+unsigned int flags,
+const char *protocol,
+unsign

[libvirt] [PATCH 2/3 v4] qemu: allow the client to choose the vmcore's format

2012-04-20 Thread Wen Congyang
This patch updates qemu driver to allow the client to choose the
vmcore's format: memory only or including device state.

---
 include/libvirt/libvirt.h.in |1 +
 src/qemu/qemu_domain.c   |1 +
 src/qemu/qemu_domain.h   |1 +
 src/qemu/qemu_driver.c   |   54 +++--
 4 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 97ad99d..5f03043 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -927,6 +927,7 @@ typedef enum {
 VIR_DUMP_LIVE = (1 << 1), /* live dump */
 VIR_DUMP_BYPASS_CACHE = (1 << 2), /* avoid file system cache pollution */
 VIR_DUMP_RESET= (1 << 3), /* reset domain after dump finishes */
+VIR_DUMP_MEMORY_ONLY  = (1 << 4), /* use dump-guest-memory */
 } virDomainCoreDumpFlags;
 
 /* Domain migration flags. */
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4dda2e0..e81f439 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -160,6 +160,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
 job->phase = 0;
 job->mask = DEFAULT_JOB_MASK;
 job->start = 0;
+job->dump_memory_only = false;
 memset(&job->info, 0, sizeof(job->info));
 }
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index ce52569..098349b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -104,6 +104,7 @@ struct qemuDomainJobObj {
 int phase;  /* Job phase (mainly for migrations) */
 unsigned long long mask;/* Jobs allowed during async job */
 unsigned long long start;   /* When the async job started */
+bool dump_memory_only;  /* use dump-guest-memory to do dump */
 virDomainJobInfo info;  /* Async job progress data */
 };
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c3555ca..779304f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2967,12 +2967,33 @@ cleanup:
 return ret;
 }
 
+static int qemuDumpToFd(struct qemud_driver *driver, virDomainObjPtr vm,
+int fd, enum qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret = -1;
+
+if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
+  fd) < 0)
+return -1;
+
+priv->job.dump_memory_only = true;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+ret = qemuMonitorDumpToFd(priv->mon, 0, fd, 0, 0);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+return ret;
+}
+
 static int
 doCoreDump(struct qemud_driver *driver,
virDomainObjPtr vm,
const char *path,
enum qemud_save_formats compress,
-   bool bypass_cache)
+   unsigned int dump_flags)
 {
 int fd = -1;
 int ret = -1;
@@ -2981,7 +3002,7 @@ doCoreDump(struct qemud_driver *driver,
 unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING;
 
 /* Create an empty file with appropriate ownership.  */
-if (bypass_cache) {
+if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
 flags |= VIR_FILE_WRAPPER_BYPASS_CACHE;
 directFlag = virFileDirectFdFlag();
 if (directFlag < 0) {
@@ -3001,14 +3022,20 @@ doCoreDump(struct qemud_driver *driver,
 if (!(wrapperFd = virFileWrapperFdNew(&fd, path, flags)))
 goto cleanup;
 
-if (qemuMigrationToFile(driver, vm, fd, 0, path,
-qemuCompressProgramName(compress), false,
-QEMU_ASYNC_JOB_DUMP) < 0)
+if (dump_flags & VIR_DUMP_MEMORY_ONLY) {
+ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP);
+} else {
+ret = qemuMigrationToFile(driver, vm, fd, 0, path,
+  qemuCompressProgramName(compress), false,
+  QEMU_ASYNC_JOB_DUMP);
+}
+
+if (ret < 0)
 goto cleanup;
 
 if (VIR_CLOSE(fd) < 0) {
 virReportSystemError(errno,
- _("unable to save file %s"),
+ _("unable to close file %s"),
  path);
 goto cleanup;
 }
@@ -3066,7 +3093,8 @@ static int qemudDomainCoreDump(virDomainPtr dom,
 virDomainEventPtr event = NULL;
 
 virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH |
-  VIR_DUMP_BYPASS_CACHE | VIR_DUMP_RESET, -1);
+  VIR_DUMP_BYPASS_CACHE | VIR_DUMP_RESET |
+  VIR_DUMP_MEMORY_ONLY, -1);
 
 qemuDriverLock(driver);
 vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -3108,8 +3136,7 @@ static int qemudDomainCoreDump(virDomainPtr dom,
 }
 }
 
-ret = doCoreDump(driver, vm, path, getCompressionType(driver),
- (flags & VIR_DUMP_BYPASS_CACHE) != 0);
+ret = doCore

[libvirt] [PATCH 0/3 v4] use qemu's dump-guest-meory when vm uses host device

2012-04-20 Thread Wen Congyang
Currently, we use migrate to dump guest's memory. There is one
restriction in migrate command: the device's status should be
stored in qemu because the device's status should be passed to
target machine.

If we passthrough a host device to guest, the device's status
is stored in the real device. So migrate command will fail.

We usually use dump when guest is panicked. So there is no need
to store device's status in the vmcore.

qemu will have a new monitor command dump-guest-memory to dump
guest memory, but it doesn't support async now(it will support
later when the common async API is implemented).

So I use dump-guest-memory only when the guest uses host device
in this patchset.

Note: the patchset for qemu is still queued. Luiz has acked,
but he said he does not wait an ACK from Jan and/or Anthony.

Changes from v3 to v4:
1. allow the user to specify the core file's format

Changes from v2 to v3:
1. qemu supports the fd that is associated with a pipe, socket, or FIFO.
   So pass a pipe fd to qemu and O_DIRECT can work now.

Change from v1 to v2:
1. remove the implemention for text mode.

Wen Congyang (3):
  qemu: implement qemu's dump-guest-memory
  qemu: allow the client to choose the vmcore's format
  virsh: allow the user to specify vmcore's format

 include/libvirt/libvirt.h.in |1 +
 src/qemu/qemu_domain.c   |1 +
 src/qemu/qemu_domain.h   |1 +
 src/qemu/qemu_driver.c   |   54 +++--
 src/qemu/qemu_monitor.c  |   38 +
 src/qemu/qemu_monitor.h  |   12 +
 src/qemu/qemu_monitor_json.c |   35 +++
 src/qemu/qemu_monitor_json.h |6 
 tools/virsh.c|3 ++
 tools/virsh.pod  |5 +++-
 10 files changed, 142 insertions(+), 14 deletions(-)

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


[libvirt] building error

2012-04-19 Thread Wen Congyang
When I build libvirt, I meet the following error message
sometimes:

make[4]: Entering directory `/home/wency/rpmbuild/BUILD/libvirt-0.9.11/docs'
  GENlibvirt-api.xml
  GENlibvirt-qemu-api.xml
  GENhtml/index.html
./libvirt-api.xml:2450: parser error : AttValue: ' expected

Re: [libvirt] [PATCHv2] virnetserver: handle sigaction correctly

2012-04-19 Thread Wen Congyang
At 04/20/2012 11:45 AM, Eric Blake Wrote:
> POSIX says that sa_sigaction is only safe to use if sa_flags
> includes SA_SIGINFO; conversely, sa_handler is only safe to
> use when flags excludes that bit.  Gnulib doesn't guarantee
> an implementation of SA_SIGINFO, but does guarantee that
> if SA_SIGINFO is undefined, we can safely define it to 0 as
> long as we don't dereference the 2nd or 3rd argument of
> any handler otherwise registered via sa_sigaction.
> 
> Based on a report by Wen Congyang.
> 
> * src/rpc/virnetserver.c (SA_SIGINFO): Stub for mingw.
> (virNetServerSignalHandler): Avoid bogus dereference.
> (virNetServerFatalSignal, virNetServerNew): Set flags properly.
> ---
> 
> v2: Simplify according to what gnulib gives us for mingw.
> 
>  src/rpc/virnetserver.c |   28 
>  1 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index 3965fc2..3f3989e 100644
> --- a/src/rpc/virnetserver.c
> +++ b/src/rpc/virnetserver.c
> @@ -1,7 +1,7 @@
>  /*
>   * virnetserver.c: generic network RPC server
>   *
> - * Copyright (C) 2006-2011 Red Hat, Inc.
> + * Copyright (C) 2006-2012 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -40,6 +40,10 @@
>  # include "virnetservermdns.h"
>  #endif
> 
> +#ifndef SA_SIGINFO
> +# define SA_SIGINFO 0
> +#endif
> +
>  #define VIR_FROM_THIS VIR_FROM_RPC
>  #define virNetError(code, ...)\
>  virReportErrorHelper(VIR_FROM_THIS, code, __FILE__,   \
> @@ -270,8 +274,9 @@ error:
>  }
> 
> 
> -static void virNetServerFatalSignal(int sig, siginfo_t *siginfo 
> ATTRIBUTE_UNUSED,
> -void *context ATTRIBUTE_UNUSED)
> +static void
> +virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED,
> +void *context ATTRIBUTE_UNUSED)
>  {
>  struct sigaction sig_action;
>  int origerrno;
> @@ -286,6 +291,7 @@ static void virNetServerFatalSignal(int sig, siginfo_t 
> *siginfo ATTRIBUTE_UNUSED
>  #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);
> @@ -363,6 +369,7 @@ virNetServerPtr virNetServerNew(size_t min_workers,
>   * 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);
> @@ -420,17 +427,24 @@ static sig_atomic_t sigErrors = 0;
>  static int sigLastErrno = 0;
>  static int sigWrite = -1;
> 
> -static void virNetServerSignalHandler(int sig, siginfo_t * siginfo,
> -  void* context ATTRIBUTE_UNUSED)
> +static void
> +virNetServerSignalHandler(int sig, siginfo_t * siginfo ATTRIBUTE_UNUSED,

Why siginfo has the attribute ATTRIBUTE_UNUSED?

> +  void* context ATTRIBUTE_UNUSED)
>  {
>  int origerrno;
>  int r;
> +siginfo_t tmp;
> +
> +if (SA_SIGINFO)
> +tmp = *siginfo;
> +else
> +memset(&tmp, 0, sizeof(tmp));
> 
>  /* set the sig num in the struct */
> -siginfo->si_signo = sig;
> +tmp->si_signo = sig;

s/->/./

> 
>  origerrno = errno;
> -r = safewrite(sigWrite, siginfo, sizeof(*siginfo));
> +r = safewrite(sigWrite, &tmp, sizeof(tmp));
>  if (r == -1) {
>  sigErrors++;
>  sigLastErrno = errno;
> @@ -533,9 +547,7 @@ int virNetServerAddSignalHandler(virNetServerPtr srv,
> 
>  memset(&sig_action, 0, sizeof(sig_action));
>  sig_action.sa_sigaction = virNetServerSignalHandler;
> -#ifdef SA_SIGINFO
>  sig_action.sa_flags = SA_SIGINFO;
> -#endif
>  sigemptyset(&sig_action.sa_mask);
> 
>  sigaction(signum, &sig_action, &sigdata->oldaction);

ACK with nit fixed.

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


Re: [libvirt] [PATCH] use sa_handler instead of sa_sigaction when SA_SIGINFO is not defined

2012-04-19 Thread Wen Congyang
At 04/20/2012 11:26 AM, Eric Blake Wrote:
> On 04/19/2012 09:10 PM, Wen Congyang wrote:
>> POSIX requires that we use sa_sigaction if sa_flags includes SA_SIGINFO,
>> and that we use sa_handler otherwise. But we still use sa_sigaction
>> when SA_SIGINFO is not defined. Practice says it will work, but theory
>> says we can't rely on it to work.
> 
> Not quite what I said.  Gnulib guarantees that sa_sigaction will work
> instead of sa_handler, _for the platforms where SA_SIGINFO is not
> defined_, and provided that your handler does not access the 2nd or 3rd
> argument in those situations.  That is, gnulib gives us some guarantees
> that POSIX does not, and that allows us to write simpler code that
> assumes SA_SIGINFO just works everywhere, as long as the handler is
> careful with the last two arguments.  For all platforms where SA_SIGINFO
> is defined, using just sa_sigaction is fine.

Sorry for my misunderstanding. I will update this patch.

Thanks
Wen Congyang

> 
> tools/virsh.c is therefore correct, and only src/rpc/virnetserver.c has
> a bug.
> 
>>
>> ---
>>  src/rpc/virnetserver.c |   25 +++--
>>  tools/virsh.c  |   35 ---
>>  2 files changed, 51 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
>> index 3965fc2..131ecb4 100644
>> --- a/src/rpc/virnetserver.c
>> +++ b/src/rpc/virnetserver.c
>> @@ -270,8 +270,12 @@ error:
>>  }
>>  
>>  
>> +#ifdef SA_SIGINFO
>>  static void virNetServerFatalSignal(int sig, siginfo_t *siginfo 
>> ATTRIBUTE_UNUSED,
>>  void *context ATTRIBUTE_UNUSED)
>> +#else
>> +static void virNetServerFatalSignal(int sig)
>> +#endif
> 
> Yuck.  I'm happy with a single three-arg definition as long as we follow
> the gnulib rules about using sa_sigaction and SA_SIGINFO as a pair,
> rather than adding ugly #ifdefs.
> 
>>  {
>>  struct sigaction sig_action;
>>  int origerrno;
>> @@ -286,6 +290,7 @@ static void virNetServerFatalSignal(int sig, siginfo_t 
>> *siginfo ATTRIBUTE_UNUSED
>>  #ifdef SIGUSR2
>>  if (sig != SIGUSR2) {
>>  #endif
>> +memset(&sig_action, 0, sizeof(sig_action));
>>  sig_action.sa_handler = SIG_DFL;
>>  sigaction(sig, &sig_action, NULL);
> 
> Good catch - that memset (or an explicit setting of sig_action.sa_flags
> = 0) is required; just because Linux behaves sanely for
> sa_handler==SIG_DFL regardless of uninitialized sa_flags does not mean
> all platforms do likewise.
> 
>>  raise(sig);
>> @@ -362,7 +367,12 @@ virNetServerPtr virNetServerNew(size_t min_workers,
>>   * catch fatal errors to dump a log, also hook to USR2 for dynamic
>>   * debugging purposes or testing
>>   */
>> +#ifdef SA_SIGINFO
>>  sig_action.sa_sigaction = virNetServerFatalSignal;
>> +sig_action.sa_flags = SA_SIGINFO;
> 
> This one line is important,
> 
>> +#else
>> +sig_action.sa_handler = virNetServerFatalSignal;
>> +#endif
> 
> this #ifdef stuff is just gross.  We should instead copy virsh.c and
> #define SA_SIGINFO 0 if it is not already defined.
> 
>>  sigaction(SIGFPE, &sig_action, NULL);
>>  sigaction(SIGSEGV, &sig_action, NULL);
>>  sigaction(SIGILL, &sig_action, NULL);
>> @@ -420,17 +430,28 @@ static sig_atomic_t sigErrors = 0;
>>  static int sigLastErrno = 0;
>>  static int sigWrite = -1;
>>  
>> +#ifdef SA_SIGINFO
>>  static void virNetServerSignalHandler(int sig, siginfo_t * siginfo,
>>void* context ATTRIBUTE_UNUSED)
>> +#else
>> +static void virNetServerSignalHandler(int sig)
>> +#endif
> 
> Not sure I like the #ifdef here,
> 
>>  {
>>  int origerrno;
>>  int r;
>> +siginfo_t temp_siginfo;
>> +
>> +#ifdef SA_SIGINFO
>> +memcpy(&temp_siginfo, siginfo, sizeof(temp_siginfo));
>> +#else
>> +memset(&temp_siginfo, 0, sizeof(temp_siginfo));
>> +#endif
> 
> Here, if you start the file with:
> 
> #ifndef SA_SIGINFO
> # define SA_SIGINFO 0
> #endif
> 
> then here, you can avoid the in-function #ifdef with:
> 
> if (SA_SIGINFO)
> memcpy(&tmp_siginfo, siginfo, sizeof(temp_siginfo));
> else
> memset(&temp_siginfo, 0, sizeof(temp_siginfo));
> 
>>  
>>  /* set the sig num in the struct */
>> -siginfo->si_signo = sig;
>> +temp_sigi

[libvirt] [PATCH] use sa_handler instead of sa_sigaction when SA_SIGINFO is not defined

2012-04-19 Thread Wen Congyang
POSIX requires that we use sa_sigaction if sa_flags includes SA_SIGINFO,
and that we use sa_handler otherwise. But we still use sa_sigaction
when SA_SIGINFO is not defined. Practice says it will work, but theory
says we can't rely on it to work.

---
 src/rpc/virnetserver.c |   25 +++--
 tools/virsh.c  |   35 ---
 2 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 3965fc2..131ecb4 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -270,8 +270,12 @@ error:
 }
 
 
+#ifdef SA_SIGINFO
 static void virNetServerFatalSignal(int sig, siginfo_t *siginfo 
ATTRIBUTE_UNUSED,
 void *context ATTRIBUTE_UNUSED)
+#else
+static void virNetServerFatalSignal(int sig)
+#endif
 {
 struct sigaction sig_action;
 int origerrno;
@@ -286,6 +290,7 @@ static void virNetServerFatalSignal(int sig, siginfo_t 
*siginfo ATTRIBUTE_UNUSED
 #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);
@@ -362,7 +367,12 @@ virNetServerPtr virNetServerNew(size_t min_workers,
  * catch fatal errors to dump a log, also hook to USR2 for dynamic
  * debugging purposes or testing
  */
+#ifdef SA_SIGINFO
 sig_action.sa_sigaction = virNetServerFatalSignal;
+sig_action.sa_flags = SA_SIGINFO;
+#else
+sig_action.sa_handler = virNetServerFatalSignal;
+#endif
 sigaction(SIGFPE, &sig_action, NULL);
 sigaction(SIGSEGV, &sig_action, NULL);
 sigaction(SIGILL, &sig_action, NULL);
@@ -420,17 +430,28 @@ static sig_atomic_t sigErrors = 0;
 static int sigLastErrno = 0;
 static int sigWrite = -1;
 
+#ifdef SA_SIGINFO
 static void virNetServerSignalHandler(int sig, siginfo_t * siginfo,
   void* context ATTRIBUTE_UNUSED)
+#else
+static void virNetServerSignalHandler(int sig)
+#endif
 {
 int origerrno;
 int r;
+siginfo_t temp_siginfo;
+
+#ifdef SA_SIGINFO
+memcpy(&temp_siginfo, siginfo, sizeof(temp_siginfo));
+#else
+memset(&temp_siginfo, 0, sizeof(temp_siginfo));
+#endif
 
 /* set the sig num in the struct */
-siginfo->si_signo = sig;
+temp_siginfo.si_signo = sig;
 
 origerrno = errno;
-r = safewrite(sigWrite, siginfo, sizeof(*siginfo));
+r = safewrite(sigWrite, &temp_siginfo, sizeof(temp_siginfo));
 if (r == -1) {
 sigErrors++;
 sigLastErrno = errno;
diff --git a/tools/virsh.c b/tools/virsh.c
index 6168a13..cb64e81 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -579,9 +579,13 @@ out:
 
 static volatile sig_atomic_t intCaught = 0;
 
+#ifdef SA_SIGINFO
 static void vshCatchInt(int sig ATTRIBUTE_UNUSED,
 siginfo_t *siginfo ATTRIBUTE_UNUSED,
 void *context ATTRIBUTE_UNUSED)
+#else
+static void vshCatchInt(int sig ATTRIBUTE_UNUSED)
+#endif
 {
 intCaught = 1;
 }
@@ -591,23 +595,25 @@ static void vshCatchInt(int sig ATTRIBUTE_UNUSED,
  */
 static int disconnected = 0; /* we may have been disconnected */
 
-/* Gnulib doesn't guarantee SA_SIGINFO support.  */
-#ifndef SA_SIGINFO
-# define SA_SIGINFO 0
-#endif
-
 /*
  * vshCatchDisconnect:
  *
  * We get here when a SIGPIPE is being raised, we can't do much in the
  * handler, just save the fact it was raised
  */
+#ifdef SA_SIGINFO
 static void vshCatchDisconnect(int sig, siginfo_t *siginfo,
void *context ATTRIBUTE_UNUSED) {
-if (sig == SIGPIPE ||
-(SA_SIGINFO && siginfo->si_signo == SIGPIPE))
+if (sig == SIGPIPE || siginfo->si_signo == SIGPIPE)
+disconnected++;
+}
+#else
+static void vshCatchDisconnect(int sig)
+{
+if (sig == SIGPIPE)
 disconnected++;
 }
+#endif
 
 /*
  * vshSetupSignals:
@@ -619,8 +625,13 @@ static void
 vshSetupSignals(void) {
 struct sigaction sig_action;
 
+#ifdef SA_SIGINFO
 sig_action.sa_sigaction = vshCatchDisconnect;
 sig_action.sa_flags = SA_SIGINFO;
+#else
+sig_action.sa_handler = vshCatchDisconnect;
+sig_action.sa_flags = 0;
+#endif
 sigemptyset(&sig_action.sa_mask);
 
 sigaction(SIGPIPE, &sig_action, NULL);
@@ -7254,8 +7265,13 @@ vshWatchJob(vshControl *ctl,
 sigaddset(&sigmask, SIGINT);
 
 intCaught = 0;
+#ifdef SA_SIGINFO
 sig_action.sa_sigaction = vshCatchInt;
 sig_action.sa_flags = SA_SIGINFO;
+#else
+sig_action.sa_handler = vshCatchInt;
+sig_action.sa_flags = 0;
+#endif
 sigemptyset(&sig_action.sa_mask);
 sigaction(SIGINT, &sig_action, &old_sig_action);
 
@@ -7631,8 +7647,13 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
 sigaddset(&sigmask, SIGINT);
 
 intCaught = 0;
+#ifdef SA_SIGINFO
 sig_action.sa_sigaction = vshCatchInt;
 sig_action.sa_flags = SA_SIGINFO;
+#else
+sig_action.sa_handler = vshCatchInt;

[libvirt] [PATCH] building: remove libvirt_dbus.syms from EXTRA_DIST

2012-04-19 Thread Wen Congyang
commit 2223ea98 removes src/libvirt_dbus.syms, but it forgets
to remove it from EXTRA_DIST. It will cause 'make dist' failed.

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

diff --git a/src/Makefile.am b/src/Makefile.am
index b8a19b4..e48dfa5 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1230,7 +1230,6 @@ EXTRA_DIST += \
   libvirt_driver_modules.syms  \
   libvirt_daemon.syms  \
   libvirt_linux.syms   \
-  libvirt_dbus.syms\
   libvirt_network.syms \
   libvirt_nwfilter.syms\
   libvirt_sasl.syms\
-- 
1.7.1

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


Re: [libvirt] [PATCHv2] virsh: avoid uninitialized memory usage

2012-04-19 Thread Wen Congyang
At 04/20/2012 04:29 AM, Eric Blake Wrote:
> On 04/19/2012 02:56 AM, Wen Congyang wrote:
>> At 04/19/2012 04:51 PM, Alex Jia Wrote:
>>> Detected by valgrind.
>>>
>>> * tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.
>>>  
> 
>>> +++ b/tools/virsh.c
>>> @@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
>>>  
>>>  intCaught = 0;
>>>  sig_action.sa_sigaction = vshCatchInt;
>>> +sig_action.sa_flags = 0;
>>>  sigemptyset(&sig_action.sa_mask);
>>>  sigaction(SIGINT, &sig_action, &old_sig_action);
>>>  
>>
>> ACK
> 
> NACK.  vshCatchInt is a 3-arg function, and therefore
> sig_action.sa_flags must include at least SA_SIGINFO.  I inadvertently
> missed a line when copying code from vshWatchJob(); I'll push the
> obvious v3 patch shortly.
> 

Sorry, I forgot this.

I search sig_action(), and found the same problem in the function
 virNetServerNew():

memset(&sig_action, 0, sizeof(sig_action));
sig_action.sa_handler = SIG_IGN;
sigaction(SIGPIPE, &sig_action, NULL);

/*
 * catch fatal errors to dump a log, also hook to USR2 for dynamic
 * debugging purposes or testing
 */
sig_action.sa_sigaction = virNetServerFatalSignal;
sigaction(SIGFPE, &sig_action, NULL);
sigaction(SIGSEGV, &sig_action, NULL);

We set sig_action.sa_flags to 0 here.

And in virsh.c, I found:

/* Gnulib doesn't guarantee SA_SIGINFO support.  */
#ifndef SA_SIGINFO
# define SA_SIGINFO 0
#endif


Is it safe to use sig_action.sa_sigaction when SA_SIGINFO is not set?
I think it is OK if we access siginfo_t if SA_SIGINFO is not 0.
But in the function virNetServerAddSignalHandler():

memset(&sig_action, 0, sizeof(sig_action));
sig_action.sa_sigaction = virNetServerSignalHandler;
#ifdef SA_SIGINFO
sig_action.sa_flags = SA_SIGINFO;
#endif
sigemptyset(&sig_action.sa_mask);

sigaction(signum, &sig_action, &sigdata->oldaction);

virNetServerSignalHandler() will access siginfo_t without any check.

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


Re: [libvirt] [PATCHv2] virsh: avoid uninitialized memory usage

2012-04-19 Thread Wen Congyang
At 04/19/2012 04:51 PM, Alex Jia Wrote:
> Detected by valgrind.
> 
> * tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.
>  
> * How to reproduce?
> $ qemu-img create /var/lib/libvirt/images/test 1M
> $ cat > /tmp/test.xml < 
>   test
>   219200
>   1
>   
> hvm
> 
>   
>   
> 
>   
>   
>   
> 
> 
> 
>   
> 
> EOF
> $ virsh define /tmp/test.xml
> $ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait
> 
> actual result:
> 
> ==10906== 1 errors in context 1 of 1:
> ==10906== Syscall param rt_sigaction(act->sa_flags) points to uninitialised 
> byte(s)
> ==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
> ==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
> ==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
> ==10906==by 0x425E73: main (virsh.c:20178)
> ==10906==  Address 0x7fefffae8 is on thread 1's stack
> 
> 
> Signed-off-by: Alex Jia 
> ---
>  tools/virsh.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 95ed7bc..4e4ca57 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
>  
>  intCaught = 0;
>  sig_action.sa_sigaction = vshCatchInt;
> +sig_action.sa_flags = 0;
>  sigemptyset(&sig_action.sa_mask);
>  sigaction(SIGINT, &sig_action, &old_sig_action);
>  

ACK

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


Re: [libvirt] [PATCH] virsh: avoid uninitialized memory usage

2012-04-19 Thread Wen Congyang
At 04/19/2012 04:40 PM, Alex Jia Wrote:
> On 04/19/2012 04:19 PM, Wen Congyang wrote:
>> At 04/19/2012 04:09 PM, Alex Jia Wrote:
>>> Detected by valgrind.
>>>
>>> * tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.
>>>
>>> * How to reproduce?
>>> $ qemu-img create /var/lib/libvirt/images/test 1M
>>> $ cat>  /tmp/test.xml<>> 
>>>test
>>>219200
>>>1
>>>
>>>  hvm
>>>  
>>>
>>>
>>>  
>>>
>>>
>>>
>>>  
>>>  
>>>  
>>>
>>> 
>>> EOF
>>> $ virsh define /tmp/test.xml
>>> $ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait
>>>
>>> actual result:
>>>
>>> ==10906== 1 errors in context 1 of 1:
>>> ==10906== Syscall param rt_sigaction(act->sa_flags) points to
>>> uninitialised byte(s)
>>> ==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
>>> ==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
>>> ==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
>>> ==10906==by 0x425E73: main (virsh.c:20178)
>>> ==10906==  Address 0x7fefffae8 is on thread 1's stack
>>>
>>>
>>> Signed-off-by: Alex Jia
>>> ---
>>>   tools/virsh.c |1 +
>>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/tools/virsh.c b/tools/virsh.c
>>> index 95ed7bc..4e4ca57 100644
>>> --- a/tools/virsh.c
>>> +++ b/tools/virsh.c
>>> @@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
>>>
>>>   intCaught = 0;
>>>   sig_action.sa_sigaction = vshCatchInt;
>>> +sigemptyset((sigset_t *)&sig_action.sa_flags);
>> Why using sigemptyset here? You should use 'sig_action.sa_flags = 0'.
> Yeah, I think 'sig_action.sa_flags = 0' is right, but I don't know what
> the difference are,
> could you explain more?

sigset_t is:
# define _SIGSET_NWORDS (1024 / (8 * sizeof (unsigned long int)))
typedef struct
  {
unsigned long int __val[_SIGSET_NWORDS];
  } __sigset_t;

The length of sigset is larger than sizeof(int)

If you use sigemptyset() to clear flags, it will affect the memory after flags.
It is very dangerous!!!

Thanks
Wen Congyang

> 
> Thanks,
> Alex
>> Thanks
>> Wen Congyang
>>
>>>   sigemptyset(&sig_action.sa_mask);
>>>   sigaction(SIGINT,&sig_action,&old_sig_action);
>>>
> 
> 

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


Re: [libvirt] [PATCH 0/2 v3 use qemu's dump-guest-meory when vm uses host device

2012-04-19 Thread Wen Congyang
At 04/19/2012 04:38 PM, Daniel P. Berrange Wrote:
> On Thu, Apr 19, 2012 at 09:03:16AM +0800, Wen Congyang wrote:
>> Currently, we use migrate to dump guest's memory. There is one
>> restriction in migrate command: the device's status should be
>> stored in qemu because the device's status should be passed to
>> target machine.
>>
>> If we passthrough a host device to guest, the device's status
>> is stored in the real device. So migrate command will fail.
>>
>> We usually use dump when guest is panicked. So there is no need
>> to store device's status in the vmcore.
>>
>> qemu will have a new monitor command dump-guest-memory to dump
>> guest memory, but it doesn't support async now(it will support
>> later when the common async API is implemented).
>>
>> So I use dump-guest-memory only when the guest uses host device
>> in this patchset.
> 
> Hmm, doesn't the new command generate dump files in a totally
> different format to the existing command ?  If so I don't
> think it is nice behaviour to silently switch between 2 different
> dump formats depending on the XML config.  I think we'd want some
> kind of flag to specify what format is desired.

Agree with it. But the new command is not a async command, and it
will block the other operation. So I only use it when the vm uses
host device(migrate command will fail in this case).

The new command will be converted to async command later(after
async API is implemented). I think we can allow the user specify
the format when the new command is async command.

Thanks
Wen Congyang

> 
> Daniel

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


Re: [libvirt] [PATCH] virsh: avoid uninitialized memory usage

2012-04-19 Thread Wen Congyang
At 04/19/2012 04:09 PM, Alex Jia Wrote:
> Detected by valgrind.
> 
> * tools/virsh.c (cmdBlockPull): fix uninitialized memory usage.
>  
> * How to reproduce?
> $ qemu-img create /var/lib/libvirt/images/test 1M
> $ cat > /tmp/test.xml < 
>   test
>   219200
>   1
>   
> hvm
> 
>   
>   
> 
>   
>   
>   
> 
> 
> 
>   
> 
> EOF
> $ virsh define /tmp/test.xml
> $ valgrind -v virsh blockpull test /var/lib/libvirt/images/test --wait
> 
> actual result:
> 
> ==10906== 1 errors in context 1 of 1:
> ==10906== Syscall param rt_sigaction(act->sa_flags) points to uninitialised 
> byte(s)
> ==10906==at 0x39CF80F5BE: __libc_sigaction (sigaction.c:67)
> ==10906==by 0x43016C: cmdBlockPull (virsh.c:7638)
> ==10906==by 0x4150D4: vshCommandRun (virsh.c:18574)
> ==10906==by 0x425E73: main (virsh.c:20178)
> ==10906==  Address 0x7fefffae8 is on thread 1's stack
> 
> 
> Signed-off-by: Alex Jia 
> ---
>  tools/virsh.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 95ed7bc..4e4ca57 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -7634,6 +7634,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
>  
>  intCaught = 0;
>  sig_action.sa_sigaction = vshCatchInt;
> +sigemptyset((sigset_t *)&sig_action.sa_flags);

Why using sigemptyset here? You should use 'sig_action.sa_flags = 0'.

Thanks
Wen Congyang

>  sigemptyset(&sig_action.sa_mask);
>  sigaction(SIGINT, &sig_action, &old_sig_action);
>  

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


[libvirt] [PATCH 1/2 v3] qemu: implement qemu's dump-guest-memory

2012-04-18 Thread Wen Congyang
---
 src/qemu/qemu_monitor.c  |   36 
 src/qemu/qemu_monitor.h  |   13 +
 src/qemu/qemu_monitor_json.c |   42 ++
 src/qemu/qemu_monitor_json.h |7 +++
 4 files changed, 98 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index e1a8d4c..81bdf07 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2018,6 +2018,42 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorDumpToFd(qemuMonitorPtr mon,
+unsigned int flags,
+int fd,
+unsigned long long begin,
+unsigned long long length)
+{
+int ret;
+VIR_DEBUG("mon=%p fd=%d flags=%x begin=%llx length=%llx",
+  mon, fd, flags, begin, length);
+
+if (!mon) {
+qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+_("monitor must not be NULL"));
+return -1;
+}
+
+if (!mon->json)
+/* dump-guest-memory is supported after qemu-1.0, and we always use 
json
+ * if qemu's version is >= 0.15. So if we use text mode, the qemu is
+ * old, and it does not support dump-guest-memory.
+ */
+return -2;
+
+if (qemuMonitorSendFileHandle(mon, "dump", fd) < 0)
+return -1;
+
+ret = qemuMonitorJSONDump(mon, flags, "fd:dump", begin, length);
+
+if (ret < 0) {
+if (qemuMonitorCloseFileHandle(mon, "dump") < 0)
+VIR_WARN("failed to close dumping handle");
+}
+
+return ret;
+}
 
 int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int type,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index b480966..315cb9e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -379,6 +379,19 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
 
 int qemuMonitorMigrateCancel(qemuMonitorPtr mon);
 
+typedef enum {
+  QEMU_MONITOR_DUMP_HAVE_FILTER  = 1 << 0,
+  QEMU_MONITOR_DUMP_PAGING   = 1 << 1,
+  QEMU_MONITOR_DUMP_FLAGS_LAST
+} QEMU_MONITOR_DUMP;
+
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorDumpToFd(qemuMonitorPtr mon,
+unsigned int flags,
+int fd,
+unsigned long long begin,
+unsigned long long length);
+
 int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int type,
 const char *hostname,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index d09d779..6709abf 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2419,6 +2419,48 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorJSONDump(qemuMonitorPtr mon,
+unsigned int flags,
+const char *protocol,
+unsigned long long begin,
+unsigned long long length)
+{
+int ret;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+
+if (flags & QEMU_MONITOR_DUMP_HAVE_FILTER)
+cmd = qemuMonitorJSONMakeCommand("dump-guest-memory",
+ "b:paging", flags & 
QEMU_MONITOR_DUMP_PAGING ? 1 : 0,
+ "s:protocol", protocol,
+ "U:begin", begin,
+ "U:length", length,
+ NULL);
+else
+cmd = qemuMonitorJSONMakeCommand("dump-guest-memory",
+ "b:paging", flags & 
QEMU_MONITOR_DUMP_PAGING ? 1 : 0,
+ "s:protocol", protocol,
+ NULL);
+if (!cmd)
+return -1;
+
+ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+if (ret == 0) {
+if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ret = -2;
+goto cleanup;
+}
+
+ret = qemuMonitorJSONCheckError(cmd, reply);
+}
+
+cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
 
 int qemuMonitorJSONGraphicsRelocate(qemuMonitorPtr mon,
 int type,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index a0f67aa..a8b70d4 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -136,6 +136,13 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon,
 
 int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon);
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorJSONDump(qemuMoni

[libvirt] [PATCH 2/2 v3] qemu: try to use qemu's dump-guest-meory when vm uses host device

2012-04-18 Thread Wen Congyang
---
 src/qemu/qemu_domain.c |1 +
 src/qemu/qemu_domain.h |1 +
 src/qemu/qemu_driver.c |   39 +--
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 69d9e6e..e3a668a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -158,6 +158,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
 job->phase = 0;
 job->mask = DEFAULT_JOB_MASK;
 job->start = 0;
+job->qemu_dump = false;
 memset(&job->info, 0, sizeof(job->info));
 }
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index adccfed..f1ab0e6 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -97,6 +97,7 @@ struct qemuDomainJobObj {
 int phase;  /* Job phase (mainly for migrations) */
 unsigned long long mask;/* Jobs allowed during async job */
 unsigned long long start;   /* When the async job started */
+bool qemu_dump; /* use qemu dump to do dump */
 virDomainJobInfo info;  /* Async job progress data */
 };
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d9e35be..651e9b8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2953,6 +2953,31 @@ cleanup:
 return ret;
 }
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+static int qemuDumpToFd(struct qemud_driver *driver, virDomainObjPtr vm,
+int fd, enum qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret = -1;
+
+if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD))
+return -2;
+
+if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
+  fd) < 0)
+return -1;
+
+priv->job.qemu_dump = true;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+ret = qemuMonitorDumpToFd(priv->mon, 0, fd, 0, 0);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+return ret;
+}
+
 static int
 doCoreDump(struct qemud_driver *driver,
virDomainObjPtr vm,
@@ -2987,6 +3012,16 @@ doCoreDump(struct qemud_driver *driver,
 if (!(wrapperFd = virFileWrapperFdNew(&fd, path, flags)))
 goto cleanup;
 
+if (vm->def->nhostdevs > 0) {
+/*
+ * If the guest uses host devices, migrate command will fail. So we
+ * should use dump command.
+ */
+ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP);
+if (ret != -2)
+goto cleanup;
+}
+
 if (qemuMigrationToFile(driver, vm, fd, 0, path,
 qemuCompressProgramName(compress), false,
 QEMU_ASYNC_JOB_DUMP) < 0)
@@ -9332,7 +9367,7 @@ static int qemuDomainGetJobInfo(virDomainPtr dom,
 priv = vm->privateData;
 
 if (virDomainObjIsActive(vm)) {
-if (priv->job.asyncJob) {
+if (priv->job.asyncJob && !priv->job.qemu_dump) {
 memcpy(info, &priv->job.info, sizeof(*info));
 
 /* Refresh elapsed time again just to ensure it
@@ -9390,7 +9425,7 @@ static int qemuDomainAbortJob(virDomainPtr dom) {
 
 priv = vm->privateData;
 
-if (!priv->job.asyncJob) {
+if (!priv->job.asyncJob || priv->job.qemu_dump) {
 qemuReportError(VIR_ERR_OPERATION_INVALID,
 "%s", _("no job is active on the domain"));
 goto endjob;
-- 
1.7.1

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


[libvirt] [PATCH 0/2 v3 use qemu's dump-guest-meory when vm uses host device

2012-04-18 Thread Wen Congyang
Currently, we use migrate to dump guest's memory. There is one
restriction in migrate command: the device's status should be
stored in qemu because the device's status should be passed to
target machine.

If we passthrough a host device to guest, the device's status
is stored in the real device. So migrate command will fail.

We usually use dump when guest is panicked. So there is no need
to store device's status in the vmcore.

qemu will have a new monitor command dump-guest-memory to dump
guest memory, but it doesn't support async now(it will support
later when the common async API is implemented).

So I use dump-guest-memory only when the guest uses host device
in this patchset.

Note: the patchset for qemu is still queued. Luiz has acked,
but he waits an ACK from Jan and/or Anthony. They are too busy,
and donot reply.

Changes from v2 to v3:
1. qemu supports the fd that is associated with a pipe, socket, or FIFO.
   So pass a pipe fd to qemu and O_DIRECT can work now.

Change from v1 to v2:
1. remove the implemention for text mode.

Wen Congyang (2):
  qemu: implement qemu's dump-guest-memory
  qemu: try to use qemu's dump-guest-meory when vm uses host device

 src/qemu/qemu_domain.c   |1 +
 src/qemu/qemu_domain.h   |1 +
 src/qemu/qemu_driver.c   |   39 +--
 src/qemu/qemu_monitor.c  |   36 
 src/qemu/qemu_monitor.h  |   13 +
 src/qemu/qemu_monitor_json.c |   42 ++
 src/qemu/qemu_monitor_json.h |7 +++
 7 files changed, 137 insertions(+), 2 deletions(-)


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


Re: [libvirt] [PATCH 0/2 v2] use qemu's dump-guest-meory when vm uses host device

2012-04-18 Thread Wen Congyang
Hi, Eric

Luiz will accept the qemu's patch:
http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg02346.html

Do you have any comment about this patchset?

Thanks
Wen Congyang

At 04/10/2012 04:03 PM, Wen Congyang Wrote:
> Currently, we use migrate to dump guest's memory. There is one
> restriction in migrate command: the device's status should be
> stored in qemu because the device's status should be passed to
> target machine.
> 
> If we passthrough a host device to guest, the device's status
> is stored in the real device. So migrate command will fail.
> 
> We usually use dump when guest is panicked. So there is no need
> to store device's status in the vmcore.
> 
> qemu will have a new monitor command dump-guest-memory to dump
> guest memory, but it doesn't support async now(it will support
> later when the common async API is implemented).
> 
> So I use dump-guest-memory only when the guest uses host device
> in this patchset.
> 
> Note: the patchset for qemu is still queued. Luiz has acked,
> but he waits an ACK from Jan and/or Anthony. They are too busy,
> and donot reply.
> 
> Change from v1 to v2:
> 1. remove the implemention for text mode.
> 
> Wen Congyang (2):
>   qemu: implement qemu's dump-guest-memory
>   qemu: try to use qemu's dump-guest-meory when vm uses host device
> 
>  src/qemu/qemu_domain.c   |1 +
>  src/qemu/qemu_domain.h   |1 +
>  src/qemu/qemu_driver.c   |   42 
> --
>  src/qemu/qemu_monitor.c  |   36 
>  src/qemu/qemu_monitor.h  |   13 +
>  src/qemu/qemu_monitor_json.c |   42 
> ++
>  src/qemu/qemu_monitor_json.h |7 +++
>  7 files changed, 140 insertions(+), 2 deletions(-)
> 
> --
> 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] qemu: optimize JSON event handler lookup

2012-04-10 Thread Wen Congyang
At 04/11/2012 06:47 AM, Eric Blake Wrote:
> Probably in the noise, but this will let us scale more efficiently
> as we recognize ever more qemu events.
> 
> * src/qemu/qemu_monitor_json.c (eventHandlers): Sort.
> (eventSearch): New helper function.
> (qemuMonitorJSONIOProcessEvent): Optimize event lookup.
> ---
> 
> In reply to:
> https://www.redhat.com/archives/libvir-list/2012-April/msg00416.html
> 
>  src/qemu/qemu_monitor_json.c |   55 +++--
>  1 files changed, 31 insertions(+), 24 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 7c893d4..d142517 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -67,37 +67,45 @@ static void qemuMonitorJSONHandlePMSuspend(qemuMonitorPtr 
> mon, virJSONValuePtr d
>  static void qemuMonitorJSONHandleBlockJobCompleted(qemuMonitorPtr mon, 
> virJSONValuePtr data);
>  static void qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon, 
> virJSONValuePtr data);
> 
> -static struct {
> +typedef struct {
>  const char *type;
>  void (*handler)(qemuMonitorPtr mon, virJSONValuePtr data);
> -} eventHandlers[] = {
> -{ "SHUTDOWN", qemuMonitorJSONHandleShutdown, },
> -{ "RESET", qemuMonitorJSONHandleReset, },
> +} qemuEventHandler;
> +static qemuEventHandler eventHandlers[] = {
> +{ "BLOCK_IO_ERROR", qemuMonitorJSONHandleIOError, },
> +{ "BLOCK_JOB_CANCELLED", qemuMonitorJSONHandleBlockJobCanceled, },

I donot find this event...

> +{ "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJobCompleted, },
> +{ "DEVICE_TRAY_MOVED", qemuMonitorJSONHandleTrayChange, },
>  { "POWERDOWN", qemuMonitorJSONHandlePowerdown, },
> -{ "STOP", qemuMonitorJSONHandleStop, },
> +{ "RESET", qemuMonitorJSONHandleReset, },
>  { "RTC_CHANGE", qemuMonitorJSONHandleRTCChange, },
> -{ "WATCHDOG", qemuMonitorJSONHandleWatchdog, },
> -{ "BLOCK_IO_ERROR", qemuMonitorJSONHandleIOError, },
> -{ "VNC_CONNECTED", qemuMonitorJSONHandleVNCConnect, },
> -{ "VNC_INITIALIZED", qemuMonitorJSONHandleVNCInitialize, },
> -{ "VNC_DISCONNECTED", qemuMonitorJSONHandleVNCDisconnect, },
> +{ "SHUTDOWN", qemuMonitorJSONHandleShutdown, },
>  { "SPICE_CONNECTED", qemuMonitorJSONHandleSPICEConnect, },
> -{ "SPICE_INITIALIZED", qemuMonitorJSONHandleSPICEInitialize, },
>  { "SPICE_DISCONNECTED", qemuMonitorJSONHandleSPICEDisconnect, },
> -{ "DEVICE_TRAY_MOVED", qemuMonitorJSONHandleTrayChange, },
> -{ "WAKEUP", qemuMonitorJSONHandlePMWakeup, },
> +{ "SPICE_INITIALIZED", qemuMonitorJSONHandleSPICEInitialize, },
> +{ "STOP", qemuMonitorJSONHandleStop, },
>  { "SUSPEND", qemuMonitorJSONHandlePMSuspend, },
> -{ "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJobCompleted, },
> -{ "BLOCK_JOB_CANCELLED", qemuMonitorJSONHandleBlockJobCanceled, },
> +{ "VNC_CONNECTED", qemuMonitorJSONHandleVNCConnect, },
> +{ "VNC_DISCONNECTED", qemuMonitorJSONHandleVNCDisconnect, },
> +{ "VNC_INITIALIZED", qemuMonitorJSONHandleVNCInitialize, },
> +{ "WAKEUP", qemuMonitorJSONHandlePMWakeup, },
> +{ "WATCHDOG", qemuMonitorJSONHandleWatchdog, },
> +/* We use bsearch, so keep this list sorted.  */
>  };
> 
> +static int
> +eventSearch(const void *key, const void *elt) {

Hmm, I think eventCompare is better than eventSearch.

> +const char *type = key;
> +const qemuEventHandler *handler = elt;
> +return strcmp(type, handler->type);
> +}
> 
>  static int
>  qemuMonitorJSONIOProcessEvent(qemuMonitorPtr mon,
>virJSONValuePtr obj)
>  {
>  const char *type;
> -int i;
> +qemuEventHandler *handler;
>  VIR_DEBUG("mon=%p obj=%p", mon, obj);
> 
>  type = virJSONValueObjectGetString(obj, "event");
> @@ -107,14 +115,13 @@ qemuMonitorJSONIOProcessEvent(qemuMonitorPtr mon,
>  return -1;
>  }
> 
> -for (i = 0 ; i < ARRAY_CARDINALITY(eventHandlers) ; i++) {
> -if (STREQ(eventHandlers[i].type, type)) {
> -virJSONValuePtr data = virJSONValueObjectGet(obj, "data");
> -VIR_DEBUG("handle %s handler=%p data=%p", type,
> -  eventHandlers[i].handler, data);
> -(eventHandlers[i].handler)(mon, data);
> -break;
> -}
> +handler = bsearch(type, eventHandlers, ARRAY_CARDINALITY(eventHandlers),
> +  sizeof(eventHandlers[0]), eventSearch);
> +if (handler) {
> +virJSONValuePtr data = virJSONValueObjectGet(obj, "data");
> +VIR_DEBUG("handle %s handler=%p data=%p", type,
> +  handler->handler, data);
> +(handler->handler)(mon, data);
>  }
>  return 0;
>  }

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


[libvirt] [PATCH 1/2 v2] qemu: implement qemu's dump-guest-memory

2012-04-10 Thread Wen Congyang
---
 src/qemu/qemu_monitor.c  |   36 
 src/qemu/qemu_monitor.h  |   13 +
 src/qemu/qemu_monitor_json.c |   42 ++
 src/qemu/qemu_monitor_json.h |7 +++
 4 files changed, 98 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index e1a8d4c..81bdf07 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2018,6 +2018,42 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorDumpToFd(qemuMonitorPtr mon,
+unsigned int flags,
+int fd,
+unsigned long long begin,
+unsigned long long length)
+{
+int ret;
+VIR_DEBUG("mon=%p fd=%d flags=%x begin=%llx length=%llx",
+  mon, fd, flags, begin, length);
+
+if (!mon) {
+qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+_("monitor must not be NULL"));
+return -1;
+}
+
+if (!mon->json)
+/* dump-guest-memory is supported after qemu-1.0, and we always use 
json
+ * if qemu's version is >= 0.15. So if we use text mode, the qemu is
+ * old, and it does not support dump-guest-memory.
+ */
+return -2;
+
+if (qemuMonitorSendFileHandle(mon, "dump", fd) < 0)
+return -1;
+
+ret = qemuMonitorJSONDump(mon, flags, "fd:dump", begin, length);
+
+if (ret < 0) {
+if (qemuMonitorCloseFileHandle(mon, "dump") < 0)
+VIR_WARN("failed to close dumping handle");
+}
+
+return ret;
+}
 
 int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int type,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index b480966..315cb9e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -379,6 +379,19 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
 
 int qemuMonitorMigrateCancel(qemuMonitorPtr mon);
 
+typedef enum {
+  QEMU_MONITOR_DUMP_HAVE_FILTER  = 1 << 0,
+  QEMU_MONITOR_DUMP_PAGING   = 1 << 1,
+  QEMU_MONITOR_DUMP_FLAGS_LAST
+} QEMU_MONITOR_DUMP;
+
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorDumpToFd(qemuMonitorPtr mon,
+unsigned int flags,
+int fd,
+unsigned long long begin,
+unsigned long long length);
+
 int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int type,
 const char *hostname,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index d09d779..6709abf 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2419,6 +2419,48 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorJSONDump(qemuMonitorPtr mon,
+unsigned int flags,
+const char *protocol,
+unsigned long long begin,
+unsigned long long length)
+{
+int ret;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+
+if (flags & QEMU_MONITOR_DUMP_HAVE_FILTER)
+cmd = qemuMonitorJSONMakeCommand("dump-guest-memory",
+ "b:paging", flags & 
QEMU_MONITOR_DUMP_PAGING ? 1 : 0,
+ "s:protocol", protocol,
+ "U:begin", begin,
+ "U:length", length,
+ NULL);
+else
+cmd = qemuMonitorJSONMakeCommand("dump-guest-memory",
+ "b:paging", flags & 
QEMU_MONITOR_DUMP_PAGING ? 1 : 0,
+ "s:protocol", protocol,
+ NULL);
+if (!cmd)
+return -1;
+
+ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+if (ret == 0) {
+if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ret = -2;
+goto cleanup;
+}
+
+ret = qemuMonitorJSONCheckError(cmd, reply);
+}
+
+cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
 
 int qemuMonitorJSONGraphicsRelocate(qemuMonitorPtr mon,
 int type,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index a0f67aa..a8b70d4 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -136,6 +136,13 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon,
 
 int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon);
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorJSONDump(qemuMoni

[libvirt] [PATCH 2/2 v2] qemu: try to use qemu's dump-guest-meory when vm uses host device

2012-04-10 Thread Wen Congyang
---
 src/qemu/qemu_domain.c |1 +
 src/qemu/qemu_domain.h |1 +
 src/qemu/qemu_driver.c |   42 --
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 69d9e6e..e3a668a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -158,6 +158,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
 job->phase = 0;
 job->mask = DEFAULT_JOB_MASK;
 job->start = 0;
+job->qemu_dump = false;
 memset(&job->info, 0, sizeof(job->info));
 }
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index adccfed..f1ab0e6 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -97,6 +97,7 @@ struct qemuDomainJobObj {
 int phase;  /* Job phase (mainly for migrations) */
 unsigned long long mask;/* Jobs allowed during async job */
 unsigned long long start;   /* When the async job started */
+bool qemu_dump; /* use qemu dump to do dump */
 virDomainJobInfo info;  /* Async job progress data */
 };
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d9e35be..fb14734 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2953,6 +2953,31 @@ cleanup:
 return ret;
 }
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+static int qemuDumpToFd(struct qemud_driver *driver, virDomainObjPtr vm,
+int fd, enum qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret = -1;
+
+if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD))
+return -2;
+
+if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
+  fd) < 0)
+return -1;
+
+priv->job.qemu_dump = true;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+ret = qemuMonitorDumpToFd(priv->mon, 0, fd, 0, 0);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+return ret;
+}
+
 static int
 doCoreDump(struct qemud_driver *driver,
virDomainObjPtr vm,
@@ -2984,6 +3009,19 @@ doCoreDump(struct qemud_driver *driver,
NULL, NULL)) < 0)
 goto cleanup;
 
+if (vm->def->nhostdevs > 0) {
+/*
+ * If the guest uses host devices, migrate command will fail. So we
+ * should use dump command.
+ *
+ * qemu dump does not support fd that is associated with a pipe,
+ * socket, or FIFO. So we should call qemuDumpTpFd() before calling
+ * virFileWrapperFdNew(). */
+ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP);
+if (ret != -2)
+goto cleanup;
+}
+
 if (!(wrapperFd = virFileWrapperFdNew(&fd, path, flags)))
 goto cleanup;
 
@@ -9332,7 +9370,7 @@ static int qemuDomainGetJobInfo(virDomainPtr dom,
 priv = vm->privateData;
 
 if (virDomainObjIsActive(vm)) {
-if (priv->job.asyncJob) {
+if (priv->job.asyncJob && !priv->job.qemu_dump) {
 memcpy(info, &priv->job.info, sizeof(*info));
 
 /* Refresh elapsed time again just to ensure it
@@ -9390,7 +9428,7 @@ static int qemuDomainAbortJob(virDomainPtr dom) {
 
 priv = vm->privateData;
 
-if (!priv->job.asyncJob) {
+if (!priv->job.asyncJob || priv->job.qemu_dump) {
 qemuReportError(VIR_ERR_OPERATION_INVALID,
 "%s", _("no job is active on the domain"));
 goto endjob;
-- 
1.7.1

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


[libvirt] [PATCH 0/2 v2] use qemu's dump-guest-meory when vm uses host device

2012-04-10 Thread Wen Congyang
Currently, we use migrate to dump guest's memory. There is one
restriction in migrate command: the device's status should be
stored in qemu because the device's status should be passed to
target machine.

If we passthrough a host device to guest, the device's status
is stored in the real device. So migrate command will fail.

We usually use dump when guest is panicked. So there is no need
to store device's status in the vmcore.

qemu will have a new monitor command dump-guest-memory to dump
guest memory, but it doesn't support async now(it will support
later when the common async API is implemented).

So I use dump-guest-memory only when the guest uses host device
in this patchset.

Note: the patchset for qemu is still queued. Luiz has acked,
but he waits an ACK from Jan and/or Anthony. They are too busy,
and donot reply.

Change from v1 to v2:
1. remove the implemention for text mode.

Wen Congyang (2):
  qemu: implement qemu's dump-guest-memory
  qemu: try to use qemu's dump-guest-meory when vm uses host device

 src/qemu/qemu_domain.c   |1 +
 src/qemu/qemu_domain.h   |1 +
 src/qemu/qemu_driver.c   |   42 --
 src/qemu/qemu_monitor.c  |   36 
 src/qemu/qemu_monitor.h  |   13 +
 src/qemu/qemu_monitor_json.c |   42 ++
 src/qemu/qemu_monitor_json.h |7 +++
 7 files changed, 140 insertions(+), 2 deletions(-)

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


Re: [libvirt] [PATCH 0/2] use qemu's dump-guest-meory when vm uses host device

2012-04-02 Thread Wen Congyang
At 04/03/2012 08:46 AM, Eric Blake Wrote:
> On 04/02/2012 06:23 PM, Wen Congyang wrote:
>>>
>>> Why are we bothering with a text implementation?  We know that the
>>> feature is only present if you have qemu 1.1 or later (assuming that
>>> qemu-devel did apply your series adding the monitor command), and
>>> therefore we know that we have QMP, so we should only implement this for
>>> JSON and not bother with the text monitor.
>>>
>>
>> In most cases, the libvirt is built with json. But if the libvirt is built
>> without json, we will use text monitor.
> 
> If libvirt is built without json, it will die a horrible death on any
> qemu newer than 0.15, thanks to commit 6e769eba.  You can safely assume
> that a qemu 1.1 feature implies that libvirt was compiled with json
> support.  The _only_ situation that requires the text monitor is when
> building libvirt for RHEL 5 where qemu is still stuck at 0.10 and where
> JSON libraries do not exist.  I suspect no one will be backporting the
> new dump-guest-monitor command to RHEL 5.
> 

I see this commit now. So I agree with removing text monitor support now.

Thanks for point it out.

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


Re: [libvirt] [PATCH 0/2] use qemu's dump-guest-meory when vm uses host device

2012-04-02 Thread Wen Congyang
At 04/02/2012 10:50 PM, Eric Blake Wrote:
> On 04/01/2012 10:01 PM, Wen Congyang wrote:
>> Currently, we use migrate to dump guest's memory. There is one
>> restriction in migrate command: the device's status should be
>> stored in qemu because the device's status should be passed to
>> target machine.
>>
>> If we passthrough a host device to guest, the device's status
>> is stored in the real device. So migrate command will fail.
>>
>> We usually use dump when guest is panicked. So there is no need
>> to store device's status in the vmcore.
>>
>> qemu will have a new monitor command dump-guest-memory to dump
>> guest memory, but it doesn't support async now(it will support
>> later when the common async API is implemented).
> 
> I've seen conversation on this patch on qemu-devel, but is it actually
> committed yet, or still pending there?  This is late enough that it
> should wait until after 0.9.11 to actually be applied, so I haven't
> quite reviewed it yet.

The patch on qemu-devel is still pending there. QMP maitainer Luiz
has acked it, but he waits an ack from Jan and/or Anthony.

> 
>>
>> So I use dump-guest-memory only when the guest uses host device
>> in this patchset.
>>
>> Wen Congyang (2):
>>   qemu: implement qemu's dump-guest-memory
>>   qemu: try to use qemu's dump-guest-meory when vm uses host device
>>
>>  src/qemu/qemu_domain.c   |1 +
>>  src/qemu/qemu_domain.h   |1 +
>>  src/qemu/qemu_driver.c   |   42 -
>>  src/qemu/qemu_monitor.c  |   32 
>>  src/qemu/qemu_monitor.h  |   13 +++
>>  src/qemu/qemu_monitor_json.c |   42 +
>>  src/qemu/qemu_monitor_json.h |7 
>>  src/qemu/qemu_monitor_text.c |   83 
>> ++
> 
> Why are we bothering with a text implementation?  We know that the
> feature is only present if you have qemu 1.1 or later (assuming that
> qemu-devel did apply your series adding the monitor command), and
> therefore we know that we have QMP, so we should only implement this for
> JSON and not bother with the text monitor.
> 

In most cases, the libvirt is built with json. But if the libvirt is built
without json, we will use text monitor.


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


Re: [libvirt] [PATCH] qemu: fix memory leak in virDomainGetVcpus

2012-04-01 Thread Wen Congyang
At 04/02/2012 01:33 PM, Laine Stump Wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=808979
> 
> The leak is really in virProcessInfoGetAffinity, as shown in the
> valgrind output given in the above bug report - it calls CPU_ALLOC(),
> but then fails to call CPU_FREE().
> 
> This leak has existed in every version of libvirt since 0.7.5.
> ---
>  src/util/processinfo.c |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/src/util/processinfo.c b/src/util/processinfo.c
> index b1b1737..af19723 100644
> --- a/src/util/processinfo.c
> +++ b/src/util/processinfo.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2009-2010 Red Hat, Inc.
> + * Copyright (C) 2009-2010, 2012 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -140,6 +140,7 @@ realloc:
>  for (i = 0 ; i < maxcpu ; i++)
>  if (CPU_ISSET_S(i, masklen, mask))
>  VIR_USE_CPU(map, i);
> +CPU_FREE(mask);
>  # else
>  /* Legacy method uses a fixed size cpu mask, only allows upto 1024 cpus 
> */
>  cpu_set_t mask;

ACK

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


[libvirt] [PATCH 1/2] qemu: implement qemu's dump-guest-memory

2012-04-01 Thread Wen Congyang
This patch introduces the API to use qemu's new monitor command 
dump-guest-memory

---
 src/qemu/qemu_monitor.c  |   32 
 src/qemu/qemu_monitor.h  |   13 +++
 src/qemu/qemu_monitor_json.c |   42 +
 src/qemu/qemu_monitor_json.h |7 
 src/qemu/qemu_monitor_text.c |   83 ++
 src/qemu/qemu_monitor_text.h |7 
 6 files changed, 184 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index e1a8d4c..69e2502 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2018,6 +2018,38 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorDumpToFd(qemuMonitorPtr mon,
+unsigned int flags,
+int fd,
+unsigned long long begin,
+unsigned long long length)
+{
+int ret;
+VIR_DEBUG("mon=%p fd=%d flags=%x begin=%llx length=%llx",
+  mon, fd, flags, begin, length);
+
+if (!mon) {
+qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+_("monitor must not be NULL"));
+return -1;
+}
+
+if (qemuMonitorSendFileHandle(mon, "dump", fd) < 0)
+return -1;
+
+if (mon->json)
+ret = qemuMonitorJSONDump(mon, flags, "fd:dump", begin, length);
+else
+ret = qemuMonitorTextDump(mon, flags, "fd:dump", begin, length);
+
+if (ret < 0) {
+if (qemuMonitorCloseFileHandle(mon, "dump") < 0)
+VIR_WARN("failed to close dumping handle");
+}
+
+return ret;
+}
 
 int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int type,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index b480966..315cb9e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -379,6 +379,19 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
 
 int qemuMonitorMigrateCancel(qemuMonitorPtr mon);
 
+typedef enum {
+  QEMU_MONITOR_DUMP_HAVE_FILTER  = 1 << 0,
+  QEMU_MONITOR_DUMP_PAGING   = 1 << 1,
+  QEMU_MONITOR_DUMP_FLAGS_LAST
+} QEMU_MONITOR_DUMP;
+
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorDumpToFd(qemuMonitorPtr mon,
+unsigned int flags,
+int fd,
+unsigned long long begin,
+unsigned long long length);
+
 int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int type,
 const char *hostname,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index eeeb6a6..57bc788 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2413,6 +2413,48 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon)
 return ret;
 }
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorJSONDump(qemuMonitorPtr mon,
+unsigned int flags,
+const char *protocol,
+unsigned long long begin,
+unsigned long long length)
+{
+int ret;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+
+if (flags & QEMU_MONITOR_DUMP_HAVE_FILTER)
+cmd = qemuMonitorJSONMakeCommand("dump-guest-memory",
+ "b:paging", flags & 
QEMU_MONITOR_DUMP_PAGING ? 1 : 0,
+ "s:protocol", protocol,
+ "U:begin", begin,
+ "U:length", length,
+ NULL);
+else
+cmd = qemuMonitorJSONMakeCommand("dump-guest-memory",
+ "b:paging", flags & 
QEMU_MONITOR_DUMP_PAGING ? 1 : 0,
+ "s:protocol", protocol,
+ NULL);
+if (!cmd)
+return -1;
+
+ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+if (ret == 0) {
+if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ret = -2;
+goto cleanup;
+}
+
+ret = qemuMonitorJSONCheckError(cmd, reply);
+}
+
+cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
 
 int qemuMonitorJSONGraphicsRelocate(qemuMonitorPtr mon,
 int type,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index a0f67aa..a8b70d4 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -136,6 +136,13 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon,
 
 int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon);
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+int qemuMonitorJSONDump(qemuMonitorPtr mon,

[libvirt] [PATCH 2/2] qemu: try to use qemu's dump-guest-meory when vm uses host device

2012-04-01 Thread Wen Congyang
---
 src/qemu/qemu_domain.c |1 +
 src/qemu/qemu_domain.h |1 +
 src/qemu/qemu_driver.c |   42 --
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 69d9e6e..e3a668a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -158,6 +158,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
 job->phase = 0;
 job->mask = DEFAULT_JOB_MASK;
 job->start = 0;
+job->qemu_dump = false;
 memset(&job->info, 0, sizeof(job->info));
 }
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index adccfed..f1ab0e6 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -97,6 +97,7 @@ struct qemuDomainJobObj {
 int phase;  /* Job phase (mainly for migrations) */
 unsigned long long mask;/* Jobs allowed during async job */
 unsigned long long start;   /* When the async job started */
+bool qemu_dump; /* use qemu dump to do dump */
 virDomainJobInfo info;  /* Async job progress data */
 };
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d503deb..0186d60 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2922,6 +2922,31 @@ cleanup:
 return ret;
 }
 
+/* Return 0 on success, -1 on failure, or -2 if not supported. */
+static int qemuDumpToFd(struct qemud_driver *driver, virDomainObjPtr vm,
+int fd, enum qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret = -1;
+
+if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD))
+return -2;
+
+if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
+  fd) < 0)
+return -1;
+
+priv->job.qemu_dump = true;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+ret = qemuMonitorDumpToFd(priv->mon, 0, fd, 0, 0);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+return ret;
+}
+
 static int
 doCoreDump(struct qemud_driver *driver,
virDomainObjPtr vm,
@@ -2953,6 +2978,19 @@ doCoreDump(struct qemud_driver *driver,
NULL, NULL)) < 0)
 goto cleanup;
 
+if (vm->def->nhostdevs > 0) {
+/*
+ * If the guest uses host devices, migrate command will fail. So we
+ * should use dump command.
+ *
+ * qemu dump does not support fd that is associated with a pipe,
+ * socket, or FIFO. So we should call qemuDumpTpFd() before calling
+ * virFileWrapperFdNew(). */
+ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP);
+if (ret != -2)
+goto cleanup;
+}
+
 if (!(wrapperFd = virFileWrapperFdNew(&fd, path, flags)))
 goto cleanup;
 
@@ -9298,7 +9336,7 @@ static int qemuDomainGetJobInfo(virDomainPtr dom,
 priv = vm->privateData;
 
 if (virDomainObjIsActive(vm)) {
-if (priv->job.asyncJob) {
+if (priv->job.asyncJob && !priv->job.qemu_dump) {
 memcpy(info, &priv->job.info, sizeof(*info));
 
 /* Refresh elapsed time again just to ensure it
@@ -9356,7 +9394,7 @@ static int qemuDomainAbortJob(virDomainPtr dom) {
 
 priv = vm->privateData;
 
-if (!priv->job.asyncJob) {
+if (!priv->job.asyncJob || priv->job.qemu_dump) {
 qemuReportError(VIR_ERR_OPERATION_INVALID,
 "%s", _("no job is active on the domain"));
 goto endjob;
-- 
1.7.1

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


[libvirt] [PATCH 0/2] use qemu's dump-guest-meory when vm uses host device

2012-04-01 Thread Wen Congyang
Currently, we use migrate to dump guest's memory. There is one
restriction in migrate command: the device's status should be
stored in qemu because the device's status should be passed to
target machine.

If we passthrough a host device to guest, the device's status
is stored in the real device. So migrate command will fail.

We usually use dump when guest is panicked. So there is no need
to store device's status in the vmcore.

qemu will have a new monitor command dump-guest-memory to dump
guest memory, but it doesn't support async now(it will support
later when the common async API is implemented).

So I use dump-guest-memory only when the guest uses host device
in this patchset.

Wen Congyang (2):
  qemu: implement qemu's dump-guest-memory
  qemu: try to use qemu's dump-guest-meory when vm uses host device

 src/qemu/qemu_domain.c   |1 +
 src/qemu/qemu_domain.h   |1 +
 src/qemu/qemu_driver.c   |   42 -
 src/qemu/qemu_monitor.c  |   32 
 src/qemu/qemu_monitor.h  |   13 +++
 src/qemu/qemu_monitor_json.c |   42 +
 src/qemu/qemu_monitor_json.h |7 
 src/qemu/qemu_monitor_text.c |   83 ++
 src/qemu/qemu_monitor_text.h |7 
 9 files changed, 226 insertions(+), 2 deletions(-)

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


Re: [libvirt] [PATCH] fix deadlock when qemu cannot start

2012-03-25 Thread Wen Congyang
ping

At 03/16/2012 02:37 PM, Wen Congyang Wrote:
> When qemu cannot start, we may call qemuProcessStop() twice.
> We have check whether the vm is running at the beginning of
> qemuProcessStop() to avoid libvirt deadlock. We call
> qemuProcessStop() with driver and vm locked. It seems that
> we can avoid libvirt deadlock. But unfortunately we may
> unlock driver and vm in the function qemuProcessKill() while
> vm->def->id is not -1. So qemuProcessStop() will be run twice,
> and monitor will be freed unexpectedly. So we should set
> vm->def->id to -1 at the beginning of qemuProcessStop().
> 
> ---
>  src/qemu/qemu_process.c |   20 ++--
>  src/qemu/qemu_process.h |2 ++
>  2 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 0af3751..44814df 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3633,10 +3633,11 @@ qemuProcessKill(struct qemud_driver *driver,
>  VIR_DEBUG("vm=%s pid=%d flags=%x",
>vm->def->name, vm->pid, flags);
>  
> -if (!virDomainObjIsActive(vm)) {
> -VIR_DEBUG("VM '%s' not active", vm->def->name);
> -return 0;
> -}
> +if (!(flags & VIR_QEMU_PROCESS_KILL_NOCHECK))
> +if (!virDomainObjIsActive(vm)) {
> +VIR_DEBUG("VM '%s' not active", vm->def->name);
> +return 0;
> +}
>  
>  /* This loop sends SIGTERM (or SIGKILL if flags has
>   * VIR_QEMU_PROCESS_KILL_FORCE and VIR_QEMU_PROCESS_KILL_NOWAIT),
> @@ -3739,6 +3740,13 @@ void qemuProcessStop(struct qemud_driver *driver,
>  return;
>  }
>  
> +/*
> + * We may unlock driver and vm in qemuProcessKill(), so the other thread
> + * can lock driver and vm, and then call qemuProcessStop(). So we should
> + * set vm->def->id to -1 here to avoid qemuProcessStop() called twice.
> + */
> +vm->def->id = -1;
> +
>  if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) {
>  /* To not break the normal domain shutdown process, skip the
>   * timestamp log writing if failed on opening log file. */
> @@ -3801,7 +3809,8 @@ void qemuProcessStop(struct qemud_driver *driver,
>  }
>  
>  /* shut it off for sure */
> -ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
> +ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE|
> + VIR_QEMU_PROCESS_KILL_NOCHECK));
>  
>  /* Stop autodestroy in case guest is restarted */
>  qemuProcessAutoDestroyRemove(driver, vm);
> @@ -3892,7 +3901,6 @@ retry:
>  
>  vm->taint = 0;
>  vm->pid = -1;
> -vm->def->id = -1;
>  virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
>  VIR_FREE(priv->vcpupids);
>  priv->nvcpupids = 0;
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index 761db6f..891f7a5 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -72,6 +72,8 @@ int qemuProcessAttach(virConnectPtr conn,
>  typedef enum {
> VIR_QEMU_PROCESS_KILL_FORCE  = 1 << 0,
> VIR_QEMU_PROCESS_KILL_NOWAIT = 1 << 1,
> +   VIR_QEMU_PROCESS_KILL_NOCHECK = 1 << 2, /* donot check whether the vm is
> +  running */
>  } virQemuProcessKillMode;
>  
>  int qemuProcessKill(struct qemud_driver *driver,

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


Re: [libvirt] problems starting several qemu VMS simultaneously

2012-03-21 Thread Wen Congyang
At 03/22/2012 06:54 AM, Serge Hallyn Wrote:
> Hi,
> 
> I grabbed today's git head of libvirt.  Created a VM (clean install of
> ubuntu oneiric, installed through virt-manager), and cloned it 3 times.
> Then I did
> 
> serge@ubuntu:~$ for i in `seq 1 4`; do virsh start o$i  > /tmp/o$i 2>&1 & done
> [1] 12184
> [2] 12185
> [3] 12186
> [4] 12187
> serge@ubuntu:~$ virsh list
> error: Failed to list active domains
> error: End of file while reading data: Input/output error

I cannot reproduce this problem on RHEL6.

> 
> serge@ubuntu:~$ virsh list
>  IdName   State
> 
>  5 o2 shut off
>  7 o3 shut off
>  8 o4 shut off
> 
> [1]   Exit 1  virsh start o$i > /tmp/o$i 2>&1
> [2]   Donevirsh start o$i > /tmp/o$i 2>&1
> serge@ubuntu:~$ virsh list
>  IdName   State
> 
>  5 o2 running
>  7 o3 running
>  8 o4 running
> 
> [3]-  Donevirsh start o$i > /tmp/o$i 2>&1
> [4]+  Donevirsh start o$i > /tmp/o$i 2>&1
> serge@ubuntu:~$ cat /tmp/o1
> error: Failed to start domain o1
> error: Unable to wait for child process: Bad file descriptor
> 
> It's quite reproducible.  An ubuntu bug report was filed at
> https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/961217
> 
> It's also very reminiscent of some lxc startup handshake issues
> which have been fixed.
> 
> You can find the last chunk of the log file at
> http://people.canonical.com/~serge/libvirtd-parallel-startup.log

This url cannot be opened:
You don't have permission to access /~serge/libvirtd-parallel-startup.log on 
this server.

Thanks
Wen Congyang

> 
> -serge
> 
> --
> 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 v2] Fixed NULL pointer check

2012-03-19 Thread Wen Congyang
At 03/19/2012 05:38 PM, Michal Privoznik Wrote:
> On 19.03.2012 08:55, Martin Kletzander wrote:
>> This patch fixes a NULL pointer check that was causing SegFault on
>> some specific configurations.
>> ---
>>  src/util/conf.c |5 -
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/util/conf.c b/src/util/conf.c
>> index 8ad60e0..3370337 100644
>> --- a/src/util/conf.c
>> +++ b/src/util/conf.c
>> @@ -1,7 +1,7 @@
>>  /**
>>   * conf.c: parser for a subset of the Python encoded Xen configuration files
>>   *
>> - * Copyright (C) 2006-2011 Red Hat, Inc.
>> + * Copyright (C) 2006-2012 Red Hat, Inc.
>>   *
>>   * See COPYING.LIB for the License of this software
>>   *
>> @@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
>>  {
>>  virConfEntryPtr cur;
>>
>> +if (conf == NULL)
>> +return NULL;
>> +
>>  cur = conf->entries;
>>  while (cur != NULL) {
>>  if ((cur->name != NULL) &&
> 
> Looking good, but I think we should revert 59d0c9801c1ab then.

I agree with reverting this commit. It does not fix all space.

> In addition - maybe we can set ATTRIBUTE_RETURN_CHECK to this function.

Agree with it.

Thanks
Wen Congyang

> 
> Michal
> 
> --
> 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] Fixed NULL pointer check

2012-03-19 Thread Wen Congyang
At 03/18/2012 08:29 AM, Martin Kletzander Wrote:
> This patch fixes a NULL pointer check that was causing SegFault on
> some specific configurations.
> ---
>  src/util/conf.c |5 -
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/src/util/conf.c b/src/util/conf.c
> index 8ad60e0..e76362c 100644
> --- a/src/util/conf.c
> +++ b/src/util/conf.c
> @@ -1,7 +1,7 @@
>  /**
>   * conf.c: parser for a subset of the Python encoded Xen configuration files
>   *
> - * Copyright (C) 2006-2011 Red Hat, Inc.
> + * Copyright (C) 2006-2012 Red Hat, Inc.
>   *
>   * See COPYING.LIB for the License of this software
>   *
> @@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
>  {
>  virConfEntryPtr cur;
> 
> +if (conf == NULL)
> +return(NULL);

Please use 'return NULL;' instead of 'return(NULL);'

'return(NULL);' is old style, and we donot use it now and later.

> +
>  cur = conf->entries;
>  while (cur != NULL) {
>  if ((cur->name != NULL) &&

ACK with the nit fixed.

Wen Congyang

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


[libvirt] [PATCH] fix deadlock when qemu cannot start

2012-03-15 Thread Wen Congyang
When qemu cannot start, we may call qemuProcessStop() twice.
We have check whether the vm is running at the beginning of
qemuProcessStop() to avoid libvirt deadlock. We call
qemuProcessStop() with driver and vm locked. It seems that
we can avoid libvirt deadlock. But unfortunately we may
unlock driver and vm in the function qemuProcessKill() while
vm->def->id is not -1. So qemuProcessStop() will be run twice,
and monitor will be freed unexpectedly. So we should set
vm->def->id to -1 at the beginning of qemuProcessStop().

---
 src/qemu/qemu_process.c |   20 ++--
 src/qemu/qemu_process.h |2 ++
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0af3751..44814df 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3633,10 +3633,11 @@ qemuProcessKill(struct qemud_driver *driver,
 VIR_DEBUG("vm=%s pid=%d flags=%x",
   vm->def->name, vm->pid, flags);
 
-if (!virDomainObjIsActive(vm)) {
-VIR_DEBUG("VM '%s' not active", vm->def->name);
-return 0;
-}
+if (!(flags & VIR_QEMU_PROCESS_KILL_NOCHECK))
+if (!virDomainObjIsActive(vm)) {
+VIR_DEBUG("VM '%s' not active", vm->def->name);
+return 0;
+}
 
 /* This loop sends SIGTERM (or SIGKILL if flags has
  * VIR_QEMU_PROCESS_KILL_FORCE and VIR_QEMU_PROCESS_KILL_NOWAIT),
@@ -3739,6 +3740,13 @@ void qemuProcessStop(struct qemud_driver *driver,
 return;
 }
 
+/*
+ * We may unlock driver and vm in qemuProcessKill(), so the other thread
+ * can lock driver and vm, and then call qemuProcessStop(). So we should
+ * set vm->def->id to -1 here to avoid qemuProcessStop() called twice.
+ */
+vm->def->id = -1;
+
 if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) {
 /* To not break the normal domain shutdown process, skip the
  * timestamp log writing if failed on opening log file. */
@@ -3801,7 +3809,8 @@ void qemuProcessStop(struct qemud_driver *driver,
 }
 
 /* shut it off for sure */
-ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
+ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE|
+ VIR_QEMU_PROCESS_KILL_NOCHECK));
 
 /* Stop autodestroy in case guest is restarted */
 qemuProcessAutoDestroyRemove(driver, vm);
@@ -3892,7 +3901,6 @@ retry:
 
 vm->taint = 0;
 vm->pid = -1;
-vm->def->id = -1;
 virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
 VIR_FREE(priv->vcpupids);
 priv->nvcpupids = 0;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 761db6f..891f7a5 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -72,6 +72,8 @@ int qemuProcessAttach(virConnectPtr conn,
 typedef enum {
VIR_QEMU_PROCESS_KILL_FORCE  = 1 << 0,
VIR_QEMU_PROCESS_KILL_NOWAIT = 1 << 1,
+   VIR_QEMU_PROCESS_KILL_NOCHECK = 1 << 2, /* donot check whether the vm is
+  running */
 } virQemuProcessKillMode;
 
 int qemuProcessKill(struct qemud_driver *driver,
-- 
1.7.1

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


[libvirt] [PATCH] hold the reference when call monitor's callback

2012-03-15 Thread Wen Congyang
When qemu cannot startup, we will call EOF notify callback.
Unfortunately, there is a bug in libvirt, and it will cause
mon->ref to be 0 while we call EOF notify callback. This
bug will cause the libvirt to be deadlock.

We call the other callback while holding the reference. So
I think we should also hold the reference here.

Note: this patch does not fix the bug. The libvirt will be
deadlock in qemuMonitorUnwatch() because the monitor is freed
unexpectedly.

I am still investigating this bug. But if I set a breakpoint
in qemuMonitorUnref(), it does not happen now. If i remove
the breakpoint, it will happen sometime(the probability is
about 20%).

The steps to reproduce this bug:
1. use newest qemu-kvm
2. add a host pci device into guest config file
3. start the guest

The qemu will fail with the following error message:
Failed to assign irq for "hostdev0": Input/output error
Perhaps you are assigning a device that shares an IRQ with another device?

I have reported qemu's problem to qemu maillist.

---
 src/qemu/qemu_monitor.c |   16 
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 78eb492..20eb9ea 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -668,10 +668,14 @@ qemuMonitorIO(int watch, int fd, int events, void 
*opaque) {
 
 /* Make sure anyone waiting wakes up now */
 virCondSignal(&mon->notify);
-if (qemuMonitorUnref(mon) > 0)
-qemuMonitorUnlock(mon);
+
+/* hold the reference when call monitor's callback to avoid deadlock */
+qemuMonitorUnlock(mon);
 VIR_DEBUG("Triggering EOF callback");
 (eofNotify)(mon, vm);
+qemuMonitorLock(mon);
+if (qemuMonitorUnref(mon) > 0)
+qemuMonitorUnlock(mon);
 } else if (error) {
 void (*errorNotify)(qemuMonitorPtr, virDomainObjPtr)
 = mon->cb->errorNotify;
@@ -679,10 +683,14 @@ qemuMonitorIO(int watch, int fd, int events, void 
*opaque) {
 
 /* Make sure anyone waiting wakes up now */
 virCondSignal(&mon->notify);
-if (qemuMonitorUnref(mon) > 0)
-qemuMonitorUnlock(mon);
+
+/* hold the reference when call monitor's callback to avoid deadlock */
+qemuMonitorUnlock(mon);
 VIR_DEBUG("Triggering error callback");
 (errorNotify)(mon, vm);
+qemuMonitorLock(mon);
+if (qemuMonitorUnref(mon) > 0)
+qemuMonitorUnlock(mon);
 } else {
 if (qemuMonitorUnref(mon) > 0)
 qemuMonitorUnlock(mon);
-- 
1.7.1

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


Re: [libvirt] question about guest status

2012-02-15 Thread Wen Congyang
At 02/16/2012 11:04 AM, KAMEZAWA Hiroyuki Wrote:
> On Wed, 15 Feb 2012 11:00:25 +
> "Daniel P. Berrange"  wrote:
> 
>> On Wed, Feb 15, 2012 at 02:44:18PM +0800, Wen Congyang wrote:
>>> Hi all:
>>>
>>> If the guest runs on xen, we can know the guest OS paniced, and the guest 
>>> status
>>> is paniced. But if the guest runs on qemu/kvm, we need the same feature. 
>>> And the
>>> feature to know guest-is-panic is a very very good feature for enterprise.
>>>
>>> We have some ways to know that guest has paniced:
>>> 1. watchdog timeouts
>>>It is not a good way, because we can not say "the guest is paniced" in 
>>> 100% accuracy
>>> 2. use a PV driver(like the XEN)
>>>The old RHELs do not have such driver...
>>>It can not work for the second kernel...
>>> 3. Adding a function for console-logging (in ring buffer or some) and check 
>>> messages
>>>given by guests. If we allow some filtering script or some, we can catch 
>>> Linux's
>>>panic messages. We can say the guest paniced if we catch such messages. 
>>> We may be
>>>able to catch other events by other script
>>>
>>> I prefer to 3 now. Does anyone have a better idea?
>>
>> I have discussed this with Anthony before and the thought was that QEMU/KVM
>> should provide some kind of paravirtualized crash notification service to
>> the guest. ie option 2.  such a device would likely be quite simple and
>> thus possible to backport to older kernels as needed
>>
> AFAIK, linux kernel has notifier at panic but it doesn't work if kdump is 
> enabled.
> And IIRC, There was a discussion in lkml that any modules should not be 
> called when

If CONFIG_KVM_GUEST is yes, the kernel can know that it runs on kvm. The 
detecting code
is also built into kernel. So if we decide to use PV driver, we can put the code
in arch/x86/kerne/kvm.c.

> crash_kexec() is called.  So, no chance to notify it from the module in the 
> 1st kernel, now.
> Calling it from 2nd kernel will work but the bahavior will be different from
> non-kdump guest.
> 
> Can't we do notify by hypercall ? But this will require guest modification..

Yes, xen requires the same modification.

Thanks
Wen Congyang

> 
> Thanks,
> -Kame
> 
> 
> 
> 
> 
> 

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


Re: [libvirt] [PATCH] daemon: fix logic bug with virAsprintf

2012-02-14 Thread Wen Congyang
At 02/15/2012 07:03 AM, Eric Blake Wrote:
> Regression introduced in commit 7033c5f2, due to improper conversion
> from snprintf to virAsprintf.
> 
> * daemon/remote.c (remoteDispatchAuthList): Check return value
> correctly.
> ---
> 
> This one's embarrassing.  I think I broke polkit authorization in
> 0.9.10.  :(
> 
>  daemon/remote.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 9c61306..724db23 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -2052,7 +2052,7 @@ remoteDispatchAuthList(virNetServerPtr server 
> ATTRIBUTE_UNUSED,
>  } else if (callerUid == 0) {
>  char *ident;
>  if (virAsprintf(&ident, "pid:%lld,uid:%d",
> -(long long) callerPid, callerUid) == 0) {
> +(long long) callerPid, callerUid) >= 0) {
>  VIR_INFO("Bypass polkit auth for privileged client %s",
>       ident);
>  if (virNetServerClientSetIdentity(client, ident) < 0)

ACK

Wen Congyang

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


[libvirt] question about guest status

2012-02-14 Thread Wen Congyang
Hi all:

If the guest runs on xen, we can know the guest OS paniced, and the guest status
is paniced. But if the guest runs on qemu/kvm, we need the same feature. And the
feature to know guest-is-panic is a very very good feature for enterprise.

We have some ways to know that guest has paniced:
1. watchdog timeouts
   It is not a good way, because we can not say "the guest is paniced" in 100% 
accuracy
2. use a PV driver(like the XEN)
   The old RHELs do not have such driver...
   It can not work for the second kernel...
3. Adding a function for console-logging (in ring buffer or some) and check 
messages
   given by guests. If we allow some filtering script or some, we can catch 
Linux's
   panic messages. We can say the guest paniced if we catch such messages. We 
may be
   able to catch other events by other script

I prefer to 3 now. Does anyone have a better idea?

Thanks
Wen Congyang

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


Re: [libvirt] [PATCH 3/3] spec: fix sanlock dependency

2011-12-13 Thread Wen Congyang
At 12/06/2011 01:38 AM, Eric Blake Write:
> * libvirt.spec.in (with_sanlock): On RHEL, don't force sanlock
> on architectures where it isn't available.
> ---
>  libvirt.spec.in |   10 --
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 35762a5..39c814a 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -174,8 +174,14 @@
>  %endif
> 
>  # Enable sanlock library for lock management with QEMU
> -%if 0%{?fedora} >= 16 || 0%{?rhel} >= 6
> -%define with_sanlock  0%{!?_without_sanlock:%{server_drivers}}
> ++# Sanlock is available only on i686 x86_64 for RHEL

According this comment, force sanlock  only on i686 x86_64 for RHEL

> +%if 0%{?fedora} >= 16
> +%define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
> +%endif
> +%if 0%{?rhel} >= 6
> +%ifnarch i386 i586 i686 x86_64

But here you force it only not on i686 x86_64 for RHEL.

s/ifnarch/ifarch/ ?

Thanks
Wen Congyang

> +%define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
> +%endif
>  %endif
> 
>  # Disable some drivers when building without libvirt daemon.

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


Re: [libvirt] [PATCH] virsh: support multifunction in attach-disk

2011-12-12 Thread Wen Congyang
At 12/13/2011 02:12 PM, KAMEZAWA Hiroyuki Write:
> From: KAMEZAWA Hiroyuki 
> 
> PCI  can be specified by attach-disk but multifunction cannot
> be specified. add --multifunction support.

IIRC, libvirt does not support hot plugging multifunction PCI device. Why
do you need this feature?

Thanks
Wen Congyang

> ---
>  tools/virsh.c   |7 ++-
>  tools/virsh.pod |3 ++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index d58b827..346b440 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -12661,6 +12661,7 @@ static const vshCmdOptDef opts_attach_disk[] = {
>  {"serial", VSH_OT_STRING, 0, N_("serial of disk device")},
>  {"shareable", VSH_OT_BOOL, 0, N_("shareable between domains")},
>  {"address", VSH_OT_STRING, 0, N_("address of disk device")},
> +{"multifunction", VSH_OT_BOOL, 0, N_("use multifunction pci under 
> specified address")},
>  {NULL, 0, 0, NULL}
>  };
>  
> @@ -12916,9 +12917,13 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>  if (diskAddr.type == DISK_ADDR_TYPE_PCI) {
>  virBufferAsprintf(&buf,
>"   -  " bus ='0x%02x' slot='0x%02x' 
> function='0x%0x' />\n",
> +  " bus ='0x%02x' slot='0x%02x' 
> function='0x%0x'",
>diskAddr.addr.pci.domain, 
> diskAddr.addr.pci.bus,
>diskAddr.addr.pci.slot, 
> diskAddr.addr.pci.function);
> +if (vshCommandOptBool(cmd, "multifunction"))
> +virBufferAsprintf(&buf, " multifunction='on' />\n");
> +else
> +virBufferAsprintf(&buf, " />\n");
>  } else {
>  vshError(ctl, "%s", _("expecting a pci:.00.00.00 
> address."));
>  goto cleanup;
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index fe92714..1a778f9 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1273,7 +1273,7 @@ the device does not use managed mode.
>  =item B I I I
>  [I<--driver driver>] [I<--subdriver subdriver>] [I<--cache cache>]
>  [I<--type type>] [I<--mode mode>] [I<--persistent>] [I<--sourcetype 
> soucetype>]
> -[I<--serial serial>] [I<--shareable>] [I<--address address>]
> +[I<--serial serial>] [I<--shareable>] [I<--address address>] 
> [I<--multifunction>]
>  
>  Attach a new disk device to the domain.
>  I and I are paths for the files and devices.
> @@ -1291,6 +1291,7 @@ I is the serial of disk device. I 
> indicates the disk device
>  is shareable between domains.
>  I is the address of disk device in the form of 
> pci:domain.bus.slot.function,
>  scsi:controller.bus.unit or ide:controller.bus.unit.
> +I indicates specified pci address is a multifunction pci 
> device address.
>  
>  =item B I I I
>  [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>]


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


Re: [libvirt] 'make check' hangs

2011-11-30 Thread Wen Congyang
At 11/30/2011 11:17 PM, Eric Blake Write:
> On 11/30/2011 04:40 AM, Daniel P. Berrange wrote:
>>> Hmm, I suspect this may be caused by
>>>
>>> commit fd7e85ac6af833845aa0eb2526158c319800a0ae
>>> Author: Jiri Denemark 
>>> Date:   Tue Oct 11 15:05:52 2011 +0200
>>>
>>> virsh: Always run event loop
>>>
>>> Since virsh already implements event loop, it has to also run it. So far
>>> the event loop was only running during virsh console command.
>>>
>>
>> Ahhh, I had discounted that because the date was Oct 11, and I figured
>> we would have seen it by now. But that's just your original commit date,
>> the merge date was Oct 24th. So yeah, I reckon this must be the culprit
> 
> I've been seeing this fairly often (about 25% of my 'make check' runs)
> since at least last Wednesday, on F16 when compiling at -O0; and the
> analysis of a race caused by not locking around ctl->quit seems like a
> reasonable culprit.
> 

This may be one reason. I think there is some bug in testdriver, and it
will also cause 'make check' hang. Here is the backtrace when 'make check'
hung:

(gdb) info threads 
  2 Thread 0x7f22e81da700 (LWP 25242)  0x003bdce0dff4 in __lll_lock_wait () 
from /lib64/libpthread.so.0
* 1 Thread 0x7f22e81db800 (LWP 25227)  0x003bdce0804d in pthread_join () 
from /lib64/libpthread.so.0
(gdb) bt
#0  0x003bdce0804d in pthread_join () from /lib64/libpthread.so.0
#1  0x0041c193 in vshDeinit (ctl=0x7fff6098df20) at virsh.c:17506
#2  0x00425251 in main (argc=, 
argv=0x7fff6098e0d8) at virsh.c:17852
(gdb) thread 2
[Switching to thread 2 (Thread 0x7f22e81da700 (LWP 25242))]#0  
0x003bdce0dff4 in __lll_lock_wait () from /lib64/libpthread.so.0
(gdb) bt
#0  0x003bdce0dff4 in __lll_lock_wait () from /lib64/libpthread.so.0
#1  0x003bdce09328 in _L_lock_854 () from /lib64/libpthread.so.0
#2  0x003bdce091f7 in pthread_mutex_lock () from /lib64/libpthread.so.0
#3  0x7f22e84f6e9c in testDriverLock (timer=, 
opaque=0x2192e10) at test/test_driver.c:127
#4  testDomainEventFlush (timer=, opaque=0x2192e10) at 
test/test_driver.c:5515
#5  0x7f22e8459a08 in virEventPollDispatchTimeouts () at 
util/event_poll.c:440
#6  virEventPollRunOnce () at util/event_poll.c:633
#7  0x7f22e8458a27 in virEventRunDefaultImpl () at util/event.c:247
#8  0x0041bf83 in vshEventLoop (opaque=0x7fff6098df20) at virsh.c:17044
#9  0x7f22e846a862 in virThreadHelper (data=) at 
util/threads-pthread.c:157
#10 0x003bdce077f1 in start_thread () from /lib64/libpthread.so.0
#11 0x003bdc2e570d in clone () from /lib64/libc.so.6

According to the backtrace, lt-virsh is deadlock.

Thanks
Wen Congyang

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


Re: [libvirt] [PATCH 2/6] qemu: Plug memory leak on qemuProcessWaitForMonitor() error path

2011-11-29 Thread Wen Congyang
At 11/30/2011 02:32 PM, Alex Jia Write:
> On 11/30/2011 02:20 PM, Wen Congyang wrote:
>> At 11/30/2011 01:57 PM, a...@redhat.com Write:
>>> From: Alex Jia
>>>
>>> Detected by Coverity. Leak introduced in commit 109efd7.
>>>
>>> Signed-off-by: Alex Jia
>>> ---
>>>   src/qemu/qemu_process.c |1 +
>>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>> index 2563f97..f3f44ca 100644
>>> --- a/src/qemu/qemu_process.c
>>> +++ b/src/qemu/qemu_process.c
>>> @@ -1224,6 +1224,7 @@ qemuProcessWaitForMonitor(struct qemud_driver*
>>> driver,
>>>
>>>   if (VIR_ALLOC_N(buf, buf_size)<  0) {
>>>   virReportOOMError();
>>> +VIR_FORCE_CLOSE(logfd);
>>>   return -1;
>> I think it is better to goto closelog
> Yeah, I think so before, but 'closelog' label will free 'buf', in fact,
> we haven't successfully allocate

buf is inited to NULL, so it is safe to use VIR_FREE(buf)

> memory to 'buf' variable, I'm not sure whether it is a issue. maybe, the
> above is a simple way, otherwise, it should be better if we add a check
> for 'buf' variable in 'closelog' label, it looks like this:
> ...
> if (buf)
>  VIR_FREE(buf);

If buf is NULL, VIR_FREE(bus) does nothing and is safe.
If you add a check for 'buf', make syntax-check will fail.

Thansk
Wen Congyang

> ...
> 
> Chongyang, please correct me if I'm wrong :)
> 
> Thanks,
> Alex
>> Thanks
>> Wen Congyang
>>
>>>   }
>>
> 
> 

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


Re: [libvirt] [PATCH 3/6] rpc: Plug memory leak on virNetClientSendInternal() error path

2011-11-29 Thread Wen Congyang
At 11/30/2011 01:57 PM, a...@redhat.com Write:
> From: Alex Jia 
> 
> Detected by Coverity. Leak introduced in commit 673adba.
> 
> Signed-off-by: Alex Jia 
> ---
>  src/rpc/virnetclient.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> index a738129..9ed03a5 100644
> --- a/src/rpc/virnetclient.c
> +++ b/src/rpc/virnetclient.c
> @@ -1740,6 +1740,7 @@ cleanup:
>  
>  unlock:
>  virNetClientUnlock(client);
> +VIR_FREE(call);

According to the comment:
/* If partially sent, then the call is still on the dispatch queue */

So I think we should not free call if it is still on the queue.

Thanks
Wen Congyang

>  return ret;
>  }
>  

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


Re: [libvirt] [PATCH 2/6] qemu: Plug memory leak on qemuProcessWaitForMonitor() error path

2011-11-29 Thread Wen Congyang
At 11/30/2011 01:57 PM, a...@redhat.com Write:
> From: Alex Jia 
> 
> Detected by Coverity. Leak introduced in commit 109efd7.
> 
> Signed-off-by: Alex Jia 
> ---
>  src/qemu/qemu_process.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 2563f97..f3f44ca 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1224,6 +1224,7 @@ qemuProcessWaitForMonitor(struct qemud_driver* driver,
>  
>  if (VIR_ALLOC_N(buf, buf_size) < 0) {
>  virReportOOMError();
> +VIR_FORCE_CLOSE(logfd);
>  return -1;

I think it is better to goto closelog

Thanks
Wen Congyang

>  }


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


[libvirt] 'make check' hangs

2011-11-29 Thread Wen Congyang
When I run 'make check', it hangs sometimes.

I use gdb to attach lt-virsh, and the following is the
backtrace:
(gdb) thread 1
[Switching to thread 1 (Thread 0x7fc6c04d5800 (LWP 24099))]#0  
0x003bdce0804d in pthread_join () from /lib64/libpthread.so.0
(gdb) bt
#0  0x003bdce0804d in pthread_join () from /lib64/libpthread.so.0
#1  0x0041c0b3 in vshDeinit (ctl=0x7fff015f4570) at virsh.c:17262
#2  0x004248d1 in main (argc=, 
argv=0x7fff015f4728) at virsh.c:17608
(gdb) thread 2
[Switching to thread 2 (Thread 0x7fc6c04d4700 (LWP 24138))]#0  
0x003bdc2dc053 in poll () from /lib64/libc.so.6
(gdb) bt
#0  0x003bdc2dc053 in poll () from /lib64/libc.so.6
#1  0x7fc6c075359c in virEventPollRunOnce () at util/event_poll.c:619
#2  0x7fc6c07527d7 in virEventRunDefaultImpl () at util/event.c:247
#3  0x0041bea3 in vshEventLoop (opaque=0x7fff015f4570) at virsh.c:16800
#4  0x7fc6c0764702 in virThreadHelper (data=) at 
util/threads-pthread.c:157
#5  0x003bdce077f1 in start_thread () from /lib64/libpthread.so.0
#6  0x003bdc2e570d in clone () from /lib64/libc.so.6
(gdb) 

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


  1   2   3   4   5   6   >