[libvirt] [PATCH 01/12] conf: numatune: Extract code for requesting memory nodeset from formatting

2015-01-28 Thread Peter Krempa
Extract the logic to determine which nodeset has to be used for a domain
from the formatting step so that it can be reused separately when the
nodeset is used in a different way.
---
 src/conf/numatune_conf.c | 33 ++---
 src/conf/numatune_conf.h |  5 +
 src/libvirt_private.syms |  1 +
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index ad928e0..16610ed 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -400,13 +400,14 @@ virDomainNumatuneFormatNodeset(virDomainNumatunePtr 
numatune,
cellid));
 }

+
 int
-virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune,
-virBitmapPtr auto_nodeset,
-char **mask,
-int cellid)
+virDomainNumatuneMaybeGetNodeset(virDomainNumatunePtr numatune,
+ virBitmapPtr auto_nodeset,
+ virBitmapPtr *retNodeset,
+ int cellid)
 {
-*mask = NULL;
+*retNodeset = NULL;

 if (!numatune)
 return 0;
@@ -424,8 +425,26 @@ virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr 
numatune,
 return -1;
 }

-*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset, cellid);
-if (!*mask)
+*retNodeset = virDomainNumatuneGetNodeset(numatune, auto_nodeset, cellid);
+
+return 0;
+}
+
+
+int
+virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune,
+virBitmapPtr auto_nodeset,
+char **mask,
+int cellid)
+{
+virBitmapPtr nodeset;
+
+if (virDomainNumatuneMaybeGetNodeset(numatune, auto_nodeset, nodeset,
+ cellid)  0)
+return -1;
+
+if (nodeset 
+!(*mask = virBitmapFormat(nodeset)))
 return -1;

 return 0;
diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
index 7ca7f97..28c4ce2 100644
--- a/src/conf/numatune_conf.h
+++ b/src/conf/numatune_conf.h
@@ -70,6 +70,11 @@ virBitmapPtr 
virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune,
  virBitmapPtr auto_nodeset,
  int cellid);

+int virDomainNumatuneMaybeGetNodeset(virDomainNumatunePtr numatune,
+ virBitmapPtr auto_nodeset,
+ virBitmapPtr *retNodeset,
+ int cellid);
+
 /*
  * Formatters
  */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 75a6d83..2bbce03 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -625,6 +625,7 @@ virDomainNumatuneGetNodeset;
 virDomainNumatuneHasPerNodeBinding;
 virDomainNumatuneHasPlacementAuto;
 virDomainNumatuneMaybeFormatNodeset;
+virDomainNumatuneMaybeGetNodeset;
 virDomainNumatuneMemModeTypeFromString;
 virDomainNumatuneMemModeTypeToString;
 virDomainNumatuneNodesetIsAvailable;
-- 
2.2.2

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


[libvirt] [PATCH 06/12] qemu: command: Add helper to format -object strings from JSON representation

2015-01-28 Thread Peter Krempa
Unlike -device, qemu uses a JSON object to add backend objects via the
monitor rather than the string that would be passed on the commandline.

To be able to reuse code parts that configure backends for various
devices, this patch adds a helper that will allow generating the command
line representations from the JSON property object.
---
 src/qemu/qemu_command.c | 111 +
 src/qemu/qemu_command.h |   4 ++
 tests/Makefile.am   |  13 -
 tests/qemucommandutiltest.c | 118 
 4 files changed, 244 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemucommandutiltest.c

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 100deed..6f298ac 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -416,6 +416,117 @@ qemuDomainSupportsNetdev(virDomainDefPtr def,
 return virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV);
 }

+
+static int
+qemuBuildObjectCommandLinePropsInternal(const char *key,
+const virJSONValue *value,
+virBufferPtr buf,
+bool nested)
+{
+virJSONValuePtr elem;
+virBitmapPtr bitmap = NULL;
+ssize_t pos = -1;
+ssize_t end;
+size_t i;
+
+switch ((virJSONType) value-type) {
+case VIR_JSON_TYPE_STRING:
+virBufferAsprintf(buf, ,%s=%s, key, value-data.string);
+break;
+
+case VIR_JSON_TYPE_NUMBER:
+virBufferAsprintf(buf, ,%s=%s, key, value-data.number);
+break;
+
+case VIR_JSON_TYPE_BOOLEAN:
+if (value-data.boolean)
+virBufferAsprintf(buf, ,%s=yes, key);
+else
+virBufferAsprintf(buf, ,%s=no, key);
+
+break;
+
+case VIR_JSON_TYPE_ARRAY:
+if (nested) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(nested -object property arrays are not 
supported));
+return -1;
+}
+
+if (virJSONValueGetArrayAsBitmap(value, bitmap) == 0) {
+while ((pos = virBitmapNextSetBit(bitmap, pos))  -1) {
+if ((end = virBitmapNextClearBit(bitmap, pos))  0)
+end = virBitmapLastSetBit(bitmap) + 1;
+
+if (end - 1  pos) {
+virBufferAsprintf(buf, ,%s=%zd-%zd, key, pos, end - 1);
+pos = end;
+} else {
+virBufferAsprintf(buf, ,%s=%zd, key, pos);
+}
+}
+} else {
+/* fallback, treat the array as a non-bitmap, adding the key
+ * for each member */
+for (i = 0; i  virJSONValueArraySize(value); i++) {
+elem = virJSONValueArrayGet((virJSONValuePtr)value, i);
+
+/* recurse to avoid duplicating code */
+if (qemuBuildObjectCommandLinePropsInternal(key, elem, buf,
+true)  0)
+return -1;
+}
+}
+break;
+
+case VIR_JSON_TYPE_OBJECT:
+case VIR_JSON_TYPE_NULL:
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(NULL and OBJECT JSON types can't be converted to 
+ commandline string));
+return -1;
+}
+
+virBitmapFree(bitmap);
+return 0;
+}
+
+
+static int
+qemuBuildObjectCommandLineProps(const char *key,
+const virJSONValue *value,
+void *opaque)
+{
+return qemuBuildObjectCommandLinePropsInternal(key, value, opaque, false);
+}
+
+
+char *
+qemuBuildObjectCommandlineFromJSON(const char *type,
+   const char *alias,
+   virJSONValuePtr props)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+char *ret = NULL;
+
+virBufferAsprintf(buf, %s,id=%s, type, alias);
+
+if (virJSONValueObjectForeachKeyValue(props,
+  qemuBuildObjectCommandLineProps,
+  buf)  0)
+goto cleanup;
+
+if (virBufferCheckError(buf)  0)
+goto cleanup;
+
+ret = virBufferContentAndReset(buf);
+
+ cleanup:
+virBufferFreeAndReset(buf);
+return ret;
+}
+
+
 /**
  * qemuOpenVhostNet:
  * @def: domain definition
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 8eaf1e4..ae36bd8 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -69,6 +69,10 @@ struct _qemuBuildCommandLineCallbacks {

 extern qemuBuildCommandLineCallbacks buildCommandLineCallbacks;

+char *qemuBuildObjectCommandlineFromJSON(const char *type,
+ const char *alias,
+ virJSONValuePtr props);
+
 virCommandPtr qemuBuildCommandLine(virConnectPtr conn,

[libvirt] [PATCH 00/12] memory hotplug: Preliminary fixes and cleanups

2015-01-28 Thread Peter Krempa
This series was split out from my memory hotplug series that is not quite ready
yet. The aim of this series is to refactor the code that creates commandline
for the memory-backend-* qemu object so that it can be later reused for hotplug
via monitor. Additionally this series also fixes a bug in NUMA memory
initialisation where qemu doesn't support a combination of old and new approach.

Peter Krempa (12):
  conf: numatune: Extract code for requesting memory nodeset from
formatting
  test: utils: Add helpers for automatic numbering of test cases
  util: json: make value object creator universal by supporting adding
  util: json: Add functions to convert JSON arrays from/to virBitmaps
  util: json: add helper to iterate JSON object key=value pairs
  qemu: command: Add helper to format -object strings from JSON
representation
  qemu: Extract code to setup memory backing objects
  qemu: command: Shuffle around formating of alias for memory backend
objs
  qemu: command: Unify values for boolean values when formating memory
backends
  qemu: command: Switch to bytes when formatting size for memory
backends
  qemu: command: Refactor NUMA backend object formatting to use JSON
objs
  qemu: command: Don't combine old and modern NUMA node creation

 src/conf/numatune_conf.c   |  33 +-
 src/conf/numatune_conf.h   |   5 +
 src/libvirt_private.syms   |   6 +
 src/qemu/qemu_command.c| 547 +++--
 src/qemu/qemu_command.h|   4 +
 src/util/virjson.c | 220 -
 src/util/virjson.h |  17 +
 tests/Makefile.am  |  13 +-
 tests/qemucommandutiltest.c| 118 +
 .../qemuxml2argv-hugepages-pages.args  |  20 +-
 .../qemuxml2argv-hugepages-pages2.args |   8 +-
 .../qemuxml2argv-hugepages-pages3.args |   7 +-
 .../qemuxml2argv-hugepages-shared.args |  20 +-
 .../qemuxml2argv-numatune-memnode-no-memory.args   |   6 +-
 .../qemuxml2argv-numatune-memnode.args |   8 +-
 tests/testutils.c  |  46 ++
 tests/testutils.h  |   3 +
 17 files changed, 871 insertions(+), 210 deletions(-)
 create mode 100644 tests/qemucommandutiltest.c

-- 
2.2.2

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


[libvirt] [PATCH 02/12] test: utils: Add helpers for automatic numbering of test cases

2015-01-28 Thread Peter Krempa
Adding or reordering test cases is usually a pain due to static test
case names that are then passed to virtTestRun(). To ease the numbering
of test cases, this patch adds two simple helpers that generate the test
names according to the order they are run. The test name can be
configured via the reset function.

This will allow us to freely add test cases in mid of test groups
without the need to re-number the rest of test cases.
---
 tests/testutils.c | 46 ++
 tests/testutils.h |  3 +++
 2 files changed, 49 insertions(+)

diff --git a/tests/testutils.c b/tests/testutils.c
index 9a79f98..c7d2615 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -986,3 +986,49 @@ virDomainXMLOptionPtr virTestGenericDomainXMLConfInit(void)
  virTestGenericPrivateDataCallbacks,
  NULL);
 }
+
+
+static int virtTestCounter;
+static char virtTestCounterStr[128];
+static char *virtTestCounterPrefixEndOffset;
+
+
+/**
+ * virtTestCounterReset:
+ * @prefix: name of the test group
+ *
+ * Resets the counter and sets up the test group name to use with
+ * virtTestCounterNext(). This function is not thread safe.
+ */
+void
+virtTestCounterReset(const char *prefix)
+{
+virtTestCounter = 0;
+
+ignore_value(virStrcpyStatic(virtTestCounterStr, prefix));
+virtTestCounterPrefixEndOffset = strchrnul(virtTestCounterStr, '\0');
+}
+
+
+/**
+ * virtTestCounterNext:
+ *
+ * This function is designed to ease test creation and reordering by adding
+ * a way to do automagic test case numbring.
+ *
+ * Returns string consisting of test name prefix configured via
+ * virtTestCounterReset() and a number that increments in every call of this
+ * function. This function is not thread safe.
+ */
+const char
+*virtTestCounterNext(void)
+{
+size_t len = ARRAY_CARDINALITY(virtTestCounterStr);
+
+/* calculate length of the rest of the string */
+len -= (virtTestCounterPrefixEndOffset - virtTestCounterStr);
+
+snprintf(virtTestCounterPrefixEndOffset, len, %d, ++virtTestCounter);
+
+return virtTestCounterStr;
+}
diff --git a/tests/testutils.h b/tests/testutils.h
index d78818d..155b30f 100644
--- a/tests/testutils.h
+++ b/tests/testutils.h
@@ -82,6 +82,9 @@ char *virtTestLogContentAndReset(void);

 void virtTestQuiesceLibvirtErrors(bool always);

+void virtTestCounterReset(const char *prefix);
+const char *virtTestCounterNext(void);
+
 int virtTestMain(int argc,
  char **argv,
  int (*func)(void));
-- 
2.2.2

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


[libvirt] [PATCH 04/12] util: json: Add functions to convert JSON arrays from/to virBitmaps

2015-01-28 Thread Peter Krempa
To be able to easily represent nodesets and other data stored in
virBitmaps in libvirt, this patch introduces a set of helpers that allow
to convert the bitmap to and from JSON value objects.
---
 src/libvirt_private.syms |   2 +
 src/util/virjson.c   | 113 +++
 src/util/virjson.h   |   4 ++
 3 files changed, 119 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cc74e35..70c81a8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1514,6 +1514,7 @@ virJSONValueArrayGet;
 virJSONValueArraySize;
 virJSONValueFree;
 virJSONValueFromString;
+virJSONValueGetArrayAsBitmap;
 virJSONValueGetBoolean;
 virJSONValueGetNumberDouble;
 virJSONValueGetNumberInt;
@@ -1523,6 +1524,7 @@ virJSONValueGetNumberUlong;
 virJSONValueGetString;
 virJSONValueIsNull;
 virJSONValueNewArray;
+virJSONValueNewArrayFromBitmap;
 virJSONValueNewBoolean;
 virJSONValueNewNull;
 virJSONValueNewNumberDouble;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 9eb1bff..3e00650 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -99,6 +99,8 @@ struct _virJSONParser {
  *
  * a: json object, must be non-NULL
  * A: json object, omitted if NULL
+ * m: a bitmap represented as a JSON array, must be non-NULL
+ * M: a bitmap represented as a JSON array, omitted if NULL
  *
  * The value corresponds to the selected type.
  *
@@ -242,6 +244,28 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj,
 rc = virJSONValueObjectAppend(obj, key, val);
 }   break;

+case 'M':
+case 'm': {
+virBitmapPtr map = va_arg(args, virBitmapPtr);
+virJSONValuePtr jsonMap;
+
+if (!map) {
+if (type == 'M')
+continue;
+
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(argument key '%s' must not have null value),
+   key);
+goto cleanup;
+}
+
+if (!(jsonMap = virJSONValueNewArrayFromBitmap(map)))
+goto cleanup;
+
+if ((rc = virJSONValueObjectAppend(obj, key, jsonMap))  0)
+virJSONValueFree(jsonMap);
+} break;
+
 default:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(unsupported data type '%c' for arg '%s'), type, 
key - 2);
@@ -941,6 +965,95 @@ virJSONValueGetBoolean(virJSONValuePtr val,
 }


+/**
+ * virJSONValueGetArrayAsBitmap:
+ * @val: JSON array to convert to bitmap
+ * @bitmap: New bitmap is allocated filled and returned via this argument
+ *
+ * Attempts a conversion of a JSON array to a bitmap. The members of the array
+ * must be non-negative integers for the conversion to succeed. This function
+ * does not report libvirt errors (except for out-of-memory) so that it can be
+ * used to probe that the array can be represented as a bitmap.
+ *
+ * Returns 0 on success and fills @bitmap; -1 on error and  @bitmap is set to
+ * NULL.
+ */
+int
+virJSONValueGetArrayAsBitmap(const virJSONValue *val,
+ virBitmapPtr *bitmap)
+{
+int ret = -1;
+virJSONValuePtr elem;
+size_t i;
+unsigned long long *elems = NULL;
+unsigned long long maxelem = 0;
+
+*bitmap = NULL;
+
+if (val-type != VIR_JSON_TYPE_ARRAY)
+return -1;
+
+if (VIR_ALLOC_N(elems, val-data.array.nvalues)  0)
+return -1;
+
+/* first pass converts array members to numbers and finds the maximum */
+for (i = 0; i  val-data.array.nvalues; i++) {
+elem = val-data.array.values[i];
+
+if (elem-type != VIR_JSON_TYPE_NUMBER ||
+virStrToLong_ullp(elem-data.number, NULL, 10, elems[i])  0)
+goto cleanup;
+
+if (elems[i]  maxelem)
+maxelem = elems[i];
+}
+
+if (!(*bitmap = virBitmapNew(maxelem + 1)))
+goto cleanup;
+
+/* second pass sets the correct bits in the map */
+for (i = 0; i  val-data.array.nvalues; i++)
+ignore_value(virBitmapSetBit(*bitmap, elems[i]));
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(elems);
+
+return ret;
+}
+
+
+virJSONValuePtr
+virJSONValueNewArrayFromBitmap(virBitmapPtr bitmap)
+{
+virJSONValuePtr ret;
+ssize_t pos = -1;
+
+if (!(ret = virJSONValueNewArray()))
+return NULL;
+
+if (!bitmap)
+return ret;
+
+while ((pos = virBitmapNextSetBit(bitmap, pos))  -1) {
+virJSONValuePtr newelem;
+
+if (!(newelem = virJSONValueNewNumberLong(pos)) ||
+virJSONValueArrayAppend(ret, newelem)  0) {
+virJSONValueFree(newelem);
+goto error;
+}
+}
+
+return ret;
+
+ error:
+virJSONValueFree(ret);
+return NULL;
+}
+
+
 int
 virJSONValueIsNull(virJSONValuePtr val)
 {
diff --git a/src/util/virjson.h b/src/util/virjson.h
index fc05ad9..57010b0 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -25,6 +25,7 @@
 # 

Re: [libvirt] [PATCHv2 1/2] storage: introduce btrfsCloneFile() for COW copy

2015-01-28 Thread Chen, Hanxiao


 -Original Message-
 From: Ján Tomko [mailto:jto...@redhat.com]
 Sent: Tuesday, January 27, 2015 8:46 PM
 To: Chen, Hanxiao/陈 晗霄; libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCHv2 1/2] storage: introduce btrfsCloneFile() for 
 COW
 copy
 
 On 01/23/2015 11:22 AM, Chen Hanxiao wrote:
  Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
  ---
   configure.ac  | 12 
   src/storage/storage_backend.c | 24 
   2 files changed, 36 insertions(+)
 
  diff --git a/configure.ac b/configure.ac
  index f370475..2498389 100644
  --- a/configure.ac
  +++ b/configure.ac
  @@ -2175,6 +2175,18 @@ fi
   AM_CONDITIONAL([WITH_HYPERV], [test $with_hyperv = yes])
 
 
  +dnl
  +dnl check for kernel headers required by btrfs ioctl
  +dnl
  +if test $with_linux = yes; then
 
  +have_btrfs=no
  +AC_CHECK_HEADER([linux/btrfs.h],[have_btrfs=yes])
  +if test ${have_btrfs} = yes; then
  +AC_DEFINE([HAVE_BTRFS_IOC_CLONE], 1,
  +  [whether have btrfs CoW clone ioctl])
 
 This macro name is misleading (it does not check for clone, just for btrfs.h).
 
 Doing just:
 AC_CHECK_HEADERS([linux/btrfs.h])
 will define HAVE_LINUX_BTRFS_H
 
  +fi
  +fi
  +
   dnl Allow perl/python overrides
   AC_PATH_PROGS([PYTHON], [python2 python])
   AC_PATH_PROG([PERL], [perl])
  diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
  index b990a82..d2a664b 100644
  --- a/src/storage/storage_backend.c
  +++ b/src/storage/storage_backend.c
  @@ -46,6 +46,10 @@
   # include selinux/selinux.h
   #endif
 
  +#if HAVE_BTRFS_IOC_CLONE
  +# include linux/btrfs.h
  +#endif
  +
   #include datatypes.h
   #include virerror.h
   #include viralloc.h
  @@ -156,6 +160,26 @@ enum {
   #define READ_BLOCK_SIZE_DEFAULT  (1024 * 1024)
   #define WRITE_BLOCK_SIZE_DEFAULT (4 * 1024)
 
  +/*
  + * Perform the O(1) btrfs clone operation, if possible.
  + * Upon success, return 0.  Otherwise, return -1 and set errno.
  + */
  +#if defined(HAVE_BTRFS_IOC_CLONE)
 
 #if HAVE_LINUX_BTRFS_H
 
  +static inline int
  +btrfsCloneFile(int dest_fd, int src_fd)
  +{
  +return ioctl(dest_fd, BTRFS_IOC_CLONE, src_fd);
  +}
  +#else
  +static inline int
  +btrfsCloneFile(int dest_fd ATTRIBUTE_UNUSED,
  +   int src_fd ATTRIBUTE_UNUSED)
  +{
  +errno = ENOTSUP;
  +return -1;
  +}
  +#endif
  +
   static int ATTRIBUTE_NONNULL(2)
   virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 virStorageVolDefPtr inputvol,
 
 
 ACK
 
 I simplified the configure check and pushed the patch.
 

Thanks for your kindly help.

Regards,
- Chen


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

Re: [libvirt] [PATCH v2 1/9] virstoragefile: Add virStorageSourceGetBackingStore

2015-01-28 Thread Michal Privoznik
On 21.01.2015 16:29, Matthias Gatto wrote:
 Create virStorageSourceGetBackingStore function in
 preparation for quorum:
 Actually, if we want to get a backing store inside a virStorageSource
 we have to do it manually(src-backingStore = backingStore).
 The problem is that with a quorum, a virStorageSource
 can contain multiple backing stores, and src-backingStore can
 be treated as a virStorageSourcePtr or a virStorageSourcePtrPtr.
 
 Due to these reason, we need to simplify the manipulation of
 virStorageSource, and create a function to get the asked
 backingStore in a virStorageSource
 
 For now, this function only return the backingStore field
 
 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---
  src/libvirt_private.syms  | 1 +
  src/util/virstoragefile.c | 8 
  src/util/virstoragefile.h | 3 +++
  3 files changed, 12 insertions(+)
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index a2eec83..3f4d02c 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -2023,6 +2023,7 @@ virStorageSourceClear;
  virStorageSourceCopy;
  virStorageSourceFree;
  virStorageSourceGetActualType;
 +virStorageSourceGetBackingStore;
  virStorageSourceGetSecurityLabelDef;
  virStorageSourceInitChainElement;
  virStorageSourceIsEmpty;
 diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
 index 7a4f9a0..dc6cf8f 100644
 --- a/src/util/virstoragefile.c
 +++ b/src/util/virstoragefile.c
 @@ -1798,6 +1798,14 @@ virStorageSourcePoolDefCopy(const 
 virStorageSourcePoolDef *src)
  }
  
  
 +virStorageSourcePtr
 +virStorageSourceGetBackingStore(const virStorageSource *src,
 +size_t pos ATTRIBUTE_UNUSED)
 +{
 +return src-backingStore;
 +}
 +
 +
  /**
   * virStorageSourcePtr:
   *
 diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
 index b4c3808..c37ddcf 100644
 --- a/src/util/virstoragefile.h
 +++ b/src/util/virstoragefile.h
 @@ -289,6 +289,9 @@ struct _virStorageSource {
  #  define DEV_BSIZE 512
  # endif
  
 +virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource 
 *src,
 +size_t pos);
 +
  int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);
  int virStorageFileProbeFormatFromBuf(const char *path,
   char *buf,
 

ACK

Michal

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


Re: [libvirt] [PATCH v2 3/9] virstoragefile: Add virStorageSourceSetBackingStore

2015-01-28 Thread Michal Privoznik
On 21.01.2015 16:29, Matthias Gatto wrote:
 As explained for virStorageSourceGetBackingStore, quorum allows
 multiple backing store, this make the operation to set bs complex
 because we have to check if the backingStore is used as an array
 or a pointer, and set it differently in both case.
 
 In order to help the manipulation of backing store, I've added a
 function virStorageSourceSetBackingStore.
 
 For now virStorageSourceSetBackingStore don't handle the case where
 we have more than one backing store in virStorageSource.
 
 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---
  src/libvirt_private.syms  |  1 +
  src/util/virstoragefile.c | 10 ++
  src/util/virstoragefile.h |  3 +++
  3 files changed, 14 insertions(+)
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 3f4d02c..980235b 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -2033,6 +2033,7 @@ virStorageSourceParseRBDColonString;
  virStorageSourcePoolDefFree;
  virStorageSourcePoolModeTypeFromString;
  virStorageSourcePoolModeTypeToString;
 +virStorageSourceSetBackingStore;
  virStorageTypeFromString;
  virStorageTypeToString;
  
 diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
 index 052debf..84eefa3 100644
 --- a/src/util/virstoragefile.c
 +++ b/src/util/virstoragefile.c
 @@ -1806,6 +1806,16 @@ virStorageSourceGetBackingStore(const virStorageSource 
 *src,
  }
  
  
 +virStorageSourcePtr
 +virStorageSourceSetBackingStore(virStorageSourcePtr src,
 +virStorageSourcePtr backingStore,
 +size_t pos ATTRIBUTE_UNUSED)
 +{
 +src-backingStore = backingStore;
 +return src-backingStore;
 +}
 +
 +
  /**
   * virStorageSourcePtr:
   *
 diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
 index c37ddcf..d5cf7e6 100644
 --- a/src/util/virstoragefile.h
 +++ b/src/util/virstoragefile.h
 @@ -289,6 +289,9 @@ struct _virStorageSource {
  #  define DEV_BSIZE 512
  # endif
  
 +virStorageSourcePtr virStorageSourceSetBackingStore(virStorageSourcePtr src,
 +virStorageSourcePtr 
 backingStore,
 +size_t pos);
  virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource 
 *src,
  size_t pos);
  
 

ACK

Michal

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


Re: [libvirt] [PATCH v2 5/9] virstoragefile: Treat backingStore as a pointer or an array

2015-01-28 Thread Michal Privoznik
On 21.01.2015 16:29, Matthias Gatto wrote:
 As explain in the former patchs, backingStore can be treat an array or
 a pointer.
 If we have only one backingStore we have to use it as a normal ptr
 but if there is more backing store, we use it as a pointer's array.
 
 Because it would be complicated to expend backingStore manually, and do
 the convertion from a pointer to an array, I've created the
 virStorageSourcePushBackingStore function to help.
 
 This function allocate an array of pointer only if there is more than 1 bs.
 
 Because we can not remove a child from a quorum, i didn't create the function
 virStorageSourcePopBackingStore.
 
 I've changed virStorageSourceBackingStoreClear, 
 virStorageSourceSetBackingStore
 and virStorageSourceGetBackingStore to handle the case where backingStore
 is an array.
 
 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---
  src/conf/storage_conf.c   |  7 ++-
  src/libvirt_private.syms  |  1 +
  src/storage/storage_backend_fs.c  |  2 +
  src/storage/storage_backend_logical.c |  2 +-
  src/util/virstoragefile.c | 89 
 ---
  src/util/virstoragefile.h |  2 +
  6 files changed, 94 insertions(+), 9 deletions(-)
 
 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index fac85fa..41887eb 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c
 @@ -1343,7 +1343,12 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
  if (VIR_ALLOC(backingStorePtr)  0)
  goto error;
  
 -backingStorePtr = virStorageSourceSetBackingStore(ret-target, 
 backingStorePtr, 0);
 +if (!virStorageSourcePushBackingStore(ret-target))
 +goto error;
 +
 +backingStorePtr = virStorageSourceSetBackingStore(ret-target,
 +  backingStorePtr,
 +  
 ret-target.nBackingStores - 1);
  backingStorePtr-path = backingStore;
  backingStore = NULL;
  
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 980235b..5752907 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -2033,6 +2033,7 @@ virStorageSourceParseRBDColonString;
  virStorageSourcePoolDefFree;
  virStorageSourcePoolModeTypeFromString;
  virStorageSourcePoolModeTypeToString;
 +virStorageSourcePushBackingStore;
  virStorageSourceSetBackingStore;
  virStorageTypeFromString;
  virStorageTypeToString;
 diff --git a/src/storage/storage_backend_fs.c 
 b/src/storage/storage_backend_fs.c
 index a3b6688..0d8c5ad 100644
 --- a/src/storage/storage_backend_fs.c
 +++ b/src/storage/storage_backend_fs.c
 @@ -112,6 +112,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
  
  if (VIR_ALLOC(backingStore)  0)
  goto cleanup;
 +if (virStorageSourcePushBackingStore(target) == false)
 +goto cleanup;
  virStorageSourceSetBackingStore(target, backingStore, 0);
  backingStore-type = VIR_STORAGE_TYPE_NETWORK;
  backingStore-path = meta-backingStoreRaw;
 diff --git a/src/storage/storage_backend_logical.c 
 b/src/storage/storage_backend_logical.c
 index fcec31f..cd8de85 100644
 --- a/src/storage/storage_backend_logical.c
 +++ b/src/storage/storage_backend_logical.c
 @@ -148,7 +148,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
   *  lv is created with --virtualsize).
   */
  if (groups[1]  !STREQ(groups[1], )  (groups[1][0] != '[')) {
 -if (VIR_ALLOC(vol-target.backingStore)  0)
 +if (virStorageSourcePushBackingStore(vol-target) == false)
  goto cleanup;
  
  if (virAsprintf(virStorageSourceGetBackingStore(vol-target, 
 0)-path, %s/%s,
 diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
 index ba38827..554aa8b 100644
 --- a/src/util/virstoragefile.c
 +++ b/src/util/virstoragefile.c
 @@ -1798,21 +1798,88 @@ virStorageSourcePoolDefCopy(const 
 virStorageSourcePoolDef *src)
  }
  
  
 +/**
 + * virStorageSourceGetBackingStore:
 + * @src: virStorageSourcePtr containing the backing stores
 + * @pos: position of the backing store to get
 + *
 + * return the backingStore at the position of @pos
 + */
  virStorageSourcePtr
 -virStorageSourceGetBackingStore(const virStorageSource *src,
 -size_t pos ATTRIBUTE_UNUSED)
 +virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos)
 +{
 +if (!src-backingStore || (pos  1  pos = src-nBackingStores))
 +return NULL;
 +if (src-nBackingStores  2)
 +return src-backingStore;
 +return ((virStorageSourcePtr *)src-backingStore)[pos];
 +}
 +
 +
 +/**
 + * virStorageSourcePushBackingStore:
 + * @src: virStorageSourcePtr to allocate the new backing store
 + *
 + * Allocate size for a new backingStorePtr in src-backingStore
 + * and update src-nBackingStores
 + * If we 

Re: [libvirt] [PATCH v2 2/9] virstoragefile: Always use virStorageSourceGetBackingStore to get backing store

2015-01-28 Thread Michal Privoznik
On 21.01.2015 16:29, Matthias Gatto wrote:
 Uniformize backing store usage by calling virStorageSourceGetBackingStore
 instead of setting backing store manually.
 
 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---
  src/conf/domain_conf.c|  7 ---
  src/conf/storage_conf.c   | 18 ++
  src/qemu/qemu_cgroup.c|  4 ++--
  src/qemu/qemu_domain.c|  2 +-
  src/qemu/qemu_driver.c| 12 ++--
  src/security/security_dac.c   |  2 +-
  src/security/security_selinux.c   |  4 ++--
  src/security/virt-aa-helper.c |  2 +-
  src/storage/storage_backend.c | 30 --
  src/storage/storage_backend_fs.c  | 31 +--
  src/storage/storage_backend_gluster.c |  8 +---
  src/storage/storage_backend_logical.c |  6 +++---
  src/util/virstoragefile.c | 20 ++--
  tests/virstoragetest.c| 14 +++---
  14 files changed, 85 insertions(+), 75 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 8792f5e..0668a5b 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -16704,7 +16704,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
  /* We currently don't output seclabels for backing chain element */
  if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true)  0 
 ||
  virDomainDiskBackingStoreFormat(buf,
 -backingStore-backingStore,
 +
 virStorageSourceGetBackingStore(backingStore, 0),
  backingStore-backingStoreRaw,
  idx + 1)  0)
  return -1;
 @@ -16826,7 +16826,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
  /* Don't format backingStore to inactive XMLs until the code for
   * persistent storage of backing chains is ready. */
  if (!(flags  VIR_DOMAIN_DEF_FORMAT_INACTIVE) 
 -virDomainDiskBackingStoreFormat(buf, def-src-backingStore,
 +virDomainDiskBackingStoreFormat(buf,
 +
 virStorageSourceGetBackingStore(def-src, 0),
  def-src-backingStoreRaw, 1)  0)
  return -1;
  
 @@ -20868,7 +20869,7 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
  }
  }
  
 -for (tmp = disk-src; tmp; tmp = tmp-backingStore) {
 +for (tmp = disk-src; tmp; tmp = virStorageSourceGetBackingStore(tmp, 
 0)) {
  int actualType = virStorageSourceGetActualType(tmp);
  /* execute the callback only for local storage */
  if (actualType != VIR_STORAGE_TYPE_NETWORK 
 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index e9aaa8a..f4f7e24 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c
 @@ -1339,20 +1339,22 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
  }
  
  if ((backingStore = virXPathString(string(./backingStore/path), 
 ctxt))) {
 +virStorageSourcePtr backingStorePtr;
  if (VIR_ALLOC(ret-target.backingStore)  0)
  goto error;
  
 -ret-target.backingStore-path = backingStore;
 +backingStorePtr = virStorageSourceGetBackingStore(ret-target, 0);
 +backingStorePtr-path = backingStore;
  backingStore = NULL;
  
  if (options-formatFromString) {
  char *format = 
 virXPathString(string(./backingStore/format/@type), ctxt);
  if (format == NULL)
 -ret-target.backingStore-format = options-defaultFormat;
 +backingStorePtr-format = options-defaultFormat;
  else
 -ret-target.backingStore-format = 
 (options-formatFromString)(format);
 +backingStorePtr-format = 
 (options-formatFromString)(format);
  
 -if (ret-target.backingStore-format  0) {
 +if (backingStorePtr-format  0) {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 _(unknown volume format type %s), format);
  VIR_FREE(format);
 @@ -1361,9 +1363,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
  VIR_FREE(format);
  }
  
 -if (VIR_ALLOC(ret-target.backingStore-perms)  0)
 +if (VIR_ALLOC(backingStorePtr-perms)  0)
  goto error;
 -if (virStorageDefParsePerms(ctxt, ret-target.backingStore-perms,
 +if (virStorageDefParsePerms(ctxt, backingStorePtr-perms,
  ./backingStore/permissions,
  DEFAULT_VOL_PERM_MODE)  0)
  goto error;
 @@ -1641,9 +1643,9 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
   def-target, target)  0)
  goto cleanup;
  
 -if (def-target.backingStore 
 

[libvirt] [PATCHv2 1/2] Split qemuDomainChrInsert into two parts

2015-01-28 Thread Ján Tomko
Do the allocation first, then add the actual device.
The second part should never fail. This is good
for live hotplug where we don't want to remove the device
on OOM after the monitor command succeeded.

The only change in behavior is that on failure, the
vmdef-consoles array is freed, not just the first console.
---
 src/conf/domain_conf.c   | 18 +---
 src/conf/domain_conf.h   |  7 +--
 src/libvirt_private.syms |  3 ++-
 src/qemu/qemu_hotplug.c  | 54 +---
 4 files changed, 68 insertions(+), 14 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10dbabd..3c6b77d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12027,15 +12027,27 @@ virDomainChrGetDomainPtrs(const virDomainDef *vmdef,
 
 
 int
-virDomainChrInsert(virDomainDefPtr vmdef,
-   virDomainChrDefPtr chr)
+virDomainChrPreAlloc(virDomainDefPtr vmdef,
+ virDomainChrDefPtr chr)
+{
+virDomainChrDefPtr **arrPtr = NULL;
+size_t *cntPtr = NULL;
+
+virDomainChrGetDomainPtrsInternal(vmdef, chr-deviceType, arrPtr, 
cntPtr);
+
+return VIR_REALLOC_N(*arrPtr, *cntPtr + 1);
+}
+
+void
+virDomainChrInsertPreAlloced(virDomainDefPtr vmdef,
+ virDomainChrDefPtr chr)
 {
 virDomainChrDefPtr **arrPtr = NULL;
 size_t *cntPtr = NULL;
 
 virDomainChrGetDomainPtrsInternal(vmdef, chr-deviceType, arrPtr, 
cntPtr);
 
-return VIR_APPEND_ELEMENT(*arrPtr, *cntPtr, chr);
+ignore_value(VIR_APPEND_ELEMENT_INPLACE(*arrPtr, *cntPtr, chr));
 }
 
 virDomainChrDefPtr
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 563c18b..93f2314 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2652,8 +2652,11 @@ bool
 virDomainChrEquals(virDomainChrDefPtr src,
virDomainChrDefPtr tgt);
 int
-virDomainChrInsert(virDomainDefPtr vmdef,
-   virDomainChrDefPtr chr);
+virDomainChrPreAlloc(virDomainDefPtr vmdef,
+ virDomainChrDefPtr chr);
+void
+virDomainChrInsertPreAlloced(virDomainDefPtr vmdef,
+ virDomainChrDefPtr chr);
 virDomainChrDefPtr
 virDomainChrRemove(virDomainDefPtr vmdef,
virDomainChrDefPtr chr);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 75a6d83..bd7870f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -153,7 +153,8 @@ virDomainChrDefNew;
 virDomainChrEquals;
 virDomainChrFind;
 virDomainChrGetDomainPtrs;
-virDomainChrInsert;
+virDomainChrInsertPreAlloced;
+virDomainChrPreAlloc;
 virDomainChrRemove;
 virDomainChrSerialTargetTypeFromString;
 virDomainChrSerialTargetTypeToString;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1dbcc5d..2ea30f5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1403,9 +1403,9 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr 
driver,
 
 }
 
-int
-qemuDomainChrInsert(virDomainDefPtr vmdef,
-virDomainChrDefPtr chr)
+static int
+qemuDomainChrPreInsert(virDomainDefPtr vmdef,
+   virDomainChrDefPtr chr)
 {
 if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
 chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
@@ -1420,25 +1420,63 @@ qemuDomainChrInsert(virDomainDefPtr vmdef,
 return -1;
 }
 
-if (virDomainChrInsert(vmdef, chr)  0)
+if (virDomainChrPreAlloc(vmdef, chr)  0)
 return -1;
 
 /* Due to some crazy backcompat stuff, the first serial device is an alias
  * to the first console too. If this is the case, the definition must be
  * duplicated as first console device. */
-if (vmdef-nserials == 1  vmdef-nconsoles == 0) {
-if ((!vmdef-consoles  VIR_ALLOC(vmdef-consoles)  0) ||
-VIR_ALLOC(vmdef-consoles[0])  0) {
-virDomainChrRemove(vmdef, chr);
+if (vmdef-nserials == 0  vmdef-nconsoles == 0 
+chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) {
+if (!vmdef-consoles  VIR_ALLOC(vmdef-consoles)  0)
+return -1;
+
+if (VIR_ALLOC(vmdef-consoles[0])  0) {
+VIR_FREE(vmdef-consoles);
 return -1;
 }
+vmdef-nconsoles++;
+}
+return 0;
+}
+
+static void
+qemuDomainChrInsertPreAlloced(virDomainDefPtr vmdef,
+  virDomainChrDefPtr chr)
+{
+virDomainChrInsertPreAlloced(vmdef, chr);
+if (vmdef-nserials == 1  vmdef-nconsoles == 0 
+chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) {
 vmdef-nconsoles = 1;
 
 /* Create an console alias for the serial port */
 vmdef-consoles[0]-deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
 vmdef-consoles[0]-targetType = 
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
 }
+}
+
+static void
+qemuDomainChrInsertPreAllocCleanup(virDomainDefPtr vmdef,
+   virDomainChrDefPtr 

[libvirt] [PATCHv2 2/2] hotplug: only add a chardev to vmdef after monitor call

2015-01-28 Thread Ján Tomko
https://bugzilla.redhat.com/show_bug.cgi?id=1161024

This way the device is in vmdef only if ret = 0 and the caller
(qemuDomainAttachDeviceFlags) does not free it.

Otherwise it might get double freed by qemuProcessStop
and qemuDomainAttachDeviceFlags if the domain crashed
in monitor after we've added it to vm-def.
---
qemuDomainChrInsertPreAllocCleanup is always called, not just when
qemuDomainChrPreInsert was called before. But unless I missed something,
the configuration where nserials == 0, nconsoles == 1 should not
happen after qemu's PostParse callback.

 src/qemu/qemu_hotplug.c | 34 +++---
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2ea30f5..033b281 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1523,59 +1523,47 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 virDomainDefPtr vmdef = vm-def;
 char *devstr = NULL;
 char *charAlias = NULL;
-bool need_remove = false;
 
 if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(qemu does not support -device));
-return ret;
+goto cleanup;
 }
 
 if (qemuAssignDeviceChrAlias(vmdef, chr, -1)  0)
-return ret;
+goto cleanup;
 
 if (qemuBuildChrDeviceStr(devstr, vm-def, chr, priv-qemuCaps)  0)
-return ret;
+goto cleanup;
 
 if (virAsprintf(charAlias, char%s, chr-info.alias)  0)
 goto cleanup;
 
-if (qemuDomainChrInsert(vmdef, chr)  0)
+if (qemuDomainChrPreInsert(vmdef, chr)  0)
 goto cleanup;
-need_remove = true;
 
 qemuDomainObjEnterMonitor(driver, vm);
 if (qemuMonitorAttachCharDev(priv-mon, charAlias, chr-source)  0) {
-if (qemuDomainObjExitMonitor(driver, vm)  0) {
-need_remove = false;
-ret = -1;
-goto cleanup;
-}
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
 goto audit;
 }
 
 if (devstr  qemuMonitorAddDevice(priv-mon, devstr)  0) {
 /* detach associated chardev on error */
 qemuMonitorDetachCharDev(priv-mon, charAlias);
-if (qemuDomainObjExitMonitor(driver, vm)  0) {
-need_remove = false;
-ret = -1;
-goto cleanup;
-}
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
 goto audit;
 }
-if (qemuDomainObjExitMonitor(driver, vm)  0) {
-need_remove = false;
-ret = -1;
-goto cleanup;
-}
+if (qemuDomainObjExitMonitor(driver, vm)  0)
+goto audit;
 
+qemuDomainChrInsertPreAlloced(vm-def, chr);
 ret = 0;
  audit:
 virDomainAuditChardev(vm, NULL, chr, attach, ret == 0);
  cleanup:
-if (ret  0  need_remove)
-qemuDomainChrRemove(vmdef, chr);
+if (ret  0  virDomainObjIsActive(vm))
+qemuDomainChrInsertPreAllocCleanup(vm-def, chr);
 VIR_FREE(charAlias);
 VIR_FREE(devstr);
 return ret;
-- 
2.0.5

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


[libvirt] [PATCHv2 0/2] Really fix the crash on qemu crash on chardev hotplug

2015-01-28 Thread Ján Tomko
An alternative solution that only adds the chardev to the definition
after the command succeeded.

Ján Tomko (2):
  Split qemuDomainChrInsert into two parts
  hotplug: only add a chardev to vmdef after monitor call

 src/conf/domain_conf.c   | 18 --
 src/conf/domain_conf.h   |  7 ++--
 src/libvirt_private.syms |  3 +-
 src/qemu/qemu_hotplug.c  | 88 +++-
 4 files changed, 79 insertions(+), 37 deletions(-)

-- 
2.0.5

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

Re: [libvirt] [PATCH v2 4/9] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store

2015-01-28 Thread Michal Privoznik
On 21.01.2015 16:29, Matthias Gatto wrote:
 Replace the parts of the code where a backing store is set manually
 with virStorageSourceSetBackingStore
 
 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---
  src/conf/domain_conf.c   | 2 +-
  src/conf/storage_conf.c  | 4 ++--
  src/qemu/qemu_driver.c   | 4 ++--
  src/storage/storage_backend_fs.c | 8 
  src/storage/storage_driver.c | 2 +-
  src/util/virstoragefile.c| 8 +---
  tests/virstoragetest.c   | 4 ++--
  7 files changed, 17 insertions(+), 15 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 0668a5b..9d6b888 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -5659,7 +5659,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
  virDomainDiskBackingStoreParse(ctxt, backingStore)  0)
  goto cleanup;
  
 -src-backingStore = backingStore;
 +virStorageSourceSetBackingStore(src, backingStore, 0);
  ret = 0;
  
   cleanup:
 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index f4f7e24..fac85fa 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c
 @@ -1340,10 +1340,10 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
  
  if ((backingStore = virXPathString(string(./backingStore/path), 
 ctxt))) {
  virStorageSourcePtr backingStorePtr;
 -if (VIR_ALLOC(ret-target.backingStore)  0)
 +if (VIR_ALLOC(backingStorePtr)  0)
  goto error;
  
 -backingStorePtr = virStorageSourceGetBackingStore(ret-target, 0);
 +backingStorePtr = virStorageSourceSetBackingStore(ret-target, 
 backingStorePtr, 0);
  backingStorePtr-path = backingStore;
  backingStore = NULL;
  
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 547d2b5..b3afccd 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -13500,13 +13500,13 @@ 
 qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
  /* Update vm in place to match changes. */
  tmp = disk-src;
  disk-src = virStorageSourceGetBackingStore(tmp, 0);
 -tmp-backingStore = NULL;
 +virStorageSourceSetBackingStore(tmp, NULL, 0);
  virStorageSourceFree(tmp);
  
  if (persistDisk) {
  tmp = persistDisk-src;
  persistDisk-src = virStorageSourceGetBackingStore(tmp, 0);
 -tmp-backingStore = NULL;
 +virStorageSourceSetBackingStore(tmp, NULL, 0);
  virStorageSourceFree(tmp);
  }
  }
 diff --git a/src/storage/storage_backend_fs.c 
 b/src/storage/storage_backend_fs.c
 index 56cfc56..a3b6688 100644
 --- a/src/storage/storage_backend_fs.c
 +++ b/src/storage/storage_backend_fs.c
 @@ -97,8 +97,9 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
  goto cleanup;
  
  if (meta-backingStoreRaw) {
 -backingStore = virStorageSourceGetBackingStore(target, 0);

See? this line ^^ is useless.

 -if (!(backingStore = virStorageSourceNewFromBacking(meta)))
 +if (!(backingStore = virStorageSourceSetBackingStore(target,
 + 
 virStorageSourceNewFromBacking(meta),
 + 0)))

Well, since I squashed in my diff in 2/9 I'm getting a merge error here.
But that's okay.

  goto cleanup;
  
  backingStore-format = backingStoreFormat;
 @@ -111,8 +112,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
  
  if (VIR_ALLOC(backingStore)  0)
  goto cleanup;
 -
 -target-backingStore = backingStore;
 +virStorageSourceSetBackingStore(target, backingStore, 0);
  backingStore-type = VIR_STORAGE_TYPE_NETWORK;
  backingStore-path = meta-backingStoreRaw;
  meta-backingStoreRaw = NULL;
 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
 index 66dc994..eeb2a4c 100644
 --- a/src/storage/storage_driver.c
 +++ b/src/storage/storage_driver.c
 @@ -2899,7 +2899,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr 
 src,
  goto cleanup;
  }
  
 -src-backingStore = backingStore;
 +virStorageSourceSetBackingStore(src, backingStore, 0);
  backingStore = NULL;
  ret = 0;
  
 diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
 index 84eefa3..ba38827 100644
 --- a/src/util/virstoragefile.c
 +++ b/src/util/virstoragefile.c
 @@ -1891,8 +1891,10 @@ virStorageSourceCopy(const virStorageSource *src,
  goto error;
  
  if (backingChain  virStorageSourceGetBackingStore(src, 0)) {
 -if (!(ret-backingStore = 
 virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0),
 -   true)))
 +if (!virStorageSourceSetBackingStore(ret,
 + 
 

[libvirt] [PATCH 12/12] qemu: command: Don't combine old and modern NUMA node creation

2015-01-28 Thread Peter Krempa
Change done by commit f309db1f4d51009bad0d32e12efc75530b66836b wrongly
assumes that qemu can start with a combination of NUMA nodes specified
with the memdev option and the appropriate backends, and the legacy
way by specifying only mem as a size argument. QEMU rejects such
commandline though:

$ /usr/bin/qemu-system-x86_64 -S -M pc -m 1024 -smp 2 \
-numa node,nodeid=0,cpus=0,mem=256 \
-object memory-backend-ram,id=ram-node1,size=12345 \
-numa node,nodeid=1,cpus=1,memdev=ram-node1
qemu-system-x86_64: -numa node,nodeid=1,cpus=1,memdev=ram-node1: qemu: memdev 
option must be specified for either all or no nodes

To fix this issue we need to check if any of the nodes requires the new
definition with the backend and if so, then all other nodes have to use
it too.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1182467
---
 src/qemu/qemu_command.c| 72 --
 .../qemuxml2argv-hugepages-pages3.args |  3 +-
 .../qemuxml2argv-numatune-memnode-no-memory.args   |  3 +-
 3 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bb58a09..8089e4e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4733,17 +4733,12 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
 backendType, props, false))  0)
 goto cleanup;

-if (rc == 1) {
-ret = 0;
-goto cleanup;
-}
-
 if (!(*backendStr = qemuBuildObjectCommandlineFromJSON(backendType,
alias,
props))  0)
 goto cleanup;

-ret = 0;
+ret = rc;

  cleanup:
 VIR_FREE(alias);
@@ -7052,7 +7047,9 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 size_t i;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 char *cpumask = NULL, *tmpmask = NULL, *next = NULL;
-char *backendStr = NULL;
+char **nodeBackends = NULL;
+bool needBackend = false;
+int rc;
 int ret = -1;
 const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;

@@ -7101,35 +7098,24 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 }
 }

+if (VIR_ALLOC_N(nodeBackends, def-cpu-ncells)  0)
+goto cleanup;
+
+/* using of -numa memdev= cannot be combined with -numa mem=, thus we
+ * need to check which approach to use */
 for (i = 0; i  def-cpu-ncells; i++) {
 unsigned long long cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024);
 def-cpu-cells[i].mem = cellmem * 1024;

-VIR_FREE(cpumask);
-VIR_FREE(backendStr);
-
-if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
-goto cleanup;
-
-if (strchr(cpumask, ',') 
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-   _(disjoint NUMA cpu ranges are not supported 
- with this QEMU));
-goto cleanup;
-}
-
-
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
-if (qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i,
-  auto_nodeset, backendStr)  0)
+if ((rc = qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i,
+auto_nodeset,
+nodeBackends[i]))  0)
 goto cleanup;

-if (backendStr) {
-virCommandAddArg(cmd, -object);
-virCommandAddArg(cmd, backendStr);
-}
+if (rc == 0)
+needBackend = true;
 } else {
 if (def-cpu-cells[i].memAccess) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
@@ -7138,6 +7124,23 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 goto cleanup;
 }
 }
+}
+
+for (i = 0; i  def-cpu-ncells; i++) {
+VIR_FREE(cpumask);
+if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
+goto cleanup;
+
+if (strchr(cpumask, ',') 
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(disjoint NUMA cpu ranges are not supported 
+ with this QEMU));
+goto cleanup;
+}
+
+if (needBackend)
+virCommandAddArgList(cmd, -object, nodeBackends[i], NULL);

 virCommandAddArg(cmd, -numa);
 virBufferAsprintf(buf, node,nodeid=%zu, i);
@@ -7149,10 +7152,10 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 virBufferAdd(buf, tmpmask, -1);
 }

-if (backendStr)
+if (needBackend)
 

[libvirt] [PATCH 11/12] qemu: command: Refactor NUMA backend object formatting to use JSON objs

2015-01-28 Thread Peter Krempa
With the new JSON to argv formatter we are now able to represent the
memory backend definitions in the JSON object format that is reusable
for monitor use (hotplug) and then convert it into the shell string.
This will avoid having two separate instances of the same code that
would create the different formats.

Previous refactors now allow to make this step without changes to the
test suite.
---
 src/qemu/qemu_command.c | 109 +++-
 1 file changed, 62 insertions(+), 47 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0c343b6..bb58a09 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4533,12 +4533,10 @@ qemuBuildMemoryBackendStr(unsigned long long size,
   virDomainDefPtr def,
   virQEMUCapsPtr qemuCaps,
   virQEMUDriverConfigPtr cfg,
-  const char *aliasPrefix,
-  size_t id,
-  char **backendStr,
+  const char **backendType,
+  virJSONValuePtr *backendProps,
   bool force)
 {
-virBuffer buf = VIR_BUFFER_INITIALIZER;
 virDomainHugePagePtr master_hugepage = NULL;
 virDomainHugePagePtr hugepage = NULL;
 virDomainNumatuneMemMode mode;
@@ -4546,11 +4544,15 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 virMemAccess memAccess = def-cpu-cells[guestNode].memAccess;
 size_t i;
 char *mem_path = NULL;
-char *nodemask = NULL;
-char *tmpmask = NULL, *next = NULL;
+virBitmapPtr nodemask = NULL;
 int ret = -1;
+virJSONValuePtr props = NULL;

-*backendStr = NULL;
+*backendProps = NULL;
+*backendType = NULL;
+
+if (!(props = virJSONValueNewObject()))
+return -1;

 mode = virDomainNumatuneGetMode(def-numatune, guestNode);

@@ -4623,16 +4625,23 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 if (!(mem_path = qemuGetHugepagePath(cfg-hugetlbfs[i])))
 goto cleanup;

-virBufferAsprintf(buf, memory-backend-file,id=%s%zu, aliasPrefix, 
id);
-virBufferAsprintf(buf, ,prealloc=yes,mem-path=%s, mem_path);
+*backendType = memory-backend-file;
+
+if (virJSONValueObjectAdd(props,
+  b:prealloc, true,
+  s:mem-path, mem_path,
+  NULL)  0)
+goto cleanup;

 switch (memAccess) {
 case VIR_MEM_ACCESS_SHARED:
-virBufferAddLit(buf, ,share=yes);
+if (virJSONValueObjectAdd(props, b:share, true, NULL)  0)
+goto cleanup;
 break;

 case VIR_MEM_ACCESS_PRIVATE:
-virBufferAddLit(buf, ,share=no);
+if (virJSONValueObjectAdd(props, b:share, false, NULL)  0)
+goto cleanup;
 break;

 case VIR_MEM_ACCESS_DEFAULT:
@@ -4647,43 +4656,30 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 goto cleanup;
 }

-virBufferAsprintf(buf, memory-backend-ram,id=%s%zu, aliasPrefix, 
id);
+*backendType = memory-backend-ram;
 }

-virBufferAsprintf(buf, ,size=%llu, size * 1024);
+if (virJSONValueObjectAdd(props, U:size, size * 1024, NULL)  0)
+goto cleanup;

 if (userNodeset) {
-if (!(nodemask = virBitmapFormat(userNodeset)))
-goto cleanup;
+nodemask = userNodeset;
 } else {
-if (virDomainNumatuneMaybeFormatNodeset(def-numatune, autoNodeset,
-nodemask, guestNode)  0)
+if (virDomainNumatuneMaybeGetNodeset(def-numatune, autoNodeset,
+ nodemask, guestNode)  0)
 goto cleanup;
 }

 if (nodemask) {
-if (strchr(nodemask, ',') 
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-   _(disjoint NUMA node ranges are not supported 
- with this QEMU));
+if (virJSONValueObjectAdd(props,
+  m:host-nodes, nodemask,
+  S:policy, qemuNumaPolicyTypeToString(mode),
+  NULL)  0)
 goto cleanup;
-}
-
-for (tmpmask = nodemask; tmpmask; tmpmask = next) {
-if ((next = strchr(tmpmask, ',')))
-*(next++) = '\0';
-virBufferAddLit(buf, ,host-nodes=);
-virBufferAdd(buf, tmpmask, -1);
-}
-
-virBufferAsprintf(buf, ,policy=%s,  
qemuNumaPolicyTypeToString(mode));
 }

-if (virBufferCheckError(buf)  0)
-goto cleanup;
-
-*backendStr = virBufferContentAndReset(buf);
+*backendProps = props;
+props = NULL;

 if (!hugepage) {
 if ((nodemask || 

[libvirt] [PATCH 09/12] qemu: command: Unify values for boolean values when formating memory backends

2015-01-28 Thread Peter Krempa
QEMU's qapi visitor code allows yes/on/y for true and no/off/n for false
value of boolean properities. Unify the used style so that we can
generate it later and fix test cases.
---
 src/qemu/qemu_command.c   | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2d89c2d..9e0b178 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4628,11 +4628,11 @@ qemuBuildMemoryBackendStr(unsigned long long size,

 switch (memAccess) {
 case VIR_MEM_ACCESS_SHARED:
-virBufferAddLit(buf, ,share=on);
+virBufferAddLit(buf, ,share=yes);
 break;

 case VIR_MEM_ACCESS_PRIVATE:
-virBufferAddLit(buf, ,share=off);
+virBufferAddLit(buf, ,share=no);
 break;

 case VIR_MEM_ACCESS_DEFAULT:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args
index b51de1b..27f476f 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args
@@ -4,11 +4,11 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test 
QEMU_AUDIO_DRV=none \
 mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \
 -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
 -object memory-backend-file,id=ram-node1,prealloc=yes,\
-mem-path=/dev/hugepages2M/libvirt/qemu,share=on,size=1024M,host-nodes=0-3,\
+mem-path=/dev/hugepages2M/libvirt/qemu,share=yes,size=1024M,host-nodes=0-3,\
 policy=bind \
 -numa node,nodeid=1,cpus=1,memdev=ram-node1 \
 -object memory-backend-file,id=ram-node2,prealloc=yes,\
-mem-path=/dev/hugepages1G/libvirt/qemu,share=off,size=1024M,host-nodes=0-3,\
+mem-path=/dev/hugepages1G/libvirt/qemu,share=no,size=1024M,host-nodes=0-3,\
 policy=bind \
 -numa node,nodeid=2,cpus=2,memdev=ram-node2 \
 -object memory-backend-file,id=ram-node3,prealloc=yes,\
-- 
2.2.2

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


[libvirt] [PATCH 10/12] qemu: command: Switch to bytes when formatting size for memory backends

2015-01-28 Thread Peter Krempa
QEMU's command line visitor as well as the JSON interface take bytes by
default for memory object sizes. Convert mebibytes to bytes so that we
can later refactor the existing code for hotplug purposes.
---
 src/qemu/qemu_command.c|  2 +-
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args   | 12 
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args  |  4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args  |  2 +-
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args  | 14 --
 .../qemuxml2argv-numatune-memnode-no-memory.args   |  3 ++-
 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args  |  8 +---
 7 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9e0b178..0c343b6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4650,7 +4650,7 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 virBufferAsprintf(buf, memory-backend-ram,id=%s%zu, aliasPrefix, 
id);
 }

-virBufferAsprintf(buf, ,size=%lluM, size / 1024);
+virBufferAsprintf(buf, ,size=%llu, size * 1024);

 if (userNodeset) {
 if (!(nodemask = virBitmapFormat(userNodeset)))
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args
index b954fbc..46ec751 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args
@@ -1,16 +1,20 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M pc -m 4096 -smp 4 \
 -object memory-backend-file,id=ram-node0,prealloc=yes,\
-mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \
+mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=0-3,\
+policy=bind \
 -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
 -object memory-backend-file,id=ram-node1,prealloc=yes,\
-mem-path=/dev/hugepages2M/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \
+mem-path=/dev/hugepages2M/libvirt/qemu,size=1073741824,host-nodes=0-3,\
+policy=bind \
 -numa node,nodeid=1,cpus=1,memdev=ram-node1 \
 -object memory-backend-file,id=ram-node2,prealloc=yes,\
-mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \
+mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=0-3,\
+policy=bind \
 -numa node,nodeid=2,cpus=2,memdev=ram-node2 \
 -object memory-backend-file,id=ram-node3,prealloc=yes,\
-mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=3,policy=bind \
+mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=3,\
+policy=bind \
 -numa node,nodeid=3,cpus=3,memdev=ram-node3 \
 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb 
\
 -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
index 90ab776..0488800 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
@@ -1,10 +1,10 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M pc -m 1024 -smp 2 \
 -object memory-backend-file,id=ram-node0,prealloc=yes,\
-mem-path=/dev/hugepages2M/libvirt/qemu,size=256M \
+mem-path=/dev/hugepages2M/libvirt/qemu,size=268435456 \
 -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
 -object memory-backend-file,id=ram-node1,prealloc=yes,\
-mem-path=/dev/hugepages2M/libvirt/qemu,size=768M \
+mem-path=/dev/hugepages2M/libvirt/qemu,size=805306368 \
 -numa node,nodeid=1,cpus=1,memdev=ram-node1 \
 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
 -usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
index b932520..3bca26c 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
@@ -2,7 +2,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test 
QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M pc -m 1024 -smp 2 \
 -numa node,nodeid=0,cpus=0,mem=256 \
 -object memory-backend-file,id=ram-node1,prealloc=yes,\
-mem-path=/dev/hugepages1G/libvirt/qemu,size=768M \
+mem-path=/dev/hugepages1G/libvirt/qemu,size=805306368 \
 -numa node,nodeid=1,cpus=1,memdev=ram-node1 \
 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb 
\
 -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args
index 27f476f..a6e4d95 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args
+++ 

Re: [libvirt] [PATCHv2 0/2] Really fix the crash on qemu crash on chardev hotplug

2015-01-28 Thread Michal Privoznik
On 28.01.2015 10:14, Ján Tomko wrote:
 An alternative solution that only adds the chardev to the definition
 after the command succeeded.
 
 Ján Tomko (2):
   Split qemuDomainChrInsert into two parts
   hotplug: only add a chardev to vmdef after monitor call
 
  src/conf/domain_conf.c   | 18 --
  src/conf/domain_conf.h   |  7 ++--
  src/libvirt_private.syms |  3 +-
  src/qemu/qemu_hotplug.c  | 88 
 +++-
  4 files changed, 79 insertions(+), 37 deletions(-)
 

ACK to both

Michal

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

Re: [libvirt] [PATCH v2 5/9] virstoragefile: Treat backingStore as a pointer or an array

2015-01-28 Thread Matthias Gatto
On Wed, Jan 28, 2015 at 11:10 AM, Michal Privoznik mpriv...@redhat.com wrote:
 On 21.01.2015 16:29, Matthias Gatto wrote:
 As explain in the former patchs, backingStore can be treat an array or
 a pointer.
 If we have only one backingStore we have to use it as a normal ptr
 but if there is more backing store, we use it as a pointer's array.

 Because it would be complicated to expend backingStore manually, and do
 the convertion from a pointer to an array, I've created the
 virStorageSourcePushBackingStore function to help.

 This function allocate an array of pointer only if there is more than 1 bs.

 Because we can not remove a child from a quorum, i didn't create the function
 virStorageSourcePopBackingStore.

 I've changed virStorageSourceBackingStoreClear, 
 virStorageSourceSetBackingStore
 and virStorageSourceGetBackingStore to handle the case where backingStore
 is an array.

 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---
  src/conf/storage_conf.c   |  7 ++-
  src/libvirt_private.syms  |  1 +
  src/storage/storage_backend_fs.c  |  2 +
  src/storage/storage_backend_logical.c |  2 +-
  src/util/virstoragefile.c | 89 
 ---
  src/util/virstoragefile.h |  2 +
  6 files changed, 94 insertions(+), 9 deletions(-)

 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index fac85fa..41887eb 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c
 @@ -1343,7 +1343,12 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
  if (VIR_ALLOC(backingStorePtr)  0)
  goto error;

 -backingStorePtr = virStorageSourceSetBackingStore(ret-target, 
 backingStorePtr, 0);
 +if (!virStorageSourcePushBackingStore(ret-target))
 +goto error;
 +
 +backingStorePtr = virStorageSourceSetBackingStore(ret-target,
 +  backingStorePtr,
 +  
 ret-target.nBackingStores - 1);
  backingStorePtr-path = backingStore;
  backingStore = NULL;

 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 980235b..5752907 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -2033,6 +2033,7 @@ virStorageSourceParseRBDColonString;
  virStorageSourcePoolDefFree;
  virStorageSourcePoolModeTypeFromString;
  virStorageSourcePoolModeTypeToString;
 +virStorageSourcePushBackingStore;
  virStorageSourceSetBackingStore;
  virStorageTypeFromString;
  virStorageTypeToString;
 diff --git a/src/storage/storage_backend_fs.c 
 b/src/storage/storage_backend_fs.c
 index a3b6688..0d8c5ad 100644
 --- a/src/storage/storage_backend_fs.c
 +++ b/src/storage/storage_backend_fs.c
 @@ -112,6 +112,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,

  if (VIR_ALLOC(backingStore)  0)
  goto cleanup;
 +if (virStorageSourcePushBackingStore(target) == false)
 +goto cleanup;
  virStorageSourceSetBackingStore(target, backingStore, 0);
  backingStore-type = VIR_STORAGE_TYPE_NETWORK;
  backingStore-path = meta-backingStoreRaw;
 diff --git a/src/storage/storage_backend_logical.c 
 b/src/storage/storage_backend_logical.c
 index fcec31f..cd8de85 100644
 --- a/src/storage/storage_backend_logical.c
 +++ b/src/storage/storage_backend_logical.c
 @@ -148,7 +148,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
   *  lv is created with --virtualsize).
   */
  if (groups[1]  !STREQ(groups[1], )  (groups[1][0] != '[')) {
 -if (VIR_ALLOC(vol-target.backingStore)  0)
 +if (virStorageSourcePushBackingStore(vol-target) == false)
  goto cleanup;

  if (virAsprintf(virStorageSourceGetBackingStore(vol-target, 
 0)-path, %s/%s,
 diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
 index ba38827..554aa8b 100644
 --- a/src/util/virstoragefile.c
 +++ b/src/util/virstoragefile.c
 @@ -1798,21 +1798,88 @@ virStorageSourcePoolDefCopy(const 
 virStorageSourcePoolDef *src)
  }


 +/**
 + * virStorageSourceGetBackingStore:
 + * @src: virStorageSourcePtr containing the backing stores
 + * @pos: position of the backing store to get
 + *
 + * return the backingStore at the position of @pos
 + */
  virStorageSourcePtr
 -virStorageSourceGetBackingStore(const virStorageSource *src,
 -size_t pos ATTRIBUTE_UNUSED)
 +virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos)
 +{
 +if (!src-backingStore || (pos  1  pos = src-nBackingStores))
 +return NULL;
 +if (src-nBackingStores  2)
 +return src-backingStore;
 +return ((virStorageSourcePtr *)src-backingStore)[pos];
 +}
 +
 +
 +/**
 + * virStorageSourcePushBackingStore:
 + * @src: virStorageSourcePtr to allocate the new backing store
 + *
 + * Allocate size for a new backingStorePtr in 

[libvirt] [PATCH 03/12] util: json: make value object creator universal by supporting adding

2015-01-28 Thread Peter Krempa
To allow constructing of value objects stepwise explode the helper into
separate steps and allow appending into existing value objects.
---
 src/libvirt_private.syms |  2 ++
 src/util/virjson.c   | 74 +++-
 src/util/virjson.h   |  5 
 3 files changed, 55 insertions(+), 26 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2bbce03..cc74e35 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1533,6 +1533,8 @@ virJSONValueNewNumberUlong;
 virJSONValueNewObject;
 virJSONValueNewString;
 virJSONValueNewStringLen;
+virJSONValueObjectAdd;
+virJSONValueObjectAddVArgs;
 virJSONValueObjectAppend;
 virJSONValueObjectAppendBoolean;
 virJSONValueObjectAppendNull;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 9f2e1cf..9eb1bff 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -64,12 +64,11 @@ struct _virJSONParser {


 /**
- * virJSONValueObjectCreateVArgs:
- * @obj: returns the created JSON object
- * @...: a key-value argument pairs, terminated by NULL
+ * virJSONValueObjectAddVArgs:
+ * @obj: JSON object to add the values to
+ * @args: a key-value argument pairs, terminated by NULL
  *
- * Creates a JSON value object filled with key-value pairs supplied as variable
- * argument list.
+ * Adds the key-value pairs supplied as variable argument list to @obj.
  *
  * Keys look like   s:name  the first letter is a type code:
  * Explanation of type codes:
@@ -104,22 +103,17 @@ struct _virJSONParser {
  * The value corresponds to the selected type.
  *
  * Returns -1 on error. 1 on success, if at least one key:pair was valid 0
- * in case of no error but nothing was filled (@obj will be NULL).
+ * in case of no error but nothing was filled.
  */
 int
-virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
+virJSONValueObjectAddVArgs(virJSONValuePtr obj,
+   va_list args)
 {
-virJSONValuePtr jargs = NULL;
 char type;
 char *key;
 int ret = -1;
 int rc;

-*obj = NULL;
-
-if (!(jargs = virJSONValueNewObject()))
-goto cleanup;
-
 while ((key = va_arg(args, char *)) != NULL) {

 if (strlen(key)  3) {
@@ -146,7 +140,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list 
args)
key);
 goto cleanup;
 }
-rc = virJSONValueObjectAppendString(jargs, key, val);
+rc = virJSONValueObjectAppendString(obj, key, val);
 }   break;

 case 'z':
@@ -165,7 +159,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list 
args)
 if (!val  (type == 'z' || type == 'y'))
 continue;

-rc = virJSONValueObjectAppendNumberInt(jargs, key, val);
+rc = virJSONValueObjectAppendNumberInt(obj, key, val);
 }   break;

 case 'p':
@@ -175,7 +169,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list 
args)
 if (!val  type == 'p')
 continue;

-rc = virJSONValueObjectAppendNumberUint(jargs, key, val);
+rc = virJSONValueObjectAppendNumberUint(obj, key, val);
 }   break;

 case 'Z':
@@ -194,7 +188,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list 
args)
 if (!val  (type == 'Z' || type == 'Y'))
 continue;

-rc = virJSONValueObjectAppendNumberLong(jargs, key, val);
+rc = virJSONValueObjectAppendNumberLong(obj, key, val);
 }   break;

 case 'P':
@@ -209,12 +203,12 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, 
va_list args)
 if (!val  type == 'P')
 continue;

-rc = virJSONValueObjectAppendNumberLong(jargs, key, val);
+rc = virJSONValueObjectAppendNumberLong(obj, key, val);
 }   break;

 case 'd': {
 double val = va_arg(args, double);
-rc = virJSONValueObjectAppendNumberDouble(jargs, key, val);
+rc = virJSONValueObjectAppendNumberDouble(obj, key, val);
 }   break;

 case 'B':
@@ -224,11 +218,11 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, 
va_list args)
 if (!val  type == 'B')
 continue;

-rc = virJSONValueObjectAppendBoolean(jargs, key, val);
+rc = virJSONValueObjectAppendBoolean(obj, key, val);
 }   break;

 case 'n': {
-rc = virJSONValueObjectAppendNull(jargs, key);
+rc = virJSONValueObjectAppendNull(obj, key);
 }   break;

 case 'A':
@@ -245,7 +239,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list 
args)
 goto cleanup;
 }

-rc = virJSONValueObjectAppend(jargs, key, val);
+rc = virJSONValueObjectAppend(obj, key, val);
 }   break;

 default:
@@ -259,17 +253,45 @@ 

[libvirt] [PATCH 05/12] util: json: add helper to iterate JSON object key=value pairs

2015-01-28 Thread Peter Krempa
This helper easies iterating all key=value pairs stored in a JSON
object. Usually we pick only certain known keys from a JSON object, but
this will allow to walk complete objects and have the callback act on
those.
---
 src/libvirt_private.syms |  1 +
 src/util/virjson.c   | 33 +
 src/util/virjson.h   |  8 
 3 files changed, 42 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 70c81a8..196b837 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1548,6 +1548,7 @@ virJSONValueObjectAppendNumberUlong;
 virJSONValueObjectAppendString;
 virJSONValueObjectCreate;
 virJSONValueObjectCreateVArgs;
+virJSONValueObjectForeachKeyValue;
 virJSONValueObjectGet;
 virJSONValueObjectGetBoolean;
 virJSONValueObjectGetKey;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 3e00650..ec1778d 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1198,6 +1198,39 @@ virJSONValueObjectIsNull(virJSONValuePtr object,
 }


+/**
+ * virJSONValueObjectForeachKeyValue:
+ * @object: JSON object to iterate
+ * @cb: callback to call on key-value pairs contained in the object
+ * @opaque: generic data for the callback
+ *
+ * Iterates all key=value pairs in @object. Iteration breaks if @cb returns
+ * negative value.
+ *
+ * Returns 0 if all elements were iterated, -2 if @cb returned negative value
+ * during iteration and -1 on generic errors.
+ */
+int
+virJSONValueObjectForeachKeyValue(virJSONValuePtr object,
+  virJSONValueObjectIteratorFunc cb,
+  void *opaque)
+{
+size_t i;
+
+if (object-type != VIR_JSON_TYPE_OBJECT)
+return -1;
+
+for (i = 0; i  object-data.object.npairs; i++) {
+virJSONObjectPairPtr elem = object-data.object.pairs + i;
+
+if (cb(elem-key, elem-value, opaque)  0)
+return -2;
+}
+
+return 0;
+}
+
+
 #if WITH_YAJL
 static int
 virJSONParserInsertValue(virJSONParserPtr parser,
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 57010b0..9bb7461 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -157,4 +157,12 @@ virJSONValuePtr virJSONValueFromString(const char 
*jsonstring);
 char *virJSONValueToString(virJSONValuePtr object,
bool pretty);

+typedef int (*virJSONValueObjectIteratorFunc)(const char *key,
+  const virJSONValue *value,
+  void *opaque);
+
+int virJSONValueObjectForeachKeyValue(virJSONValuePtr object,
+  virJSONValueObjectIteratorFunc cb,
+  void *opaque);
+
 #endif /* __VIR_JSON_H_ */
-- 
2.2.2

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


[libvirt] [PATCH 08/12] qemu: command: Shuffle around formating of alias for memory backend objs

2015-01-28 Thread Peter Krempa
Move the alias as the second formated argument and tweak the tests so
that a future refactor that will change the order doesn't break tests.
---
 src/qemu/qemu_command.c|  9 -
 .../qemuxml2argvdata/qemuxml2argv-hugepages-pages.args | 16 
 .../qemuxml2argv-hugepages-pages2.args |  8 
 .../qemuxml2argv-hugepages-pages3.args |  4 ++--
 .../qemuxml2argv-hugepages-shared.args | 18 ++
 .../qemuxml2argv-numatune-memnode-no-memory.args   |  2 +-
 .../qemuxml2argv-numatune-memnode.args |  6 +++---
 7 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c65f187..2d89c2d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4623,8 +4623,8 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 if (!(mem_path = qemuGetHugepagePath(cfg-hugetlbfs[i])))
 goto cleanup;

-virBufferAsprintf(buf, memory-backend-file,prealloc=yes,mem-path=%s,
-  mem_path);
+virBufferAsprintf(buf, memory-backend-file,id=%s%zu, aliasPrefix, 
id);
+virBufferAsprintf(buf, ,prealloc=yes,mem-path=%s, mem_path);

 switch (memAccess) {
 case VIR_MEM_ACCESS_SHARED:
@@ -4647,11 +4647,10 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 goto cleanup;
 }

-virBufferAddLit(buf, memory-backend-ram);
+virBufferAsprintf(buf, memory-backend-ram,id=%s%zu, aliasPrefix, 
id);
 }

-virBufferAsprintf(buf, ,size=%lluM,id=%s%zu, size / 1024,
-  aliasPrefix, id);
+virBufferAsprintf(buf, ,size=%lluM, size / 1024);

 if (userNodeset) {
 if (!(nodemask = virBitmapFormat(userNodeset)))
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args
index 042683a..b954fbc 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args
@@ -1,16 +1,16 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M pc -m 4096 -smp 4 \
--object 
memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\
-size=1024M,id=ram-node0,host-nodes=0-3,policy=bind \
+-object memory-backend-file,id=ram-node0,prealloc=yes,\
+mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \
 -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
--object 
memory-backend-file,prealloc=yes,mem-path=/dev/hugepages2M/libvirt/qemu,\
-size=1024M,id=ram-node1,host-nodes=0-3,policy=bind \
+-object memory-backend-file,id=ram-node1,prealloc=yes,\
+mem-path=/dev/hugepages2M/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \
 -numa node,nodeid=1,cpus=1,memdev=ram-node1 \
--object 
memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\
-size=1024M,id=ram-node2,host-nodes=0-3,policy=bind \
+-object memory-backend-file,id=ram-node2,prealloc=yes,\
+mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \
 -numa node,nodeid=2,cpus=2,memdev=ram-node2 \
--object 
memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\
-size=1024M,id=ram-node3,host-nodes=3,policy=bind \
+-object memory-backend-file,id=ram-node3,prealloc=yes,\
+mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=3,policy=bind \
 -numa node,nodeid=3,cpus=3,memdev=ram-node3 \
 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb 
\
 -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
index 9211bc6..90ab776 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
@@ -1,10 +1,10 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M pc -m 1024 -smp 2 \
--object memory-backend-file,prealloc=yes,\
-mem-path=/dev/hugepages2M/libvirt/qemu,size=256M,id=ram-node0 \
+-object memory-backend-file,id=ram-node0,prealloc=yes,\
+mem-path=/dev/hugepages2M/libvirt/qemu,size=256M \
 -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
--object memory-backend-file,prealloc=yes,\
-mem-path=/dev/hugepages2M/libvirt/qemu,size=768M,id=ram-node1 \
+-object memory-backend-file,id=ram-node1,prealloc=yes,\
+mem-path=/dev/hugepages2M/libvirt/qemu,size=768M \
 -numa node,nodeid=1,cpus=1,memdev=ram-node1 \
 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
 -usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
index f81947e..b932520 100644
--- 

[libvirt] [PATCH 07/12] qemu: Extract code to setup memory backing objects

2015-01-28 Thread Peter Krempa
Extract the memory backend device code into a separate function so that
it can be later easily refactored and reused.

Few small changes for future reusability, namely:
- new (currently unused) parameter for user specified page size
- size of the memory is specified in kibibytes, divided up in the
function
- new (currently unused) parameter for user specifed source nodeset
- option to enforce capability check
---
 src/qemu/qemu_command.c | 382 +++-
 1 file changed, 250 insertions(+), 132 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6f298ac..c65f187 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4502,6 +4502,243 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
 }


+/**
+ * qemuBuildMemoryBackendStr:
+ * @size: size of the memory device in kibibytes
+ * @pagesize: size of the requested memory page in KiB, 0 for default
+ * @guestNode: NUMA node in the guest that the memory object will be attached 
to
+ * @hostNodes: map of host nodes to alloc the memory in, NULL for default
+ * @autoNodeset: fallback nodeset in case of automatic numa placement
+ * @def: domain definition object
+ * @qemuCaps: qemu capabilities object
+ * @cfg: qemu driver config object
+ * @aliasPrefix: prefix string of the alias (to allow for multiple frontents)
+ * @id: index of the device (to construct the alias)
+ * @backendStr: returns the object string
+ *
+ * Formats the configuration string for the memory device backend according
+ * to the configuration. @pagesize and @hostNodes can be used to override the
+ * default source configuration, both are optional.
+ *
+ * Returns 0 on success, 1 if only the implicit memory-device-ram with no
+ * other configuration was used (to detect legacy configurations). Returns
+ * -1 in case of an error.
+ */
+static int
+qemuBuildMemoryBackendStr(unsigned long long size,
+  unsigned long long pagesize,
+  int guestNode,
+  virBitmapPtr userNodeset,
+  virBitmapPtr autoNodeset,
+  virDomainDefPtr def,
+  virQEMUCapsPtr qemuCaps,
+  virQEMUDriverConfigPtr cfg,
+  const char *aliasPrefix,
+  size_t id,
+  char **backendStr,
+  bool force)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+virDomainHugePagePtr master_hugepage = NULL;
+virDomainHugePagePtr hugepage = NULL;
+virDomainNumatuneMemMode mode;
+const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+virMemAccess memAccess = def-cpu-cells[guestNode].memAccess;
+size_t i;
+char *mem_path = NULL;
+char *nodemask = NULL;
+char *tmpmask = NULL, *next = NULL;
+int ret = -1;
+
+*backendStr = NULL;
+
+mode = virDomainNumatuneGetMode(def-numatune, guestNode);
+
+if (pagesize == 0 || pagesize != system_page_size) {
+/* Find the huge page size we want to use */
+for (i = 0; i  def-mem.nhugepages; i++) {
+bool thisHugepage = false;
+
+hugepage = def-mem.hugepages[i];
+
+if (!hugepage-nodemask) {
+master_hugepage = hugepage;
+continue;
+}
+
+if (virBitmapGetBit(hugepage-nodemask, guestNode,
+thisHugepage)  0) {
+/* Ignore this error. It's not an error after all. Well,
+ * the nodemask for this page/ can contain lower NUMA
+ * nodes than we are querying in here. */
+continue;
+}
+
+if (thisHugepage) {
+/* Hooray, we've found the page size */
+break;
+}
+}
+
+if (i == def-mem.nhugepages) {
+/* We have not found specific huge page to be used with this
+ * NUMA node. Use the generic setting then (page/ without any
+ * @nodemask) if possible. */
+hugepage = master_hugepage;
+}
+
+if (hugepage)
+pagesize = hugepage-size;
+
+if (hugepage  hugepage-size == system_page_size) {
+/* However, if user specified to use huge page
+ * of regular system page size, it's as if they
+ * hasn't specified any huge pages at all. */
+hugepage = NULL;
+}
+}
+
+if (hugepage) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(this qemu doesn't support hugepage memory 
backing));
+goto cleanup;
+}
+
+/* Now lets see, if the huge page we want to use is even mounted
+ * and ready to use */
+for (i = 0; i  cfg-nhugetlbfs; i++) {
+if (cfg-hugetlbfs[i].size == 

Re: [libvirt] [PATCHv2 2/2] hotplug: only add a chardev to vmdef after monitor call

2015-01-28 Thread Michal Privoznik
On 28.01.2015 10:14, Ján Tomko wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1161024
 
 This way the device is in vmdef only if ret = 0 and the caller
 (qemuDomainAttachDeviceFlags) does not free it.
 
 Otherwise it might get double freed by qemuProcessStop
 and qemuDomainAttachDeviceFlags if the domain crashed
 in monitor after we've added it to vm-def.
 ---
 qemuDomainChrInsertPreAllocCleanup is always called, not just when
 qemuDomainChrPreInsert was called before. But unless I missed something,
 the configuration where nserials == 0, nconsoles == 1 should not
 happen after qemu's PostParse callback.
 
  src/qemu/qemu_hotplug.c | 34 +++---
  1 file changed, 11 insertions(+), 23 deletions(-)
 
 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index 2ea30f5..033b281 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -1523,59 +1523,47 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
  virDomainDefPtr vmdef = vm-def;
  char *devstr = NULL;
  char *charAlias = NULL;
 -bool need_remove = false;
  
  if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
  virReportError(VIR_ERR_OPERATION_INVALID, %s,
 _(qemu does not support -device));
 -return ret;
 +goto cleanup;
  }
  
  if (qemuAssignDeviceChrAlias(vmdef, chr, -1)  0)
 -return ret;
 +goto cleanup;
  
  if (qemuBuildChrDeviceStr(devstr, vm-def, chr, priv-qemuCaps)  0)
 -return ret;
 +goto cleanup;
  
  if (virAsprintf(charAlias, char%s, chr-info.alias)  0)
  goto cleanup;
  
 -if (qemuDomainChrInsert(vmdef, chr)  0)
 +if (qemuDomainChrPreInsert(vmdef, chr)  0)
  goto cleanup;
 -need_remove = true;
  
  qemuDomainObjEnterMonitor(driver, vm);
  if (qemuMonitorAttachCharDev(priv-mon, charAlias, chr-source)  0) {
 -if (qemuDomainObjExitMonitor(driver, vm)  0) {
 -need_remove = false;
 -ret = -1;
 -goto cleanup;
 -}
 +ignore_value(qemuDomainObjExitMonitor(driver, vm));
  goto audit;
  }
  
  if (devstr  qemuMonitorAddDevice(priv-mon, devstr)  0) {
  /* detach associated chardev on error */
  qemuMonitorDetachCharDev(priv-mon, charAlias);
 -if (qemuDomainObjExitMonitor(driver, vm)  0) {
 -need_remove = false;
 -ret = -1;
 -goto cleanup;
 -}
 +ignore_value(qemuDomainObjExitMonitor(driver, vm));
  goto audit;
  }
 -if (qemuDomainObjExitMonitor(driver, vm)  0) {
 -need_remove = false;
 -ret = -1;
 -goto cleanup;
 -}
 +if (qemuDomainObjExitMonitor(driver, vm)  0)
 +goto audit;
  
 +qemuDomainChrInsertPreAlloced(vm-def, chr);
  ret = 0;
   audit:
  virDomainAuditChardev(vm, NULL, chr, attach, ret == 0);
   cleanup:
 -if (ret  0  need_remove)
 -qemuDomainChrRemove(vmdef, chr);
 +if (ret  0  virDomainObjIsActive(vm))
 +qemuDomainChrInsertPreAllocCleanup(vm-def, chr);

It took me a while to see if this is safe. We can jump here even if
vm-def hasn't been touched at all, e.g. if qemu is missing the DEVICE
capability. However, if that's the case, there's currently no way for
vm-def to contain one console but no serial line.


  VIR_FREE(charAlias);
  VIR_FREE(devstr);
  return ret;
 

Michal

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


[libvirt] [java] [PATCH 0/6] Fix JNA wrapping, fix memory leaks and wrap security model / label function

2015-01-28 Thread Claudio Bley
Hi.

First and foremost, this series fixes a few mistakes in the wrapping
code found by inspecting the org.libvirt.jna.Libvirt interface and the
corresponding C types of the XML API file.

The last two patches add two missing functions introduced in libvirt
0.6.1.

At the end of the day, this means libvirt-java has gained full
coverage of the libvirt functions up to and including version
0.8.5. Yay!

Claudio Bley (6):
  JNA: fix wrong return type void vs. int
  JNA: add CString class and fix memory leaks
  JNA: simplify freeing memory for C strings
  Use the CString class for Arrays of CStrings too
  Implement Domain.getSecurityLabel and add SecurityLabel class
  Implement Connect.getSecurityModel and add SecurityModel class

 src/main/java/org/libvirt/Connect.java|  74 +
 src/main/java/org/libvirt/Device.java |   9 +-
 src/main/java/org/libvirt/Domain.java |  57 ++---
 src/main/java/org/libvirt/DomainSnapshot.java |   8 +-
 src/main/java/org/libvirt/Interface.java  |   7 +-
 src/main/java/org/libvirt/Library.java|  44 +++---
 src/main/java/org/libvirt/Network.java|  14 +---
 src/main/java/org/libvirt/NetworkFilter.java  |   2 +-
 src/main/java/org/libvirt/Secret.java |   2 +-
 src/main/java/org/libvirt/SecurityLabel.java  |  49 +++
 src/main/java/org/libvirt/SecurityModel.java  |  37 +
 src/main/java/org/libvirt/StoragePool.java|   9 +-
 src/main/java/org/libvirt/StorageVol.java |  16 +---
 src/main/java/org/libvirt/jna/CString.java|  85 +++
 src/main/java/org/libvirt/jna/Libvirt.java| 114 +-
 15 files changed, 343 insertions(+), 184 deletions(-)
 create mode 100644 src/main/java/org/libvirt/SecurityLabel.java
 create mode 100644 src/main/java/org/libvirt/SecurityModel.java
 create mode 100644 src/main/java/org/libvirt/jna/CString.java

-- 
2.2.2

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


[libvirt] [java] [PATCH 4/6] Use the CString class for Arrays of CStrings too

2015-01-28 Thread Claudio Bley
Change the prototypes of the JNA library interface in order to take
advantage of the CString class.

This removes a fair amount of code in the org.libvirt.Library class
and puts the decoding of C string data into a single place.
---
 src/main/java/org/libvirt/Connect.java | 20 +++---
 src/main/java/org/libvirt/Device.java  |  7 +++--
 src/main/java/org/libvirt/Domain.java  |  6 ++--
 src/main/java/org/libvirt/Library.java | 44 --
 src/main/java/org/libvirt/StoragePool.java |  7 +++--
 src/main/java/org/libvirt/jna/Libvirt.java | 26 +-
 6 files changed, 46 insertions(+), 64 deletions(-)

diff --git a/src/main/java/org/libvirt/Connect.java 
b/src/main/java/org/libvirt/Connect.java
index 437f423..1d37c22 100644
--- a/src/main/java/org/libvirt/Connect.java
+++ b/src/main/java/org/libvirt/Connect.java
@@ -1183,7 +1183,7 @@ public class Connect {
 public String[] listDefinedDomains() throws LibvirtException {
 int maxnames = numOfDefinedDomains();
 if (maxnames  0) {
-final Pointer[] names = new Pointer[maxnames];
+final CString[] names = new CString[maxnames];
 final int n = 
processError(libvirt.virConnectListDefinedDomains(VCP, names, maxnames));
 return Library.toStringArray(names, n);
 } else {
@@ -1201,7 +1201,7 @@ public class Connect {
 public String[] listDefinedInterfaces() throws LibvirtException {
 final int max = numOfDefinedInterfaces();
 if (max  0) {
-final Pointer[] ifs = new Pointer[max];
+final CString[] ifs = new CString[max];
 final int n = 
processError(libvirt.virConnectListDefinedInterfaces(VCP, ifs, max));
 return Library.toStringArray(ifs, n);
 } else {
@@ -1219,7 +1219,7 @@ public class Connect {
 public String[] listDefinedNetworks() throws LibvirtException {
 int maxnames = numOfDefinedNetworks();
 if (maxnames  0) {
-final Pointer[] names = new Pointer[maxnames];
+final CString[] names = new CString[maxnames];
 final int n = 
processError(libvirt.virConnectListDefinedNetworks(VCP, names, maxnames));
 return Library.toStringArray(names, n);
 } else {
@@ -1237,7 +1237,7 @@ public class Connect {
 public String[] listDefinedStoragePools() throws LibvirtException {
 int num = numOfDefinedStoragePools();
 if (num  0) {
-Pointer[] pools = new Pointer[num];
+CString[] pools = new CString[num];
 final int n = 
processError(libvirt.virConnectListDefinedStoragePools(VCP, pools, num));
 return Library.toStringArray(pools, n);
 } else {
@@ -1254,7 +1254,7 @@ public class Connect {
 public String[] listDevices(String capabilityName) throws LibvirtException 
{
 int maxDevices = numOfDevices(capabilityName);
 if (maxDevices  0) {
-Pointer[] names = new Pointer[maxDevices];
+CString[] names = new CString[maxDevices];
 final int n = processError(libvirt.virNodeListDevices(VCP, 
capabilityName, names, maxDevices, 0));
 return Library.toStringArray(names, n);
 } else {
@@ -1288,7 +1288,7 @@ public class Connect {
 public String[] listInterfaces() throws LibvirtException {
 int num = numOfInterfaces();
 if (num  0) {
-Pointer[] ifs = new Pointer[num];
+CString[] ifs = new CString[num];
 final int n = processError(libvirt.virConnectListInterfaces(VCP, 
ifs, num));
 return Library.toStringArray(ifs, n);
 } else {
@@ -1305,7 +1305,7 @@ public class Connect {
 public String[] listNetworkFilters() throws LibvirtException {
 int maxnames = numOfNetworkFilters();
 if (maxnames  0) {
-Pointer[] names = new Pointer[maxnames];
+CString[] names = new CString[maxnames];
 final int n = processError(libvirt.virConnectListNWFilters(VCP, 
names, maxnames));
 return Library.toStringArray(names, n);
 } else {
@@ -1323,7 +1323,7 @@ public class Connect {
 public String[] listNetworks() throws LibvirtException {
 int maxnames = numOfNetworks();
 if (maxnames  0) {
-Pointer[] names = new Pointer[maxnames];
+CString[] names = new CString[maxnames];
 final int n = processError(libvirt.virConnectListNetworks(VCP, 
names, maxnames));
 return Library.toStringArray(names, n);
 } else {
@@ -1340,7 +1340,7 @@ public class Connect {
 public String[] listSecrets() throws LibvirtException {
 int num = numOfSecrets();
 if (num  0) {
-Pointer[] returnValue = new Pointer[num];
+CString[] returnValue = new CString[num];
 final int n = processError(libvirt.virConnectListSecrets(VCP, 
returnValue, num));
 

Re: [libvirt] [PATCH] qemu: fix cannot set graphic passwd via qemuDomainSaveImageDefineXML

2015-01-28 Thread lhuang


On 01/29/2015 12:25 AM, Michal Privoznik wrote:

On 20.01.2015 10:04, Luyao Huang wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1183890

When we try to update a xml to a image file, we will clear the
graphics passwd settings, because we do not pass VIR_DOMAIN_XML_SECURE
to qemuDomainDefCopy, qemuDomainDefFormatBuf won't format the passwd.

Add VIR_DOMAIN_XML_SECURE flag when we call qemuDomainDefCopy
in qemuDomainSaveImageUpdateDef.

Signed-off-by: Luyao Huang lhu...@redhat.com
---
  src/qemu/qemu_driver.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5994558..abe3b9f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5613,7 +5613,8 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,
  
  if (!(newdef_migr = qemuDomainDefCopy(driver,

newdef,
-  VIR_DOMAIN_XML_MIGRATABLE)))
+  VIR_DOMAIN_XML_MIGRATABLE |
+  VIR_DOMAIN_XML_SECURE)))
  goto cleanup;
  
  if (!virDomainDefCheckABIStability(def, newdef_migr)) {



ACKed and pushed. Thanks for catching that.


Thanks for your review and push.


Michal


Luyao

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


[libvirt] [java] [PATCH 5/6] Implement Domain.getSecurityLabel and add SecurityLabel class

2015-01-28 Thread Claudio Bley
This wraps the native virDomainGetSecurityLabel libvirt function available
since version 0.6.1.
---
 src/main/java/org/libvirt/Domain.java| 18 ++
 src/main/java/org/libvirt/SecurityLabel.java | 49 
 src/main/java/org/libvirt/jna/Libvirt.java   | 24 +-
 3 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 src/main/java/org/libvirt/SecurityLabel.java

diff --git a/src/main/java/org/libvirt/Domain.java 
b/src/main/java/org/libvirt/Domain.java
index ed6690c..83a500c 100644
--- a/src/main/java/org/libvirt/Domain.java
+++ b/src/main/java/org/libvirt/Domain.java
@@ -637,6 +637,24 @@ public class Domain {
 }
 
 /**
+ * Get the security label of an active domain.
+ *
+ * @return the SecurityLabel or {@code null} if the domain is not
+ * running under a security model
+ * @throws LibvirtException
+ */
+public SecurityLabel getSecurityLabel() throws LibvirtException {
+Libvirt.SecurityLabel seclabel = new Libvirt.SecurityLabel();
+
+processError(libvirt.virDomainGetSecurityLabel(this.VDP, seclabel));
+
+if (seclabel.label[0] == 0)
+return null;
+else
+return new SecurityLabel(seclabel);
+}
+
+/**
  * Get the UUID for this domain.
  *
  * @return the UUID as an unpacked int array
diff --git a/src/main/java/org/libvirt/SecurityLabel.java 
b/src/main/java/org/libvirt/SecurityLabel.java
new file mode 100644
index 000..60132ba
--- /dev/null
+++ b/src/main/java/org/libvirt/SecurityLabel.java
@@ -0,0 +1,49 @@
+package org.libvirt;
+
+import org.libvirt.jna.Libvirt;
+import com.sun.jna.Native;
+
+/**
+ * Represents a security label used for mandatory access control.
+ *
+ * @see Domain#getSecurityLabel
+ */
+public final class SecurityLabel {
+private String label;
+private boolean enforced;
+private static byte NUL = 0;
+
+SecurityLabel(Libvirt.SecurityLabel seclabel) {
+label = Native.toString(seclabel.label, UTF-8);
+enforced = seclabel.enforcing == 1;
+}
+
+/**
+ * Returns the label of this SecurityLabel.
+ *
+ * @return the security label string
+ */
+public String getLabel() {
+return label;
+}
+
+/**
+ * Returns true if the security policy is being enforced.
+ *
+ * @return true if the policy is enforced, false otherwise
+ */
+public boolean isEnforced() {
+return enforced;
+}
+
+@Override
+public String toString() {
+return new StringBuilder()
+.append((label=)
+.append(label)
+.append(, enforced=)
+.append(enforced)
+.append())
+.toString();
+}
+}
diff --git a/src/main/java/org/libvirt/jna/Libvirt.java 
b/src/main/java/org/libvirt/jna/Libvirt.java
index 3589525..2958233 100644
--- a/src/main/java/org/libvirt/jna/Libvirt.java
+++ b/src/main/java/org/libvirt/jna/Libvirt.java
@@ -8,9 +8,13 @@ import com.sun.jna.Native;
 import com.sun.jna.NativeLong;
 import com.sun.jna.Platform;
 import com.sun.jna.Pointer;
+import com.sun.jna.Structure;
 import com.sun.jna.ptr.IntByReference;
 import com.sun.jna.ptr.LongByReference;
 
+import java.util.Arrays;
+import java.util.List;
+
 /**
  * The libvirt interface which is exposed via JNA. The complete API is
  * documented at http://www.libvirt.org/html/libvirt-libvirt.html.
@@ -36,7 +40,6 @@ import com.sun.jna.ptr.LongByReference;
  * LIBVIRT_0.6.1
  * virFreeError
  * virSaveLastError
- * virDomainGetSecurityLabel;
  * virNodeGetSecurityModel;
  *
  * LIBVIRT_0.6.4
@@ -152,6 +155,24 @@ public interface Libvirt extends Library {
 // Connection Functions
 CString virConnectBaselineCPU(ConnectionPointer virConnectPtr, String[] 
xmlCPUs, int ncpus, int flags);
 
+///
+/// Structure definitions
+///
+
+static class SecurityLabel extends Structure {
+private static final int VIR_SECURITY_LABEL_BUFLEN = 4096 + 1;
+private static final ListString fields = Arrays.asList(label, 
enforcing);
+
+public byte label[] = new byte[VIR_SECURITY_LABEL_BUFLEN];
+public int enforcing;
+
+@Override
+protected ListString getFieldOrder() {
+return fields;
+}
+};
+
+
 /**
  * @deprecated as of libvirt 0.6.0, all errors reported in the
  * per-connection object are also duplicated in the global error
@@ -265,6 +286,7 @@ public interface Libvirt extends Library {
 int virDomainGetSchedulerParameters(DomainPointer virDomainPtr, 
virSchedParameter[] params,
 IntByReference nparams);
 CString virDomainGetSchedulerType(DomainPointer virDomainPtr, 
IntByReference nparams);
+int virDomainGetSecurityLabel(DomainPointer virDomainPtr, SecurityLabel 
seclabel);
 int virDomainGetUUID(DomainPointer virDomainPtr, byte[] uuidString);
 int virDomainGetUUIDString(DomainPointer virDomainPtr, 

[libvirt] [java] [PATCH 3/6] JNA: simplify freeing memory for C strings

2015-01-28 Thread Claudio Bley
Make use of the CString class introduced recently and get rid of some
boilerplate code required to release memory at the calling side.
---
 src/main/java/org/libvirt/Connect.java| 22 +++---
 src/main/java/org/libvirt/Domain.java | 33 +++
 src/main/java/org/libvirt/DomainSnapshot.java |  8 +--
 src/main/java/org/libvirt/Interface.java  |  7 +-
 src/main/java/org/libvirt/Network.java| 14 ++--
 src/main/java/org/libvirt/StorageVol.java | 16 ++---
 src/main/java/org/libvirt/jna/Libvirt.java| 26 ++---
 7 files changed, 30 insertions(+), 96 deletions(-)

diff --git a/src/main/java/org/libvirt/Connect.java 
b/src/main/java/org/libvirt/Connect.java
index d0bdc4d..437f423 100644
--- a/src/main/java/org/libvirt/Connect.java
+++ b/src/main/java/org/libvirt/Connect.java
@@ -978,12 +978,7 @@ public class Connect {
  *  description/a
  */
 public String getCapabilities() throws LibvirtException {
-Pointer ptr = processError(libvirt.virConnectGetCapabilities(VCP));
-try {
-return Library.getString(ptr);
-} finally {
-Library.free(ptr);
-}
+return processError(libvirt.virConnectGetCapabilities(VCP)).toString();
 }
 
 /**
@@ -1014,12 +1009,7 @@ public class Connect {
  * @throws LibvirtException
  */
 public String getHostName() throws LibvirtException {
-Pointer ptr = processError(libvirt.virConnectGetHostname(VCP));
-try {
-return Library.getString(ptr);
-} finally {
-Library.free(ptr);
-}
+return processError(libvirt.virConnectGetHostname(VCP)).toString();
 }
 
 /**
@@ -1083,13 +1073,7 @@ public class Connect {
  * @since 1.5.2
  */
 public String getSysinfo() throws LibvirtException {
-Pointer p = processError(libvirt.virConnectGetSysinfo(this.VCP, 0));
-
-try {
-return Library.getString(p);
-} finally {
-Library.free(p);
-}
+return processError(libvirt.virConnectGetSysinfo(this.VCP, 
0)).toString();
 }
 
 /**
diff --git a/src/main/java/org/libvirt/Domain.java 
b/src/main/java/org/libvirt/Domain.java
index ab646de..087a06f 100644
--- a/src/main/java/org/libvirt/Domain.java
+++ b/src/main/java/org/libvirt/Domain.java
@@ -4,6 +4,7 @@ import java.nio.ByteBuffer;
 import java.util.concurrent.TimeUnit;
 
 import org.libvirt.event.IOErrorListener;
+import org.libvirt.jna.CString;
 import org.libvirt.jna.DomainPointer;
 import org.libvirt.jna.DomainSnapshotPointer;
 import org.libvirt.jna.Libvirt;
@@ -589,12 +590,7 @@ public class Domain {
  * @throws LibvirtException
  */
 public String getOSType() throws LibvirtException {
-Pointer ptr = processError(libvirt.virDomainGetOSType(VDP));
-try {
-return Library.getString(ptr);
-} finally {
-Library.free(ptr);
-}
+return processError(libvirt.virDomainGetOSType(VDP)).toString();
 }
 
 /**
@@ -605,7 +601,7 @@ public class Domain {
  */
 public SchedParameter[] getSchedulerParameters() throws LibvirtException {
 IntByReference nParams = new IntByReference();
-Library.free(processError(libvirt.virDomainGetSchedulerType(VDP, 
nParams)));
+processError(libvirt.virDomainGetSchedulerType(VDP, nParams));
 
 int n = nParams.getValue();
 
@@ -637,12 +633,7 @@ public class Domain {
  * @throws LibvirtException
  */
 public String getSchedulerType() throws LibvirtException {
-Pointer pScheduler = 
processError(libvirt.virDomainGetSchedulerType(VDP, null));
-try {
-return Library.getString(pScheduler);
-} finally {
-Library.free(pScheduler);
-}
+return processError(libvirt.virDomainGetSchedulerType(VDP, 
null)).toString();
 }
 
 /**
@@ -725,12 +716,7 @@ public class Domain {
  *  Description format /a
  */
 public String getXMLDesc(int flags) throws LibvirtException {
-Pointer ptr = processError(libvirt.virDomainGetXMLDesc(VDP, flags));
-try {
-return Library.getString(ptr);
-} finally {
-Library.free(ptr);
-}
+return processError(libvirt.virDomainGetXMLDesc(VDP, 
flags)).toString();
 }
 
 /**
@@ -1290,13 +1276,10 @@ public class Domain {
 }
 
 public String screenshot(Stream stream, int screen) throws 
LibvirtException {
-Pointer ptr = processError(libvirt.virDomainScreenshot(this.VDP, 
stream.getVSP(), screen, 0));
+CString mimeType = libvirt.virDomainScreenshot(this.VDP, 
stream.getVSP(), screen, 0);
+processError(mimeType);
 stream.markReadable();
-try {
-return Library.getString(ptr);
-} finally {
-Library.free(ptr);
-}
+return 

[libvirt] [java] [PATCH 6/6] Implement Connect.getSecurityModel and add SecurityModel class

2015-01-28 Thread Claudio Bley
---
 src/main/java/org/libvirt/Connect.java   | 15 +++
 src/main/java/org/libvirt/SecurityModel.java | 37 
 src/main/java/org/libvirt/jna/Libvirt.java   | 16 +++-
 3 files changed, 67 insertions(+), 1 deletion(-)
 create mode 100644 src/main/java/org/libvirt/SecurityModel.java

diff --git a/src/main/java/org/libvirt/Connect.java 
b/src/main/java/org/libvirt/Connect.java
index 1d37c22..04de4fd 100644
--- a/src/main/java/org/libvirt/Connect.java
+++ b/src/main/java/org/libvirt/Connect.java
@@ -1062,6 +1062,21 @@ public class Connect {
 }
 
 /**
+ * Returns the security model of the connected node.
+ */
+public SecurityModel getSecurityModel() throws LibvirtException {
+Libvirt.SecurityModel secmodel = new Libvirt.SecurityModel();
+
+processError(libvirt.virNodeGetSecurityModel(this.VCP, secmodel));
+
+if (secmodel.model[0] == 0)
+return null;
+else
+return new SecurityModel(secmodel);
+
+}
+
+/**
  * Returns the XML description of the sysinfo details for the host
  * on which the hypervisor is running.
  * p
diff --git a/src/main/java/org/libvirt/SecurityModel.java 
b/src/main/java/org/libvirt/SecurityModel.java
new file mode 100644
index 000..b41835f
--- /dev/null
+++ b/src/main/java/org/libvirt/SecurityModel.java
@@ -0,0 +1,37 @@
+package org.libvirt;
+
+import org.libvirt.jna.Libvirt;
+import com.sun.jna.Native;
+
+/**
+ * A security model used for mandatory access control.
+ *
+ * @see Connect#getSecurityModel
+ */
+public final class SecurityModel {
+private String model;
+private String doi;
+
+SecurityModel(Libvirt.SecurityModel secmodel) {
+model = Native.toString(secmodel.model, UTF-8);
+doi = Native.toString(secmodel.doi, UTF-8);
+}
+
+/**
+ * Returns the model of this SecurityModel.
+ *
+ * @return the model string
+ */
+public String getModel() {
+return model;
+}
+
+/**
+ * Returns the DOI, domain of interpretation of this security model.
+ *
+ * @return the DOI
+ */
+public String getDomainOfInterpretation() {
+return doi;
+}
+}
diff --git a/src/main/java/org/libvirt/jna/Libvirt.java 
b/src/main/java/org/libvirt/jna/Libvirt.java
index 2958233..024e268 100644
--- a/src/main/java/org/libvirt/jna/Libvirt.java
+++ b/src/main/java/org/libvirt/jna/Libvirt.java
@@ -40,7 +40,6 @@ import java.util.List;
  * LIBVIRT_0.6.1
  * virFreeError
  * virSaveLastError
- * virNodeGetSecurityModel;
  *
  * LIBVIRT_0.6.4
  * virInterfaceRef
@@ -172,6 +171,20 @@ public interface Libvirt extends Library {
 }
 };
 
+static class SecurityModel extends Structure {
+private static final int VIR_SECURITY_MODEL_BUFLEN = 256 + 1;
+private static final int VIR_SECURITY_DOI_BUFLEN = 256 + 1;
+
+private static final ListString fields = Arrays.asList(model, 
doi);
+
+public byte model[] = new byte[VIR_SECURITY_MODEL_BUFLEN];
+public byte doi[] = new byte[VIR_SECURITY_DOI_BUFLEN];
+
+@Override
+protected ListString getFieldOrder() {
+return fields;
+}
+}
 
 /**
  * @deprecated as of libvirt 0.6.0, all errors reported in the
@@ -362,6 +375,7 @@ public interface Libvirt extends Library {
 int virNodeGetCellsFreeMemory(ConnectionPointer virConnectPtr, 
LongByReference freeMems, int startCell,
 int maxCells);
 long virNodeGetFreeMemory(ConnectionPointer virConnectPtr);
+int virNodeGetSecurityModel(ConnectionPointer virConnectPtr, SecurityModel 
secmodel);
 
 // Node/Device functions
 int virNodeNumOfDevices(ConnectionPointer virConnectPtr, String 
capabilityName, int flags);
-- 
2.2.2

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


[libvirt] [java] [PATCH 2/6] JNA: add CString class and fix memory leaks

2015-01-28 Thread Claudio Bley
Some libvirt functions were wrapped using a Java String as return
type, although they return a char*. That means the memory allocated
by libvirt for those strings is never freed because JNA assumes a
const char*.

Start to refactor the String handling by using a type-safe Pointer for
C Strings, which automatically frees the memory when converting from
the native pointer to the Java type.
---
 src/main/java/org/libvirt/Connect.java   | 17 +++---
 src/main/java/org/libvirt/Device.java|  2 +-
 src/main/java/org/libvirt/NetworkFilter.java |  2 +-
 src/main/java/org/libvirt/Secret.java|  2 +-
 src/main/java/org/libvirt/StoragePool.java   |  2 +-
 src/main/java/org/libvirt/jna/CString.java   | 85 
 src/main/java/org/libvirt/jna/Libvirt.java   | 18 +++---
 7 files changed, 108 insertions(+), 20 deletions(-)
 create mode 100644 src/main/java/org/libvirt/jna/CString.java

diff --git a/src/main/java/org/libvirt/Connect.java 
b/src/main/java/org/libvirt/Connect.java
index e86fd83..d0bdc4d 100644
--- a/src/main/java/org/libvirt/Connect.java
+++ b/src/main/java/org/libvirt/Connect.java
@@ -7,6 +7,7 @@ import java.util.UUID;
 
 import org.libvirt.event.*;
 import org.libvirt.jna.ConnectionPointer;
+import org.libvirt.jna.CString;
 import org.libvirt.jna.DevicePointer;
 import org.libvirt.jna.DomainPointer;
 import org.libvirt.jna.InterfacePointer;
@@ -364,7 +365,8 @@ public class Connect {
  * @throws LibvirtException
  */
 public String baselineCPU(String[] xmlCPUs) throws LibvirtException {
-return processError(libvirt.virConnectBaselineCPU(VCP, xmlCPUs, 
xmlCPUs.length, 0));
+CString result = libvirt.virConnectBaselineCPU(VCP, xmlCPUs, 
xmlCPUs.length, 0);
+return processError(result).toString();
 }
 
 /**
@@ -920,7 +922,8 @@ public class Connect {
  * @throws LibvirtException
  */
 public String domainXMLFromNative(String nativeFormat, String 
nativeConfig, int flags) throws LibvirtException {
-return processError(libvirt.virConnectDomainXMLFromNative(VCP, 
nativeFormat, nativeConfig, 0));
+CString result = libvirt.virConnectDomainXMLFromNative(VCP, 
nativeFormat, nativeConfig, 0);
+return processError(result).toString();
 }
 
 /**
@@ -932,8 +935,8 @@ public class Connect {
  * @throws LibvirtException
  */
 public String domainXMLToNative(String nativeFormat, String domainXML, int 
flags) throws LibvirtException {
-String returnValue = libvirt.virConnectDomainXMLToNative(VCP, 
nativeFormat, domainXML, 0);
-return processError(returnValue);
+CString returnValue = libvirt.virConnectDomainXMLToNative(VCP, 
nativeFormat, domainXML, 0);
+return processError(returnValue).toString();
 }
 
 @Override
@@ -962,8 +965,8 @@ public class Connect {
  * @throws LibvirtException
  */
 public String findStoragePoolSources(String type, String srcSpecs, int 
flags) throws LibvirtException {
-String returnValue = libvirt.virConnectFindStoragePoolSources(VCP, 
type, srcSpecs, flags);
-return processError(returnValue);
+CString returnValue = libvirt.virConnectFindStoragePoolSources(VCP, 
type, srcSpecs, flags);
+return processError(returnValue).toString();
 }
 
 /**
@@ -1109,7 +1112,7 @@ public class Connect {
  * @throws LibvirtException
  */
 public String getURI() throws LibvirtException {
-return processError(libvirt.virConnectGetURI(VCP));
+return processError(libvirt.virConnectGetURI(VCP)).toString();
 }
 
 /**
diff --git a/src/main/java/org/libvirt/Device.java 
b/src/main/java/org/libvirt/Device.java
index 3c876c6..04f373e 100644
--- a/src/main/java/org/libvirt/Device.java
+++ b/src/main/java/org/libvirt/Device.java
@@ -118,7 +118,7 @@ public class Device {
  * @throws LibvirtException
  */
 public String getXMLDescription() throws LibvirtException {
-return processError(libvirt.virNodeDeviceGetXMLDesc(VDP, 0));
+return processError(libvirt.virNodeDeviceGetXMLDesc(VDP, 
0)).toString();
 }
 
 /**
diff --git a/src/main/java/org/libvirt/NetworkFilter.java 
b/src/main/java/org/libvirt/NetworkFilter.java
index 4f4bc6c..094a5f6 100644
--- a/src/main/java/org/libvirt/NetworkFilter.java
+++ b/src/main/java/org/libvirt/NetworkFilter.java
@@ -90,7 +90,7 @@ public class NetworkFilter {
  * @return the XML document
  */
 public String getXMLDesc() throws LibvirtException {
-return processError(libvirt.virNWFilterGetXMLDesc(NFP, 0));
+return processError(libvirt.virNWFilterGetXMLDesc(NFP, 0)).toString();
 }
 
 /**
diff --git a/src/main/java/org/libvirt/Secret.java 
b/src/main/java/org/libvirt/Secret.java
index e0ded73..a5f13ac 100644
--- a/src/main/java/org/libvirt/Secret.java
+++ b/src/main/java/org/libvirt/Secret.java
@@ -127,7 +127,7 @@ public class Secret {
  * @return the XML document
  

[libvirt] [java] [PATCH 1/6] JNA: fix wrong return type void vs. int

2015-01-28 Thread Claudio Bley
The functions virEventUpdateTimeout and virConnResetLastError have no
return value, they return void. In Java they had been mistakenly
wrapped as returning int.
---
 src/main/java/org/libvirt/jna/Libvirt.java | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/libvirt/jna/Libvirt.java 
b/src/main/java/org/libvirt/jna/Libvirt.java
index a17bfc7..a3cee0a 100644
--- a/src/main/java/org/libvirt/jna/Libvirt.java
+++ b/src/main/java/org/libvirt/jna/Libvirt.java
@@ -217,7 +217,7 @@ public interface Libvirt extends Library {
  */
 @Deprecated
 virError virConnGetLastError(ConnectionPointer virConnectPtr);
-int virConnResetLastError(ConnectionPointer virConnectPtr);
+void virConnResetLastError(ConnectionPointer virConnectPtr);
 String virConnectDomainXMLFromNative(ConnectionPointer virConnectPtr, 
String nativeFormat,
 String nativeConfig, int flags);
 String virConnectDomainXMLToNative(ConnectionPointer virConnectPtr, String 
nativeFormat, String domainXML,
@@ -469,5 +469,5 @@ public interface Libvirt extends Library {
 // Event functions
 int virEventAddTimeout(int milliSeconds, VirEventTimeoutCallback cb, 
Pointer opaque, Pointer ff);
 int virEventRemoveTimeout(int timer);
-int virEventUpdateTimeout(int timer, int timeout);
+void virEventUpdateTimeout(int timer, int timeout);
 }
-- 
2.2.2

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


Re: [libvirt] [PATCHv2 0/6] Resolve issues in disk pool backend

2015-01-28 Thread Michal Privoznik
On 22.01.2015 20:20, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1138516
 
 v1: http://www.redhat.com/archives/libvir-list/2015-January/msg00465.html
 
 In my previous attempt to resolve the issue, I created a stateDir and
 saved the volume XML for each pool in order to attempt to preserve the
 volume name and target format type. However, it was pointed out during
 review that having a different name was not the original design intention.
 The intention was the name would be the partition name rather than something
 the user fabricates and expects to be saved/used. Since partition naming
 is handled by parted, the control over the name is not ours. Additionally,
 a pool refresh or libvirtd restart/reload would use the 'default' of the
 source device path and the partition number from the partition table.
 
 The simple fix to the issue is in patch 6; however, as I was going through
 fixing this I realized a few things and found a few other issues along the
 way, namely:
 
 Patches #1  #2:
 After creating the partition if we fail something within the callback to
 virStorageBackendDiskMakeDataVol as a result of parsing the partition table
 from libvirt_parthelper, then the partition wasn't removed. So, patches
 1  2 work at moving the DeleteVol code and then calling it if something
 in virStorageBackendDiskReadPartitions fails.
 
 Patch #3:
 When determining what type of partitions currently exist in the pool we
 compared against the target.format which is supposed to be the file system
 format type of the partition on the target rather than whether the partition
 is a primary, extended, or logical partition on the source. Since we set
 the partType on the source while reading the partitions, that's what we
 should use.
 
 Patch #4:
 During a refresh of the pool, we use virStorageBackendDiskReadPartitions
 in order to determine what types of partitions exist; however, if we
 have an extended partition and have created either a logical partition
 within or another primary partition after the extended one, then the
 open() will fail with ENXIO.  So I special cased that.
 
 Patch #5:
 When we delete an extended partition, any logical partitions that existed
 in the pool would still be listed even though as part of the delete
 partition logic all the logical partitions were also deleted.
 
 Patch #6:
 So here is essentially the replacement for the previous patch series.
 Since the theory is one is supposed to know what they are doing and
 will provide the correct vol-name, make sure that they do so and cause
 a failure if they don't (indicating what the name should be). As an
 alternative we could just overwrite the name like we did anyway with
 pool refresh or libvirtd restart/reload by doing the following instead:
 
 /* Like the reload/restart/refresh path - use the name created by
  * parted rather than the API/user provided name
  */
 if (STRNEQ(vol-name, partname)) {
 VIR_FREE(vol-name);
 if (VIR_STRDUP(vol-name, partname)  0)
 return -1;
 }
 
 I also added details to the virsh.pod and formatstorage.html.in for
 the 'name' and the intersting side effect I found using 'virsh
 vol-create-as $pool $name $size --format extended'.  I'd remove it,
 but I fear that someone else found this and has made use of it. The
 reality is that '--format' is supposed to be the file system format
 of the partition. However, the way it's used in the code will take
 what is supposed to be target format and allow creation of an extended
 partition. In virStorageBackendDiskPartFormat, if the pool source.format
 is DOS and the vol-target.format is VIR_STORAGE_VOL_DISK_EXTENDED, then
 we create an extended source partition as long as one doesn't already exist.
 
 John Ferlan (6):
   storage: Move virStorageBackendDiskDeleteVol
   storage: Attempt error recovery in virStorageBackendDiskCreateVol
   storage: Fix check for partition type for disk backing volumes
   storage: Adjust how to refresh extended partition disk data
   storage: When delete extended partition, need to refresh pool
   storage: Check the partition name against provided name
 
  docs/formatstorage.html.in |  12 +-
  src/storage/storage_backend.c  |   4 +
  src/storage/storage_backend_disk.c | 235 
 +++--
  tools/virsh.pod|  17 ++-
  4 files changed, 174 insertions(+), 94 deletions(-)
 


ACK series

Michal

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


[libvirt] [PATCH 0/7] Resolve issues saving domain to NFS root squashed client

2015-01-28 Thread John Ferlan
Lots of details here:

https://bugzilla.redhat.com/show_bug.cgi?id=1158034

During my investigation I uncovered some other issues which are covered
by the first 4 patches. The issues are error path related.

The first two patches could be combined, but for now they're apart so
that it's proveable that they resolve a single issue. Hopefully mkletzan
can take a look at these to ensure they fit with the spirit of the
changes they're fixing. With these fixes in place the error from virfile
through qemu will be displayed...

The third and fourth patches can be combined.  Realistically only the
third one is necessary as it resolves an issue introduced by the ref'd
gnulib change (eblake has already read about it). The fourth takes eblake's
suggestion to me in order to take the next step to avoid the throw-away
waitpid loop in virFileOpenForked and replace it by more inline logic.
Since this is code introduced by laine, I'm hoping he'll be able to take
a look-see. It's not required for the series, but nice to have. The
third patch allows us to be able to get the correct EACCES error back
from the vfork'd child, while the forth tries to make it clearer.

The fifth and sixth patches resolve the problem from the above ref'd
bz. Essentially what's happening is that when there's a root squashed
nfs client to which the 'virsh save $dom $file' is being saved, the
attempt by the second virOpenFileAs to open or chmod the $file can
fail because the file generated by the first call may not be able to
be opened, written, or chown'd due to whatever was set by that first call.
The fifth patch is there because I found that if a 'save' file existed
initially and a 'virsh save $dom $file' failed - the $file was deleted
(and of course the subsequent 'virsh save $dom $file' would succeed).
Since we didn't create the file, we shouldn't delete it 

The seventh patch isn't required, but I was hoping it might generate
some discussion as to whether it's the right thing to do. If we don't
set the VIR_FILE_OPEN_FORCE_MODE flag on the virFileOpenAs call, then
the subsequent virFileOpenForceOwnerMode will not change the protection
bits. So why pass something different if the end result is not change?
I looked at other virOpenFileAs paths that are creating and found only
one where the bit was set in virStorageBackendCreateRaw.  Perhaps this
patch generates other patches to really save mode bits when CREAT is
being used.

An eight patch was considered, but I dropped it. I was going to add
an S_IROTH to the bits being passed on that nfs root squash path. This
was being considered because the above ref'd bz uses an nfs pool in
order to save the file into. That pool used different protections on
files when first created and once/if a pool-refresh or libvirtd restart/
reload is done - the pool will fail to start because of the save file's
protections. Setting other+read resolved or work-around that, but
I don't really think that's the right thing to do.  Altough, it is
NFS and well not necessarily secure.


John Ferlan (7):
  qemu: Save/restore error in error path for qemuDomainSaveInternal
  qemu: Adjust EndAsyncJob for qemuDomainSaveInternal error path
  virfile: Need to check for ENOTCONN from recvfd failure
  virfile: Adjust error path for virFileOpenForked
  qemu: Don't unconditionally delete file in qemuOpenFileAs
  qemu: remove failed create prior to root squash share path
  qemu: qemuOpenFileAs - set flag VIR_FILE_OPEN_FORCE_MODE

 src/qemu/qemu_driver.c | 24 +++---
 src/util/virfile.c | 66 +-
 2 files changed, 60 insertions(+), 30 deletions(-)

-- 
2.1.0

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


[libvirt] [PATCH 7/7] qemu: qemuOpenFileAs - set flag VIR_FILE_OPEN_FORCE_MODE

2015-01-28 Thread John Ferlan
In the event we're falling into the code that tries to create the file
in a forked environment (VIR_FILE_OPEN_FORK) we pass different mode bits,
but those are never set because the virFileOpenForceOwnerMode has a check
if the OPEN_FORCE_MODE bit is set before attempting to change the mode.

Since this is a special case it seems reasonable to set u+rw,g+rw,o

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 91fefa9..651f84b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2965,6 +2965,9 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
 
 /* Retry creating the file as qemu user */
 
+/* Since we're passing different modes... */
+vfoflags |= VIR_FILE_OPEN_FORCE_MODE;
+
 if ((fd = virFileOpenAs(path, oflags,
 S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
 fallback_uid, fallback_gid,
-- 
2.1.0

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


[libvirt] [PATCH 5/7] qemu: Don't unconditionally delete file in qemuOpenFileAs

2015-01-28 Thread John Ferlan
If we're expecting to create a file somewhere and that fails for some
reason during qemuOpenFileAs, then we unlink the path we're attempting
to create leaving no way to determine what the existing privileges,
protections, or labels are that caused the failure (open, change owner
and group, change mode, etc)

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0174c87..89b54c8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2899,6 +2899,9 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
 vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
 
 if (stat(path, sb) == 0) {
+/* It already exists, we don't want to delete it on error */
+need_unlink = false;
+
 is_reg = !!S_ISREG(sb.st_mode);
 /* If the path is regular file which exists
  * already and dynamic_ownership is off, we don't
-- 
2.1.0

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


[libvirt] [PATCH 6/7] qemu: remove failed create prior to root squash share path

2015-01-28 Thread John Ferlan
https://bugzilla.redhat.com/show_bug.cgi?id=1158034

In qemuOpenFileAs if we fall into the path where we'll be opening / creating
the file using VIR_FILE_OPEN_FORK, we need to first unlink/delete the file
we created in the first path; otherwise, the attempt by the child process
to open as some specific user:group may fail because the file was already
created using nfsnobody:nfsnobody.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_driver.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 89b54c8..91fefa9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2954,6 +2954,15 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
 goto error;
 }
 
+/* If we created the file above, then we need to remove it;
+ * otherwise, the next attempt to create will fail. If the
+ * file had already existed before we got here, then we also
+ * don't want to delete it and allow the following to succeed
+ * or fail based on existing protections
+ */
+if (need_unlink)
+unlink(path);
+
 /* Retry creating the file as qemu user */
 
 if ((fd = virFileOpenAs(path, oflags,
-- 
2.1.0

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


[libvirt] [PATCH 1/7] qemu: Save/restore error in error path for qemuDomainSaveInternal

2015-01-28 Thread John Ferlan
If we get to endjob: because of some error earlier such as perhaps
failure from qemuDomainSaveMemory to open/create the save file and
the attempt to restart the CPU's fails, then we get the error from
that restart attempt displayed to the user rather than the error
from the failed attempt to create a save file.

Upstream commit id '540c339a' changed the flow of the code moving
the EndAsyncJob call and thus exposing the issue where an error in
restarting CPUs resulted in the following:

error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
error: internal error: unexpected async job 3

where /tmp/pl is a NFS root squashed client where the failure to save
the file (EPERM or ENOTCONN) should result in a failure:

error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
error: Error from child process creating '/tmp/pl/rhel70.save': Transport 
endpoint is not connected

or (REVISIT ORDER - that is if I move the virfile check for ENOTCONN
to sooner, then we'll never see ENOTCONN)

error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
error: Error from child process creating '/tmp/pl/rhel70.save': Operation not 
permitted
Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1d3bee6..190d472 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3211,6 +3211,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
 qemuDomainObjEndAsyncJob(driver, vm);
 if (ret != 0) {
 if (was_running  virDomainObjIsActive(vm)) {
+virErrorPtr save_err = virSaveLastError();
 rc = qemuProcessStartCPUs(driver, vm, dom-conn,
   VIR_DOMAIN_RUNNING_SAVE_CANCELED,
   QEMU_ASYNC_JOB_SAVE);
@@ -3220,6 +3221,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
   
VIR_DOMAIN_EVENT_SUSPENDED,
   
VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR);
 }
+virSetError(save_err);
+virFreeError(save_err);
 }
 } else if (!vm-persistent) {
 qemuDomainRemoveInactive(driver, vm);
-- 
2.1.0

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


[libvirt] [PATCH 4/7] virfile: Adjust error path for virFileOpenForked

2015-01-28 Thread John Ferlan
Rather than have a dummy waitpid loop and return of the failure status
from recvfd, adjust the logic to save the recvfd error  fd and then
in priority order:

if waitpid failed, use that
if waitpid succeeded, but the child reported failure, use that
regardless of recvfd status
if waitpid succeeded and reported success, but recvfd failed with
anything other than EACCES or ENOTCONN, use recvfd's result
otherwise, waitpid and recvfd succeeded, return the fd

NOTE: Original logic to retry the open and force owner mode was
documented as only being attempted if we had already tried opening
with the fork+setuid, but checked flags vs. VIR_FILE_OPEN_NOFORK which
is counter to how we would get to that point. So that code was removed.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/util/virfile.c | 67 +++---
 1 file changed, 39 insertions(+), 28 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 4024f3d..cf6819f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2034,6 +2034,7 @@ virFileOpenForked(const char *path, int openflags, mode_t 
mode,
 {
 pid_t pid;
 int waitret, status, ret = 0;
+int recvfd_errno;
 int fd = -1;
 int pair[2] = { -1, -1 };
 gid_t *groups;
@@ -2124,15 +2125,11 @@ virFileOpenForked(const char *path, int openflags, 
mode_t mode,
 fd = recvfd(pair[0], 0);
 } while (fd  0  errno == EINTR);
 VIR_FORCE_CLOSE(pair[0]); /* NB: this preserves errno */
+recvfd_errno = errno;
 
-/* gnulib will return ENOTCONN in certain instances */
-if (fd  0  !(errno == EACCES || errno == ENOTCONN)) {
-ret = -errno;
-while (waitpid(pid, NULL, 0) == -1  errno == EINTR);
-return ret;
-}
-
-/* wait for child to complete, and retrieve its exit code */
+/* wait for child to complete, and retrieve its exit code
+ * if waitpid fails, use that status
+ */
 while ((waitret = waitpid(pid, status, 0)) == -1  errno == EINTR);
 if (waitret == -1) {
 ret = -errno;
@@ -2142,28 +2139,42 @@ virFileOpenForked(const char *path, int openflags, 
mode_t mode,
 VIR_FORCE_CLOSE(fd);
 return ret;
 }
-if (!WIFEXITED(status) || (ret = -WEXITSTATUS(status)) == -EACCES ||
-fd == -1) {
-/* fall back to the simpler method, which works better in
- * some cases */
+
+/*
+ * if waitpid succeeded, but the child reported failure, use that
+ * regardless of recvfd status
+ */
+if (WIFEXITED(status)  WEXITSTATUS(status) != 0) {
+ret = WEXITSTATUS(status);
+virReportSystemError(ret,
+ _(child failed to create '%s'),
+ path);
 VIR_FORCE_CLOSE(fd);
-if (flags  VIR_FILE_OPEN_NOFORK) {
-/* If we had already tried opening w/o fork+setuid and
- * failed, no sense trying again. Just set return the
- * original errno that we got at that time (by
- * definition, always either EACCES or EPERM - EACCES
- * is close enough).
- */
-return -EACCES;
-}
-if ((fd = open(path, openflags, mode))  0)
-return -errno;
-ret = virFileOpenForceOwnerMode(path, fd, mode, uid, gid, flags);
-if (ret  0) {
-VIR_FORCE_CLOSE(fd);
-return ret;
-}
+return -ret;
+}
+
+/* if waitpid succeeded and reported success, but recvfd failed with
+ * anything other than EACCES or ENOTCONN, use recvfd's result
+ */
+if (WIFEXITED(status)  WEXITSTATUS(status) == 0  fd  0 
+!(recvfd_errno == EACCES || recvfd_errno == ENOTCONN)) {
+virReportSystemError(recvfd_errno,
+ _(failed recvfd for child creating '%s'),
+ path);
+return -recvfd_errno;
+}
+
+/* Some sort of abnormal child termination */
+if (!WIFEXITED(status) || fd == -1) {
+VIR_FORCE_CLOSE(fd);
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(child exited abnormally, failed to create '%s'),
+   path);
+return -EACCES;
 }
+
+/* otherwise, waitpid and recvfd succeeded, return the fd
+ */
 return fd;
 }
 
-- 
2.1.0

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


[libvirt] [PATCH 3/7] virfile: Need to check for ENOTCONN from recvfd failure

2015-01-28 Thread John Ferlan
A gnulib change (commit id 'beae0bdc') causes ENOTCONN to be returned
from recvfd which causes us to fall into the throwaway waitpid() call
and return ENOTCONN to the caller, this then gets displayed during
a 'virsh save' when using a root squashed NFS environment that's trying
to save the file as something other than root:root.

This patch will add the additional check for ENOTCONN to force the code
into the waitpid loop looking for the actual status from the _exit()'d
child fork.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/util/virfile.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 5f56005..4024f3d 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2125,7 +2125,8 @@ virFileOpenForked(const char *path, int openflags, mode_t 
mode,
 } while (fd  0  errno == EINTR);
 VIR_FORCE_CLOSE(pair[0]); /* NB: this preserves errno */
 
-if (fd  0  errno != EACCES) {
+/* gnulib will return ENOTCONN in certain instances */
+if (fd  0  !(errno == EACCES || errno == ENOTCONN)) {
 ret = -errno;
 while (waitpid(pid, NULL, 0) == -1  errno == EINTR);
 return ret;
-- 
2.1.0

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


[libvirt] [PATCH 2/7] qemu: Adjust EndAsyncJob for qemuDomainSaveInternal error path

2015-01-28 Thread John Ferlan
Commit id '540c339a' to fix issues with reference counting and transient
domains moved the qemuDomainObjEndAsyncJob call prior to the attempt to
restart the guest CPU's resulting in an error:

error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
error: internal error: unexpected async job 3

when (ret != 0) - eg, the error path from qemuDomainSaveMemory.

This patch will adjust the logic to only call the EndAsyncJob after
we've tried to restart the guest CPUs. It also needs to adjust the
test for qemuDomainRemoveInactive to add the ret == 0 condition.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_driver.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 190d472..0174c87 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3208,7 +3208,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
  VIR_DOMAIN_EVENT_STOPPED,
  VIR_DOMAIN_EVENT_STOPPED_SAVED);
  endjob:
-qemuDomainObjEndAsyncJob(driver, vm);
 if (ret != 0) {
 if (was_running  virDomainObjIsActive(vm)) {
 virErrorPtr save_err = virSaveLastError();
@@ -3224,9 +3223,10 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
 virSetError(save_err);
 virFreeError(save_err);
 }
-} else if (!vm-persistent) {
-qemuDomainRemoveInactive(driver, vm);
 }
+qemuDomainObjEndAsyncJob(driver, vm);
+if (ret == 0  !vm-persistent)
+qemuDomainRemoveInactive(driver, vm);
 
  cleanup:
 VIR_FREE(xml);
-- 
2.1.0

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


[libvirt] [PATCH] virsh: man: Document behavior of some blkdeviotune's flags when querying

2015-01-28 Thread Peter Krempa
--live and --config can't be specified together when querying the
configuration, but are valid when setting. The man page was hinting that
they are valid always.
---
 tools/virsh.pod | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 1591341..804458e 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1100,9 +1100,10 @@ cannot be mixed with non-zero values for read or write.
 If I--live is specified, affect a running guest.
 If I--config is specified, affect the next boot of a persistent guest.
 If I--current is specified, affect the current guest state.
-Both I--live and I--config flags may be given, but I--current is
-exclusive. If no flag is specified, behavior is different depending
-on hypervisor.
+When setting the disk io parameters both I--live and I--config flags may be
+given, but I--current is exclusive. For querying only one of I--live,
+I--config or I--current can be specified. If no flag is specified, behavior
+is different depending on hypervisor.

 =item Bblockjob Idomain Ipath { [I--abort] [I--async] [I--pivot] |
 [I--info] [I--raw] [I--bytes] | [Ibandwidth] }
-- 
2.2.2

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


Re: [libvirt] [PATCH] qemu: fix cannot set graphic passwd via qemuDomainSaveImageDefineXML

2015-01-28 Thread Michal Privoznik
On 20.01.2015 10:04, Luyao Huang wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1183890
 
 When we try to update a xml to a image file, we will clear the
 graphics passwd settings, because we do not pass VIR_DOMAIN_XML_SECURE
 to qemuDomainDefCopy, qemuDomainDefFormatBuf won't format the passwd.
 
 Add VIR_DOMAIN_XML_SECURE flag when we call qemuDomainDefCopy
 in qemuDomainSaveImageUpdateDef.
 
 Signed-off-by: Luyao Huang lhu...@redhat.com
 ---
  src/qemu/qemu_driver.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 5994558..abe3b9f 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -5613,7 +5613,8 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,
  
  if (!(newdef_migr = qemuDomainDefCopy(driver,
newdef,
 -  VIR_DOMAIN_XML_MIGRATABLE)))
 +  VIR_DOMAIN_XML_MIGRATABLE |
 +  VIR_DOMAIN_XML_SECURE)))
  goto cleanup;
  
  if (!virDomainDefCheckABIStability(def, newdef_migr)) {
 

ACKed and pushed. Thanks for catching that.

Michal

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


[libvirt] [PATCH] libvirt.spec: remove vbox storage and network .so files

2015-01-28 Thread Pavel Hrdina
Commit 55ea7be7 removed separated modules for vbox_network and
vbox_storage drivers but forget to update libvirt.spec.in file. This
patch will fix rpm build.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---

Pushed under trivial rule.

 libvirt.spec.in | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index b2db80e..7f8be38 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -2117,8 +2117,6 @@ exit 0
 %files daemon-driver-vbox
 %defattr(-, root, root)
 %{_libdir}/%{name}/connection-driver/libvirt_driver_vbox.so
-%{_libdir}/%{name}/connection-driver/libvirt_driver_vbox_network.so
-%{_libdir}/%{name}/connection-driver/libvirt_driver_vbox_storage.so
 %endif
 %endif # %{with_driver_modules}
 
-- 
2.0.5

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


Re: [libvirt] [PATCHv2] util: bitmap: Tolerate NULL bitmaps in virBitmapEqual

2015-01-28 Thread Ján Tomko
On 01/26/2015 10:39 AM, Peter Krempa wrote:
 After virBitmapEqual is able to compare NULL bitmaps few bits of code
 can be cleaned up.
 ---
 Version 2 cleans up code paths that would do a duplicate check now.
 
  src/conf/domain_conf.c   | 40 ++--
  src/conf/numatune_conf.c |  3 ---
  src/util/virbitmap.c |  6 ++
  src/util/virbitmap.h |  3 +--
  4 files changed, 17 insertions(+), 35 deletions(-)

ACK

Jan



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

[libvirt] [PATCH] schemas: Allow all generic elements and attributes for all interfaces

2015-01-28 Thread Michal Privoznik
There are some interface types (notably 'server' and 'client')
which instead of allowing the default set of elements and
attributes (like the rest do), try to enumerate only the elements
they know of. This way it's, however, easy to miss something. For
instance, the address/ element was not mentioned at all. This
resulted in a strange behavior: when such interface was added
into XML, the address was automatically generated by parsing
code. Later, the formatted XML hasn't passed the RNG schema. This
became more visible once we've turned on the XML validation on
domain XML changes: appending an empty line at the end of
formatted XML (to trick virsh think the XML had changed) made
libvirt to refuse the very same XML it formatted.

Instead of trying to find each element and attribute we are
missing in the schema, lets just allow all the elements and
attributes like we're doing that for the rest of types. It's no
harm if the schema is wider than our parser allows.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 docs/schemas/domaincommon.rng  |  28 +---
 .../qemuxml2argv-interface-server.xml  | 157 +
 2 files changed, 159 insertions(+), 26 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-interface-server.xml

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 9d6c1ee..d467dce 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2173,14 +2173,7 @@
   /attribute
   empty/
 /element
-optional
-  element name=mac
-attribute name=address
-  ref name=uniMacAddr/
-/attribute
-empty/
-  /element
-/optional
+ref name=interface-options/
   /interleave
 /group
 group
@@ -2199,24 +2192,7 @@
   /attribute
   empty/
 /element
-optional
-  element name=mac
-attribute name=address
-  ref name=uniMacAddr/
-/attribute
-empty/
-  /element
-/optional
-optional
-  element name=model
-attribute name=type
-  data type=string
-param name='pattern'[a-zA-Z0-9\-_]+/param
-  /data
-/attribute
-empty/
-  /element
-/optional
+ref name=interface-options/
   /interleave
 /group
 group
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-interface-server.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-interface-server.xml
new file mode 100644
index 000..9edf773
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-interface-server.xml
@@ -0,0 +1,157 @@
+domain type='kvm'
+  namegentoo/name
+  uuida75aca4b-a02f-2bcb-4a91-c93cd848c34b/uuid
+  memory unit='KiB'4194304/memory
+  currentMemory unit='KiB'4194304/currentMemory
+  memoryBacking
+hugepages
+  page size='1048576' unit='KiB' nodeset='0-3'/
+/hugepages
+  /memoryBacking
+  vcpu placement='static'4/vcpu
+  os
+type arch='x86_64' machine='pc-i440fx-1.4'hvm/type
+boot dev='hd'/
+boot dev='cdrom'/
+  /os
+  features
+acpi/
+apic/
+pae/
+  /features
+  cpu mode='custom' match='exact'
+model fallback='allow'Haswell/model
+vendorIntel/vendor
+feature policy='require' name='tm2'/
+feature policy='require' name='est'/
+feature policy='require' name='vmx'/
+feature policy='require' name='osxsave'/
+feature policy='require' name='smx'/
+feature policy='require' name='ss'/
+feature policy='require' name='ds'/
+feature policy='require' name='vme'/
+feature policy='require' name='dtes64'/
+feature policy='require' name='abm'/
+feature policy='require' name='ht'/
+feature policy='require' name='acpi'/
+feature policy='require' name='pbe'/
+feature policy='require' name='tm'/
+feature policy='require' name='pdcm'/
+feature policy='require' name='pdpe1gb'/
+feature policy='require' name='ds_cpl'/
+feature policy='require' name='rdrand'/
+feature policy='require' name='f16c'/
+feature policy='require' name='xtpr'/
+feature policy='require' name='monitor'/
+numa
+  cell id='0' cpus='0' memory='1048576' unit='KiB'/
+  cell id='1' cpus='1' memory='1048576' unit='KiB'/
+  cell id='2' cpus='2' memory='1048576' unit='KiB'/
+  cell id='3' cpus='3' memory='1048576' unit='KiB'/
+/numa
+  /cpu
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashrestart/on_crash
+  pm
+suspend-to-mem enabled='yes'/
+suspend-to-disk enabled='yes'/
+  /pm
+  devices
+emulator/usr/bin/qemu-system-x86_64/emulator
+disk type='file' device='floppy'
+  driver name='qemu' type='raw'