Re: [libvirt] [PATCH] Don't use AI_ADDRCONFIG when binding to wildcard addresses

2014-05-30 Thread Ján Tomko
On 05/29/2014 04:47 PM, Eric Blake wrote:
 On 05/29/2014 03:32 AM, Ján Tomko wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1098659

 With parallel boot, network addresses might not yet be assigned [1],
 but binding to wildcard addresses should work.

 For non-wildcard addresses, ADDRCONFIG is still used. Document this
 in libvirtd.conf.

 [1] http://www.freedesktop.org/wiki/Software/systemd/NetworkTarget/
 ---
 
 -hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
 +hints.ai_flags = AI_PASSIVE;
  hints.ai_socktype = SOCK_STREAM;
  
 +/* Don't use ADDRCONFIG for binding to the wildcard address.
 + * Just catch the error returned by socket() if the system has
 + * no IPv6 support.
 + *
 + * This allows libvirtd to be started in parallel with the network
 + * startup in most cases.
 + */
 +if (nodename 
 +!(virSocketAddrParse(tmp_addr, nodename, AF_UNSPEC)  0 
 +  virSocketAddrIsWildcard(tmp_addr)))
 +hints.ai_flags = AI_ADDRCONFIG;
 
 Shouldn't this be |= ?
 

Functionally it's the same, AI_PASSIVE is ignored if nodename is non-NULL.

But it should be |= if we were using other flags.

Jan



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

[libvirt] Libvrt Migrate an Offline VM

2014-05-30 Thread Sijo Jose
Hi,

Is it possible to migrate an Offline VM using libvirt API ?

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

[libvirt] [PATCH] manual: Add virsh manual about specified migration host

2014-05-30 Thread Chen Fan
the 'migration_host' description maybe have a bit of difficulty to
understand for user, so add this manual for them.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
---
 tools/virsh.pod | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index de9a4f7..8d77a2f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1238,6 +1238,11 @@ seen from the source machine.
 
 When Imigrateuri is not specified, libvirt will automatically determine the
 hypervisor specific URI, by looking up the target host's configured hostname.
+In particular, some hypervisors support having this migration hostname 
specified
+separately by setting 'migration_host' in definition file, if 'migration_host'
+is specified, the hostname or IP address will be used to as the default 
Imigrateuri
+while running migration from source host. if 'migration_host' is not specified,
+the migration hostname is set to the host's configured hostname by default.
 There are a few scenarios where specifying Imigrateuri may help:
 
 =over 4
@@ -1251,7 +1256,9 @@ explicitly specified, using an IP address, or a correct 
hostname.
 interfaces, it might be desirable for the migration data stream to be sent over
 a specific interface for either security or performance reasons.  In this case
 Imigrateuri should be explicitly specified, using an IP address associated
-with the network to be used.
+with the network to be used. In particular, Some hypervisors could be easy to
+specify the default network interface by setting 'migration_host'. then the
+Imigrateuri can be omitted.
 
 =item * The firewall restricts what ports are available.  When libvirt 
generates
 a migration URI, it will pick a port number using hypervisor specific rules.
-- 
1.9.3

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


[libvirt] [PATCHv3 10/36] storage: Add infrastructure to parse remote network backing names

2014-05-30 Thread Peter Krempa
Add parsers for relative and absolute backing names for local and remote
storage files.

This parser parses relative paths as relative to their parents and
absolute paths according to the protocol or local access.

For remote storage volumes, all URI based backing file names are
supported and for the qemu colon syntax the NBD protocol is supported.
---
 src/libvirt_private.syms  |   1 +
 src/util/virstoragefile.c | 364 ++
 src/util/virstoragefile.h |   4 +
 3 files changed, 369 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 57312c3..e1702db 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1883,6 +1883,7 @@ virStorageSourceClear;
 virStorageSourceClearBackingStore;
 virStorageSourceFree;
 virStorageSourceGetActualType;
+virStorageSourceNewFromBacking;
 virStorageSourcePoolDefFree;
 virStorageSourcePoolModeTypeFromString;
 virStorageSourcePoolModeTypeToString;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index a80131a..43b7a5a 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -38,6 +38,8 @@
 #include virendian.h
 #include virstring.h
 #include virutil.h
+#include viruri.h
+#include dirname.h
 #if HAVE_SYS_SYSCALL_H
 # include sys/syscall.h
 #endif
@@ -660,6 +662,19 @@ virStorageIsFile(const char *backing)
 }


+static bool
+virStorageIsRelative(const char *backing)
+{
+if (backing[0] == '/')
+return false;
+
+if (!virStorageIsFile(backing))
+return false;
+
+return true;
+}
+
+
 static int
 virStorageFileProbeFormatFromBuf(const char *path,
  char *buf,
@@ -1563,3 +1578,352 @@ virStorageSourceFree(virStorageSourcePtr def)
 virStorageSourceClear(def);
 VIR_FREE(def);
 }
+
+
+static virStorageSourcePtr
+virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
+   const char *rel)
+{
+char *dirname = NULL;
+const char *parentdir = ;
+virStorageSourcePtr ret;
+
+if (VIR_ALLOC(ret)  0)
+return NULL;
+
+ret-backingRelative = true;
+
+/* XXX Once we get rid of the need to use canonical names in path, we will 
be
+ * able to use mdir_name on parent-path instead of using parent-relDir */
+if (STRNEQ(parent-relDir, /))
+parentdir = parent-relDir;
+
+if (virAsprintf(ret-path, %s/%s, parentdir, rel)  0)
+goto error;
+
+if (virStorageSourceGetActualType(parent) != VIR_STORAGE_TYPE_NETWORK) {
+ret-type = VIR_STORAGE_TYPE_FILE;
+
+/* XXX store the relative directory name for test's sake */
+if (!(ret-relDir = mdir_name(ret-path))) {
+virReportOOMError();
+goto error;
+}
+
+/* XXX we don't currently need to store the canonical path but the
+ * change would break the test suite. Get rid of this later */
+char *tmp = ret-path;
+if (!(ret-path = canonicalize_file_name(tmp))) {
+ret-path = tmp;
+tmp = NULL;
+}
+VIR_FREE(tmp);
+} else {
+ret-type = VIR_STORAGE_TYPE_NETWORK;
+
+/* copy the host network part */
+ret-protocol = parent-protocol;
+if (!(ret-hosts = virStorageNetHostDefCopy(parent-nhosts, 
parent-hosts)))
+goto error;
+ret-nhosts = parent-nhosts;
+
+if (VIR_STRDUP(ret-volume, parent-volume)  0)
+goto error;
+
+/* XXX store the relative directory name for test's sake */
+if (!(ret-relDir = mdir_name(ret-path))) {
+virReportOOMError();
+goto error;
+}
+}
+
+ cleanup:
+VIR_FREE(dirname);
+return ret;
+
+ error:
+virStorageSourceFree(ret);
+ret = NULL;
+goto cleanup;
+}
+
+
+static int
+virStorageSourceParseBackingURI(virStorageSourcePtr src,
+const char *path)
+{
+virURIPtr uri = NULL;
+char **scheme = NULL;
+int ret = -1;
+
+if (!(uri = virURIParse(path))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(failed to parse backing file location '%s'),
+   path);
+goto cleanup;
+}
+
+if (!(scheme = virStringSplit(uri-scheme, +, 2)))
+goto cleanup;
+
+if (!scheme[0] ||
+(src-protocol = virStorageNetProtocolTypeFromString(scheme[0]))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(invalid backing protocol '%s'),
+   NULLSTR(scheme[0]));
+goto cleanup;
+}
+
+if (scheme[1] 
+(src-hosts-transport = 
virStorageNetHostTransportTypeFromString(scheme[1]))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(invalid protocol transport type '%s'),
+   scheme[1]);
+goto cleanup;
+}
+
+/* handle socket stored as a query */
+if (uri-query) {
+if 

[libvirt] [PATCHv3 01/36] storage: backend: Add unique id retrieval API

2014-05-30 Thread Peter Krempa
Different protocols have different means to uniquely identify a storage
file. This patch implements a storage driver API to retrieve a unique
string describing a volume. The current implementation works for local
storage only and returns the canonical path of the volume.

To add caching support the local filesystem driver now has a private
structure holding the cached string, which is created only when it's
initially accessed.

This patch provides the implementation for local files only for start.
---

Notes:
Version 3:
- changed name of variable from uid to canonpath
- documented validity scope of the returned name

 src/storage/storage_backend.h|  3 +++
 src/storage/storage_backend_fs.c | 49 
 src/storage/storage_driver.c | 30 
 src/storage/storage_driver.h |  1 +
 4 files changed, 83 insertions(+)

diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 6be5ca7..5d71cde 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -195,6 +195,8 @@ typedef ssize_t
ssize_t max_len,
char **buf);

+typedef const char *
+(*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src);

 virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);

@@ -211,6 +213,7 @@ struct _virStorageFileBackend {
 virStorageFileBackendInit backendInit;
 virStorageFileBackendDeinit backendDeinit;
 virStorageFileBackendReadHeader storageFileReadHeader;
+virStorageFileBackendGetUniqueIdentifier storageFileGetUniqueIdentifier;

 /* The following group of callbacks is expected to set errno
  * and return -1 on error. No libvirt error shall be reported */
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 003c6df..aed07a6 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1333,6 +1333,14 @@ virStorageBackend virStorageBackendNetFileSystem = {
 };


+typedef struct _virStorageFileBackendFsPriv virStorageFileBackendFsPriv;
+typedef virStorageFileBackendFsPriv *virStorageFileBackendFsPrivPtr;
+
+struct _virStorageFileBackendFsPriv {
+char *canonpath; /* unique file identifier (canonical path) */
+};
+
+
 static void
 virStorageFileBackendFileDeinit(virStorageSourcePtr src)
 {
@@ -1340,16 +1348,27 @@ virStorageFileBackendFileDeinit(virStorageSourcePtr src)
   virStorageTypeToString(virStorageSourceGetActualType(src)),
   src-path);

+virStorageFileBackendFsPrivPtr priv = src-drv-priv;
+
+VIR_FREE(priv-canonpath);
+VIR_FREE(priv);
 }


 static int
 virStorageFileBackendFileInit(virStorageSourcePtr src)
 {
+virStorageFileBackendFsPrivPtr priv = NULL;
+
 VIR_DEBUG(initializing FS storage file %p (%s:%s), src,
   virStorageTypeToString(virStorageSourceGetActualType(src)),
   src-path);

+if (VIR_ALLOC(priv)  0)
+return -1;
+
+src-drv-priv = priv;
+
 return 0;
 }

@@ -1397,6 +1416,23 @@ virStorageFileBackendFileReadHeader(virStorageSourcePtr 
src,
 }


+static const char *
+virStorageFileBackendFileGetUniqueIdentifier(virStorageSourcePtr src)
+{
+virStorageFileBackendFsPrivPtr priv = src-drv-priv;
+
+if (!priv-canonpath) {
+if (!(priv-canonpath = canonicalize_file_name(src-path))) {
+virReportSystemError(errno, _(can't canonicalize path '%s'),
+ src-path);
+return NULL;
+}
+}
+
+return priv-canonpath;
+}
+
+
 virStorageFileBackend virStorageFileBackendFile = {
 .type = VIR_STORAGE_TYPE_FILE,

@@ -1406,6 +1442,8 @@ virStorageFileBackend virStorageFileBackendFile = {
 .storageFileUnlink = virStorageFileBackendFileUnlink,
 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
+
+.storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };


@@ -1417,7 +1455,18 @@ virStorageFileBackend virStorageFileBackendBlock = {

 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
+
+.storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };


+virStorageFileBackend virStorageFileBackendDir = {
+.type = VIR_STORAGE_TYPE_DIR,
+
+.backendInit = virStorageFileBackendFileInit,
+.backendDeinit = virStorageFileBackendFileDeinit,
+
+.storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
+};
+
 #endif /* WITH_STORAGE_FS */
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index cd6babe..0c779c5 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2987,3 +2987,33 @@ virStorageFileReadHeader(virStorageSourcePtr src,

 return ret;
 }
+
+
+/*
+ * 

[libvirt] [PATCHv3 14/36] util: string: Add helper to free non-NULL terminated string arrays

2014-05-30 Thread Peter Krempa
Sometimes the length of the string list is known but the array isn't
NULL terminated. Add helper to free the array in such cases.
---
 src/libvirt_private.syms |  1 +
 src/util/virstring.c | 20 
 src/util/virstring.h |  1 +
 3 files changed, 22 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4c1f61c..4491262 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1901,6 +1901,7 @@ virStrcpy;
 virStrdup;
 virStringArrayHasString;
 virStringFreeList;
+virStringFreeListCount;
 virStringJoin;
 virStringListLength;
 virStringReplace;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 6dcc7a8..35b99a5 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -187,6 +187,26 @@ void virStringFreeList(char **strings)
 }


+/**
+ * virStringFreeListCount:
+ * @strings: array of strings to free
+ * @count: number of elements in the array
+ *
+ * Frees a string array of @count length.
+ */
+void
+virStringFreeListCount(char **strings,
+   size_t count)
+{
+size_t i;
+
+for (i = 0; i  count; i++)
+VIR_FREE(strings[i]);
+
+VIR_FREE(strings);
+}
+
+
 bool
 virStringArrayHasString(char **strings, const char *needle)
 {
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 0ab9d96..42d68b5 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -42,6 +42,7 @@ char *virStringJoin(const char **strings,
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

 void virStringFreeList(char **strings);
+void virStringFreeListCount(char **strings, size_t count);

 bool virStringArrayHasString(char **strings, const char *needle);

-- 
1.9.3

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


[libvirt] [PATCHv3 21/36] tests: virstoragetest: Remove expBackingStore field

2014-05-30 Thread Peter Krempa
Now that we changed ordering of the stored metadata so that the backing
store is described by the child element the test should reflect this
change too.

Remove the expected backing store field as it's actually described by
the next element in the backing chain, so there's no need for
duplication.
---
 tests/virstoragetest.c | 19 +++
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 3c892d5..03f8552 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -282,7 +282,6 @@ testPrepImages(void)
 typedef struct _testFileData testFileData;
 struct _testFileData
 {
-const char *expBackingStore;
 const char *expBackingStoreRaw;
 unsigned long long expCapacity;
 bool expEncrypted;
@@ -314,12 +313,11 @@ struct testChainData


 static const char testStorageChainFormat[] =
-store: %s\n
+path:%s\n
 backingStoreRaw: %s\n
 capacity: %lld\n
 encryption: %d\n
 relPath:%s\n
-path:%s\n
 relDir:%s\n
 type:%d\n
 format:%d\n;
@@ -386,23 +384,21 @@ testStorageChain(const void *args)
 : data-files[i]-relDirRel;
 if (virAsprintf(expect,
 testStorageChainFormat,
-NULLSTR(data-files[i]-expBackingStore),
+NULLSTR(data-files[i]-path),
 NULLSTR(data-files[i]-expBackingStoreRaw),
 data-files[i]-expCapacity,
 data-files[i]-expEncrypted,
 NULLSTR(expPath),
-NULLSTR(data-files[i]-path),
 NULLSTR(expRelDir),
 data-files[i]-type,
 data-files[i]-format)  0 ||
 virAsprintf(actual,
 testStorageChainFormat,
-NULLSTR(elt-backingStore ? elt-backingStore-path : 
NULL),
+NULLSTR(elt-path),
 NULLSTR(elt-backingStoreRaw),
 elt-capacity,
 !!elt-encryption,
 NULLSTR(elt-relPath),
-NULLSTR(elt-path),
 NULLSTR(elt-relDir),
 elt-type,
 elt-format)  0) {
@@ -793,7 +789,6 @@ mymain(void)
 /* Qcow2 file with relative raw backing, format provided */
 raw.pathAbs = raw;
 testFileData qcow2 = {
-.expBackingStore = canonraw,
 .expBackingStoreRaw = raw,
 .expCapacity = 1024,
 .pathRel = qcow2,
@@ -849,7 +844,6 @@ mymain(void)

 /* Wrapped file access */
 testFileData wrap = {
-.expBackingStore = canonqcow2,
 .expBackingStoreRaw = absqcow2,
 .expCapacity = 1024,
 .pathRel = wrap,
@@ -885,7 +879,6 @@ mymain(void)

 /* Qcow2 file with raw as absolute backing, backing format omitted */
 testFileData wrap_as_raw = {
-.expBackingStore = canonqcow2,
 .expBackingStoreRaw = absqcow2,
 .expCapacity = 1024,
 .pathRel = wrap,
@@ -909,7 +902,6 @@ mymain(void)
qcow2, NULL);
 if (virCommandRun(cmd, NULL)  0)
 ret = -1;
-qcow2.expBackingStore = NULL;
 qcow2.expBackingStoreRaw = datadir /bogus;
 qcow2.pathRel = qcow2;
 qcow2.relDirRel = .;
@@ -942,7 +934,6 @@ mymain(void)
qcow2, NULL);
 if (virCommandRun(cmd, NULL)  0)
 ret = -1;
-qcow2.expBackingStore = blah;
 qcow2.expBackingStoreRaw = nbd:example.org:6000:exportname=blah;

 /* Qcow2 file with backing protocol instead of file */
@@ -963,7 +954,6 @@ mymain(void)

 /* qed file */
 testFileData qed = {
-.expBackingStore = canonraw,
 .expBackingStoreRaw = absraw,
 .expCapacity = 1024,
 .pathRel = qed,
@@ -1028,7 +1018,6 @@ mymain(void)

 /* Behavior of symlinks to qcow2 with relative backing files */
 testFileData link1 = {
-.expBackingStore = canonraw,
 .expBackingStoreRaw = ../raw,
 .expCapacity = 1024,
 .pathRel = ../sub/link1,
@@ -1040,7 +1029,6 @@ mymain(void)
 .format = VIR_STORAGE_FILE_QCOW2,
 };
 testFileData link2 = {
-.expBackingStore = canonqcow2,
 .expBackingStoreRaw = ../sub/link1,
 .expCapacity = 1024,
 .pathRel = sub/link2,
@@ -1068,7 +1056,6 @@ mymain(void)
-F, qcow2, -b, qcow2, qcow2, NULL);
 if (virCommandRun(cmd, NULL)  0)
 ret = -1;
-qcow2.expBackingStore = NULL;
 qcow2.expBackingStoreRaw = qcow2;

 /* Behavior of an infinite loop chain */
-- 
1.9.3

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


[libvirt] [PATCHv3 23/36] storage: Store relative path only for relatively backed storage

2014-05-30 Thread Peter Krempa
Due to various refactors and compatibility with the virstoragetest the
relPath field of the virStorageSource structure was always filled either
with the relative name or the full path in case of abslutely backed
storage. Return it's original purpose to store only the relative name of
the disk if it is backed relatively and tweak the tests.
---
 src/storage/storage_driver.c |  4 
 src/util/virstoragefile.c| 21 +
 src/util/virstoragefile.h|  4 ++--
 tests/virstoragetest.c   | 25 +++--
 4 files changed, 14 insertions(+), 40 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 4f51517..9ce3b62 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3189,10 +3189,6 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
 if (!(cycle = virHashCreate(5, NULL)))
 return -1;

-if (!src-relPath 
-VIR_STRDUP(src-relPath, src-path)  0)
-goto cleanup;
-
 if (!src-relDir 
 !(src-relDir = mdir_name(src-path))) {
 virReportOOMError();
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 301452e..b33bc1a 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -764,11 +764,11 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr 
meta,
 {
 int ret = -1;

-VIR_DEBUG(relPath=%s, buf=%p, len=%zu, meta-format=%d,
-  meta-relPath, buf, len, meta-format);
+VIR_DEBUG(path=%s, buf=%p, len=%zu, meta-format=%d,
+  meta-path, buf, len, meta-format);

 if (meta-format == VIR_STORAGE_FILE_AUTO)
-meta-format = virStorageFileProbeFormatFromBuf(meta-relPath, buf, 
len);
+meta-format = virStorageFileProbeFormatFromBuf(meta-path, buf, len);

 if (meta-format = VIR_STORAGE_FILE_NONE ||
 meta-format = VIR_STORAGE_FILE_LAST) {
@@ -907,9 +907,6 @@ virStorageFileMetadataNew(const char *path,
 ret-format = format;
 ret-type = VIR_STORAGE_TYPE_FILE;

-if (VIR_STRDUP(ret-relPath, path)  0)
-goto error;
-
 if (VIR_STRDUP(ret-path, path)  0)
 goto error;

@@ -1375,7 +1372,8 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
 if (idx == i)
 break;
 } else {
-if (STREQ_NULLABLE(name, chain-relPath))
+if (STREQ_NULLABLE(name, chain-relPath) ||
+STREQ(name, chain-path))
 break;
 if (nameIsFile  (chain-type == VIR_STORAGE_TYPE_FILE ||
chain-type == VIR_STORAGE_TYPE_BLOCK)) {
@@ -1594,6 +1592,10 @@ 
virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,

 ret-backingRelative = true;

+/* store relative name */
+if (VIR_STRDUP(ret-relPath, parent-backingStoreRaw)  0)
+goto error;
+
 /* XXX Once we get rid of the need to use canonical names in path, we will 
be
  * able to use mdir_name on parent-path instead of using parent-relDir */
 if (STRNEQ(parent-relDir, /))
@@ -1908,11 +1910,6 @@ virStorageSourceNewFromBacking(virStorageSourcePtr 
parent)
 ret = virStorageSourceNewFromBackingAbsolute(parent-backingStoreRaw);

 if (ret) {
-if (VIR_STRDUP(ret-relPath, parent-backingStoreRaw)  0) {
-virStorageSourceFree(ret);
-return NULL;
-}
-
 /* possibly update local type */
 if (ret-type == VIR_STORAGE_TYPE_FILE) {
 if (stat(ret-path, st) == 0) {
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 70deaef..1f2ae53 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -247,8 +247,8 @@ struct _virStorageSource {
 virStorageDriverDataPtr drv;

 /* metadata about storage image which need separate fields */
-/* Name of the current file as spelled by the user (top level) or
- * metadata of the overlay (if this is a backing store).  */
+/* Relative path of the backing image from the parent NULL if
+ * backed by absolute path */
 char *relPath;
 /* Directory to start from if backingStoreRaw is a relative file
  * name.  */
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index e1adce9..5fd2530 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -116,9 +116,6 @@ testStorageFileGetMetadata(const char *path,
 }
 }

-if (VIR_STRDUP(ret-relPath, path)  0)
-goto error;
-
 if (!(ret-relDir = mdir_name(path))) {
 virReportOOMError();
 goto error;
@@ -371,7 +368,6 @@ testStorageChain(const void *args)
 while (elt) {
 char *expect = NULL;
 char *actual = NULL;
-const char *expPath;
 const char *expRelDir;

 if (i == data-nfiles) {
@@ -379,8 +375,6 @@ testStorageChain(const void *args)
 goto cleanup;
 }

-expPath = isAbs ? data-files[i]-pathAbs
-: data-files[i]-pathRel;
 expRelDir = 

[libvirt] [PATCHv3 00/36] Network backed backing chains and block jobs

2014-05-30 Thread Peter Krempa
First 7 patches of v2 are already pushed. First ~6 patches of this series
were already ACKed, but can't be pushed due to the freeze and
rebase conflicts of changing order of the patches. Thus I'm reposting
them.

Peter Krempa (36):
  storage: backend: Add unique id retrieval API
  storage: Add API to check accessibility of storage volumes
  storage: Move virStorageFileGetMetadata to the storage driver
  storage: Determine the local storage type right away
  test: storage: Initialize storage source to correct type
  storage: backend: Add possibility to suppress errors from backend
lookup
  storage: Switch metadata crawler to use storage driver to get unique
path
  storage: Switch metadata crawler to use storage driver to read headers
  storage: Switch metadata crawler to use storage driver file access
check
  storage: Add infrastructure to parse remote network backing names
  storage: Change to new backing store parser
  storage: Traverse backing chains of network disks
  util: string: Return element count from virStringSplit
  util: string: Add helper to free non-NULL terminated string arrays
  util: storagefile: Add helper to resolve ../, ./ and  in
paths
  util: storage: Add helper to resolve relative path difference
  util: storagefile: Add canonicalization to virStorageFileSimplifyPath
  storage: gluster: Add backend to return unique storage file path
  qemu: json: Add format strings for optional command arguments
  tests: storagetest: Unify and reformat storage chain format string
  tests: virstoragetest: Remove expBackingStore field
  tests: virstoragetest: Fix output when hitting errors
  storage: Store relative path only for relatively backed storage
  tests: virstoragetest: Remove now unused pathAbs
  util: storage: Remove now redundant backingRelative from
virStorageSource
  tests: virstoragetest: Don't test relative start of backing chains
  tests: virstoragetest: Remove unneeded relative test plumbing
  storage: Don't canonicalize paths unnecessarily
  storage: Don't store parent directory of an image explicitly
  qemu: monitor: Add argument for specifying backing name for block
commit
  qemu: monitor: Add support for backing name specification for
block-stream
  lib: Introduce flag VIR_DOMAIN_BLOCK_COMMIT_RELATIVE
  lib: Introduce flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE
  qemu: caps: Add capability for change-backing-file command
  qemu: Add support for networked disks for block commit
  qemu: Add support for networked disks for block pull/block rebase

 cfg.mk|   2 +-
 include/libvirt/libvirt.h.in  |   6 +
 src/Makefile.am   |   2 +
 src/libvirt_private.syms  |   7 +-
 src/qemu/qemu_capabilities.c  |   2 +
 src/qemu/qemu_capabilities.h  |   1 +
 src/qemu/qemu_domain.c|  10 +-
 src/qemu/qemu_driver.c|  79 +++-
 src/qemu/qemu_migration.c |   6 +-
 src/qemu/qemu_monitor.c   |  21 +-
 src/qemu/qemu_monitor.h   |   4 +-
 src/qemu/qemu_monitor_json.c  | 139 --
 src/qemu/qemu_monitor_json.h  |   2 +
 src/security/virt-aa-helper.c |   2 +
 src/storage/storage_backend.c |  16 +-
 src/storage/storage_backend.h |  12 +-
 src/storage/storage_backend_fs.c  |  62 +++
 src/storage/storage_backend_gluster.c |  92 
 src/storage/storage_driver.c  | 212 +
 src/storage/storage_driver.h  |   7 +
 src/util/virstoragefile.c | 822 +++---
 src/util/virstoragefile.h |  34 +-
 src/util/virstring.c  |  44 +-
 src/util/virstring.h  |   7 +
 tests/Makefile.am |   8 +-
 tests/qemumonitorjsontest.c   |   2 +-
 tests/virstoragetest.c| 491 +---
 tests/virstringtest.c |  14 +-
 tools/virsh-domain.c  |  29 +-
 tools/virsh.pod   |  10 +-
 30 files changed, 1633 insertions(+), 512 deletions(-)

-- 
1.9.3

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


[libvirt] [PATCHv3 28/36] storage: Don't canonicalize paths unnecessarily

2014-05-30 Thread Peter Krempa
Store backing chain paths as non-cannonical. The cannonicalization step
will be already taken. This will allow to avoid storing unnecessary
amounts of data.
---
 src/util/virstoragefile.c | 33 ++---
 tests/virstoragetest.c| 10 +-
 2 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 6482689..02b4c73 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1043,25 +1043,17 @@ virStorageFileGetMetadataFromFD(const char *path,
 int *backingFormat)

 {
-virStorageSourcePtr ret = NULL;
-char *canonPath = NULL;
-
-if (!(canonPath = canonicalize_file_name(path))) {
-virReportSystemError(errno, _(unable to resolve '%s'), path);
-goto cleanup;
-}
+virStorageSourcePtr ret;

-if (!(ret = virStorageFileMetadataNew(canonPath, format)))
-goto cleanup;
+if (!(ret = virStorageFileMetadataNew(path, format)))
+return NULL;


 if (virStorageFileGetMetadataFromFDInternal(ret, fd, backingFormat)  0) {
 virStorageSourceFree(ret);
-ret = NULL;
+return NULL;
 }

- cleanup:
-VIR_FREE(canonPath);
 return ret;
 }

@@ -1610,15 +1602,6 @@ 
virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
 virReportOOMError();
 goto error;
 }
-
-/* XXX we don't currently need to store the canonical path but the
- * change would break the test suite. Get rid of this later */
-char *tmp = ret-path;
-if (!(ret-path = canonicalize_file_name(tmp))) {
-ret-path = tmp;
-tmp = NULL;
-}
-VIR_FREE(tmp);
 } else {
 ret-type = VIR_STORAGE_TYPE_NETWORK;

@@ -1857,12 +1840,8 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
 goto error;
 }

-/* XXX we don't currently need to store the canonical path but the
- * change would break the test suite. Get rid of this later */
-if (!(ret-path = canonicalize_file_name(path))) {
-if (VIR_STRDUP(ret-path, path)  0)
-goto error;
-}
+if (VIR_STRDUP(ret-path, path)  0)
+goto error;
 } else {
 ret-type = VIR_STORAGE_TYPE_NETWORK;

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 3c262e2..5ce42d3 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -121,10 +121,8 @@ testStorageFileGetMetadata(const char *path,
 goto error;
 }

-if (!(ret-path = canonicalize_file_name(path))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, failed to resolve '%s', path);
+if (VIR_STRDUP(ret-path, path)  0)
 goto error;
-}

 if (virStorageFileGetMetadata(ret, uid, gid, allow_probe)  0)
 goto error;
@@ -937,7 +935,7 @@ mymain(void)
 .expBackingStoreRaw = ../raw,
 .expCapacity = 1024,
 .pathRel = ../sub/link1,
-.path = canonqcow2,
+.path = datadir /sub/../sub/link1,
 .relDir = datadir /sub/../sub,
 .type = VIR_STORAGE_TYPE_FILE,
 .format = VIR_STORAGE_FILE_QCOW2,
@@ -945,11 +943,13 @@ mymain(void)
 testFileData link2 = {
 .expBackingStoreRaw = ../sub/link1,
 .expCapacity = 1024,
-.path = canonwrap,
+.path = abslink2,
 .relDir = datadir /sub,
 .type = VIR_STORAGE_TYPE_FILE,
 .format = VIR_STORAGE_FILE_QCOW2,
 };
+
+raw.path = datadir /sub/../sub/../raw;
 raw.pathRel = ../raw;
 raw.relDir = datadir /sub/../sub/..;
 TEST_CHAIN(15, abslink2, VIR_STORAGE_FILE_QCOW2,
-- 
1.9.3

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


[libvirt] [PATCHv3 03/36] storage: Move virStorageFileGetMetadata to the storage driver

2014-05-30 Thread Peter Krempa
My future work will modify the metadata crawler function to use the
storage driver file APIs to access the files instead of accessing them
directly so that we will be able to request the metadata for remote
files too. To avoid linking the storage driver to every helper file
using the utils code, the backing chain traversal function needs to be
moved to the storage driver source.

Additionally the virt-aa-helper and virstoragetest programs need to be
linked with the storage driver as a result of this change.
---

Notes:
Version 3:
- fixed whitespace issue
- ACKed by Eric

 cfg.mk|   2 +-
 src/Makefile.am   |   2 +
 src/libvirt_private.syms  |   2 +-
 src/qemu/qemu_domain.c|   2 +
 src/security/virt-aa-helper.c |   2 +
 src/storage/storage_driver.c  | 233 ++
 src/storage/storage_driver.h  |   5 +
 src/util/virstoragefile.c | 233 +-
 src/util/virstoragefile.h |   7 +-
 tests/Makefile.am |   8 +-
 tests/virstoragetest.c|   2 +
 11 files changed, 259 insertions(+), 239 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 5ff2721..9e8fcec 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -774,7 +774,7 @@ sc_prohibit_cross_inclusion:
access/ | conf/) safe=($$dir|conf|util);; \
locking/) safe=($$dir|util|conf|rpc);;\
cpu/| network/| node_device/| rpc/| security/| storage/)\
- safe=($$dir|util|conf);;\
+ safe=($$dir|util|conf|storage);;\
xenapi/ | xenxs/ ) safe=($$dir|util|conf|xen);;   \
*) safe=($$dir|$(mid_dirs)|util);;\
  esac; \
diff --git a/src/Makefile.am b/src/Makefile.am
index cfb7097..5028f0d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2565,8 +2565,10 @@ virt_aa_helper_LDFLAGS = \
$(PIE_LDFLAGS) \
$(NULL)
 virt_aa_helper_LDADD = \
+   libvirt.la  \
libvirt_conf.la \
libvirt_util.la \
+   libvirt_driver_storage_impl.la  \
../gnulib/lib/libgnu.la
 if WITH_DTRACE_PROBES
 virt_aa_helper_LDADD += libvirt_probes.lo
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cb635cd..91f13a4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1863,9 +1863,9 @@ virStorageFileFeatureTypeToString;
 virStorageFileFormatTypeFromString;
 virStorageFileFormatTypeToString;
 virStorageFileGetLVMKey;
-virStorageFileGetMetadata;
 virStorageFileGetMetadataFromBuf;
 virStorageFileGetMetadataFromFD;
+virStorageFileGetMetadataFromFDInternal;
 virStorageFileGetSCSIKey;
 virStorageFileIsClusterFS;
 virStorageFileParseChainIndex;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 78cfdc6..502b7bd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -40,6 +40,8 @@
 #include virstoragefile.h
 #include virstring.h

+#include storage/storage_driver.h
+
 #include sys/time.h
 #include fcntl.h

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 32fc04a..bf540b4 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -55,6 +55,8 @@
 #include virrandom.h
 #include virstring.h

+#include storage/storage_driver.h
+
 #define VIR_FROM_THIS VIR_FROM_SECURITY

 static char *progname;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index f66c259..59f6fa8 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -49,6 +49,7 @@
 #include configmake.h
 #include virstring.h
 #include viraccessapicheck.h
+#include dirname.h

 #define VIR_FROM_THIS VIR_FROM_STORAGE

@@ -3041,3 +3042,235 @@ virStorageFileAccess(virStorageSourcePtr src,

 return src-drv-backend-storageFileAccess(src, mode);
 }
+
+
+/**
+ * Given a starting point START (a directory containing the original
+ * file, if the original file was opened via a relative path; ignored
+ * if NAME is absolute), determine the location of the backing file
+ * NAME (possibly relative), and compute the relative DIRECTORY
+ * (optional) and CANONICAL (mandatory) location of the backing file.
+ * Return 0 on success, negative on error.
+ */
+static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
+virFindBackingFile(const char *start, const char *path,
+   char **directory, char **canonical)
+{
+/* FIXME - when we eventually allow non-raw network devices, we
+ * must ensure that we handle backing files the same way as qemu.
+ * For a qcow2 top file of gluster://server/vol/img, qemu treats
+ * the 

[libvirt] [PATCHv3 05/36] test: storage: Initialize storage source to correct type

2014-05-30 Thread Peter Krempa
Stat the path of the storage file being tested to set the correct type
into the virStorageSource. This will avoid breaking the test suite when
inquiring metadata of directory paths in the next patches.
---

Notes:
Version 3:
- ACKed by eric

 tests/virstoragetest.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index fb96c71..746bf63 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -98,6 +98,7 @@ testStorageFileGetMetadata(const char *path,
uid_t uid, gid_t gid,
bool allow_probe)
 {
+struct stat st;
 virStorageSourcePtr ret = NULL;

 if (VIR_ALLOC(ret)  0)
@@ -106,6 +107,15 @@ testStorageFileGetMetadata(const char *path,
 ret-type = VIR_STORAGE_TYPE_FILE;
 ret-format = format;

+if (stat(path, st) == 0) {
+if (S_ISDIR(st.st_mode)) {
+ret-type = VIR_STORAGE_TYPE_DIR;
+ret-format = VIR_STORAGE_FILE_DIR;
+} else if (S_ISBLK(st.st_mode)) {
+ret-type = VIR_STORAGE_TYPE_BLOCK;
+}
+}
+
 if (VIR_STRDUP(ret-relPath, path)  0)
 goto error;

-- 
1.9.3

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


[libvirt] [PATCHv3 09/36] storage: Switch metadata crawler to use storage driver file access check

2014-05-30 Thread Peter Krempa
Use virStorageFileAccess() to to check whether the file is accessible in
the main part of the metadata crawler.
---

Notes:
Version 3:
- fixed typo
- ACKed

 src/storage/storage_driver.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index b074047..787171d 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3103,13 +3103,6 @@ virFindBackingFile(const char *start, const char *path,
 goto cleanup;
 }

-if (virFileAccessibleAs(combined, F_OK, geteuid(), getegid())  0) {
-virReportSystemError(errno,
- _(Cannot access backing file '%s'),
- combined);
-goto cleanup;
-}
-
 if (!(*canonical = canonicalize_file_name(combined))) {
 virReportSystemError(errno,
  _(Can't canonicalize path '%s'), path);
@@ -3151,6 +3144,13 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 if (virStorageFileInitAs(src, uid, gid)  0)
 return -1;

+if (virStorageFileAccess(src, F_OK)  0) {
+virReportSystemError(errno,
+ _(Cannot access backing file %s),
+ src-path);
+goto cleanup;
+}
+
 if (!(uniqueName = virStorageFileGetUniqueIdentifier(src)))
 goto cleanup;

-- 
1.9.3

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


[libvirt] [PATCHv3 17/36] util: storagefile: Add canonicalization to virStorageFileSimplifyPath

2014-05-30 Thread Peter Krempa
The recently introduced virStorageFileSimplifyPath is good at resolving
relative portions of a path. To add full path canonicalization
capability we need to be able to resolve symlinks in the path too.

This patch adds a callback to that function so that arbitrary storage
systems can use this functionality.
---
 src/libvirt_private.syms  |  1 +
 src/util/virstoragefile.c | 82 +--
 src/util/virstoragefile.h |  9 ++
 tests/virstoragetest.c| 80 +
 4 files changed, 170 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8e00d8c..7de04ed 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1872,6 +1872,7 @@ virStorageFileIsClusterFS;
 virStorageFileParseChainIndex;
 virStorageFileProbeFormat;
 virStorageFileResize;
+virStorageFileSimplifyPathInternal;
 virStorageIsFile;
 virStorageNetHostDefClear;
 virStorageNetHostDefCopy;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index d8b4b54..301452e 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1962,13 +1962,47 @@ virStorageFileExportPath(char **components,
 }


+static int
+virStorageFileExplodePath(const char *path,
+  size_t at,
+  char ***components,
+  size_t *ncomponents)
+{
+char **tmp = NULL;
+char **next;
+size_t ntmp = 0;
+int ret = -1;
+
+if (!(tmp = virStringSplitCount(path, /, 0, ntmp)))
+goto cleanup;
+
+/* prepend */
+for (next = tmp; *next; next++) {
+if (VIR_INSERT_ELEMENT(*components, at, *ncomponents, *next)  0)
+goto cleanup;
+
+at++;
+}
+
+ret = 0;
+
+ cleanup:
+virStringFreeListCount(tmp, ntmp);
+return ret;
+}
+
+
 char *
-virStorageFileSimplifyPath(const char *path,
-   bool allow_relative)
+virStorageFileSimplifyPathInternal(const char *path,
+   bool allow_relative,
+   virStorageFileSimplifyPathReadlinkCallback 
cb,
+   void *cbdata)
 {
 bool beginSlash = false;
 char **components = NULL;
 size_t ncomponents = 0;
+char *linkpath = NULL;
+char *currentpath = NULL;
 size_t i;
 char *ret = NULL;

@@ -2012,6 +2046,40 @@ virStorageFileSimplifyPath(const char *path,
 continue;
 }

+/* read link and stuff */
+if (cb) {
+int rc;
+if (!(currentpath = virStorageFileExportPath(components, i + 1,
+ beginSlash)))
+goto cleanup;
+
+if ((rc = cb(currentpath, linkpath, cbdata))  0)
+goto cleanup;
+
+if (rc == 0) {
+if (linkpath[0] == '/') {
+/* start from a clean slate */
+virStringFreeListCount(components, ncomponents);
+ncomponents = 0;
+components = NULL;
+beginSlash = true;
+i = 0;
+} else {
+VIR_FREE(components[i]);
+VIR_DELETE_ELEMENT(components, i, ncomponents);
+}
+
+if (virStorageFileExplodePath(linkpath, i, components,
+  ncomponents)  0)
+goto cleanup;
+
+VIR_FREE(linkpath);
+VIR_FREE(currentpath);
+
+continue;
+}
+}
+
 i++;
 }

@@ -2019,11 +2087,21 @@ virStorageFileSimplifyPath(const char *path,

  cleanup:
 virStringFreeListCount(components, ncomponents);
+VIR_FREE(linkpath);
+VIR_FREE(currentpath);

 return ret;
 }


+char *
+virStorageFileSimplifyPath(const char *path,
+   bool allow_relative)
+{
+return virStorageFileSimplifyPathInternal(path, allow_relative, NULL, 
NULL);
+}
+
+
 int
 virStorageFileGetRelativeBackingPath(virStorageSourcePtr from,
  virStorageSourcePtr to,
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index cbac30b..70deaef 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -325,6 +325,15 @@ void virStorageSourceFree(virStorageSourcePtr def);
 void virStorageSourceClearBackingStore(virStorageSourcePtr def);
 virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent);

+typedef int
+(*virStorageFileSimplifyPathReadlinkCallback)(const char *path,
+  char **link,
+  void *data);
+char *virStorageFileSimplifyPathInternal(const char *path,
+ bool allow_relative,
+ 

[libvirt] [PATCHv3 25/36] util: storage: Remove now redundant backingRelative from virStorageSource

2014-05-30 Thread Peter Krempa
Now that we store only relative names in virStorageSource's member
relPath the backingRelative member is obsolete. Remove it and adapt the
code to the removal.
---
 src/util/virstoragefile.c |  4 +---
 src/util/virstoragefile.h |  2 --
 tests/virstoragetest.c| 23 +++
 3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index b33bc1a..6482689 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1590,8 +1590,6 @@ 
virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
 if (VIR_ALLOC(ret)  0)
 return NULL;

-ret-backingRelative = true;
-
 /* store relative name */
 if (VIR_STRDUP(ret-relPath, parent-backingStoreRaw)  0)
 goto error;
@@ -2112,7 +2110,7 @@ virStorageFileGetRelativeBackingPath(virStorageSourcePtr 
from,
 *relpath = NULL;

 for (next = from; next; next = next-backingStore) {
-if (!next-backingRelative || !next-relPath) {
+if (!next-relPath) {
 ret = 1;
 goto cleanup;
 }
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 1f2ae53..37f4e66 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -256,8 +256,6 @@ struct _virStorageSource {
 /* Name of the child backing store recorded in metadata of the
  * current file.  */
 char *backingStoreRaw;
-/* is backing store identified as relative */
-bool backingRelative;
 };


diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index af565c9..a787cc9 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -570,34 +570,33 @@ testPathRelativePrepare(void)

 for (i = 0; i  ARRAY_CARDINALITY(backingchain) - 1; i++) {
 backingchain[i].backingStore = backingchain[i+1];
+backingchain[i].relPath = NULL;
 }

-backingchain[0].relPath = (char *) /path/to/some/img;
-backingchain[0].backingRelative = false;
+backingchain[0].path = (char *) /path/to/disk;

+backingchain[1].path = (char *) /path/to/asdf;
 backingchain[1].relPath = (char *) asdf;
-backingchain[1].backingRelative = true;

+backingchain[2].path = (char *) /path/to/test;
 backingchain[2].relPath = (char *) test;
-backingchain[2].backingRelative = true;

+backingchain[3].path = (char *) /path/to/blah;
 backingchain[3].relPath = (char *) blah;
-backingchain[3].backingRelative = true;

-backingchain[4].relPath = (char *) /path/to/some/other/img;
-backingchain[4].backingRelative = false;
+backingchain[4].path = (char *) /path/to/some/other/img;

+backingchain[5].path = (char *) 
/path/to/some/other/relative/in/other/path;
 backingchain[5].relPath = (char *) ../relative/in/other/path;
-backingchain[5].backingRelative = true;

+backingchain[6].path = (char *) 
/path/to/some/other/relative/in/other/test;
 backingchain[6].relPath = (char *) test;
-backingchain[6].backingRelative = true;

+backingchain[7].path = (char *) /path/to/below;
 backingchain[7].relPath = (char *) ../../../../../below;
-backingchain[7].backingRelative = true;

+backingchain[8].path = (char *) /path/to/a/little/more/upwards;
 backingchain[8].relPath = (char *) a/little/more/upwards;
-backingchain[8].backingRelative = true;
 }


@@ -626,7 +625,7 @@ testPathRelative(const void *args)
 if (STRNEQ_NULLABLE(data-expect, actual)) {
 fprintf(stderr, relative path resolution from '%s' to '%s': 
 expected '%s', got '%s'\n,
-data-from-relPath, data-to-relPath,
+data-from-path, data-to-path,
 NULLSTR(data-expect), NULLSTR(actual));
 goto cleanup;
 }
-- 
1.9.3

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


[libvirt] [PATCHv3 04/36] storage: Determine the local storage type right away

2014-05-30 Thread Peter Krempa
When walking the backing chain we previously set the storage type to
_FILE and let the virStorageFileGetMetadataFromFDInternal update it to
the correct type later on.

This patch moves the actual storage type determination to the place
where we parse the backing store name so that the code can later be
switched to use virStorageFileReadHeader() directly.
---

Notes:
Version 3:
- ACKed by Eric

 src/storage/storage_driver.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 59f6fa8..f92a553 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3111,6 +3111,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 {
 int fd;
 int ret = -1;
+struct stat st;
 virStorageSourcePtr backingStore = NULL;
 int backingFormat;

@@ -3173,6 +3174,16 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 ret = 0;
 goto cleanup;
 }
+
+/* update the type for local storage */
+if (stat(backingStore-path, st) == 0) {
+if (S_ISDIR(st.st_mode)) {
+backingStore-type = VIR_STORAGE_TYPE_DIR;
+backingStore-format = VIR_STORAGE_FILE_DIR;
+} else if (S_ISBLK(st.st_mode)) {
+backingStore-type = VIR_STORAGE_TYPE_BLOCK;
+}
+}
 } else {
 /* TODO: To satisfy the test case, copy the network URI as path. This
  * will be removed later. */
-- 
1.9.3

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


[libvirt] [PATCHv3 30/36] qemu: monitor: Add argument for specifying backing name for block commit

2014-05-30 Thread Peter Krempa
To allow changing the name that is recorded in the overlay of the TOP
image used in a block commit operation, we need to specify the backing
name to qemu. This is done via the backing-file attribute to the
block-commit commad.
---
 src/qemu/qemu_driver.c   | 1 +
 src/qemu/qemu_monitor.c  | 9 ++---
 src/qemu/qemu_monitor.h  | 1 +
 src/qemu/qemu_monitor_json.c | 2 ++
 src/qemu/qemu_monitor_json.h | 1 +
 tests/qemumonitorjsontest.c  | 2 +-
 6 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 59185c6..0619156 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15529,6 +15529,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
 ret = qemuMonitorBlockCommit(priv-mon, device,
  top  !topIndex ? top : topSource-path,
  base  !baseIndex ? base : baseSource-path,
+ NULL,
  bandwidth);
 qemuDomainObjExitMonitor(driver, vm);

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 912bea1..590081c 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3234,13 +3234,15 @@ qemuMonitorTransaction(qemuMonitorPtr mon, 
virJSONValuePtr actions)
 int
 qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device,
const char *top, const char *base,
+   const char *backingName,
unsigned long bandwidth)
 {
 int ret = -1;
 unsigned long long speed;

-VIR_DEBUG(mon=%p, device=%s, top=%s, base=%s, bandwidth=%ld,
-  mon, device, top, base, bandwidth);
+VIR_DEBUG(mon=%p, device=%s, top=%s, base=%s, backingName=%s, 
+  bandwidth=%ld,
+  mon, device, top, base, backingName, bandwidth);

 /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
  * limited to LLONG_MAX also for unsigned values */
@@ -3254,7 +3256,8 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char 
*device,
 speed = 20;

 if (mon-json)
-ret = qemuMonitorJSONBlockCommit(mon, device, top, base, speed);
+ret = qemuMonitorJSONBlockCommit(mon, device, top, base,
+ backingName, speed);
 else
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
_(block-commit requires JSON monitor));
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index de724bf..2cac1b3 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -662,6 +662,7 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon,
const char *device,
const char *top,
const char *base,
+   const char *backingName,
unsigned long bandwidth)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
 ATTRIBUTE_NONNULL(4);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index d78bd30..8d69b96 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3451,6 +3451,7 @@ qemuMonitorJSONTransaction(qemuMonitorPtr mon, 
virJSONValuePtr actions)
 int
 qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device,
const char *top, const char *base,
+   const char *backingName,
unsigned long long speed)
 {
 int ret = -1;
@@ -3462,6 +3463,7 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char 
*device,
  U:speed, speed,
  s:top, top,
  s:base, base,
+ S:backing-file, backingName,
  NULL);
 if (!cmd)
 return -1;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 5dda0ba..c30803f 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -261,6 +261,7 @@ int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
const char *device,
const char *top,
const char *base,
+   const char *backingName,
unsigned long long bandwidth)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
 ATTRIBUTE_NONNULL(4);
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 47d7481..e618c67 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1164,7 +1164,7 @@ GEN_TEST_FUNC(qemuMonitorJSONAddDevice, 
some_dummy_devicestr)
 GEN_TEST_FUNC(qemuMonitorJSONSetDrivePassphrase, vda, secret_passhprase)
 GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, vdb, /foo/bar, NULL, 1024,
   VIR_DOMAIN_BLOCK_REBASE_SHALLOW | 

[libvirt] [PATCHv3 06/36] storage: backend: Add possibility to suppress errors from backend lookup

2014-05-30 Thread Peter Krempa
Add a new function wrapper and tweak the storage file backend lookup
function so that it can be used without reporting error. This will be
useful in the metadata crawler code where we need silently break if
metadata retrieval is not supported for the current storage type.
---

Notes:
Version 3:
- ACKed by Eric

 src/storage/storage_backend.c | 16 ++--
 src/storage/storage_backend.h |  4 +++-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index e016cc8..380ca58 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1173,8 +1173,9 @@ virStorageBackendForType(int type)


 virStorageFileBackendPtr
-virStorageFileBackendForType(int type,
- int protocol)
+virStorageFileBackendForTypeInternal(int type,
+ int protocol,
+ bool report)
 {
 size_t i;

@@ -1188,6 +1189,9 @@ virStorageFileBackendForType(int type,
 }
 }

+if (!report)
+return NULL;
+
 if (type == VIR_STORAGE_TYPE_NETWORK) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(missing storage backend for network files 
@@ -1203,6 +1207,14 @@ virStorageFileBackendForType(int type,
 }


+virStorageFileBackendPtr
+virStorageFileBackendForType(int type,
+ int protocol)
+{
+return virStorageFileBackendForTypeInternal(type, protocol, true);
+}
+
+
 struct diskType {
 int part_table_type;
 unsigned short offset;
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 56643fb..76c1afa 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -203,7 +203,9 @@ typedef int
int mode);

 virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);
-
+virStorageFileBackendPtr virStorageFileBackendForTypeInternal(int type,
+  int protocol,
+  bool report);


 struct _virStorageFileBackend {
-- 
1.9.3

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


[libvirt] [PATCHv3 12/36] storage: Traverse backing chains of network disks

2014-05-30 Thread Peter Krempa
Now we don't need to skip backing chain detection for remote disks.
---
 src/qemu/qemu_domain.c   |  8 +++-
 src/storage/storage_driver.c | 18 ++
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 502b7bd..1d6f7be 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2428,12 +2428,10 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
 int ret = 0;
 uid_t uid;
 gid_t gid;
-const char *src = virDomainDiskGetSource(disk);
-int type = virDomainDiskGetType(disk);
+int type = virStorageSourceGetActualType(disk-src);

-if (!src ||
-type == VIR_STORAGE_TYPE_NETWORK ||
-type == VIR_STORAGE_TYPE_VOLUME)
+if (type != VIR_STORAGE_TYPE_NETWORK 
+!disk-src.path)
 goto cleanup;

 if (disk-src.backingStore) {
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 4d96070..4f51517 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3189,19 +3189,13 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
 if (!(cycle = virHashCreate(5, NULL)))
 return -1;

-if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
-if (!src-relPath 
-VIR_STRDUP(src-relPath, src-path)  0)
-goto cleanup;
+if (!src-relPath 
+VIR_STRDUP(src-relPath, src-path)  0)
+goto cleanup;

-if (!src-relDir 
-!(src-relDir = mdir_name(src-path))) {
-virReportOOMError();
-goto cleanup;
-}
-} else {
-/* TODO: currently unimplemented for non-local storage */
-ret = 0;
+if (!src-relDir 
+!(src-relDir = mdir_name(src-path))) {
+virReportOOMError();
 goto cleanup;
 }

-- 
1.9.3

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


[libvirt] [PATCHv3 34/36] qemu: caps: Add capability for change-backing-file command

2014-05-30 Thread Peter Krempa
This command allows to change the backing file name recorded in the
metadata of a qcow (or other) image. The capability also notifies that
the block-stream and block-commit commands understand the
backing-file attribute.
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 08c3d04..05613e5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -256,6 +256,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   usb-kbd, /* 165 */
   host-pci-multidomain,
   msg-timestamp,
+  change-backing-file,
 );


@@ -1382,6 +1383,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
 { blockdev-snapshot-sync, QEMU_CAPS_DISK_SNAPSHOT },
 { add-fd, QEMU_CAPS_ADD_FD },
 { nbd-server-start, QEMU_CAPS_NBD_SERVER },
+{ change-backing-file, QEMU_CAPS_CHANGE_BACKING_FILE },
 };

 struct virQEMUCapsStringFlags virQEMUCapsEvents[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d755caa..f6bca5d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -206,6 +206,7 @@ enum virQEMUCapsFlags {
 QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */
 QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain  0 in host pci 
address */
 QEMU_CAPS_MSG_TIMESTAMP  = 167, /* -msg timestamp */
+QEMU_CAPS_CHANGE_BACKING_FILE = 168, /* change namen of backing file in 
metadata */

 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
-- 
1.9.3

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


[libvirt] [PATCHv3 36/36] qemu: Add support for networked disks for block pull/block rebase

2014-05-30 Thread Peter Krempa
Now that we are able to select images from the backing chain via indexed
access we should also convert possible network sources to
qemu-compatible strings before passing them to qemu.
---
 src/qemu/qemu_driver.c | 43 +++
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 424be4a..feef9fa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14980,6 +14980,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 virDomainDiskDefPtr disk;
 virStorageSourcePtr baseSource = NULL;
 unsigned int baseIndex = 0;
+char *basePath = NULL;
+char *backingPath = NULL;

 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
@@ -14987,6 +14989,13 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 goto cleanup;
 }

+if (flags  VIR_DOMAIN_BLOCK_REBASE_RELATIVE  !base) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+   _(flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE is valid only 
+  with non-null base ));
+goto cleanup;
+}
+
 priv = vm-privateData;
 if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) {
 async = true;
@@ -15048,10 +15057,33 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
   base, baseIndex, NULL
 goto endjob;

+if (baseSource) {
+if (qemuGetDriveSourceString(baseSource, NULL, basePath)  0)
+goto endjob;
+
+if (flags  VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {
+if (!virQEMUCapsGet(priv-qemuCaps, 
QEMU_CAPS_CHANGE_BACKING_FILE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(this QEMU binary doesn't support relative 
+ block pull/rebase));
+goto endjob;
+}
+
+if (disk-src.backingStore == baseSource) {
+if (VIR_STRDUP(backingPath, baseSource-relPath)  0)
+goto endjob;
+} else {
+if 
(virStorageFileGetRelativeBackingPath(disk-src.backingStore,
+ baseSource,
+ backingPath)  0)
+goto endjob;
+}
+}
+}
+
 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJob(priv-mon, device,
-  baseIndex ? baseSource-path : base,
-  NULL, bandwidth, info, mode, async);
+ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath,
+  bandwidth, info, mode, async);
 qemuDomainObjExitMonitor(driver, vm);
 if (ret  0)
 goto endjob;
@@ -15121,6 +15153,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 }

  cleanup:
+VIR_FREE(basePath);
+VIR_FREE(backingPath);
 VIR_FREE(device);
 if (vm)
 virObjectUnlock(vm);
@@ -15361,7 +15395,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char 
*path, const char *base,
 virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
   VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
   VIR_DOMAIN_BLOCK_REBASE_COPY |
-  VIR_DOMAIN_BLOCK_REBASE_COPY_RAW, -1);
+  VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |
+  VIR_DOMAIN_BLOCK_REBASE_RELATIVE, -1);

 if (!(vm = qemuDomObjFromDomain(dom)))
 return -1;
-- 
1.9.3

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


[libvirt] [PATCHv3 35/36] qemu: Add support for networked disks for block commit

2014-05-30 Thread Peter Krempa
Now that we are able to select images from the backing chain via indexed
access we should also convert possible network sources to
qemu-compatible strings before passing them to qemu.
---
 src/qemu/qemu_driver.c | 33 +
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 72e92cc..424be4a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15425,8 +15425,12 @@ qemuDomainBlockCommit(virDomainPtr dom,
 unsigned int baseIndex = 0;
 const char *top_parent = NULL;
 bool clean_access = false;
+char *topPath = NULL;
+char *basePath = NULL;
+char *backingPath = NULL;

-virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);
+virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
+  VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, -1);

 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;
@@ -15520,6 +15524,26 @@ qemuDomainBlockCommit(virDomainPtr dom,
VIR_DISK_CHAIN_READ_WRITE)  0))
 goto endjob;

+if (qemuGetDriveSourceString(topSource, NULL, topPath)  0)
+goto endjob;
+
+if (qemuGetDriveSourceString(baseSource, NULL, basePath)  0)
+goto endjob;
+
+if (flags  VIR_DOMAIN_BLOCK_COMMIT_RELATIVE) {
+if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(this qemu doesn't support relative blockpull));
+goto endjob;
+}
+
+if (virStorageFileGetRelativeBackingPath(topSource, baseSource,
+ backingPath)  0)
+goto endjob;
+
+/* XXX should we error out? */
+}
+
 /* Start the commit operation.  Pass the user's original spelling,
  * if any, through to qemu, since qemu may behave differently
  * depending on whether the input was specified as relative or
@@ -15527,9 +15551,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
  * thing if the user specified a relative name). */
 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorBlockCommit(priv-mon, device,
- top  !topIndex ? top : topSource-path,
- base  !baseIndex ? base : baseSource-path,
- NULL,
+ topPath, basePath, backingPath,
  bandwidth);
 qemuDomainObjExitMonitor(driver, vm);

@@ -15547,6 +15569,9 @@ qemuDomainBlockCommit(virDomainPtr dom,
 vm = NULL;

  cleanup:
+VIR_FREE(topPath);
+VIR_FREE(basePath);
+VIR_FREE(backingPath);
 VIR_FREE(device);
 if (vm)
 virObjectUnlock(vm);
-- 
1.9.3

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


[libvirt] [PATCHv3 24/36] tests: virstoragetest: Remove now unused pathAbs

2014-05-30 Thread Peter Krempa
Separately remove the now unused variable.
---
 tests/virstoragetest.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 5fd2530..af565c9 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -283,7 +283,6 @@ struct _testFileData
 unsigned long long expCapacity;
 bool expEncrypted;
 const char *pathRel;
-const char *pathAbs;
 const char *path;
 const char *relDirRel;
 const char *relDirAbs;
@@ -761,7 +760,6 @@ mymain(void)

 /* Raw image, whether with right format or no specified format */
 testFileData raw = {
-.pathAbs = canonraw,
 .path = canonraw,
 .relDirRel = .,
 .relDirAbs = datadir,
@@ -780,12 +778,10 @@ mymain(void)
(raw), ALLOW_PROBE | EXP_PASS);

 /* Qcow2 file with relative raw backing, format provided */
-raw.pathAbs = raw;
 raw.pathRel = raw;
 testFileData qcow2 = {
 .expBackingStoreRaw = raw,
 .expCapacity = 1024,
-.pathAbs = canonqcow2,
 .path = canonqcow2,
 .relDirRel = .,
 .relDirAbs = datadir,
@@ -793,7 +789,6 @@ mymain(void)
 .format = VIR_STORAGE_FILE_QCOW2,
 };
 testFileData qcow2_as_raw = {
-.pathAbs = canonqcow2,
 .path = canonqcow2,
 .relDirRel = .,
 .relDirAbs = datadir,
@@ -819,7 +814,6 @@ mymain(void)
 ret = -1;
 qcow2.expBackingStoreRaw = absraw;
 raw.pathRel = NULL;
-raw.pathAbs = absraw;
 raw.relDirRel = datadir;

 /* Qcow2 file with raw as absolute backing, backing format provided */
@@ -838,7 +832,6 @@ mymain(void)
 testFileData wrap = {
 .expBackingStoreRaw = absqcow2,
 .expCapacity = 1024,
-.pathAbs = abswrap,
 .path = canonwrap,
 .relDirRel = .,
 .relDirAbs = datadir,
@@ -870,7 +863,6 @@ mymain(void)
 testFileData wrap_as_raw = {
 .expBackingStoreRaw = absqcow2,
 .expCapacity = 1024,
-.pathAbs = abswrap,
 .path = canonwrap,
 .relDirRel = .,
 .relDirAbs = datadir,
@@ -925,7 +917,6 @@ mymain(void)

 /* Qcow2 file with backing protocol instead of file */
 testFileData nbd = {
-.pathAbs = nbd:example.org:6000:exportname=blah,
 .path = blah,
 .type = VIR_STORAGE_TYPE_NETWORK,
 .format = VIR_STORAGE_FILE_RAW,
@@ -942,7 +933,6 @@ mymain(void)
 testFileData qed = {
 .expBackingStoreRaw = absraw,
 .expCapacity = 1024,
-.pathAbs = absqed,
 .path = canonqed,
 .relDirRel = .,
 .relDirAbs = datadir,
@@ -950,7 +940,6 @@ mymain(void)
 .format = VIR_STORAGE_FILE_QED,
 };
 testFileData qed_as_raw = {
-.pathAbs = absqed,
 .path = canonqed,
 .relDirRel = .,
 .relDirAbs = datadir,
@@ -965,7 +954,6 @@ mymain(void)

 /* directory */
 testFileData dir = {
-.pathAbs = absdir,
 .path = canondir,
 .relDirRel = .,
 .relDirAbs = datadir,
@@ -1004,7 +992,6 @@ mymain(void)
 .expBackingStoreRaw = ../raw,
 .expCapacity = 1024,
 .pathRel = ../sub/link1,
-.pathAbs = ../sub/link1,
 .path = canonqcow2,
 .relDirRel = sub/../sub,
 .relDirAbs = datadir /sub/../sub,
@@ -1014,7 +1001,6 @@ mymain(void)
 testFileData link2 = {
 .expBackingStoreRaw = ../sub/link1,
 .expCapacity = 1024,
-.pathAbs = abslink2,
 .path = canonwrap,
 .relDirRel = sub,
 .relDirAbs = datadir /sub,
@@ -1022,7 +1008,6 @@ mymain(void)
 .format = VIR_STORAGE_FILE_QCOW2,
 };
 raw.pathRel = ../raw;
-raw.pathAbs = ../raw;
 raw.relDirRel = sub/../sub/..;
 raw.relDirAbs = datadir /sub/../sub/..;
 TEST_CHAIN(15, sub/link2, abslink2, VIR_STORAGE_FILE_QCOW2,
-- 
1.9.3

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


[libvirt] [PATCHv3 31/36] qemu: monitor: Add support for backing name specification for block-stream

2014-05-30 Thread Peter Krempa
To allow changing the name that is recorded in the top of the current
image chain used in a block pull/rebase operation, we need to specify
the backing name to qemu. This is done via the backing-file attribute
to the block-stream commad.
---
 src/qemu/qemu_driver.c   |  8 
 src/qemu/qemu_migration.c|  6 +++---
 src/qemu/qemu_monitor.c  | 12 +++-
 src/qemu/qemu_monitor.h  |  3 ++-
 src/qemu/qemu_monitor_json.c | 15 +++
 src/qemu/qemu_monitor_json.h |  1 +
 6 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0619156..72e92cc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14834,7 +14834,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
 /* Probe the status, if needed.  */
 if (!disk-mirroring) {
 qemuDomainObjEnterMonitor(driver, vm);
-rc = qemuMonitorBlockJob(priv-mon, device, NULL, 0, info,
+rc = qemuMonitorBlockJob(priv-mon, device, NULL, NULL, 0, info,
   BLOCK_JOB_INFO, true);
 qemuDomainObjExitMonitor(driver, vm);
 if (rc  0)
@@ -15051,7 +15051,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorBlockJob(priv-mon, device,
   baseIndex ? baseSource-path : base,
-  bandwidth, info, mode, async);
+  NULL, bandwidth, info, mode, async);
 qemuDomainObjExitMonitor(driver, vm);
 if (ret  0)
 goto endjob;
@@ -15091,8 +15091,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 virDomainBlockJobInfo dummy;

 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJob(priv-mon, device, NULL, 0, dummy,
-  BLOCK_JOB_INFO, async);
+ret = qemuMonitorBlockJob(priv-mon, device, NULL, NULL, 0,
+  dummy, BLOCK_JOB_INFO, async);
 qemuDomainObjExitMonitor(driver, vm);

 if (ret = 0)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5754f73..327679d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1308,7 +1308,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
_(canceled by client));
 goto error;
 }
-mon_ret = qemuMonitorBlockJob(priv-mon, diskAlias, NULL, 0,
+mon_ret = qemuMonitorBlockJob(priv-mon, diskAlias, NULL, NULL, 0,
   info, BLOCK_JOB_INFO, true);
 qemuDomainObjExitMonitor(driver, vm);

@@ -1360,7 +1360,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
 continue;
 if (qemuDomainObjEnterMonitorAsync(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_OUT) == 0) 
{
-if (qemuMonitorBlockJob(priv-mon, diskAlias, NULL, 0,
+if (qemuMonitorBlockJob(priv-mon, diskAlias, NULL, NULL, 0,
 NULL, BLOCK_JOB_ABORT, true)  0) {
 VIR_WARN(Unable to cancel block-job on '%s', diskAlias);
 }
@@ -1426,7 +1426,7 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig,
QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
 goto cleanup;

-if (qemuMonitorBlockJob(priv-mon, diskAlias, NULL, 0,
+if (qemuMonitorBlockJob(priv-mon, diskAlias, NULL, NULL, 0,
 NULL, BLOCK_JOB_ABORT, true)  0)
 VIR_WARN(Unable to stop block job on %s, diskAlias);
 qemuDomainObjExitMonitor(driver, vm);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 590081c..df5dc3a 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3354,6 +3354,7 @@ int qemuMonitorScreendump(qemuMonitorPtr mon,
 int qemuMonitorBlockJob(qemuMonitorPtr mon,
 const char *device,
 const char *base,
+const char *backingName,
 unsigned long bandwidth,
 virDomainBlockJobInfoPtr info,
 qemuMonitorBlockJobCmd mode,
@@ -3362,9 +3363,10 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon,
 int ret = -1;
 unsigned long long speed;

-VIR_DEBUG(mon=%p, device=%s, base=%s, bandwidth=%luM, info=%p, mode=%o, 
-  modern=%d, mon, device, NULLSTR(base), bandwidth, info, mode,
-  modern);
+VIR_DEBUG(mon=%p, device=%s, base=%s, backingName=%s, bandwidth=%luM, 
+  info=%p, mode=%o, modern=%d,
+  mon, device, NULLSTR(base), NULLSTR(backingName),
+  bandwidth, info, mode, modern);

 /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
  * limited to LLONG_MAX also for 

[libvirt] [PATCHv3 16/36] util: storage: Add helper to resolve relative path difference

2014-05-30 Thread Peter Krempa
This patch introduces a function that will allow us to resolve a
relative difference between two elements of a disk backing chain. This
fucntion will be used to allow relative block commit and block pull
where we need to specify the new relative name of the image to qemu.

This patch also adds unit tests for the function to verify that it works
correctly.
---
 src/libvirt_private.syms  |   1 +
 src/util/virstoragefile.c |  45 +
 src/util/virstoragefile.h |   4 ++
 tests/virstoragetest.c| 101 ++
 4 files changed, 151 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4491262..8e00d8c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1866,6 +1866,7 @@ virStorageFileGetLVMKey;
 virStorageFileGetMetadataFromBuf;
 virStorageFileGetMetadataFromFD;
 virStorageFileGetMetadataInternal;
+virStorageFileGetRelativeBackingPath;
 virStorageFileGetSCSIKey;
 virStorageFileIsClusterFS;
 virStorageFileParseChainIndex;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index a9c6a13..d8b4b54 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2022,3 +2022,48 @@ virStorageFileSimplifyPath(const char *path,

 return ret;
 }
+
+
+int
+virStorageFileGetRelativeBackingPath(virStorageSourcePtr from,
+ virStorageSourcePtr to,
+ char **relpath)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+virStorageSourcePtr next;
+char *tmp = NULL;
+char ret = -1;
+
+*relpath = NULL;
+
+for (next = from; next; next = next-backingStore) {
+if (!next-backingRelative || !next-relPath) {
+ret = 1;
+goto cleanup;
+}
+
+if (next != from)
+virBufferAddLit(buf, /../);
+
+virBufferAdd(buf, next-relPath, -1);
+
+if (next == to)
+break;
+}
+
+if (next != to)
+goto cleanup;
+
+if (!(tmp = virBufferContentAndReset(buf)))
+goto cleanup;
+
+if (!(*relpath = virStorageFileSimplifyPath(tmp, true)))
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virBufferFreeAndReset(buf);
+VIR_FREE(tmp);
+return ret;
+}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 3e9e5c8..cbac30b 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -328,4 +328,8 @@ virStorageSourcePtr 
virStorageSourceNewFromBacking(virStorageSourcePtr parent);
 char *virStorageFileSimplifyPath(const char *path,
  bool allow_relative);

+int virStorageFileGetRelativeBackingPath(virStorageSourcePtr from,
+ virStorageSourcePtr to,
+ char **relpath);
+
 #endif /* __VIR_STORAGE_FILE_H__ */
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 57f16ca..80d73ca 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -560,6 +560,85 @@ testPathSimplify(const void *args)
 }


+virStorageSource backingchain[9];
+
+static void
+testPathRelativePrepare(void)
+{
+size_t i;
+
+for (i = 0; i  ARRAY_CARDINALITY(backingchain) - 1; i++) {
+backingchain[i].backingStore = backingchain[i+1];
+}
+
+backingchain[0].relPath = (char *) /path/to/some/img;
+backingchain[0].backingRelative = false;
+
+backingchain[1].relPath = (char *) asdf;
+backingchain[1].backingRelative = true;
+
+backingchain[2].relPath = (char *) test;
+backingchain[2].backingRelative = true;
+
+backingchain[3].relPath = (char *) blah;
+backingchain[3].backingRelative = true;
+
+backingchain[4].relPath = (char *) /path/to/some/other/img;
+backingchain[4].backingRelative = false;
+
+backingchain[5].relPath = (char *) ../relative/in/other/path;
+backingchain[5].backingRelative = true;
+
+backingchain[6].relPath = (char *) test;
+backingchain[6].backingRelative = true;
+
+backingchain[7].relPath = (char *) ../../../../../below;
+backingchain[7].backingRelative = true;
+
+backingchain[8].relPath = (char *) a/little/more/upwards;
+backingchain[8].backingRelative = true;
+}
+
+
+struct testPathRelativeBacking
+{
+virStorageSourcePtr from;
+virStorageSourcePtr to;
+
+const char *expect;
+};
+
+static int
+testPathRelative(const void *args)
+{
+const struct testPathRelativeBacking *data = args;
+char *actual = NULL;
+int ret = -1;
+
+if (virStorageFileGetRelativeBackingPath(data-from,
+ data-to,
+ actual)  0) {
+fprintf(stderr, relative backing path resolution failed\n);
+goto cleanup;
+}
+
+if (STRNEQ_NULLABLE(data-expect, actual)) {
+fprintf(stderr, relative path resolution from '%s' to '%s': 
+expected '%s', got '%s'\n,
+

[libvirt] [PATCHv3 02/36] storage: Add API to check accessibility of storage volumes

2014-05-30 Thread Peter Krempa
Add a storage driver API equivalent of the access() function.
Implementations for the filesystem and gluster backends are provided.
---

Notes:
Version 3:
- fixed typos
- ACKed by Eric

 src/storage/storage_backend.h |  5 +
 src/storage/storage_backend_fs.c  | 13 +
 src/storage/storage_backend_gluster.c | 11 +++
 src/storage/storage_driver.c  | 24 
 src/storage/storage_driver.h  |  1 +
 5 files changed, 54 insertions(+)

diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 5d71cde..56643fb 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -198,6 +198,10 @@ typedef ssize_t
 typedef const char *
 (*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src);

+typedef int
+(*virStorageFileBackendAccess)(virStorageSourcePtr src,
+   int mode);
+
 virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);


@@ -220,6 +224,7 @@ struct _virStorageFileBackend {
 virStorageFileBackendCreate storageFileCreate;
 virStorageFileBackendUnlink storageFileUnlink;
 virStorageFileBackendStat   storageFileStat;
+virStorageFileBackendAccess storageFileAccess;
 };

 #endif /* __VIR_STORAGE_BACKEND_H__ */
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index aed07a6..414f2c7 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1433,6 +1433,15 @@ 
virStorageFileBackendFileGetUniqueIdentifier(virStorageSourcePtr src)
 }


+static int
+virStorageFileBackendFileAccess(virStorageSourcePtr src,
+int mode)
+{
+return virFileAccessibleAs(src-path, mode,
+   src-drv-uid, src-drv-gid);
+}
+
+
 virStorageFileBackend virStorageFileBackendFile = {
 .type = VIR_STORAGE_TYPE_FILE,

@@ -1442,6 +1451,7 @@ virStorageFileBackend virStorageFileBackendFile = {
 .storageFileUnlink = virStorageFileBackendFileUnlink,
 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
+.storageFileAccess = virStorageFileBackendFileAccess,

 .storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };
@@ -1455,6 +1465,7 @@ virStorageFileBackend virStorageFileBackendBlock = {

 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
+.storageFileAccess = virStorageFileBackendFileAccess,

 .storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };
@@ -1466,6 +1477,8 @@ virStorageFileBackend virStorageFileBackendDir = {
 .backendInit = virStorageFileBackendFileInit,
 .backendDeinit = virStorageFileBackendFileDeinit,

+.storageFileAccess = virStorageFileBackendFileAccess,
+
 .storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };

diff --git a/src/storage/storage_backend_gluster.c 
b/src/storage/storage_backend_gluster.c
index af6792f..3db4e66 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -703,6 +703,16 @@ virStorageFileBackendGlusterReadHeader(virStorageSourcePtr 
src,
 }


+static int
+virStorageFileBackendGlusterAccess(virStorageSourcePtr src,
+   int mode)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+
+return glfs_access(priv-vol, src-path, mode);
+}
+
+
 virStorageFileBackend virStorageFileBackendGluster = {
 .type = VIR_STORAGE_TYPE_NETWORK,
 .protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER,
@@ -713,4 +723,5 @@ virStorageFileBackend virStorageFileBackendGluster = {
 .storageFileUnlink = virStorageFileBackendGlusterUnlink,
 .storageFileStat = virStorageFileBackendGlusterStat,
 .storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
+.storageFileAccess = virStorageFileBackendGlusterAccess,
 };
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 0c779c5..f66c259 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3017,3 +3017,27 @@ virStorageFileGetUniqueIdentifier(virStorageSourcePtr 
src)

 return src-drv-backend-storageFileGetUniqueIdentifier(src);
 }
+
+
+/**
+ * virStorageFileAccess: Check accessibility of a storage file
+ *
+ * @src: storage file to check access permissions
+ * @mode: accessibility check options (see man 2 access)
+ *
+ * Returns 0 on success, -1 on error and sets errno. No libvirt
+ * error is reported. Returns -2 if the operation isn't supported
+ * by libvirt storage backend.
+ */
+int
+virStorageFileAccess(virStorageSourcePtr src,
+ int mode)
+{
+if (!virStorageFileIsInitialized(src) ||
+!src-drv-backend-storageFileAccess) {
+errno = ENOSYS;
+return -2;
+}
+
+return 

[libvirt] [PATCHv3 33/36] lib: Introduce flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE

2014-05-30 Thread Peter Krempa
Introduce flag for the block rebase API to allow the rebase operation to
leave the chain relatively addressed. Also adds a virsh switch to enable
this behavior.
---
 include/libvirt/libvirt.h.in |  2 ++
 tools/virsh-domain.c | 22 +++---
 tools/virsh.pod  |  4 
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 2df7fc8..a4fe175 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2571,6 +2571,8 @@ typedef enum {
file for a copy */
 VIR_DOMAIN_BLOCK_REBASE_COPY_RAW  = 1  2, /* Make destination file raw */
 VIR_DOMAIN_BLOCK_REBASE_COPY  = 1  3, /* Start a copy job */
+VIR_DOMAIN_BLOCK_REBASE_RELATIVE  = 1  4, /* Keep backing chain relative
+   if possible */
 } virDomainBlockRebaseFlags;

 int   virDomainBlockRebase(virDomainPtr dom, const char *disk,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 5c3a142..74473a8 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1479,10 +1479,14 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
 case VSH_CMD_BLOCK_JOB_PULL:
 if (vshCommandOptStringReq(ctl, cmd, base, base)  0)
 goto cleanup;
-if (base)
-ret = virDomainBlockRebase(dom, path, base, bandwidth, 0);
-else
+if (base) {
+  if (vshCommandOptBool(cmd, keep-relative))
+  flags |= VIR_DOMAIN_BLOCK_REBASE_RELATIVE;
+
+ret = virDomainBlockRebase(dom, path, base, bandwidth, flags);
+} else {
 ret = virDomainBlockPull(dom, path, bandwidth, 0);
+}
 break;
 case VSH_CMD_BLOCK_JOB_COMMIT:
 if (vshCommandOptStringReq(ctl, cmd, base, base)  0 ||
@@ -2071,6 +2075,11 @@ static const vshCmdOptDef opts_block_pull[] = {
  .type = VSH_OT_BOOL,
  .help = N_(with --wait, don't wait for cancel to finish)
 },
+{.name = keep-relative,
+ .type = VSH_OT_BOOL,
+ .help = N_(keep the backing chain relative if it was relatively 
+referenced if it was before)
+},
 {.name = NULL}
 };

@@ -2091,6 +2100,13 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
 bool quit = false;
 int abort_flags = 0;

+if (vshCommandOptBool(cmd, keep-relative) 
+!vshCommandOptBool(cmd, base)) {
+vshError(ctl, %s, _(--keep-relative is supported only with partial 
+  pull operations with --base specified));
+return false;
+}
+
 if (blocking) {
 if (vshCommandOptTimeoutToMs(ctl, cmd, timeout)  0)
 return false;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 03b94a5..e252d62 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -844,6 +844,7 @@ Ibandwidth specifies copying bandwidth limit in MiB/s.

 =item Bblockpull Idomain Ipath [Ibandwidth] [Ibase]
 [I--wait [I--verbose] [I--timeout Bseconds] [I--async]]
+[I--keep-relative]

 Populate a disk from its backing image chain. By default, this command
 flattens the entire chain; but if Ibase is specified, containing the
@@ -863,6 +864,9 @@ is triggered, I--async will return control to the user as 
fast as
 possible, otherwise the command may continue to block a little while
 longer until the job is done cleaning up.

+Using the I--keep-relative flag will try to keep the backing chain names
+relative (if they were relative before).
+
 Ipath specifies fully-qualified path of the disk; it corresponds
 to a unique target name (target dev='name'/) or source file (source
 file='name'/) for one of the disk devices attached to Idomain (see
-- 
1.9.3

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


[libvirt] [PATCHv3 18/36] storage: gluster: Add backend to return unique storage file path

2014-05-30 Thread Peter Krempa
Use virStorageFileSimplifyPathInternal to canonicalize gluster paths
via a callback and use it for the unique volume path retrieval API.
---
 src/storage/storage_backend_gluster.c | 81 +++
 1 file changed, 81 insertions(+)

diff --git a/src/storage/storage_backend_gluster.c 
b/src/storage/storage_backend_gluster.c
index 3db4e66..1e86383 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -533,6 +533,7 @@ typedef virStorageFileBackendGlusterPriv 
*virStorageFileBackendGlusterPrivPtr;

 struct _virStorageFileBackendGlusterPriv {
 glfs_t *vol;
+char *uid;
 };


@@ -547,6 +548,7 @@ virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)

 if (priv-vol)
 glfs_fini(priv-vol);
+VIR_FREE(priv-uid);

 VIR_FREE(priv);
 src-drv-priv = NULL;
@@ -712,6 +714,81 @@ virStorageFileBackendGlusterAccess(virStorageSourcePtr src,
 return glfs_access(priv-vol, src-path, mode);
 }

+static int
+virStorageFileBackendGlusterReadlinkCallback(const char *path,
+ char **link,
+ void *data)
+{
+virStorageFileBackendGlusterPrivPtr priv = data;
+char *buf = NULL;
+size_t bufsiz = 0;
+ssize_t ret;
+struct stat st;
+
+*link = NULL;
+
+if (glfs_stat(priv-vol, path, st)  0) {
+virReportSystemError(errno,
+ _(failed to stat gluster path '%s'),
+ path);
+return -1;
+}
+
+if (!S_ISLNK(st.st_mode))
+return 1;
+
+ realloc:
+if (VIR_EXPAND_N(buf, bufsiz, 256)  0)
+goto error;
+
+if ((ret = glfs_readlink(priv-vol, path, buf, bufsiz))  0) {
+virReportSystemError(errno,
+ _(failed to read link of gluster file '%s'),
+ path);
+goto error;
+}
+
+if (ret == bufsiz)
+goto realloc;
+
+buf[ret] = '\0';
+
+*link = buf;
+
+return 0;
+
+ error:
+VIR_FREE(buf);
+return -1;
+}
+
+
+static const char *
+virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+char *canonPath = NULL;
+
+if (priv-uid)
+return priv-uid;
+
+if (!(canonPath = virStorageFileSimplifyPathInternal(src-path,
+ false,
+ 
virStorageFileBackendGlusterReadlinkCallback,
+ priv)))
+return NULL;
+
+ignore_value(virAsprintf(priv-uid, gluster://%s:%s/%s/%s,
+ src-hosts-name,
+ src-hosts-port,
+ src-volume,
+ canonPath));
+
+VIR_FREE(canonPath);
+
+return priv-uid;
+}
+

 virStorageFileBackend virStorageFileBackendGluster = {
 .type = VIR_STORAGE_TYPE_NETWORK,
@@ -724,4 +801,8 @@ virStorageFileBackend virStorageFileBackendGluster = {
 .storageFileStat = virStorageFileBackendGlusterStat,
 .storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
 .storageFileAccess = virStorageFileBackendGlusterAccess,
+
+.storageFileGetUniqueIdentifier = 
virStorageFileBackendGlusterGetUniqueIdentifier,
+
+
 };
-- 
1.9.3

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


[libvirt] [PATCHv3 20/36] tests: storagetest: Unify and reformat storage chain format string

2014-05-30 Thread Peter Krempa
All the fields crammed into two lines weren't easy to parse by human
eyes. Split up the format string into lines and put it into a central
variable so that changes in two places aren't necessary.
---
 tests/virstoragetest.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 771df0b..3c892d5 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -312,6 +312,18 @@ struct testChainData
 unsigned int flags;
 };

+
+static const char testStorageChainFormat[] =
+store: %s\n
+backingStoreRaw: %s\n
+capacity: %lld\n
+encryption: %d\n
+relPath:%s\n
+path:%s\n
+relDir:%s\n
+type:%d\n
+format:%d\n;
+
 static int
 testStorageChain(const void *args)
 {
@@ -373,8 +385,7 @@ testStorageChain(const void *args)
 expRelDir = isAbs ? data-files[i]-relDirAbs
 : data-files[i]-relDirRel;
 if (virAsprintf(expect,
-store:%s\nraw:%s\nother:%lld %d\n
-relPath:%s\npath:%s\nrelDir:%s\ntype:%d %d\n,
+testStorageChainFormat,
 NULLSTR(data-files[i]-expBackingStore),
 NULLSTR(data-files[i]-expBackingStoreRaw),
 data-files[i]-expCapacity,
@@ -385,15 +396,16 @@ testStorageChain(const void *args)
 data-files[i]-type,
 data-files[i]-format)  0 ||
 virAsprintf(actual,
-store:%s\nraw:%s\nother:%lld %d\n
-relPath:%s\npath:%s\nrelDir:%s\ntype:%d %d\n,
+testStorageChainFormat,
 NULLSTR(elt-backingStore ? elt-backingStore-path : 
NULL),
 NULLSTR(elt-backingStoreRaw),
-elt-capacity, !!elt-encryption,
+elt-capacity,
+!!elt-encryption,
 NULLSTR(elt-relPath),
 NULLSTR(elt-path),
 NULLSTR(elt-relDir),
-elt-type, elt-format)  0) {
+elt-type,
+elt-format)  0) {
 VIR_FREE(expect);
 VIR_FREE(actual);
 goto cleanup;
-- 
1.9.3

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


[libvirt] [PATCHv3 22/36] tests: virstoragetest: Fix output when hitting errors

2014-05-30 Thread Peter Krempa
When the test is failing but the debug output isn't enabled the
resulting line would look ugly like and would not contain the actual
difference.

TEST: virstoragetest
  .chain member 1!chain member 1!chain member 1!

Store the member index in the actual checked string to hide this problem
---
 tests/virstoragetest.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 03f8552..e1adce9 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -313,6 +313,7 @@ struct testChainData


 static const char testStorageChainFormat[] =
+chain member: %zu\n
 path:%s\n
 backingStoreRaw: %s\n
 capacity: %lld\n
@@ -383,7 +384,7 @@ testStorageChain(const void *args)
 expRelDir = isAbs ? data-files[i]-relDirAbs
 : data-files[i]-relDirRel;
 if (virAsprintf(expect,
-testStorageChainFormat,
+testStorageChainFormat, i,
 NULLSTR(data-files[i]-path),
 NULLSTR(data-files[i]-expBackingStoreRaw),
 data-files[i]-expCapacity,
@@ -393,7 +394,7 @@ testStorageChain(const void *args)
 data-files[i]-type,
 data-files[i]-format)  0 ||
 virAsprintf(actual,
-testStorageChainFormat,
+testStorageChainFormat, i,
 NULLSTR(elt-path),
 NULLSTR(elt-backingStoreRaw),
 elt-capacity,
@@ -407,7 +408,6 @@ testStorageChain(const void *args)
 goto cleanup;
 }
 if (STRNEQ(expect, actual)) {
-fprintf(stderr, chain member %zu, i);
 virtTestDifference(stderr, expect, actual);
 VIR_FREE(expect);
 VIR_FREE(actual);
-- 
1.9.3

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


[libvirt] [PATCHv3 13/36] util: string: Return element count from virStringSplit

2014-05-30 Thread Peter Krempa
To allow using the array manipulation macros on the arrays returned by
virStringSplit we need to know the count of the elements in the array.
Modify virStringSplit to return this value, rename it and add a helper
with the old name so that we don't need to update all the code.
---

Notes:
Version 3:
- added test coverage

 src/libvirt_private.syms |  1 +
 src/util/virstring.c | 24 
 src/util/virstring.h |  6 ++
 tests/virstringtest.c| 14 +-
 4 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e1702db..4c1f61c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1908,6 +1908,7 @@ virStringSearch;
 virStringSortCompare;
 virStringSortRevCompare;
 virStringSplit;
+virStringSplitCount;
 virStrncpy;
 virStrndup;
 virStrToDouble;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 7a8430e..6dcc7a8 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -43,13 +43,15 @@ VIR_LOG_INIT(util.string);
  */

 /**
- * virStringSplit:
+ * virStringSplitCount:
  * @string: a string to split
  * @delim: a string which specifies the places at which to split
  * the string. The delimiter is not included in any of the resulting
  * strings, unless @max_tokens is reached.
  * @max_tokens: the maximum number of pieces to split @string into.
  * If this is 0, the string is split completely.
+ * @tokcount: If provided, the value is set to the count of pieces the string
+ *was split to excluding the terminating NULL element.
  *
  * Splits a string into a maximum of @max_tokens pieces, using the given
  * @delim. If @max_tokens is reached, the remainder of @string is
@@ -65,9 +67,11 @@ VIR_LOG_INIT(util.string);
  * Return value: a newly-allocated NULL-terminated array of strings. Use
  *virStringFreeList() to free it.
  */
-char **virStringSplit(const char *string,
-  const char *delim,
-  size_t max_tokens)
+char **
+virStringSplitCount(const char *string,
+const char *delim,
+size_t max_tokens,
+size_t *tokcount)
 {
 char **tokens = NULL;
 size_t ntokens = 0;
@@ -109,6 +113,9 @@ char **virStringSplit(const char *string,
 goto error;
 tokens[ntokens++] = NULL;

+if (tokcount)
+*tokcount = ntokens - 1;
+
 return tokens;

  error:
@@ -119,6 +126,15 @@ char **virStringSplit(const char *string,
 }


+char **
+virStringSplit(const char *string,
+   const char *delim,
+   size_t max_tokens)
+{
+return virStringSplitCount(string, delim, max_tokens, NULL);
+}
+
+
 /**
  * virStringJoin:
  * @strings: a NULL-terminated array of strings to join
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 1ed1046..0ab9d96 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -26,6 +26,12 @@

 # include internal.h

+char **virStringSplitCount(const char *string,
+   const char *delim,
+   size_t max_tokens,
+   size_t *tokcount)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
 char **virStringSplit(const char *string,
   const char *delim,
   size_t max_tokens)
diff --git a/tests/virstringtest.c b/tests/virstringtest.c
index 1e330f9..291e505 100644
--- a/tests/virstringtest.c
+++ b/tests/virstringtest.c
@@ -53,11 +53,14 @@ static int testSplit(const void *args)
 {
 const struct testSplitData *data = args;
 char **got;
+size_t ntokens;
+size_t exptokens = 0;
 char **tmp1;
 const char **tmp2;
 int ret = -1;

-if (!(got = virStringSplit(data-string, data-delim, data-max_tokens))) {
+if (!(got = virStringSplitCount(data-string, data-delim,
+data-max_tokens, ntokens))) {
 VIR_DEBUG(Got no tokens at all);
 return -1;
 }
@@ -71,6 +74,7 @@ static int testSplit(const void *args)
 }
 tmp1++;
 tmp2++;
+exptokens++;
 }
 if (*tmp1) {
 virFilePrintf(stderr, Too many pieces returned\n);
@@ -81,6 +85,14 @@ static int testSplit(const void *args)
 goto cleanup;
 }

+if (ntokens != exptokens) {
+virFilePrintf(stderr,
+  Returned token count (%zu) doesn't match 
+  expected count (%zu),
+  ntokens, exptokens);
+goto cleanup;
+}
+
 ret = 0;
  cleanup:
 virStringFreeList(got);
-- 
1.9.3

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


[libvirt] [PATCHv3 27/36] tests: virstoragetest: Remove unneeded relative test plumbing

2014-05-30 Thread Peter Krempa
After we don't test relative paths, remove even more unnecessary cruft
from the test code.
---
 tests/virstoragetest.c | 61 +++---
 1 file changed, 18 insertions(+), 43 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 002df68..3c262e2 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -284,8 +284,7 @@ struct _testFileData
 bool expEncrypted;
 const char *pathRel;
 const char *path;
-const char *relDirRel;
-const char *relDirAbs;
+const char *relDir;
 int type;
 int format;
 };
@@ -295,7 +294,6 @@ enum {
 EXP_FAIL = 1,
 EXP_WARN = 2,
 ALLOW_PROBE = 4,
-ABS_START = 8,
 };

 struct testChainData
@@ -328,7 +326,6 @@ testStorageChain(const void *args)
 virStorageSourcePtr elt;
 size_t i = 0;
 char *broken = NULL;
-bool isAbs = !!(data-flags  ABS_START);

 meta = testStorageFileGetMetadata(data-start, data-format, -1, -1,
   (data-flags  ALLOW_PROBE) != 0);
@@ -367,15 +364,12 @@ testStorageChain(const void *args)
 while (elt) {
 char *expect = NULL;
 char *actual = NULL;
-const char *expRelDir;

 if (i == data-nfiles) {
 fprintf(stderr, probed chain was too long\n);
 goto cleanup;
 }

-expRelDir = isAbs ? data-files[i]-relDirAbs
-: data-files[i]-relDirRel;
 if (virAsprintf(expect,
 testStorageChainFormat, i,
 NULLSTR(data-files[i]-path),
@@ -383,7 +377,7 @@ testStorageChain(const void *args)
 data-files[i]-expCapacity,
 data-files[i]-expEncrypted,
 NULLSTR(data-files[i]-pathRel),
-NULLSTR(expRelDir),
+NULLSTR(data-files[i]-relDir),
 data-files[i]-type,
 data-files[i]-format)  0 ||
 virAsprintf(actual,
@@ -739,12 +733,10 @@ mymain(void)
 #define VIR_FLATTEN_2(...) __VA_ARGS__
 #define VIR_FLATTEN_1(_1) VIR_FLATTEN_2 _1

-#define TEST_CHAIN(id, path, format, chain1, flags1, chain2, flags2) \
-do { \
-TEST_ONE_CHAIN(#id a, path, format, flags1 | ABS_START,\
-   VIR_FLATTEN_1(chain1));   \
-TEST_ONE_CHAIN(#id b, path, format, flags2 | ABS_START,\
-   VIR_FLATTEN_1(chain2));   \
+#define TEST_CHAIN(id, path, format, chain1, flags1, chain2, flags2)   
\
+do {   
\
+TEST_ONE_CHAIN(#id a, path, format, flags1, VIR_FLATTEN_1(chain1));  
\
+TEST_ONE_CHAIN(#id b, path, format, flags2, VIR_FLATTEN_1(chain2));  
\
 } while (0)

 /* The actual tests, in several groups. */
@@ -755,8 +747,7 @@ mymain(void)
 /* Raw image, whether with right format or no specified format */
 testFileData raw = {
 .path = canonraw,
-.relDirRel = .,
-.relDirAbs = datadir,
+.relDir = datadir,
 .type = VIR_STORAGE_TYPE_FILE,
 .format = VIR_STORAGE_FILE_RAW,
 };
@@ -773,15 +764,13 @@ mymain(void)
 .expBackingStoreRaw = raw,
 .expCapacity = 1024,
 .path = canonqcow2,
-.relDirRel = .,
-.relDirAbs = datadir,
+.relDir = datadir,
 .type = VIR_STORAGE_TYPE_FILE,
 .format = VIR_STORAGE_FILE_QCOW2,
 };
 testFileData qcow2_as_raw = {
 .path = canonqcow2,
-.relDirRel = .,
-.relDirAbs = datadir,
+.relDir = datadir,
 .type = VIR_STORAGE_TYPE_FILE,
 .format = VIR_STORAGE_FILE_RAW,
 };
@@ -800,7 +789,6 @@ mymain(void)
 ret = -1;
 qcow2.expBackingStoreRaw = absraw;
 raw.pathRel = NULL;
-raw.relDirRel = datadir;

 /* Qcow2 file with raw as absolute backing, backing format provided */
 TEST_CHAIN(5, absqcow2, VIR_STORAGE_FILE_QCOW2,
@@ -815,12 +803,10 @@ mymain(void)
 .expBackingStoreRaw = absqcow2,
 .expCapacity = 1024,
 .path = canonwrap,
-.relDirRel = .,
-.relDirAbs = datadir,
+.relDir = datadir,
 .type = VIR_STORAGE_TYPE_FILE,
 .format = VIR_STORAGE_FILE_QCOW2,
 };
-qcow2.relDirRel = datadir;
 TEST_CHAIN(7, abswrap, VIR_STORAGE_FILE_QCOW2,
(wrap, qcow2, raw), EXP_PASS,
(wrap, qcow2, raw), ALLOW_PROBE | EXP_PASS);
@@ -837,15 +823,13 @@ mymain(void)
-b, absqcow2, wrap, NULL);
 if (virCommandRun(cmd, NULL)  0)
 ret = -1;
-qcow2_as_raw.relDirRel = datadir;

 /* Qcow2 file with raw as absolute backing, backing format omitted */
 testFileData wrap_as_raw = {
 .expBackingStoreRaw = absqcow2,
 .expCapacity = 

[libvirt] [PATCHv3 26/36] tests: virstoragetest: Don't test relative start of backing chains

2014-05-30 Thread Peter Krempa
libvirt always uses an absolute path to address the top image of an
image chain. Our storage test tests also the relative path which won't
ever be used. Additionally it makes the test more complicated.
---
 tests/virstoragetest.c | 79 +-
 1 file changed, 20 insertions(+), 59 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index a787cc9..002df68 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -739,17 +739,12 @@ mymain(void)
 #define VIR_FLATTEN_2(...) __VA_ARGS__
 #define VIR_FLATTEN_1(_1) VIR_FLATTEN_2 _1

-#define TEST_CHAIN(id, relstart, absstart, format, chain1, flags1,   \
-   chain2, flags2, chain3, flags3, chain4, flags4)   \
+#define TEST_CHAIN(id, path, format, chain1, flags1, chain2, flags2) \
 do { \
-TEST_ONE_CHAIN(#id a, relstart, format, flags1,\
+TEST_ONE_CHAIN(#id a, path, format, flags1 | ABS_START,\
VIR_FLATTEN_1(chain1));   \
-TEST_ONE_CHAIN(#id b, relstart, format, flags2,\
+TEST_ONE_CHAIN(#id b, path, format, flags2 | ABS_START,\
VIR_FLATTEN_1(chain2));   \
-TEST_ONE_CHAIN(#id c, absstart, format, flags3 | ABS_START,\
-   VIR_FLATTEN_1(chain3));   \
-TEST_ONE_CHAIN(#id d, absstart, format, flags4 | ABS_START,\
-   VIR_FLATTEN_1(chain4));   \
 } while (0)

 /* The actual tests, in several groups. */
@@ -765,14 +760,10 @@ mymain(void)
 .type = VIR_STORAGE_TYPE_FILE,
 .format = VIR_STORAGE_FILE_RAW,
 };
-TEST_CHAIN(1, raw, absraw, VIR_STORAGE_FILE_RAW,
-   (raw), EXP_PASS,
-   (raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(1, absraw, VIR_STORAGE_FILE_RAW,
(raw), EXP_PASS,
(raw), ALLOW_PROBE | EXP_PASS);
-TEST_CHAIN(2, raw, absraw, VIR_STORAGE_FILE_AUTO,
-   (raw), EXP_PASS,
-   (raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(2, absraw, VIR_STORAGE_FILE_AUTO,
(raw), EXP_PASS,
(raw), ALLOW_PROBE | EXP_PASS);

@@ -794,14 +785,10 @@ mymain(void)
 .type = VIR_STORAGE_TYPE_FILE,
 .format = VIR_STORAGE_FILE_RAW,
 };
-TEST_CHAIN(3, qcow2, absqcow2, VIR_STORAGE_FILE_QCOW2,
-   (qcow2, raw), EXP_PASS,
-   (qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(3, absqcow2, VIR_STORAGE_FILE_QCOW2,
(qcow2, raw), EXP_PASS,
(qcow2, raw), ALLOW_PROBE | EXP_PASS);
-TEST_CHAIN(4, qcow2, absqcow2, VIR_STORAGE_FILE_AUTO,
-   (qcow2_as_raw), EXP_PASS,
-   (qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(4, absqcow2, VIR_STORAGE_FILE_AUTO,
(qcow2_as_raw), EXP_PASS,
(qcow2, raw), ALLOW_PROBE | EXP_PASS);

@@ -816,14 +803,10 @@ mymain(void)
 raw.relDirRel = datadir;

 /* Qcow2 file with raw as absolute backing, backing format provided */
-TEST_CHAIN(5, qcow2, absqcow2, VIR_STORAGE_FILE_QCOW2,
-   (qcow2, raw), EXP_PASS,
-   (qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(5, absqcow2, VIR_STORAGE_FILE_QCOW2,
(qcow2, raw), EXP_PASS,
(qcow2, raw), ALLOW_PROBE | EXP_PASS);
-TEST_CHAIN(6, qcow2, absqcow2, VIR_STORAGE_FILE_AUTO,
-   (qcow2_as_raw), EXP_PASS,
-   (qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(6, absqcow2, VIR_STORAGE_FILE_AUTO,
(qcow2_as_raw), EXP_PASS,
(qcow2, raw), ALLOW_PROBE | EXP_PASS);

@@ -838,9 +821,7 @@ mymain(void)
 .format = VIR_STORAGE_FILE_QCOW2,
 };
 qcow2.relDirRel = datadir;
-TEST_CHAIN(7, wrap, abswrap, VIR_STORAGE_FILE_QCOW2,
-   (wrap, qcow2, raw), EXP_PASS,
-   (wrap, qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(7, abswrap, VIR_STORAGE_FILE_QCOW2,
(wrap, qcow2, raw), EXP_PASS,
(wrap, qcow2, raw), ALLOW_PROBE | EXP_PASS);

@@ -868,9 +849,7 @@ mymain(void)
 .type = VIR_STORAGE_TYPE_FILE,
 .format = VIR_STORAGE_FILE_QCOW2,
 };
-TEST_CHAIN(8, wrap, abswrap, VIR_STORAGE_FILE_QCOW2,
-   (wrap_as_raw, qcow2_as_raw), EXP_PASS,
-   (wrap, qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(8, abswrap, VIR_STORAGE_FILE_QCOW2,
(wrap_as_raw, qcow2_as_raw), EXP_PASS,
(wrap, qcow2, raw), ALLOW_PROBE | EXP_PASS);

@@ -885,9 +864,7 @@ mymain(void)
 qcow2.relDirRel = .;

 /* Qcow2 file with missing backing file but specified type */
-TEST_CHAIN(9, qcow2, absqcow2, VIR_STORAGE_FILE_QCOW2,
-   (qcow2), EXP_WARN,
-   (qcow2), ALLOW_PROBE | EXP_WARN,
+

[libvirt] [PATCHv3 08/36] storage: Switch metadata crawler to use storage driver to read headers

2014-05-30 Thread Peter Krempa
Use virStorageFileReadHeader() to read headers of storage files possibly
on remote storage to retrieve the image metadata.

The backend information is now parsed by
virStorageFileGetMetadataInternal which is now exported from the util
source and virStorageFileGetMetadataFromFDInternal now doesn't need to
be exported.
---

Notes:
Version 3:
- fixed typo
- ACKed

 src/libvirt_private.syms |  2 +-
 src/storage/storage_driver.c | 26 +++---
 src/util/virstoragefile.c|  5 ++---
 src/util/virstoragefile.h|  9 ++---
 4 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 91f13a4..57312c3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1865,7 +1865,7 @@ virStorageFileFormatTypeToString;
 virStorageFileGetLVMKey;
 virStorageFileGetMetadataFromBuf;
 virStorageFileGetMetadataFromFD;
-virStorageFileGetMetadataFromFDInternal;
+virStorageFileGetMetadataInternal;
 virStorageFileGetSCSIKey;
 virStorageFileIsClusterFS;
 virStorageFileParseChainIndex;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 5c4188f..b074047 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3132,10 +3132,11 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr 
src,
  bool allow_probe,
  virHashTablePtr cycle)
 {
-int fd;
 int ret = -1;
 struct stat st;
 const char *uniqueName;
+char *buf = NULL;
+ssize_t headerLen;
 virStorageSourcePtr backingStore = NULL;
 int backingFormat;

@@ -3163,26 +3164,13 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr 
src,
 if (virHashAddEntry(cycle, uniqueName, (void *)1)  0)
 goto cleanup;

-if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
-if ((fd = virFileOpenAs(src-path, O_RDONLY, 0, uid, gid, 0))  0) {
-virReportSystemError(-fd, _(Failed to open file '%s'),
- src-path);
-goto cleanup;
-}
-
-if (virStorageFileGetMetadataFromFDInternal(src, fd,
-backingFormat)  0) {
-VIR_FORCE_CLOSE(fd);
-goto cleanup;
-}
+if ((headerLen = virStorageFileReadHeader(src, VIR_STORAGE_MAX_HEADER,
+  buf))  0)
+goto cleanup;

-if (VIR_CLOSE(fd)  0)
-VIR_WARN(could not close file %s, src-path);
-} else {
-/* TODO: currently we call this only for local storage */
-ret = 0;
+if (virStorageFileGetMetadataInternal(src, buf, headerLen,
+  backingFormat)  0)
 goto cleanup;
-}

 /* check whether we need to go deeper */
 if (!src-backingStoreRaw) {
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 4956808..a80131a 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -740,8 +740,7 @@ qcow2GetFeatures(virBitmapPtr *features,
  * with information about the file and its backing store. Return format
  * of the backing store as BACKING_FORMAT. PATH and FORMAT have to be
  * pre-populated in META */
-static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(4)
+int
 virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
   char *buf,
   size_t len,
@@ -955,7 +954,7 @@ virStorageFileGetMetadataFromBuf(const char *path,


 /* Internal version that also supports a containing directory name.  */
-int
+static int
 virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta,
 int fd,
 int *backingFormat)
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 9947203..b516fbe 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -265,9 +265,12 @@ struct _virStorageSource {

 int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);

-int virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta,
-int fd,
-int *backingFormat);
+int virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
+  char *buf,
+  size_t len,
+  int *backingFormat)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
+
 virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path,
 int fd,
 int format,
-- 
1.9.3

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


[libvirt] [PATCHv3 19/36] qemu: json: Add format strings for optional command arguments

2014-05-30 Thread Peter Krempa
This patch adds option to specify that a json qemu command argument is
optional without the need to use if's or ternary operators to pass the
list. Additionally all the modifier characters are documented to avoid
user confusion.
---
 src/qemu/qemu_monitor_json.c | 122 +--
 1 file changed, 84 insertions(+), 38 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 914f3ef..d78bd30 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -451,7 +451,28 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char 
*cmdname, ...)
 goto error;
 }

-/* Keys look like   s:name  the first letter is a type code */
+/* Keys look like   s:name  the first letter is a type code:
+ * Explanation of type codes:
+ * s: string value, must be non-null
+ * S: string value, signed, omitted if null
+ *
+ * i: signed integer value
+ * z: signed integer value, omitted if zero
+ *
+ * I: signed long integer value
+ * Z: integer value, signed, omitted if zero
+ *
+ * u: unsigned integer value
+ * p: unsigned integer value, omitted if zero
+ *
+ * U: unsigned long integer value (see below for quirks)
+ * P: unsigned long integer value, omitted if zero,
+ *
+ * d: double precision floating point number
+ * b: boolean value
+ * n: json null value
+ * a: json array
+ */
 type = key[0];
 key += 2;

@@ -461,9 +482,13 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char 
*cmdname, ...)

 /* This doesn't support maps, but no command uses those.  */
 switch (type) {
+case 'S':
 case 's': {
 char *val = va_arg(args, char *);
 if (!val) {
+if (type == 'S')
+continue;
+
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(argument key '%s' must not have null value),
key);
@@ -471,18 +496,38 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char 
*cmdname, ...)
 }
 ret = virJSONValueObjectAppendString(jargs, key, val);
 }   break;
+
+case 'z':
 case 'i': {
 int val = va_arg(args, int);
+
+if (!val  type == 'z')
+continue;
+
 ret = virJSONValueObjectAppendNumberInt(jargs, key, val);
 }   break;
+
+case 'p':
 case 'u': {
 unsigned int val = va_arg(args, unsigned int);
+
+if (!val  type == 'p')
+continue;
+
 ret = virJSONValueObjectAppendNumberUint(jargs, key, val);
 }   break;
+
+case 'Z':
 case 'I': {
 long long val = va_arg(args, long long);
+
+if (!val  type == 'Z')
+continue;
+
 ret = virJSONValueObjectAppendNumberLong(jargs, key, val);
 }   break;
+
+case 'P':
 case 'U': {
 /* qemu silently truncates numbers larger than LLONG_MAX,
  * so passing the full range of unsigned 64 bit integers
@@ -490,23 +535,32 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char 
*cmdname, ...)
  * instead.
  */
 long long val = va_arg(args, long long);
+
+if (!val  type == 'P')
+continue;
+
 ret = virJSONValueObjectAppendNumberLong(jargs, key, val);
 }   break;
+
 case 'd': {
 double val = va_arg(args, double);
 ret = virJSONValueObjectAppendNumberDouble(jargs, key, val);
 }   break;
+
 case 'b': {
 int val = va_arg(args, int);
 ret = virJSONValueObjectAppendBoolean(jargs, key, val);
 }   break;
+
 case 'n': {
 ret = virJSONValueObjectAppendNull(jargs, key);
 }   break;
+
 case 'a': {
 virJSONValuePtr val = va_arg(args, virJSONValuePtr);
 ret = virJSONValueObjectAppend(jargs, key, val);
 }   break;
+
 default:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(unsupported data type '%c' for arg '%s'), type, 
key - 2);
@@ -2203,19 +2257,14 @@ int qemuMonitorJSONChangeMedia(qemuMonitorPtr mon,
 {
 int ret;
 virJSONValuePtr cmd;
-if (format)
-cmd = qemuMonitorJSONMakeCommand(change,
- s:device, dev_name,
- s:target, newmedia,
- s:arg, format,
- NULL);
-else
-cmd = qemuMonitorJSONMakeCommand(change,
- s:device, dev_name,
- s:target, newmedia,
-   

[libvirt] [PATCHv3 07/36] storage: Switch metadata crawler to use storage driver to get unique path

2014-05-30 Thread Peter Krempa
Use the virStorageFileGetUniqueIdentifier() function to get a unique
identifier regardless of the target storage type instead of relying on
canonicalize_path().

A new function that checks whether we support a given image is
introduced to avoid errors for unimplemented backends.
---

Notes:
Version 3:
- fixed typo
- ACKed by Eric

 src/storage/storage_driver.c | 77 +++-
 1 file changed, 54 insertions(+), 23 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index f92a553..5c4188f 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2788,6 +2788,30 @@ virStorageFileIsInitialized(virStorageSourcePtr src)
 return !!src-drv;
 }

+
+static bool
+virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
+{
+int actualType = virStorageSourceGetActualType(src);
+virStorageFileBackendPtr backend;
+
+if (!src)
+return false;
+
+if (src-drv) {
+backend = src-drv-backend;
+} else {
+if (!(backend = virStorageFileBackendForTypeInternal(actualType,
+ src-protocol,
+ false)))
+return false;
+}
+
+return backend-storageFileGetUniqueIdentifier 
+   backend-storageFileReadHeader 
+   backend-storageFileAccess;
+}
+
 void
 virStorageFileDeinit(virStorageSourcePtr src)
 {
@@ -3104,7 +3128,6 @@ virFindBackingFile(const char *start, const char *path,
 /* Recursive workhorse for virStorageFileGetMetadata.  */
 static int
 virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
- const char *canonPath,
  uid_t uid, gid_t gid,
  bool allow_probe,
  virHashTablePtr cycle)
@@ -3112,49 +3135,63 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr 
src,
 int fd;
 int ret = -1;
 struct stat st;
+const char *uniqueName;
 virStorageSourcePtr backingStore = NULL;
 int backingFormat;

-VIR_DEBUG(path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d,
-  src-path, canonPath, NULLSTR(src-relDir), src-format,
+VIR_DEBUG(path=%s dir=%s format=%d uid=%d gid=%d probe=%d,
+  src-path, NULLSTR(src-relDir), src-format,
   (int)uid, (int)gid, allow_probe);

-if (virHashLookup(cycle, canonPath)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(backing store for %s is self-referential),
-   src-path);
+/* exit if we can't load information about the current image */
+if (!virStorageFileSupportsBackingChainTraversal(src))
+return 0;
+
+if (virStorageFileInitAs(src, uid, gid)  0)
 return -1;
+
+if (!(uniqueName = virStorageFileGetUniqueIdentifier(src)))
+goto cleanup;
+
+if (virHashLookup(cycle, uniqueName)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(backing store for %s (%s) is self-referential),
+   src-path, uniqueName);
+goto cleanup;
 }

-if (virHashAddEntry(cycle, canonPath, (void *)1)  0)
-return -1;
+if (virHashAddEntry(cycle, uniqueName, (void *)1)  0)
+goto cleanup;

 if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
 if ((fd = virFileOpenAs(src-path, O_RDONLY, 0, uid, gid, 0))  0) {
 virReportSystemError(-fd, _(Failed to open file '%s'),
  src-path);
-return -1;
+goto cleanup;
 }

 if (virStorageFileGetMetadataFromFDInternal(src, fd,
 backingFormat)  0) {
 VIR_FORCE_CLOSE(fd);
-return -1;
+goto cleanup;
 }

 if (VIR_CLOSE(fd)  0)
 VIR_WARN(could not close file %s, src-path);
 } else {
 /* TODO: currently we call this only for local storage */
-return 0;
+ret = 0;
+goto cleanup;
 }

 /* check whether we need to go deeper */
-if (!src-backingStoreRaw)
-return 0;
+if (!src-backingStoreRaw) {
+ret = 0;
+goto cleanup;
+}

 if (VIR_ALLOC(backingStore)  0)
-return -1;
+goto cleanup;

 if (VIR_STRDUP(backingStore-relPath, src-backingStoreRaw)  0)
 goto cleanup;
@@ -3201,7 +3238,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 backingStore-format = backingFormat;

 if (virStorageFileGetMetadataRecurse(backingStore,
- backingStore-path,
  uid, gid, allow_probe,
  cycle)  0) {
 /* if we fail somewhere midway, just accept and return a
@@ -3215,6 +3251,7 @@ 

[libvirt] [PATCHv3 32/36] lib: Introduce flag VIR_DOMAIN_BLOCK_COMMIT_RELATIVE

2014-05-30 Thread Peter Krempa
Introduce flag for the block commit API to allow the commit operation to
leave the chain relatively addressed. Also adds a virsh switch to enable
this behavior.
---
 include/libvirt/libvirt.h.in | 4 
 tools/virsh-domain.c | 7 +++
 tools/virsh.pod  | 6 --
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 260c971..2df7fc8 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2588,6 +2588,10 @@ typedef enum {
 VIR_DOMAIN_BLOCK_COMMIT_DELETE  = 1  1, /* Delete any files that are now
  invalid after their contents
  have been committed */
+VIR_DOMAIN_BLOCK_COMMIT_RELATIVE = 1  2, /* try to keep the backing chain
+  relative if the components
+  removed by the commit are
+  already relative */
 } virDomainBlockCommitFlags;

 int virDomainBlockCommit(virDomainPtr dom, const char *disk, const char *base,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 84a6706..5c3a142 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1492,6 +1492,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
 flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW;
 if (vshCommandOptBool(cmd, delete))
 flags |= VIR_DOMAIN_BLOCK_COMMIT_DELETE;
+if (vshCommandOptBool(cmd, keep-relative))
+flags |= VIR_DOMAIN_BLOCK_COMMIT_RELATIVE;
 ret = virDomainBlockCommit(dom, path, base, top, bandwidth, flags);
 break;
 case VSH_CMD_BLOCK_JOB_COPY:
@@ -1612,6 +1614,11 @@ static const vshCmdOptDef opts_block_commit[] = {
  .type = VSH_OT_BOOL,
  .help = N_(with --wait, don't wait for cancel to finish)
 },
+{.name = keep-relative,
+ .type = VSH_OT_BOOL,
+ .help = N_(keep the backing chain relative if it was relatively 
+referenced if it was before)
+},
 {.name = NULL}
 };

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 02671b4..03b94a5 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -767,7 +767,7 @@ address of virtual interface (such as Idetach-interface or
 Idomif-setlink) will accept the MAC address printed by this command.

 =item Bblockcommit Idomain Ipath [Ibandwidth]
-{[Ibase] | [I--shallow]} [Itop] [I--delete]
+{[Ibase] | [I--shallow]} [Itop] [I--delete] [I--keep-relative]
 [I--wait [I--verbose] [I--timeout Bseconds] [I--async]]

 Reduce the length of a backing image chain, by committing changes at the
@@ -779,7 +779,9 @@ I--shallow can be used instead of Ibase to specify the 
immediate
 backing file of the resulting top image to be committed.  The files
 being committed are rendered invalid, possibly as soon as the operation
 starts; using the I--delete flag will remove these files at the successful
-completion of the commit operation.
+completion of the commit operation. Using the I--keep-relative flag
+will try to keep the backing chain names relative (if they were
+relative before).

 By default, this command returns as soon as possible, and data for
 the entire disk is committed in the background; the progress of the
-- 
1.9.3

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


[libvirt] [PATCHv3 11/36] storage: Change to new backing store parser

2014-05-30 Thread Peter Krempa
Use the new backing store parser in the backing chain crawler. This
change needs one test change where information about the NBD image are
now parsed differently.
---
 src/storage/storage_driver.c | 90 +---
 tests/virstoragetest.c   | 14 ---
 2 files changed, 9 insertions(+), 95 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 787171d..4d96070 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3068,56 +3068,6 @@ virStorageFileAccess(virStorageSourcePtr src,
 }


-/**
- * Given a starting point START (a directory containing the original
- * file, if the original file was opened via a relative path; ignored
- * if NAME is absolute), determine the location of the backing file
- * NAME (possibly relative), and compute the relative DIRECTORY
- * (optional) and CANONICAL (mandatory) location of the backing file.
- * Return 0 on success, negative on error.
- */
-static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
-virFindBackingFile(const char *start, const char *path,
-   char **directory, char **canonical)
-{
-/* FIXME - when we eventually allow non-raw network devices, we
- * must ensure that we handle backing files the same way as qemu.
- * For a qcow2 top file of gluster://server/vol/img, qemu treats
- * the relative backing file 'rel' as meaning
- * 'gluster://server/vol/rel', while the backing file '/abs' is
- * used as a local file.  But we cannot canonicalize network
- * devices via canonicalize_file_name(), because they are not part
- * of the local file system.  */
-char *combined = NULL;
-int ret = -1;
-
-if (*path == '/') {
-/* Safe to cast away const */
-combined = (char *)path;
-} else if (virAsprintf(combined, %s/%s, start, path)  0) {
-goto cleanup;
-}
-
-if (directory  !(*directory = mdir_name(combined))) {
-virReportOOMError();
-goto cleanup;
-}
-
-if (!(*canonical = canonicalize_file_name(combined))) {
-virReportSystemError(errno,
- _(Can't canonicalize path '%s'), path);
-goto cleanup;
-}
-
-ret = 0;
-
- cleanup:
-if (combined != path)
-VIR_FREE(combined);
-return ret;
-}
-
-
 /* Recursive workhorse for virStorageFileGetMetadata.  */
 static int
 virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
@@ -3126,7 +3076,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
  virHashTablePtr cycle)
 {
 int ret = -1;
-struct stat st;
 const char *uniqueName;
 char *buf = NULL;
 ssize_t headerLen;
@@ -3178,46 +3127,9 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 goto cleanup;
 }

-if (VIR_ALLOC(backingStore)  0)
+if (!(backingStore = virStorageSourceNewFromBacking(src)))
 goto cleanup;

-if (VIR_STRDUP(backingStore-relPath, src-backingStoreRaw)  0)
-goto cleanup;
-
-if (virStorageIsFile(src-backingStoreRaw)) {
-backingStore-type = VIR_STORAGE_TYPE_FILE;
-
-if (virFindBackingFile(src-relDir,
-   src-backingStoreRaw,
-   backingStore-relDir,
-   backingStore-path)  0) {
-/* the backing file is (currently) unavailable, treat this
- * file as standalone:
- * backingStoreRaw is kept to mark broken image chains */
-VIR_WARN(Backing file '%s' of image '%s' is missing.,
- src-backingStoreRaw, src-path);
-ret = 0;
-goto cleanup;
-}
-
-/* update the type for local storage */
-if (stat(backingStore-path, st) == 0) {
-if (S_ISDIR(st.st_mode)) {
-backingStore-type = VIR_STORAGE_TYPE_DIR;
-backingStore-format = VIR_STORAGE_FILE_DIR;
-} else if (S_ISBLK(st.st_mode)) {
-backingStore-type = VIR_STORAGE_TYPE_BLOCK;
-}
-}
-} else {
-/* TODO: To satisfy the test case, copy the network URI as path. This
- * will be removed later. */
-backingStore-type = VIR_STORAGE_TYPE_NETWORK;
-
-if (VIR_STRDUP(backingStore-path, src-backingStoreRaw)  0)
-goto cleanup;
-}
-
 if (backingFormat == VIR_STORAGE_FILE_AUTO  !allow_probe)
 backingStore-format = VIR_STORAGE_FILE_RAW;
 else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 746bf63..29297ef 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -730,20 +730,22 @@ mymain(void)
 /* Rewrite qcow2 to use an nbd: protocol as backend */
 virCommandFree(cmd);
 cmd = virCommandNewArgList(qemuimg, rebase, -u, -f, qcow2,
-   -F, raw, -b, 

[libvirt] [PATCHv3 29/36] storage: Don't store parent directory of an image explicitly

2014-05-30 Thread Peter Krempa
The parent directory doesn't necessarily need to be stored after we
don't mangle the path stored in the image. Remove it and tweak the code
to avoid using it.
---
 src/storage/storage_driver.c | 11 ++-
 src/util/virstoragefile.c| 68 ++--
 src/util/virstoragefile.h|  3 --
 tests/virstoragetest.c   | 21 --
 4 files changed, 30 insertions(+), 73 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 9ce3b62..1c8ad1e 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3082,8 +3082,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 virStorageSourcePtr backingStore = NULL;
 int backingFormat;

-VIR_DEBUG(path=%s dir=%s format=%d uid=%d gid=%d probe=%d,
-  src-path, NULLSTR(src-relDir), src-format,
+VIR_DEBUG(path=%s format=%d uid=%d gid=%d probe=%d,
+  src-path, src-format,
   (int)uid, (int)gid, allow_probe);

 /* exit if we can't load information about the current image */
@@ -3189,19 +3189,12 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
 if (!(cycle = virHashCreate(5, NULL)))
 return -1;

-if (!src-relDir 
-!(src-relDir = mdir_name(src-path))) {
-virReportOOMError();
-goto cleanup;
-}
-
 if (src-format = VIR_STORAGE_FILE_NONE)
 src-format = allow_probe ? VIR_STORAGE_FILE_AUTO : 
VIR_STORAGE_FILE_RAW;

 ret = virStorageFileGetMetadataRecurse(src, uid, gid,
allow_probe, cycle);

- cleanup:
 VIR_FREE(canonPath);
 virHashFree(cycle);
 return ret;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 02b4c73..2a45e58 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1337,9 +1337,10 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
   unsigned int idx,
   const char **parent)
 {
+virStorageSourcePtr prev = NULL;
 const char *start = chain-path;
 const char *tmp;
-const char *parentDir = .;
+char *parentDir = NULL;
 bool nameIsFile = virStorageIsFile(name);
 size_t i;

@@ -1369,8 +1370,20 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
 break;
 if (nameIsFile  (chain-type == VIR_STORAGE_TYPE_FILE ||
chain-type == VIR_STORAGE_TYPE_BLOCK)) {
+if (prev) {
+if (!(parentDir = mdir_name(prev-path))) {
+virReportOOMError();
+goto error;
+}
+} else {
+if (VIR_STRDUP(parentDir, .)  0)
+goto error;
+}
+
 int result = virFileRelLinkPointsTo(parentDir, name,
 chain-path);
+
+VIR_FREE(parentDir);
 if (result  0)
 goto error;
 if (result  0)
@@ -1378,7 +1391,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
 }
 }
 *parent = chain-path;
-parentDir = chain-relDir;
+prev = chain;
 chain = chain-backingStore;
 i++;
 }
@@ -1517,7 +1530,6 @@ virStorageSourceClearBackingStore(virStorageSourcePtr def)
 return;

 VIR_FREE(def-relPath);
-VIR_FREE(def-relDir);
 VIR_FREE(def-backingStoreRaw);

 /* recursively free backing chain */
@@ -1576,7 +1588,6 @@ 
virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
const char *rel)
 {
 char *dirname = NULL;
-const char *parentdir = ;
 virStorageSourcePtr ret;

 if (VIR_ALLOC(ret)  0)
@@ -1586,23 +1597,20 @@ 
virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
 if (VIR_STRDUP(ret-relPath, parent-backingStoreRaw)  0)
 goto error;

-/* XXX Once we get rid of the need to use canonical names in path, we will 
be
- * able to use mdir_name on parent-path instead of using parent-relDir */
-if (STRNEQ(parent-relDir, /))
-parentdir = parent-relDir;
-
-if (virAsprintf(ret-path, %s/%s, parentdir, rel)  0)
+if (!(dirname = mdir_name(parent-path))) {
+virReportOOMError();
 goto error;
+}

-if (virStorageSourceGetActualType(parent) != VIR_STORAGE_TYPE_NETWORK) {
-ret-type = VIR_STORAGE_TYPE_FILE;
-
-/* XXX store the relative directory name for test's sake */
-if (!(ret-relDir = mdir_name(ret-path))) {
-virReportOOMError();
+if (STRNEQ(dirname, /)) {
+if (virAsprintf(ret-path, %s/%s, dirname, rel)  0)
 goto error;
-}
 } else {
+if (virAsprintf(ret-path, /%s, rel)  0)
+goto error;
+}
+
+if (virStorageSourceGetActualType(parent) == VIR_STORAGE_TYPE_NETWORK) 

[libvirt] [PATCHv3 15/36] util: storagefile: Add helper to resolve ../, ./ and //// in paths

2014-05-30 Thread Peter Krempa
As gnulib's canonicalize_filename_mode isn't LGPL and relative snapshot
resolution code will need to clean paths with relative path components
this patch adds a libvirt's own implementation of that functionality and
tests to make sure everything works well.
---
 src/util/virstoragefile.c | 95 +++
 src/util/virstoragefile.h |  2 +
 tests/virstoragetest.c| 83 +
 3 files changed, 180 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 43b7a5a..a9c6a13 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -40,6 +40,7 @@
 #include virutil.h
 #include viruri.h
 #include dirname.h
+#include virbuffer.h
 #if HAVE_SYS_SYSCALL_H
 # include sys/syscall.h
 #endif
@@ -1927,3 +1928,97 @@ virStorageSourceNewFromBacking(virStorageSourcePtr 
parent)

 return ret;
 }
+
+
+static char *
+virStorageFileExportPath(char **components,
+ size_t ncomponents,
+ bool beginSlash)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+size_t i;
+char *ret = NULL;
+
+if (beginSlash)
+virBufferAddLit(buf, /);
+
+for (i = 0; i  ncomponents; i++) {
+if (i != 0)
+virBufferAddLit(buf, /);
+
+virBufferAdd(buf, components[i], -1);
+}
+
+if (virBufferError(buf) != 0) {
+virReportOOMError();
+return NULL;
+}
+
+/* if the output string is empty ... wel just return an empty string */
+if (!(ret = virBufferContentAndReset(buf)))
+ignore_value(VIR_STRDUP(ret, ));
+
+return ret;
+}
+
+
+char *
+virStorageFileSimplifyPath(const char *path,
+   bool allow_relative)
+{
+bool beginSlash = false;
+char **components = NULL;
+size_t ncomponents = 0;
+size_t i;
+char *ret = NULL;
+
+/* special cases are  and //, return them as they are */
+if (STREQ(path, ) || STREQ(path, //)) {
+ignore_value(VIR_STRDUP(ret, path));
+goto cleanup;
+}
+
+if (path[0] == '/')
+beginSlash = true;
+
+if (!(components = virStringSplitCount(path, /, 0, ncomponents)))
+goto cleanup;
+
+i = 0;
+while (i  ncomponents) {
+if (STREQ(components[i], ) ||
+STREQ(components[i], .)) {
+VIR_FREE(components[i]);
+VIR_DELETE_ELEMENT(components, i, ncomponents);
+continue;
+}
+
+if (STREQ(components[i], ..)) {
+if (allow_relative  !beginSlash 
+(i == 0 || STREQ(components[i - 1], ..))) {
+i++;
+continue;
+}
+
+VIR_FREE(components[i]);
+VIR_DELETE_ELEMENT(components, i, ncomponents);
+
+if (i != 0) {
+VIR_FREE(components[i - 1]);
+VIR_DELETE_ELEMENT(components, i - 1, ncomponents);
+i--;
+}
+
+continue;
+}
+
+i++;
+}
+
+ret = virStorageFileExportPath(components, ncomponents, beginSlash);
+
+ cleanup:
+virStringFreeListCount(components, ncomponents);
+
+return ret;
+}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 34b3625..3e9e5c8 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -325,5 +325,7 @@ void virStorageSourceFree(virStorageSourcePtr def);
 void virStorageSourceClearBackingStore(virStorageSourcePtr def);
 virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent);

+char *virStorageFileSimplifyPath(const char *path,
+ bool allow_relative);

 #endif /* __VIR_STORAGE_FILE_H__ */
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 29297ef..57f16ca 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -512,12 +512,61 @@ testStorageLookup(const void *args)
 return ret;
 }

+
+struct testPathSimplifyData
+{
+const char *path;
+const char *exp_abs;
+const char *exp_rel;
+};
+
+
+static int
+testPathSimplify(const void *args)
+{
+const struct testPathSimplifyData *data = args;
+char *simple;
+int ret = -1;
+
+if (!(simple = virStorageFileSimplifyPath(data-path, false))) {
+fprintf(stderr, path simplification returned NULL\n);
+goto cleanup;
+}
+
+if (STRNEQ(simple, data-exp_abs)) {
+fprintf(stderr, simplify path (absolute) '%s': expected: '%s', got 
'%s'\n,
+data-path, data-exp_abs, simple);
+goto cleanup;
+}
+
+VIR_FREE(simple);
+
+if (!(simple = virStorageFileSimplifyPath(data-path, true))) {
+fprintf(stderr, path simplification returned NULL\n);
+goto cleanup;
+}
+
+if (STRNEQ(simple, data-exp_rel)) {
+fprintf(stderr, simplify path (relative) '%s': expected: '%s', got 
'%s'\n,
+data-path, data-exp_rel, simple);
+goto cleanup;
+}
+
+ret = 

Re: [libvirt] [PATCH 01/12] Introduce virNodeHugeTLB

2014-05-30 Thread Daniel P. Berrange
On Thu, May 29, 2014 at 10:32:35AM +0200, Michal Privoznik wrote:
  /**
 + * virNodeHugeTLB:
 + * @conn: pointer to the hypervisor connection
 + * @type: type
 + * @params: pointer to memory parameter object
 + *  (return value, allocated by the caller)
 + * @nparams: pointer to number of memory parameters; input and output
 + * @flags: extra flags; not used yet, so callers should always pass 0
 + *
 + * Get information about host's huge pages. On input, @nparams
 + * gives the size of the @params array; on output, @nparams gives
 + * how many slots were filled with parameter information, which
 + * might be less but will not exceed the input value.
 + *
 + * As a special case, calling with @params as NULL and @nparams
 + * as 0 on input will cause @nparams on output to contain the
 + * number of parameters supported by the hypervisor. The caller
 + * should then allocate @params array, i.e.
 + * (sizeof(@virTypedParameter) * @nparams) bytes and call the API
 + * again.  See virDomainGetMemoryParameters() for an equivalent
 + * usage example.
 + *
 + * Returns 0 in case of success, and -1 in case of failure.
 + */
 +int
 +virNodeHugeTLB(virConnectPtr conn,
 +   int type,
 +   virTypedParameterPtr params,
 +   int *nparams,
 +   unsigned int flags)

What is the 'type' parameter doing ?

I think in general this API needs a different design. I'd like to have
an API that can request info for all page sizes on all NUMA nods in a
single call. I also think the static unchanging data should be part of
the cpu + NUMA info in the capabilities XML. So the API only reports
info which is changing - ie the available pages.

In the cpu element, we should report which page sizes are available
for the CPU model.

In the topology element we should report the number of pages of each
size that are present in that node. We shouldn't treat huge pages
separately from small pages in this respect.

So as an example

 - CPU supports 3 page sizes 4k, 2MB, 1GB
 - 2 numa nodes
 - 3 GB of memory per numa node
 - First node has
- 262144  * 4k pages
- 2 * 1 GB pages
 - Second node has
- 1536 * 2 MB pages

This would look like this

  host
cpu
  archx86_64/arch
  modelWestmere/model
  vendorIntel/vendor
  topology sockets='1' cores='6' threads='2'/
  feature name='rdtscp'/
  feature name='pdpe1gb'/
  feature name='dca'/
  feature name='pdcm'/
  feature name='xtpr'/
  pages units=KiB size=4/
  pages units=MiB size=2/
  pages units=GiB size=1/
/cpu

topology
  cells num='2'
cell id='0'
  memory unit='KiB'3221225472/memory
  pages unit=KiB  size=4262144/pages
  pages unit=MiB  size=20/pages
  pages unit=GiB  size=12/pages
  cpus num='4'
cpu id='0'/
cpu id='2'/
cpu id='4'/
cpu id='6'/
  /cpus
/cell
cell id='1'
  memory unit='KiB'3221225472/memory
  pages unit=KiB  size=40/pages
  pages unit=MiB  size=21536/pages
  pages unit=GiB  size=12/pages
  cpus num='4'
cpu id='1'/
cpu id='3'/
cpu id='5'/
cpu id='7'/
  /cpus
/cell
  /cells
/topology


So then an API call to request the available pages on all nodes
would look something like

   virNodeGetFreePages(virConnectPtr conn,
   unsigned int *pages,
   unsigned int npages,
   unsigned int startcell,
   unsigned int cellcount,
   unsigned long long *counts);

In this API

 @pages - array whose elements are the page sizes to request info for
 @npages - number of elements in @pages
 @startcell - ID of first NUMA cell to request data for
 @cellcount - number of cells to request data for
 @counts - array which is @npages * @cellcount in length

So if you want free count for all page sizes on all NUMA nodes
you might use this as

   unsigned int pages[] = { 4096, 2097152, 1073741824}
   unsigned int npages = ARRAY_CARDINALITY(pages);
   unsigned int startcell = 0;
   unsigned int cellcount = 2;

   unsigned long long counts = malloc(sizeof(long long) * npages * cellcount);

   virNodeGetFreePages(conn, pages, npages,
   startcell, cellcount, counts);

   for (i = 0 ; i  cellcount ; i++) {
   fprintf(stdout, Cell %d\n, startcell + i);
   for (j = 0 ; j  npages ; j++) {
  fprintf(stdout,   Page size=%d count=%d bytes=%llu\n,
  pages[j], counts[(i * npages) +  j],
  pages[j] * counts[(i * npages) +  j]);
   }
   fprintf(stderr, \n);
   }

 Cell 0
Page size=4096 count=300 bytes=1228800
Page size=2097152 count=0 bytes=0
Page size=1073741824 count=1 bytes=1073741824
 Cell 1
Page size=4096 count=0 bytes=0
Page size=2097152 count=20 bytes=41943040
Page 

Re: [libvirt] [PATCH 06/12] virInterface: Expose link state speed

2014-05-30 Thread Daniel P. Berrange
On Thu, May 29, 2014 at 10:32:40AM +0200, Michal Privoznik wrote:
 Currently it is not possible to determine the speed of an interface
 and whether a link is actually detected from the API. Orchestrating
 platforms want to be able to determine when the link has failed and
 where multiple speeds may be available which one the interface is
 actually connected at. This commit introduces an extension to our
 interface XML (without implementation to interface driver backends):
 
   interface type='ethernet' name='eth0'
 start mode='none'/
 mac address='aa:bb:cc:dd:ee:ff'/
 link speed='1000' state='up'/
 mtu size='1492'/
 ...
   /interface
 
 Where @speed is negotiated link speed in Mbits per second, and state
 is the current NIC state (can be one of the following:  unknown,
 notpresent, down, lowerlayerdown,testing, dormant, up).

This is fine for the the interface objects, but it is limited
in usefulness for SRIOV use cases. The interface objects only
exist for interfaces which are configured for the host. With
SRIOV passthrough some of the interfaces we're interested in
are not going to be configured - they're just bare devices
waiting to be given to a guest.

To deal with that, we need report all this info on the node device
APIs which let us list all NICs, regardless of whether they are
configured on the host or not.

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

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


Re: [libvirt] [PATCH 10/12] virCaps: Introduce IOMMU and VFIO capabilities

2014-05-30 Thread Daniel P. Berrange
On Thu, May 29, 2014 at 10:32:44AM +0200, Michal Privoznik wrote:
 There's no need to check for these two host capabilities on each
 device attach or detach. It's sufficient to check them on the daemon
 start and then just query them from virCaps when needed. Moreover,
 this way it's fairly simple to expose them in capabilities XML.

Unless I'm missing something, this patch is not exposing them in
the capabilities XML, as it hasn't modified the XML formatting
code at all ?


 diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
 index cf474d7..9561ba3 100644
 --- a/src/conf/capabilities.c
 +++ b/src/conf/capabilities.c
 @@ -1068,3 +1068,49 @@ virCapabilitiesGetCpusForNodemask(virCapsPtr caps,
  
  return ret;
  }
 +
 +
 +int
 +virCapabilitiesGetKVMLegacy(virCapsPtr caps,
 +bool *legacy)
 +{
 +if (!caps)
 +return -1;
 +
 +*legacy = caps-host.legacyKVMPassthrough;
 +return 0;
 +}
 +
 +int
 +virCapabilitiesSetKVMLegacy(virCapsPtr caps,
 +bool legacy)
 +{
 +if (!caps)
 +return -1;
 +
 +caps-host.legacyKVMPassthrough = legacy;
 +return 0;
 +}
 +
 +
 +int
 +virCapabilitiesGetVFIO(virCapsPtr caps,
 +   bool *vfio)
 +{
 +if (!caps)
 +return -1;
 +
 +*vfio = caps-host.VFIOPassthrough;
 +return 0;
 +}
 +
 +int
 +virCapabilitiesSetVFIO(virCapsPtr caps,
 +   bool vfio)
 +{
 +if (!caps)
 +return -1;
 +
 +caps-host.VFIOPassthrough = vfio;
 +return 0;
 +}

I'd expect this file to have modified the XML formatter.


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

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


Re: [libvirt] [PATCH 11/12] Expose IOMMU and VFIO capabilities from virCaps

2014-05-30 Thread Daniel P. Berrange
On Thu, May 29, 2014 at 10:32:45AM +0200, Michal Privoznik wrote:
 This piece of information may be useful for management application to
 decide if a domain with a device passthrough can be started on given
 host or not.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
 
 Notes:
 I'm not very happy with the element names, but they're the best I
 could come up with so far. If you have any better suggestion I am
 all ears.
 
  docs/formatcaps.html.in  |  8 +++-
  docs/schemas/capability.rng  | 12 
  src/conf/capabilities.c  |  4 
  tests/capabilityschemadata/caps-qemu-kvm.xml |  2 ++
  tests/capabilityschemadata/caps-test.xml |  2 ++
  tests/capabilityschemadata/caps-test2.xml|  2 ++
  tests/capabilityschemadata/caps-test3.xml|  2 ++
  tests/xencapsdata/xen-i686-pae-hvm.xml   |  2 ++
  tests/xencapsdata/xen-i686-pae.xml   |  2 ++
  tests/xencapsdata/xen-i686.xml   |  2 ++
  tests/xencapsdata/xen-ia64-be-hvm.xml|  2 ++
  tests/xencapsdata/xen-ia64-be.xml|  2 ++
  tests/xencapsdata/xen-ia64-hvm.xml   |  2 ++
  tests/xencapsdata/xen-ia64.xml   |  2 ++
  tests/xencapsdata/xen-ppc64.xml  |  2 ++
  tests/xencapsdata/xen-x86_64-hvm.xml |  2 ++
  tests/xencapsdata/xen-x86_64.xml |  2 ++
  17 files changed, 51 insertions(+), 1 deletion(-)
 
 diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
 index d060a5b..eb8c905 100644
 --- a/docs/formatcaps.html.in
 +++ b/docs/formatcaps.html.in
 @@ -35,6 +35,8 @@ BIOS you will see/p
lt;suspend_disk/gt;
lt;suspend_hybrid/gt;
  lt;power_management/gt;
 +lt;kvmgt;truelt;/kvmgt;
 +lt;vfiogt;truelt;/vfiogt;
lt;/hostgt;/span
  
lt;!-- xen-3.0-x86_64 --gt;
 @@ -78,7 +80,11 @@ BIOS you will see/p
Suspend-to-Disk (S4) and Hybrid-Suspend (a combination of S3
and S4). In case the host does not support
any such feature, then an empty lt;power_management/gt;
 -  tag will be shown. /p
 +  tag will be shown. Then, two elements
 +  codelt;kvm/gt;/code and codelt;vfio/gt;/code
 +  expose the fact, whether the host supports legacy device
 +  passthrough with IOMMU cooperation or newer Virtual function
 +  I/O./p
  pThe second block (in blue) indicates the paravirtualization
support of the Xen support, you will see the os_type of xen
to indicate a paravirtual kernel, then architecture
 diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
 index d2d9776..3b378eb 100644
 --- a/docs/schemas/capability.rng
 +++ b/docs/schemas/capability.rng
 @@ -48,6 +48,18 @@
zeroOrMore
  ref name='secmodel'/
/zeroOrMore
 +  element name='kvm'
 +  choice
 +  valuefalse/value
 +  valuetrue/value
 +  /choice
 +  /element
 +  element name='vfio'
 +  choice
 +  valuefalse/value
 +  valuetrue/value
 +  /choice
 +  /element
  /element
/define
  
 diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
 index 9561ba3..a91f37b 100644
 --- a/src/conf/capabilities.c
 +++ b/src/conf/capabilities.c
 @@ -901,6 +901,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
  virBufferAddLit(buf, /secmodel\n);
  }
  
 +/* KVM and VFIO features */
 +virBufferAsprintf(buf, kvm%s/kvm\n, 
 caps-host.legacyKVMPassthrough ? true : false);
 +virBufferAsprintf(buf, vfio%s/vfio\n, caps-host.VFIOPassthrough 
 ? true : false);

Ah, so this is missing from the previous patch.

In fact the splt between this  previous patch is a bit confusing.

I'd expect the previous patch to only change the src/conf/capabilities*
files and generic test cases. While this patch only change the driver
code and driver specific test cases.


So this XML is really trying to provide a list of the valid enumeration
options for the driver element of hostdev devices. This is quite a
common request for many domain XML elements, so I feel we ought to do
something that is not so ad-hoc here. When we get into this world of
providing info about supported options for devices, we really need to
be able to express this on a fine granularity that the capabilities
XML allows for.

For example, different emulator binaries will support different options,
as will many different architectures, or even different machine types of
virtualization types.

So it feels like we need a new API for this, that accepts info about
the machine we're trying to launch. eg

  char * virConnectGetEmulatorCapabilties(virConnectPtr conn,
  const char *emulatorbin,
  const char *machine,
  const char *virttype);

NB I didn't include 'architecture' since 

Re: [libvirt] [libvirt-users] Host OS, Storage Info

2014-05-30 Thread Martin Kletzander

On Thu, May 29, 2014 at 01:31:13PM -0400, Ainsworth, Thomas wrote:

Martin, et al,

Sorry for the lag in response.

So I started playing with the various virsh commands.  Awesome.
Been doing some reading and I believe I have some things configured not so
well.
As I stated earlier in the thread, we have all of the VM image files on one
RAID5.  Very fast machine.

When using top, the load average is a stable 5.xx.  No I/O wait. GB's of
free memory.  Swap has not been touched.
Using vmstat, I am writing to the RAID5 volume at a constant 150MB/s and
reading at a constant 275MB/s.

With all of that said, here are some results from virsh commands:

# virsh pool-list --all
Name State  Autostart
--
default  active yes


# virsh pool-info default
Name:   default
UUID:x
State:  running
Persistent: yes
Autostart:  yes
Capacity:   30.76 GiB
Allocation: 2.10 GiB
Available:  28.66 GiB


Now, is that ok to have all of the VM's using a default pool?
Or should a pool be created for each VM instance.
I honestly am not even sure what a pool references...?...



Pool is a set of volumes of the same type (iscsi, LVs, files in a
folder, etc.) in the same place.  Example is the default pool which is,
by default, in /var/lib/libvirt/images and volumes there are files
(pool type is dir).  If you want to have all the domain disks in
that place and all the disks (volumes) should be files then default
pool is enough.


The more I read, the more I am moving away from thinking something in the
OS is the cause of my sluggishness.



I haven't read your previous mail before, so I've found that now.  How
often are you dropping those caches?  That won't help you not to use
swap.  Having memory occupied by buffers and caches is good if you
read/write from/to disks.  Even when the reads/writes are as fast as
you mentioned, reading/writing from/to RAM is way faster and until
there's some free memory, why not use that?

Martin


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

Re: [libvirt] [PATCH 12/12] nodedev: Export NUMA node locality for PCI devices

2014-05-30 Thread Daniel P. Berrange
On Thu, May 29, 2014 at 10:32:46AM +0200, Michal Privoznik wrote:
 A PCI device can be associated with a specific NUMA node. Later, when
 a guest is pinned to one NUMA node the PCI device can be assigned on
 different NUMA node. This makes DMA transfers travel across nodes and
 thus results in suboptimal performance. We should expose the NUMA node
 locality for PCI devices so management applications can make better
 decisions.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
 
 Notes:
 All the machines I have tried this on had only -1 in the
 numa_node file. From the kernel sources it seems that this is the
 default, so I'm not printing the numa/ element into the XML in
 this case. But I'd like to hear your opinion.

Yes, I believe '-1' means that there is no NUMA locality info
available for the device, so it makes sense to skip this.

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

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


Re: [libvirt] [PATCH] docs: bhyve driver documentation improvements

2014-05-30 Thread Ján Tomko
On 05/11/2014 09:30 AM, Roman Bogorodskiy wrote:
 - Document 'domxml-to-native' command
 - Mention that the nmdm console support needs an appropriate
   kernel module loaded
 ---
  docs/drvbhyve.html.in | 21 +
  1 file changed, 21 insertions(+)

ACK from a non-native speaker. (and safe for 1.2.5)

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] libxl: add discard support to libxl_device_disk

2014-05-30 Thread Olaf Hering
Translate libvirt discard settings into libxl-4.5 discard settings.

Signed-off-by: Olaf Hering o...@aepfle.de
---

default means leave decision to libxl.
Not sure if that is what default in libvirt terms really means.


 src/libxl/libxl_conf.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b7fed7f..4cb062e 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -713,6 +713,33 @@ libxlMakeDomBuildInfo(virDomainObjPtr vm, 
libxl_domain_config *d_config)
 return -1;
 }
 
+static void
+libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
+{
+if (!x_disk-readwrite)
+return;
+#if defined(LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE)
+switch (discard) {
+case VIR_DOMAIN_DISK_DISCARD_DEFAULT:
+break;
+case VIR_DOMAIN_DISK_DISCARD_UNMAP:
+libxl_defbool_set(x_disk-discard_enable, true);
+break;
+default:
+case VIR_DOMAIN_DISK_DISCARD_IGNORE:
+libxl_defbool_set(x_disk-discard_enable, false);
+break;
+}
+#else
+if (discard == VIR_DOMAIN_DISK_DISCARD_DEFAULT)
+return;
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(This version of libxenlight does not support 
+ discard= option passing));
+#endif
+}
+
+
 int
 libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
 {
@@ -827,6 +854,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk 
*x_disk)
 x_disk-removable = 1;
 x_disk-readwrite = !l_disk-readonly;
 x_disk-is_cdrom = l_disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 0;
+libxlDiskSetDiscard(x_disk, l_disk-discard);
 /* An empty CDROM must have the empty format, otherwise libxl fails. */
 if (x_disk-is_cdrom  !x_disk-pdev_path)
 x_disk-format = LIBXL_DISK_FORMAT_EMPTY;

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


Re: [libvirt] [PATCH 12/12] nodedev: Export NUMA node locality for PCI devices

2014-05-30 Thread Daniel P. Berrange
On Fri, May 30, 2014 at 10:14:17AM +0100, Daniel P. Berrange wrote:
 On Thu, May 29, 2014 at 10:32:46AM +0200, Michal Privoznik wrote:
  A PCI device can be associated with a specific NUMA node. Later, when
  a guest is pinned to one NUMA node the PCI device can be assigned on
  different NUMA node. This makes DMA transfers travel across nodes and
  thus results in suboptimal performance. We should expose the NUMA node
  locality for PCI devices so management applications can make better
  decisions.
  
  Signed-off-by: Michal Privoznik mpriv...@redhat.com
  ---
  
  Notes:
  All the machines I have tried this on had only -1 in the
  numa_node file. From the kernel sources it seems that this is the
  default, so I'm not printing the numa/ element into the XML in
  this case. But I'd like to hear your opinion.
 
 Yes, I believe '-1' means that there is no NUMA locality info
 available for the device, so it makes sense to skip this.

Confirmed in the kernel source 

  include/linux/numa.h:#defineNUMA_NO_NODE(-1)

Is used when the ACPI tables don't specify any NUMA node for the
PCI device, or when the NUMA node is not online.

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

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


Re: [libvirt] [libvirt-users] Host OS, Storage Info

2014-05-30 Thread Ainsworth, Thomas
Martin,

Thanks for the information.  That makes sense.  I *believe* we are good
there.

I noticed something weird yesterday.  After a clone (via the virt-manager
GUI) it seems libvirtd locked up.   A force quit pop up appeared - I had to
kill it.  Then I restarted libvirtd.  Then I did a ps -edf | grep libvirt
and there were three (3) libvirtd --daemon processes.  Then any virsh
commands or virt-manager GUI (when it finally would come up) was very
sluggish.  By the end of the day I had four (4) of the processes running.
Keep in mind, whilst all of this is going on the VM's were just cranking
along fine.  I could not find any dead PID files elated to the processes to
kill...

...we rebooted the server at the end of the day.  It should be fine until
the next time I attempt a clone operation - which I am hesitant to do for
obvious reasons...

Any ideas?

Thanks,

Tom




On Fri, May 30, 2014 at 5:12 AM, Martin Kletzander mklet...@redhat.com
wrote:

 On Thu, May 29, 2014 at 01:31:13PM -0400, Ainsworth, Thomas wrote:

 Martin, et al,

 Sorry for the lag in response.

 So I started playing with the various virsh commands.  Awesome.
 Been doing some reading and I believe I have some things configured not so
 well.
 As I stated earlier in the thread, we have all of the VM image files on
 one
 RAID5.  Very fast machine.

 When using top, the load average is a stable 5.xx.  No I/O wait. GB's of
 free memory.  Swap has not been touched.
 Using vmstat, I am writing to the RAID5 volume at a constant 150MB/s and
 reading at a constant 275MB/s.

 With all of that said, here are some results from virsh commands:

 # virsh pool-list --all
 Name State  Autostart
 --
 default  active yes


 # virsh pool-info default
 Name:   default
 UUID:x
 State:  running
 Persistent: yes
 Autostart:  yes
 Capacity:   30.76 GiB
 Allocation: 2.10 GiB
 Available:  28.66 GiB


 Now, is that ok to have all of the VM's using a default pool?
 Or should a pool be created for each VM instance.
 I honestly am not even sure what a pool references...?...


 Pool is a set of volumes of the same type (iscsi, LVs, files in a
 folder, etc.) in the same place.  Example is the default pool which is,
 by default, in /var/lib/libvirt/images and volumes there are files
 (pool type is dir).  If you want to have all the domain disks in
 that place and all the disks (volumes) should be files then default
 pool is enough.


  The more I read, the more I am moving away from thinking something in the
 OS is the cause of my sluggishness.


 I haven't read your previous mail before, so I've found that now.  How
 often are you dropping those caches?  That won't help you not to use
 swap.  Having memory occupied by buffers and caches is good if you
 read/write from/to disks.  Even when the reads/writes are as fast as
 you mentioned, reading/writing from/to RAM is way faster and until
 there's some free memory, why not use that?

 Martin

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

Re: [libvirt] [PATCH 06/12] virInterface: Expose link state speed

2014-05-30 Thread Michal Privoznik

On 30.05.2014 10:56, Daniel P. Berrange wrote:

On Thu, May 29, 2014 at 10:32:40AM +0200, Michal Privoznik wrote:

Currently it is not possible to determine the speed of an interface
and whether a link is actually detected from the API. Orchestrating
platforms want to be able to determine when the link has failed and
where multiple speeds may be available which one the interface is
actually connected at. This commit introduces an extension to our
interface XML (without implementation to interface driver backends):

   interface type='ethernet' name='eth0'
 start mode='none'/
 mac address='aa:bb:cc:dd:ee:ff'/
 link speed='1000' state='up'/
 mtu size='1492'/
 ...
   /interface

Where @speed is negotiated link speed in Mbits per second, and state
is the current NIC state (can be one of the following:  unknown,
notpresent, down, lowerlayerdown,testing, dormant, up).


This is fine for the the interface objects, but it is limited
in usefulness for SRIOV use cases. The interface objects only
exist for interfaces which are configured for the host. With
SRIOV passthrough some of the interfaces we're interested in
are not going to be configured - they're just bare devices
waiting to be given to a guest.


I hear what you're saying, but unless a PCI device is given interface 
name I am afraid we can't do anything. For instance, if you have a NIC 
but detach it from the driver (echo ${PCI_ADDR}  
/sys/bus/pci/drivers/driver/unbind), kernel still sees the PCI device 
(it's shown in lspci output for instance), but it's not configured 
anymore - kernel doesn't know device link state, hence it's not aware if 
NIC's link speed, etc. So tools like ethtool, ip, ifconfing won't show 
the device.




To deal with that, we need report all this info on the node device
APIs which let us list all NICs, regardless of whether they are
configured on the host or not.


Due the reasons written above I don't see much benefit in moving this to 
nodedev.


Michal

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


Re: [libvirt] [PATCH 01/12] Introduce virNodeHugeTLB

2014-05-30 Thread Michal Privoznik

On 30.05.2014 10:52, Daniel P. Berrange wrote:

On Thu, May 29, 2014 at 10:32:35AM +0200, Michal Privoznik wrote:

  /**
+ * virNodeHugeTLB:
+ * @conn: pointer to the hypervisor connection
+ * @type: type
+ * @params: pointer to memory parameter object
+ *  (return value, allocated by the caller)
+ * @nparams: pointer to number of memory parameters; input and output
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Get information about host's huge pages. On input, @nparams
+ * gives the size of the @params array; on output, @nparams gives
+ * how many slots were filled with parameter information, which
+ * might be less but will not exceed the input value.
+ *
+ * As a special case, calling with @params as NULL and @nparams
+ * as 0 on input will cause @nparams on output to contain the
+ * number of parameters supported by the hypervisor. The caller
+ * should then allocate @params array, i.e.
+ * (sizeof(@virTypedParameter) * @nparams) bytes and call the API
+ * again.  See virDomainGetMemoryParameters() for an equivalent
+ * usage example.
+ *
+ * Returns 0 in case of success, and -1 in case of failure.
+ */
+int
+virNodeHugeTLB(virConnectPtr conn,
+   int type,
+   virTypedParameterPtr params,
+   int *nparams,
+   unsigned int flags)


What is the 'type' parameter doing ?


Ah, it should be named numa_node rather than type. If type==-1, then 
overall statistics are returned (number of {available,free} pages 
accumulated across all NUMA nodes), if type = 0, info on the specific 
NUMA node is returned.




I think in general this API needs a different design. I'd like to have
an API that can request info for all page sizes on all NUMA nods in a
single call. I also think the static unchanging data should be part of
the cpu + NUMA info in the capabilities XML. So the API only reports
info which is changing - ie the available pages.


The only problem is, the size of huge pages pool is not immutable. Now 
it's possible for 2M huge pages to be allocated dynamically:


# echo 8  /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages

and it may be possible for 1GB too in future (what if kernel learns how 
to do it?). In general, the only thing that we can take unalterable for 
now is the default size of huge pages. And I wouldn't bet on that either.




In the cpu element, we should report which page sizes are available
for the CPU model.


Okay.



In the topology element we should report the number of pages of each
size that are present in that node. We shouldn't treat huge pages
separately from small pages in this respect.


Well, if we do that you'll still need two API calls (and hence you'll 
lack atomicity): virConnectGetCapabilities() followed by 
virNodeGetFreePages(). On the other hand, the virNodeGetFreePages() 
itself is not atomic either (from system POV - other processes can jump 
in and allocate huge pages as libvirtd is executing the API).




So as an example

  - CPU supports 3 page sizes 4k, 2MB, 1GB
  - 2 numa nodes
  - 3 GB of memory per numa node
  - First node has
 - 262144  * 4k pages
 - 2 * 1 GB pages
  - Second node has
 - 1536 * 2 MB pages

This would look like this

   host
 cpu
   archx86_64/arch
   modelWestmere/model
   vendorIntel/vendor
   topology sockets='1' cores='6' threads='2'/
   feature name='rdtscp'/
   feature name='pdpe1gb'/
   feature name='dca'/
   feature name='pdcm'/
   feature name='xtpr'/
   pages units=KiB size=4/
   pages units=MiB size=2/
   pages units=GiB size=1/
 /cpu


Right. This makes sense.



 topology
   cells num='2'
 cell id='0'
   memory unit='KiB'3221225472/memory
   pages unit=KiB  size=4262144/pages
   pages unit=MiB  size=20/pages
   pages unit=GiB  size=12/pages
   cpus num='4'
 cpu id='0'/
 cpu id='2'/
 cpu id='4'/
 cpu id='6'/
   /cpus
 /cell
 cell id='1'
   memory unit='KiB'3221225472/memory
   pages unit=KiB  size=40/pages
   pages unit=MiB  size=21536/pages
   pages unit=GiB  size=12/pages
   cpus num='4'
 cpu id='1'/
 cpu id='3'/
 cpu id='5'/
 cpu id='7'/
   /cpus
 /cell
   /cells
 /topology



Well, and this to some extent too. We can report the pool size, but 
since it may vary it's on the same level as number of free pages. What 
about:


pages unit='MiB' size='2' free='8' avail='8'/

That way we don't even need a new API (not saying we shouldn't implement 
it though - API is still better than updating capabilities each single 
time).





So then an API call to request the available pages on all nodes
would look something like

virNodeGetFreePages(virConnectPtr conn,
unsigned int *pages,

Re: [libvirt] [libvirt-glib] [PATCH] GVirDomainSnapshot: add gvir_domain_snapshot_delete

2014-05-30 Thread Christophe Fergeau
On Thu, May 29, 2014 at 01:46:02PM +0200, Timm Bäder wrote:
 ---
  libvirt-gobject/libvirt-gobject-domain-snapshot.c | 21 +
  libvirt-gobject/libvirt-gobject-domain-snapshot.h |  3 +++
  libvirt-gobject/libvirt-gobject.sym   |  5 +
  3 files changed, 29 insertions(+)
 
 diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c 
 b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
 index ab23342..f46c99b 100644
 --- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
 +++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
 @@ -206,3 +206,24 @@ GVirConfigDomainSnapshot *gvir_domain_snapshot_get_config
  free(xml);
  return conf;
  }
 +
 +/**
 + * gvir_domain_snapshot_delete:
 + * @snapshot: the domain_snapshot

no need for '_' here

 + * @error: (allow-none): Place-holder for error or NULL
 + */
 +void gvir_domain_snapshot_delete(GVirDomainSnapshot *snapshot, GError 
 **error)
 +{
 +GVirDomainSnapshotPrivate *priv;
 +int status;
 +
 +g_return_if_fail(GVIR_IS_DOMAIN_SNAPSHOT (snapshot));
 +g_return_if_fail(error == NULL || *error == NULL);
 +
 +priv = snapshot-priv;
 +status = virDomainSnapshotDelete(priv-handle, 0);

virDomainSnapshotDelete flags seem to provide useful behaviour, so we
should have a 'flags' argument here as well

 +if (status  0) {
 +gvir_set_error_literal(error, GVIR_DOMAIN_SNAPSHOT_ERROR, 0,
 +   Unable to delete snapshot);

I'd include the name of the snapshot in the error message.

Christophe


pgpyH_Hn3I6eN.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 06/12] virInterface: Expose link state speed

2014-05-30 Thread Daniel P. Berrange
On Fri, May 30, 2014 at 01:41:08PM +0200, Michal Privoznik wrote:
 On 30.05.2014 10:56, Daniel P. Berrange wrote:
 On Thu, May 29, 2014 at 10:32:40AM +0200, Michal Privoznik wrote:
 Currently it is not possible to determine the speed of an interface
 and whether a link is actually detected from the API. Orchestrating
 platforms want to be able to determine when the link has failed and
 where multiple speeds may be available which one the interface is
 actually connected at. This commit introduces an extension to our
 interface XML (without implementation to interface driver backends):
 
interface type='ethernet' name='eth0'
  start mode='none'/
  mac address='aa:bb:cc:dd:ee:ff'/
  link speed='1000' state='up'/
  mtu size='1492'/
  ...
/interface
 
 Where @speed is negotiated link speed in Mbits per second, and state
 is the current NIC state (can be one of the following:  unknown,
 notpresent, down, lowerlayerdown,testing, dormant, up).
 
 This is fine for the the interface objects, but it is limited
 in usefulness for SRIOV use cases. The interface objects only
 exist for interfaces which are configured for the host. With
 SRIOV passthrough some of the interfaces we're interested in
 are not going to be configured - they're just bare devices
 waiting to be given to a guest.
 
 I hear what you're saying, but unless a PCI device is given interface name I
 am afraid we can't do anything. For instance, if you have a NIC but detach
 it from the driver (echo ${PCI_ADDR} 
 /sys/bus/pci/drivers/driver/unbind), kernel still sees the PCI device
 (it's shown in lspci output for instance), but it's not configured anymore -
 kernel doesn't know device link state, hence it's not aware if NIC's link
 speed, etc. So tools like ethtool, ip, ifconfing won't show the device.

IIUC, We have three classes of device

 1 Devices not bound - no NIC visible in the host OS

 2 Devices bound but not configured. NIC visible in host OS, but no
   /etc/sysconfig/networking/ifcfg-XXX file

 3 Devices bound and configured. NIC visible in host OS, and has a
   /etc/sysconfig/networking/ifcfg-XXX file

The interface configs only let you deal with NIC devices in class 3.

The nodedev XML / APIs let you see NIC devices in class 2 + 3.



 
 
 To deal with that, we need report all this info on the node device
 APIs which let us list all NICs, regardless of whether they are
 configured on the host or not.
 
 Due the reasons written above I don't see much benefit in moving this to
 nodedev.

NB, I don't mean we should move it. I mean we should report the data in
both places - nodedev and interface XML


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

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


Re: [libvirt] [PATCH 01/12] Introduce virNodeHugeTLB

2014-05-30 Thread Daniel P. Berrange
On Fri, May 30, 2014 at 01:41:06PM +0200, Michal Privoznik wrote:
 On 30.05.2014 10:52, Daniel P. Berrange wrote:
 On Thu, May 29, 2014 at 10:32:35AM +0200, Michal Privoznik wrote:
   /**
 + * virNodeHugeTLB:
 + * @conn: pointer to the hypervisor connection
 + * @type: type
 + * @params: pointer to memory parameter object
 + *  (return value, allocated by the caller)
 + * @nparams: pointer to number of memory parameters; input and output
 + * @flags: extra flags; not used yet, so callers should always pass 0
 + *
 + * Get information about host's huge pages. On input, @nparams
 + * gives the size of the @params array; on output, @nparams gives
 + * how many slots were filled with parameter information, which
 + * might be less but will not exceed the input value.
 + *
 + * As a special case, calling with @params as NULL and @nparams
 + * as 0 on input will cause @nparams on output to contain the
 + * number of parameters supported by the hypervisor. The caller
 + * should then allocate @params array, i.e.
 + * (sizeof(@virTypedParameter) * @nparams) bytes and call the API
 + * again.  See virDomainGetMemoryParameters() for an equivalent
 + * usage example.
 + *
 + * Returns 0 in case of success, and -1 in case of failure.
 + */
 +int
 +virNodeHugeTLB(virConnectPtr conn,
 +   int type,
 +   virTypedParameterPtr params,
 +   int *nparams,
 +   unsigned int flags)
 
 What is the 'type' parameter doing ?
 
 Ah, it should be named numa_node rather than type. If type==-1, then overall
 statistics are returned (number of {available,free} pages accumulated across
 all NUMA nodes), if type = 0, info on the specific NUMA node is returned.
 
 
 I think in general this API needs a different design. I'd like to have
 an API that can request info for all page sizes on all NUMA nods in a
 single call. I also think the static unchanging data should be part of
 the cpu + NUMA info in the capabilities XML. So the API only reports
 info which is changing - ie the available pages.
 
 The only problem is, the size of huge pages pool is not immutable. Now it's
 possible for 2M huge pages to be allocated dynamically:
 
 # echo 8  /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages
 
 and it may be possible for 1GB too in future (what if kernel learns how to
 do it?). In general, the only thing that we can take unalterable for now is
 the default size of huge pages. And I wouldn't bet on that either.

Yes, you can in theory change the number of huge pages at an arbitrary
time, but realistically people mostly only do it immediately at boot.
With 1 GB pages is will be impossible todo it any time except immediately
at boot. If you wait a little while, then memory will be too fragmented
for you to be able to dynamically allocate more 1 GB pages. The same
is true of 2MB pages to a lesser degree.

 In the topology element we should report the number of pages of each
 size that are present in that node. We shouldn't treat huge pages
 separately from small pages in this respect.
 
 Well, if we do that you'll still need two API calls (and hence you'll lack
 atomicity): virConnectGetCapabilities() followed by virNodeGetFreePages().
 On the other hand, the virNodeGetFreePages() itself is not atomic either
 (from system POV - other processes can jump in and allocate huge pages as
 libvirtd is executing the API).

I'm not worried about atomicity wrt to free pages - no matter what
you do you will always race, because there's inherantly a gap between
when you check the free pages and when the VM boots. I'm more interested
in the fact that the capabilities shows you the overall NUMA topology
of the host, and the pages allocated per node are inherantly part of
that model IMHO.

host
  cpu
archx86_64/arch
modelWestmere/model
vendorIntel/vendor
topology sockets='1' cores='6' threads='2'/
feature name='rdtscp'/
feature name='pdpe1gb'/
feature name='dca'/
feature name='pdcm'/
feature name='xtpr'/
pages units=KiB size=4/
pages units=MiB size=2/
pages units=GiB size=1/
  /cpu
 
 Right. This makes sense.
 
 
  topology
cells num='2'
  cell id='0'
memory unit='KiB'3221225472/memory
pages unit=KiB  size=4262144/pages
pages unit=MiB  size=20/pages
pages unit=GiB  size=12/pages
cpus num='4'
  cpu id='0'/
  cpu id='2'/
  cpu id='4'/
  cpu id='6'/
/cpus
  /cell
  cell id='1'
memory unit='KiB'3221225472/memory
pages unit=KiB  size=40/pages
pages unit=MiB  size=21536/pages
pages unit=GiB  size=12/pages
cpus num='4'
  cpu id='1'/
  cpu id='3'/
  cpu id='5'/
  cpu id='7'/
/cpus
  /cell
/cells
  /topology
 
 

[libvirt] [PATCH] virsh: Check wether found volume is member of the specified storage pool

2014-05-30 Thread Peter Krempa
When looking up storage volumes virsh uses multiple lookup steps. Some
of the steps don't require a pool name specified. This resulted into a
possibility that a volume would be part of a different pool than the
user specified:

Let's have a /var/lib/libvirt/images/test.qcow image in the 'default'
pool and a second pool 'emptypool':

Currently we'd return:
  $ virsh vol-info --pool emptypool /var/lib/libvirt/images/test.qcow
  Name:   test.qcow
  Type:   file
  Capacity:   100.00 MiB
  Allocation: 212.00 KiB

After the fix:
 $ tools/virsh vol-info --pool emptypool /var/lib/libvirt/images/test.qcow
 error: Requested volume '/var/lib/libvirt/images/test.qcow' found in a 
different pool (default) than specified

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1088667
---
 tools/virsh-volume.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 55bf6f0..6416ba6 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -104,6 +104,26 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
 might help), n, pooloptname);
 }

+/* If the pool was specified, then make sure that the returned
+ * volume is from the given pool */
+if (pool  vol) {
+virStoragePoolPtr volpool = NULL;
+
+if ((volpool = virStoragePoolLookupByVolume(vol))) {
+if (STRNEQ(virStoragePoolGetName(volpool),
+   virStoragePoolGetName(pool))) {
+vshResetLibvirtError();
+vshError(ctl,
+ _(Requested volume '%s' found in a different 
+   pool (%s) than specified),
+ n, virStoragePoolGetName(volpool));
+virStorageVolFree(vol);
+vol = NULL;
+}
+virStoragePoolFree(volpool);
+}
+}
+
 if (pool)
 virStoragePoolFree(pool);

-- 
1.9.3

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


Re: [libvirt] [PATCH 01/12] Introduce virNodeHugeTLB

2014-05-30 Thread Michal Privoznik

On 30.05.2014 14:46, Daniel P. Berrange wrote:

On Fri, May 30, 2014 at 01:41:06PM +0200, Michal Privoznik wrote:

On 30.05.2014 10:52, Daniel P. Berrange wrote:

On Thu, May 29, 2014 at 10:32:35AM +0200, Michal Privoznik wrote:

  /**
+ * virNodeHugeTLB:
+ * @conn: pointer to the hypervisor connection
+ * @type: type
+ * @params: pointer to memory parameter object
+ *  (return value, allocated by the caller)
+ * @nparams: pointer to number of memory parameters; input and output
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Get information about host's huge pages. On input, @nparams
+ * gives the size of the @params array; on output, @nparams gives
+ * how many slots were filled with parameter information, which
+ * might be less but will not exceed the input value.
+ *
+ * As a special case, calling with @params as NULL and @nparams
+ * as 0 on input will cause @nparams on output to contain the
+ * number of parameters supported by the hypervisor. The caller
+ * should then allocate @params array, i.e.
+ * (sizeof(@virTypedParameter) * @nparams) bytes and call the API
+ * again.  See virDomainGetMemoryParameters() for an equivalent
+ * usage example.
+ *
+ * Returns 0 in case of success, and -1 in case of failure.
+ */
+int
+virNodeHugeTLB(virConnectPtr conn,
+   int type,
+   virTypedParameterPtr params,
+   int *nparams,
+   unsigned int flags)


What is the 'type' parameter doing ?


Ah, it should be named numa_node rather than type. If type==-1, then overall
statistics are returned (number of {available,free} pages accumulated across
all NUMA nodes), if type = 0, info on the specific NUMA node is returned.



I think in general this API needs a different design. I'd like to have
an API that can request info for all page sizes on all NUMA nods in a
single call. I also think the static unchanging data should be part of
the cpu + NUMA info in the capabilities XML. So the API only reports
info which is changing - ie the available pages.


The only problem is, the size of huge pages pool is not immutable. Now it's
possible for 2M huge pages to be allocated dynamically:

# echo 8  /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages

and it may be possible for 1GB too in future (what if kernel learns how to
do it?). In general, the only thing that we can take unalterable for now is
the default size of huge pages. And I wouldn't bet on that either.


Yes, you can in theory change the number of huge pages at an arbitrary
time, but realistically people mostly only do it immediately at boot.
With 1 GB pages is will be impossible todo it any time except immediately
at boot. If you wait a little while, then memory will be too fragmented
for you to be able to dynamically allocate more 1 GB pages. The same
is true of 2MB pages to a lesser degree.


IMO no. Processes never ever see physical address (PA). All they see are 
virtual addresses (VA). So there is possibility for kernel to rearrange 
physical memory without effect on the processes in order to gain bigger 
segments of free memory. The only problem are DMA transfers which access 
PA directly but that can be resolved by reserving first X {M,G}B of RAM 
for the transfers. And kernel is doing something similar already - 
Kernel SamePage Merging (KSM). On the other hand, I don't think huge 
page pool size is going to be changed too often. Especially when 
compared to huge page allocation in a userspace processes. So whenever 
mgmt application changes the pool size, it knows it has to refetch 
capabilities. Well, if we document it :-)





In the topology element we should report the number of pages of each
size that are present in that node. We shouldn't treat huge pages
separately from small pages in this respect.


Well, if we do that you'll still need two API calls (and hence you'll lack
atomicity): virConnectGetCapabilities() followed by virNodeGetFreePages().
On the other hand, the virNodeGetFreePages() itself is not atomic either
(from system POV - other processes can jump in and allocate huge pages as
libvirtd is executing the API).


I'm not worried about atomicity wrt to free pages - no matter what
you do you will always race, because there's inherantly a gap between
when you check the free pages and when the VM boots. I'm more interested
in the fact that the capabilities shows you the overall NUMA topology
of the host, and the pages allocated per node are inherantly part of
that model IMHO.


   host
 cpu
   archx86_64/arch
   modelWestmere/model
   vendorIntel/vendor
   topology sockets='1' cores='6' threads='2'/
   feature name='rdtscp'/
   feature name='pdpe1gb'/
   feature name='dca'/
   feature name='pdcm'/
   feature name='xtpr'/
   pages units=KiB size=4/
   pages units=MiB size=2/
   pages units=GiB size=1/
 /cpu


Right. This makes sense.



 topology
   cells num='2'
 cell id='0'
 

Re: [libvirt] [PATCH 06/12] virInterface: Expose link state speed

2014-05-30 Thread Michal Privoznik

On 30.05.2014 14:35, Daniel P. Berrange wrote:

On Fri, May 30, 2014 at 01:41:08PM +0200, Michal Privoznik wrote:

On 30.05.2014 10:56, Daniel P. Berrange wrote:

On Thu, May 29, 2014 at 10:32:40AM +0200, Michal Privoznik wrote:

Currently it is not possible to determine the speed of an interface
and whether a link is actually detected from the API. Orchestrating
platforms want to be able to determine when the link has failed and
where multiple speeds may be available which one the interface is
actually connected at. This commit introduces an extension to our
interface XML (without implementation to interface driver backends):

   interface type='ethernet' name='eth0'
 start mode='none'/
 mac address='aa:bb:cc:dd:ee:ff'/
 link speed='1000' state='up'/
 mtu size='1492'/
 ...
   /interface

Where @speed is negotiated link speed in Mbits per second, and state
is the current NIC state (can be one of the following:  unknown,
notpresent, down, lowerlayerdown,testing, dormant, up).


This is fine for the the interface objects, but it is limited
in usefulness for SRIOV use cases. The interface objects only
exist for interfaces which are configured for the host. With
SRIOV passthrough some of the interfaces we're interested in
are not going to be configured - they're just bare devices
waiting to be given to a guest.


I hear what you're saying, but unless a PCI device is given interface name I
am afraid we can't do anything. For instance, if you have a NIC but detach
it from the driver (echo ${PCI_ADDR} 
/sys/bus/pci/drivers/driver/unbind), kernel still sees the PCI device
(it's shown in lspci output for instance), but it's not configured anymore -
kernel doesn't know device link state, hence it's not aware if NIC's link
speed, etc. So tools like ethtool, ip, ifconfing won't show the device.


IIUC, We have three classes of device

  1 Devices not bound - no NIC visible in the host OS

  2 Devices bound but not configured. NIC visible in host OS, but no
/etc/sysconfig/networking/ifcfg-XXX file

  3 Devices bound and configured. NIC visible in host OS, and has a
/etc/sysconfig/networking/ifcfg-XXX file

The interface configs only let you deal with NIC devices in class 3.

The nodedev XML / APIs let you see NIC devices in class 2 + 3.



Right. The netcf world. Okay, makes sense now. In udev we have 2nd and 
3rd classes merged into one.


Michal

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


Re: [libvirt] [PATCH 08/12] interface_backend_netcf: Implement link speed state

2014-05-30 Thread Laine Stump
On 05/29/2014 11:32 AM, Michal Privoznik wrote:
 While the previous commit was pretty straightforward, things are
 different with netcf as it doesn't exposed the bits we need yet.

Rather than tacking these onto the XML returned from netcf after the
fact, we should instead modify netcf to directly provide the desired
information in the XML returned from ncf_if_xml_state(). I can help with
that.

Also, note that the interface XML only returns items that can be
configured when the VIR_INTERFACE_XML_INACTIVE flag is set, or when the
interface isn't active. If these values are read-only, I think they
should only be present when the interface is active and the INACTIVE
flag isn't set (i.e. they are part of the interface status rather than
config).

Of course netcf has a separate API (ncf_if_status) that checks the
interface IFF_UP and IFF_RUNNING flags, and uses that to determine what
is returned from libvirt's virInterfaceIsActive() API. We should
probably try to at least be sure that what is returned from that is
enough differentiated in documentation from what is returned in this new
state element that they aren't confused with each other (if they are
in fact different in practice).

And that brings up another possible issue (depending on exactly how
link state='up' meshes with IFF_UP|IFF_RUNNING) - within the current
virInterfaceGetXMLDesc() paradigm of interface *status* is only
returned when an interface is 'active' (which was directly copied from
libvirt's behavior in the case of domains and networks), 1) it will
never be possible to learn about the state of the interface when the
interface is inactive (and if the cable is unplugged, it is considered
inactive), and 2) it will then be a bit pointless to report the
active/inactive status of the interface in the XML since it will by
definition always report active (state='up') if the element is present
at all. In order to make it useful, we'll need to modify the semantics
of virInterfaceGetXMLDesc() in some way such that it is always possible
to get the current status even when the interface is down; for example,
possibly we could add a VIR_INTERFACE_XML_STATUS flag that would
indicate to always return the status of the interface, even when it is
inactive.

 However, we can work around it by fetching the info we need from
 SYSFS.

 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/interface/interface_backend_netcf.c | 99 
 +
  1 file changed, 99 insertions(+)

 diff --git a/src/interface/interface_backend_netcf.c 
 b/src/interface/interface_backend_netcf.c
 index 1b9ace5..40181ef 100644
 --- a/src/interface/interface_backend_netcf.c
 +++ b/src/interface/interface_backend_netcf.c
 @@ -33,6 +33,7 @@
  #include virlog.h
  #include virstring.h
  #include viraccessapicheck.h
 +#include virfile.h
  
  #define VIR_FROM_THIS VIR_FROM_INTERFACE
  
 @@ -240,6 +241,96 @@ static struct netcf_if *interfaceDriverGetNetcfIF(struct 
 netcf *ncf, virInterfac
  return iface;
  }
  
 +/* Okay, the following two doesn't really belong here as they dump the info
 + * from SYSFS rather than netcf. But netcf is not yet exposing the info we
 + * need, so what. */
 +#define SYSFS_PREFIX /sys/class/net/
 +
 +static int
 +interfaceGetState(const char *name,
 +  virInterfaceState *state)
 +{
 +int ret = -1;
 +char *path = NULL;
 +char *buf = NULL;
 +char *tmp;
 +
 +if (virAsprintf(path, SYSFS_PREFIX %s/operstate, name)  0)
 +goto cleanup;
 +
 +if (virFileReadAll(path, 1024, buf)  0) {
 +virReportSystemError(errno,
 + _(unable to read: %s),
 + path);
 +goto cleanup;
 +}
 +
 +if (!(tmp = strchr(buf, '\n'))) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Unable to parse: %s),
 +   buf);
 +goto cleanup;
 +}
 +
 +*tmp = '\0';
 +
 +/* We shouldn't allow 0 here, because
 + * virInterfaceState enum starts from 1. */
 +if ((*state = virInterfaceStateTypeFromString(buf)) = 0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Unable to parse: %s),
 +   buf);
 +goto cleanup;
 +}
 +
 +ret = 0;
 + cleanup:
 +VIR_FREE(buf);
 +VIR_FREE(path);
 +return ret;
 +}
 +
 +static int
 +interfaceGetSpeed(const char *name,
 +  unsigned long *speed)
 +{
 +int ret = -1;
 +char *path = NULL;
 +char *buf = NULL;
 +char *tmp;
 +
 +if (virAsprintf(path, SYSFS_PREFIX %s/speed, name)  0)
 +goto cleanup;
 +
 +if (virFileReadAll(path, 1024, buf)  0) {
 +/* Some devices doesn't report speed, in which case we get EINVAL */
 +if (errno == EINVAL) {
 +ret = 0;
 +goto cleanup;
 +}
 +virReportSystemError(errno,
 + _(unable to read: %s),
 + 

Re: [libvirt] [libvirt-users] Host OS, Storage Info

2014-05-30 Thread Martin Kletzander

On Fri, May 30, 2014 at 07:31:28AM -0400, Ainsworth, Thomas wrote:

Martin,

Thanks for the information.  That makes sense.  I *believe* we are good
there.

I noticed something weird yesterday.  After a clone (via the virt-manager
GUI) it seems libvirtd locked up.   A force quit pop up appeared - I had to
kill it.  Then I restarted libvirtd.  Then I did a ps -edf | grep libvirt
and there were three (3) libvirtd --daemon processes.  Then any virsh
commands or virt-manager GUI (when it finally would come up) was very
sluggish.  By the end of the day I had four (4) of the processes running.
Keep in mind, whilst all of this is going on the VM's were just cranking
along fine.  I could not find any dead PID files elated to the processes to
kill...

...we rebooted the server at the end of the day.  It should be fine until
the next time I attempt a clone operation - which I am hesitant to do for
obvious reasons...

Any ideas?



I'd definitely try looking at the debug logs to see what the daemon is
doing, when there are more processes I'd try looking what the others
are doing by attaching with strace/gdb/whatever.

As a way out you can always stop the daemon, (kill all remaining ones
in your case) and start the daemon again.

Martin


Thanks,

Tom




On Fri, May 30, 2014 at 5:12 AM, Martin Kletzander mklet...@redhat.com
wrote:


On Thu, May 29, 2014 at 01:31:13PM -0400, Ainsworth, Thomas wrote:


Martin, et al,

Sorry for the lag in response.

So I started playing with the various virsh commands.  Awesome.
Been doing some reading and I believe I have some things configured not so
well.
As I stated earlier in the thread, we have all of the VM image files on
one
RAID5.  Very fast machine.

When using top, the load average is a stable 5.xx.  No I/O wait. GB's of
free memory.  Swap has not been touched.
Using vmstat, I am writing to the RAID5 volume at a constant 150MB/s and
reading at a constant 275MB/s.

With all of that said, here are some results from virsh commands:

# virsh pool-list --all
Name State  Autostart
--
default  active yes


# virsh pool-info default
Name:   default
UUID:x
State:  running
Persistent: yes
Autostart:  yes
Capacity:   30.76 GiB
Allocation: 2.10 GiB
Available:  28.66 GiB


Now, is that ok to have all of the VM's using a default pool?
Or should a pool be created for each VM instance.
I honestly am not even sure what a pool references...?...



Pool is a set of volumes of the same type (iscsi, LVs, files in a
folder, etc.) in the same place.  Example is the default pool which is,
by default, in /var/lib/libvirt/images and volumes there are files
(pool type is dir).  If you want to have all the domain disks in
that place and all the disks (volumes) should be files then default
pool is enough.


 The more I read, the more I am moving away from thinking something in the

OS is the cause of my sluggishness.



I haven't read your previous mail before, so I've found that now.  How
often are you dropping those caches?  That won't help you not to use
swap.  Having memory occupied by buffers and caches is good if you
read/write from/to disks.  Even when the reads/writes are as fast as
you mentioned, reading/writing from/to RAM is way faster and until
there's some free memory, why not use that?

Martin




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


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

Re: [libvirt] [PATCH 06/12] virInterface: Expose link state speed

2014-05-30 Thread Laine Stump
On 05/30/2014 04:42 PM, Michal Privoznik wrote:
 On 30.05.2014 14:35, Daniel P. Berrange wrote:
 On Fri, May 30, 2014 at 01:41:08PM +0200, Michal Privoznik wrote:
 On 30.05.2014 10:56, Daniel P. Berrange wrote:
 On Thu, May 29, 2014 at 10:32:40AM +0200, Michal Privoznik wrote:
 Currently it is not possible to determine the speed of an interface
 and whether a link is actually detected from the API. Orchestrating
 platforms want to be able to determine when the link has failed and
 where multiple speeds may be available which one the interface is
 actually connected at. This commit introduces an extension to our
 interface XML (without implementation to interface driver backends):

interface type='ethernet' name='eth0'
  start mode='none'/
  mac address='aa:bb:cc:dd:ee:ff'/
  link speed='1000' state='up'/
  mtu size='1492'/
  ...
/interface

 Where @speed is negotiated link speed in Mbits per second, and state
 is the current NIC state (can be one of the following:  unknown,
 notpresent, down, lowerlayerdown,testing, dormant, up).

 This is fine for the the interface objects, but it is limited
 in usefulness for SRIOV use cases. The interface objects only
 exist for interfaces which are configured for the host. With
 SRIOV passthrough some of the interfaces we're interested in
 are not going to be configured - they're just bare devices
 waiting to be given to a guest.

 I hear what you're saying, but unless a PCI device is given
 interface name I
 am afraid we can't do anything. For instance, if you have a NIC but
 detach
 it from the driver (echo ${PCI_ADDR} 
 /sys/bus/pci/drivers/driver/unbind), kernel still sees the PCI device
 (it's shown in lspci output for instance), but it's not configured
 anymore -
 kernel doesn't know device link state, hence it's not aware if NIC's
 link
 speed, etc. So tools like ethtool, ip, ifconfing won't show the device.

 IIUC, We have three classes of device

   1 Devices not bound - no NIC visible in the host OS

   2 Devices bound but not configured. NIC visible in host OS, but no
 /etc/sysconfig/networking/ifcfg-XXX file

   3 Devices bound and configured. NIC visible in host OS, and has a
 /etc/sysconfig/networking/ifcfg-XXX file

 The interface configs only let you deal with NIC devices in class 3.

 The nodedev XML / APIs let you see NIC devices in class 2 + 3.


 Right. The netcf world. Okay, makes sense now. In udev we have 2nd and
 3rd classes merged into one.

Unfortunately yes. The udev backend was added for distros that didn't
support netcf, and doesn't exactly fit the original semantics of the
virInterface API. In particular, virConnect*Interfaces() lists the class
2 devices, which was not intended to be the case. I guess that's our
fault for adding the ability to report device status into the netcf API
at all - if I recall correctly, it wasn't in the initial requirements of
virInterface (which were only to provide a way to *configure* host
interfaces and report on their configuration), but somebody (I forget
who, I could have been one of the guilty parties) requested that it also
provide interface status, and we thought sounds useful, what harm could
it possibly do?. Then once it became an essential part of
virt-manager's guest network config (in order to get a list of host
interfaces the guest could connect to), distros without netcf felt the
pain of missing this feature that was present in other distros, and saw
the udev backend as a way to sidestep the problems of making a netcf
port. It has definitely been useful, but has kind of messed up the
clarity of the virinterface APIs.



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


Re: [libvirt] [PATCH] virsh: Check wether found volume is member of the specified storage pool

2014-05-30 Thread Martin Kletzander

On Fri, May 30, 2014 at 03:39:36PM +0200, Peter Krempa wrote:

When looking up storage volumes virsh uses multiple lookup steps. Some
of the steps don't require a pool name specified. This resulted into a
possibility that a volume would be part of a different pool than the
user specified:

Let's have a /var/lib/libvirt/images/test.qcow image in the 'default'
pool and a second pool 'emptypool':

Currently we'd return:
 $ virsh vol-info --pool emptypool /var/lib/libvirt/images/test.qcow
 Name:   test.qcow
 Type:   file
 Capacity:   100.00 MiB
 Allocation: 212.00 KiB



I believed that the --pool parameter for vol-info (and *some* others)
was only a hint in case you had more volumes with the same name
(specifying absolute path with a pool doesn't make any sense).  That
would mean the BZ is notabug actually, but let's assume such users
exist...


After the fix:
$ tools/virsh vol-info --pool emptypool /var/lib/libvirt/images/test.qcow
error: Requested volume '/var/lib/libvirt/images/test.qcow' found in a 
different pool (default) than specified



I'd say this is rather noisy.  How about changing it to ...'%s' not
found in pool '%s'...  or is not in pool '%s'?

ACK after release with or without the change mentioned above.

Martin


Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1088667
---
tools/virsh-volume.c | 20 
1 file changed, 20 insertions(+)

diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 55bf6f0..6416ba6 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -104,6 +104,26 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
might help), n, portamento);
}

+/* If the pool was specified, then make sure that the returned
+ * volume is from the given pool */
+if (pool  vol) {
+virStoragePoolPtr volpool = NULL;
+
+if ((volpool = virStoragePoolLookupByVolume(vol))) {
+if (STRNEQ(virStoragePoolGetName(volpool),
+   virStoragePoolGetName(pool))) {
+vshResetLibvirtError();
+vshError(ctl,
+ _(Requested volume '%s' found in a different 
+   pool (%s) than specified),
+ n, virStoragePoolGetName(volpool));
+virStorageVolFree(vol);
+vol = NULL;
+}
+virStoragePoolFree(volpool);
+}
+}
+
if (pool)
virStoragePoolFree(pool);

--
1.9.3

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


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

[libvirt] [PATCH] libxl: add support for cache=directsync to to libxl_device_disk

2014-05-30 Thread Olaf Hering
Up to now xend and libxl did not have any way and need to specify a cache=
mode for block devices. At least the libxl driver did just ignore the
cachemode value.

With xen-4.5 a new knob was added to libxl which instructs qemu to use
threads=native cache=directsync in the qdisk driver. This mode was disabled
early due to a pvops kernel bug. As a workaround cache=writeback was set as
default. The kernel bug is long fixed. I assume the forward ported xenlinux
kernel never had this bug.

This change exposes the knob to libvirt. If cache=directsync is set qemu is
instructed to use native AIO.

This change also lays the ground work to set cache=unsafe at some point.
It is currently under investigation whether libxl should expose such knob.

Signed-off-by: Olaf Hering o...@aepfle.de
---
 src/libxl/libxl_conf.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 4cb062e..79f4a82 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -739,6 +739,20 @@ libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
 #endif
 }
 
+static void
+libxlDiskSetCacheMode(libxl_device_disk *x_disk, int cachemode)
+{
+switch (cachemode) {
+#if defined(LIBXL_HAVE_DEVICE_DISK_DIRECT_IO_SAFE)
+case VIR_DOMAIN_DISK_CACHE_DIRECTSYNC:
+x_disk-direct_io_safe = 1;
+break;
+#endif
+default:
+break;
+}
+}
+
 
 int
 libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
@@ -855,6 +869,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk 
*x_disk)
 x_disk-readwrite = !l_disk-readonly;
 x_disk-is_cdrom = l_disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 0;
 libxlDiskSetDiscard(x_disk, l_disk-discard);
+libxlDiskSetCacheMode(x_disk, l_disk-cachemode);
 /* An empty CDROM must have the empty format, otherwise libxl fails. */
 if (x_disk-is_cdrom  !x_disk-pdev_path)
 x_disk-format = LIBXL_DISK_FORMAT_EMPTY;

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


Re: [libvirt] [PATCH 01/12] Introduce virNodeHugeTLB

2014-05-30 Thread Daniel P. Berrange
On Fri, May 30, 2014 at 03:42:01PM +0200, Michal Privoznik wrote:
 On 30.05.2014 14:46, Daniel P. Berrange wrote:
 On Fri, May 30, 2014 at 01:41:06PM +0200, Michal Privoznik wrote:
 On 30.05.2014 10:52, Daniel P. Berrange wrote:
 On Thu, May 29, 2014 at 10:32:35AM +0200, Michal Privoznik wrote:
   /**
 + * virNodeHugeTLB:
 + * @conn: pointer to the hypervisor connection
 + * @type: type
 + * @params: pointer to memory parameter object
 + *  (return value, allocated by the caller)
 + * @nparams: pointer to number of memory parameters; input and output
 + * @flags: extra flags; not used yet, so callers should always pass 0
 + *
 + * Get information about host's huge pages. On input, @nparams
 + * gives the size of the @params array; on output, @nparams gives
 + * how many slots were filled with parameter information, which
 + * might be less but will not exceed the input value.
 + *
 + * As a special case, calling with @params as NULL and @nparams
 + * as 0 on input will cause @nparams on output to contain the
 + * number of parameters supported by the hypervisor. The caller
 + * should then allocate @params array, i.e.
 + * (sizeof(@virTypedParameter) * @nparams) bytes and call the API
 + * again.  See virDomainGetMemoryParameters() for an equivalent
 + * usage example.
 + *
 + * Returns 0 in case of success, and -1 in case of failure.
 + */
 +int
 +virNodeHugeTLB(virConnectPtr conn,
 +   int type,
 +   virTypedParameterPtr params,
 +   int *nparams,
 +   unsigned int flags)
 
 What is the 'type' parameter doing ?
 
 Ah, it should be named numa_node rather than type. If type==-1, then overall
 statistics are returned (number of {available,free} pages accumulated across
 all NUMA nodes), if type = 0, info on the specific NUMA node is returned.
 
 
 I think in general this API needs a different design. I'd like to have
 an API that can request info for all page sizes on all NUMA nods in a
 single call. I also think the static unchanging data should be part of
 the cpu + NUMA info in the capabilities XML. So the API only reports
 info which is changing - ie the available pages.
 
 The only problem is, the size of huge pages pool is not immutable. Now it's
 possible for 2M huge pages to be allocated dynamically:
 
 # echo 8  /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages
 
 and it may be possible for 1GB too in future (what if kernel learns how to
 do it?). In general, the only thing that we can take unalterable for now is
 the default size of huge pages. And I wouldn't bet on that either.
 
 Yes, you can in theory change the number of huge pages at an arbitrary
 time, but realistically people mostly only do it immediately at boot.
 With 1 GB pages is will be impossible todo it any time except immediately
 at boot. If you wait a little while, then memory will be too fragmented
 for you to be able to dynamically allocate more 1 GB pages. The same
 is true of 2MB pages to a lesser degree.
 
 IMO no. Processes never ever see physical address (PA). All they see are
 virtual addresses (VA). So there is possibility for kernel to rearrange
 physical memory without effect on the processes in order to gain bigger
 segments of free memory. 

Applications aren't typically the problem - it is the kernels' own data
structures that often cannot be moved at all, so over time they will
cause free physical RAM regions to be very fragmented. It is a real
problem with huge page usage which will be an order of magnitude worse
for 1GB size pages.

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

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


[libvirt] [PATCH 0/5] Testing libvirt XML - libxl_domain_config conversion

2014-05-30 Thread Daniel P. Berrange
At the Xen Hackathon I learnt that libxl provides an API which
can serialize a libxl_domain_config instance to a JSON document.
This is exactly what we need for testing the XML - libxl_domain_config
conversion process, so I spent the afternoon trying to get such a test
working. The result is that we can now just add pairs of XML, JSON
files to libvirt to test handling of new config features.

I hit a couple of small issues with libxl, which I worked around, when
writing this test which I why I'm copying xen-devel

 - libxl_ctx_alloc() will call xs_daemon_open and xc_interface_open,
   and stat /var/run/xenstored.pid to see if Xen is actually running.
   This fails when run on non-Xen hosts (and also possibly if run
   unprivileged).

   I used an evil LD_PRELOAD hack to stub out xs_daemon_open and
   xc_interface_open to return (void*)0x1, and also turn
   xc_interface_close and xs_daemon_close to no-ops, and make
   stat() always return success for xenstored.pid.

   This works (evidenced by the fact that if something was needing
   these xs/xc handles they would have crashed referencing 0x1), 
   but at the same time it might be an idea to have an officially
   supported  non live mode for libxl_ctx_alloc() turned on by a
   flag of some sort.


 - The libxl_json.h header file is relying on conditionals that
   are only set by Xen's build process

eg HAVE_YAJL_YAJL_VERSION_H

   I hacked around this, but it is a little dirty too. libvirt
   already links to libyajl for the QEMU driver, but we  don't
   really need the raw YAJL objects. It'd be nice to have a

  char * libxl_domain_config_as_json(libxl_domain_config *p)

   as a higher level wrapper around libxl_domain_config_gen_json
   avoiding the pain of dealing with YAJL's different APIs.

   Ian J mentioned to me that he thought there was already such a
   method, but AFAICT, the only such code is in the 'xl' command
   line tool itself (xl_cmdimpl.c - printf_info_one_json)


A few further ideas that could be done as a followup

 - Make virConnectDomainXMLToNative accept 'xl-json as a
   data format, so you can feed in a libvirt XML and get back
   out a JSON document. This could be a useful debugging tool
   for Xen developers trying to identify bugs in libvirt.

 - Write out the JSON document to /var/log/libvirt/libxl/$GUEST.log
   whenever starting a guest. Again this would be a useful debugging
   aid to Xen developers / support people trying to identify why a
   guest might be mis-behaving


Regards,
Daniel

Daniel P. Berrange (5):
  Don't pass virDomainObjPtr to libxlBuildDomainConfig
  Don't pass libxlDriverPrivatePtr into libxlBuildDomainConfig
  libxl: Move virDomainXMLOptionNew into libxlCreateXMLConf
  Add more test suite mock helpers
  Add a test suite for libxl option generator

 src/libxl/libxl_conf.c   |  38 +++---
 src/libxl/libxl_conf.h   |  10 +-
 src/libxl/libxl_domain.c |   7 +-
 src/libxl/libxl_driver.c |   4 +-
 tests/Makefile.am|  25 +++-
 tests/libxlxml2jsondata/minimal.json | 172 +++
 tests/libxlxml2jsondata/minimal.xml  |  36 ++
 tests/libxlxml2jsontest.c| 219 +++
 tests/virfirewalltest.c  |   4 +-
 tests/virmock.h  |  54 ++---
 tests/virmocklibxl.c |  87 ++
 tests/virsystemdtest.c   |   4 +-
 12 files changed, 617 insertions(+), 43 deletions(-)
 create mode 100644 tests/libxlxml2jsondata/minimal.json
 create mode 100644 tests/libxlxml2jsondata/minimal.xml
 create mode 100644 tests/libxlxml2jsontest.c
 create mode 100644 tests/virmocklibxl.c

-- 
1.9.3

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


[libvirt] [PATCH 2/5] Don't pass libxlDriverPrivatePtr into libxlBuildDomainConfig

2014-05-30 Thread Daniel P. Berrange
To make it easier to test, change libxlBuildDomainConfig so
that it takes a virPortAllocatorPtr instead of the larger
libxlDriverPrivatePtr object.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/libxl/libxl_conf.c   | 12 ++--
 src/libxl/libxl_conf.h   |  4 ++--
 src/libxl/libxl_domain.c |  2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index a98e27a..f9e3a1b 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -959,7 +959,7 @@ libxlMakeNicList(virDomainDefPtr def,  libxl_domain_config 
*d_config)
 }
 
 int
-libxlMakeVfb(libxlDriverPrivatePtr driver,
+libxlMakeVfb(virPortAllocatorPtr graphicsports,
  virDomainGraphicsDefPtr l_vfb,
  libxl_device_vfb *x_vfb)
 {
@@ -982,7 +982,7 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
 libxl_defbool_set(x_vfb-vnc.findunused, 0);
 if (l_vfb-data.vnc.autoport) {
 
-if (virPortAllocatorAcquire(driver-reservedVNCPorts, port)  
0)
+if (virPortAllocatorAcquire(graphicsports, port)  0)
 return -1;
 l_vfb-data.vnc.port = port;
 }
@@ -1004,7 +1004,7 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
 }
 
 static int
-libxlMakeVfbList(libxlDriverPrivatePtr driver,
+libxlMakeVfbList(virPortAllocatorPtr graphicsports,
  virDomainDefPtr def,
  libxl_domain_config *d_config)
 {
@@ -1027,7 +1027,7 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver,
 for (i = 0; i  nvfbs; i++) {
 libxl_device_vkb_init(x_vkbs[i]);
 
-if (libxlMakeVfb(driver, l_vfbs[i], x_vfbs[i])  0)
+if (libxlMakeVfb(graphicsports, l_vfbs[i], x_vfbs[i])  0)
 goto error;
 }
 
@@ -1305,7 +1305,7 @@ libxlMakeCapabilities(libxl_ctx *ctx)
 }
 
 int
-libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
+libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
virDomainDefPtr def,
libxl_ctx *ctx,
libxl_domain_config *d_config)
@@ -1324,7 +1324,7 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
 if (libxlMakeNicList(def, d_config)  0)
 return -1;
 
-if (libxlMakeVfbList(driver, def, d_config)  0)
+if (libxlMakeVfbList(graphicsports, def, d_config)  0)
 return -1;
 
 if (libxlMakePCIList(def, d_config)  0)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 1dd9de3..2dcd0b8 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -152,14 +152,14 @@ libxlMakeNic(virDomainDefPtr def,
  virDomainNetDefPtr l_nic,
  libxl_device_nic *x_nic);
 int
-libxlMakeVfb(libxlDriverPrivatePtr driver,
+libxlMakeVfb(virPortAllocatorPtr graphicsports,
  virDomainGraphicsDefPtr l_vfb, libxl_device_vfb *x_vfb);
 
 int
 libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev);
 
 int
-libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
+libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
virDomainDefPtr def,
libxl_ctx *ctx,
libxl_domain_config *d_config);
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index f95f8ce..e00a3fb 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1151,7 +1151,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 
 libxl_domain_config_init(d_config);
 
-if (libxlBuildDomainConfig(driver, vm-def,
+if (libxlBuildDomainConfig(driver-reservedVNCPorts, vm-def,
priv-ctx, d_config)  0)
 goto endjob;
 
-- 
1.9.3

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


[libvirt] [PATCH 1/5] Don't pass virDomainObjPtr to libxlBuildDomainConfig

2014-05-30 Thread Daniel P. Berrange
To make it easier to unit test, change libxlBuildDomainConfig
so that it takes 'virDomainDefPtr' and 'libxl_ctx *' objects
as separate parameters.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/libxl/libxl_conf.c   | 19 +--
 src/libxl/libxl_conf.h   |  4 +++-
 src/libxl/libxl_domain.c |  3 ++-
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b7fed7f..a98e27a 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -567,10 +567,10 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
 }
 
 static int
-libxlMakeDomBuildInfo(virDomainObjPtr vm, libxl_domain_config *d_config)
+libxlMakeDomBuildInfo(virDomainDefPtr def,
+  libxl_ctx *ctx,
+  libxl_domain_config *d_config)
 {
-virDomainDefPtr def = vm-def;
-libxlDomainObjPrivatePtr priv = vm-privateData;
 libxl_domain_build_info *b_info = d_config-b_info;
 int hvm = STREQ(def-os.type, hvm);
 size_t i;
@@ -583,7 +583,7 @@ libxlMakeDomBuildInfo(virDomainObjPtr vm, 
libxl_domain_config *d_config)
 libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV);
 
 b_info-max_vcpus = def-maxvcpus;
-if (libxl_cpu_bitmap_alloc(priv-ctx, b_info-avail_vcpus, def-maxvcpus))
+if (libxl_cpu_bitmap_alloc(ctx, b_info-avail_vcpus, def-maxvcpus))
 goto error;
 libxl_bitmap_set_none(b_info-avail_vcpus);
 for (i = 0; i  def-vcpus; i++)
@@ -1306,17 +1306,16 @@ libxlMakeCapabilities(libxl_ctx *ctx)
 
 int
 libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
-   virDomainObjPtr vm, libxl_domain_config *d_config)
+   virDomainDefPtr def,
+   libxl_ctx *ctx,
+   libxl_domain_config *d_config)
 {
-virDomainDefPtr def = vm-def;
-libxlDomainObjPrivatePtr priv = vm-privateData;
-
 libxl_domain_config_init(d_config);
 
-if (libxlMakeDomCreateInfo(priv-ctx, def, d_config-c_info)  0)
+if (libxlMakeDomCreateInfo(ctx, def, d_config-c_info)  0)
 return -1;
 
-if (libxlMakeDomBuildInfo(vm, d_config)  0)
+if (libxlMakeDomBuildInfo(def, ctx, d_config)  0)
 return -1;
 
 if (libxlMakeDiskList(def, d_config)  0)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 24e1720..1dd9de3 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -160,7 +160,9 @@ libxlMakePCI(virDomainHostdevDefPtr hostdev, 
libxl_device_pci *pcidev);
 
 int
 libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
-   virDomainObjPtr vm, libxl_domain_config *d_config);
+   virDomainDefPtr def,
+   libxl_ctx *ctx,
+   libxl_domain_config *d_config);
 
 static inline void
 libxlDriverLock(libxlDriverPrivatePtr driver)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 80d5280..f95f8ce 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1151,7 +1151,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 
 libxl_domain_config_init(d_config);
 
-if (libxlBuildDomainConfig(driver, vm, d_config)  0)
+if (libxlBuildDomainConfig(driver, vm-def,
+   priv-ctx, d_config)  0)
 goto endjob;
 
 if (cfg-autoballoon  libxlDomainFreeMem(priv, d_config)  0) {
-- 
1.9.3

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


[libvirt] [PATCH 3/5] libxl: Move virDomainXMLOptionNew into libxlCreateXMLConf

2014-05-30 Thread Daniel P. Berrange
To allow the test suite to creat the XML option object,
move the virDomainXMLOptionNew call into a libxlCreateXMLConf
method.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/libxl/libxl_conf.c   | 7 +++
 src/libxl/libxl_conf.h   | 2 ++
 src/libxl/libxl_domain.c | 4 ++--
 src/libxl/libxl_driver.c | 4 +---
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f9e3a1b..967759c 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1336,3 +1336,10 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
 
 return 0;
 }
+
+virDomainXMLOptionPtr libxlCreateXMLConf(void)
+{
+return virDomainXMLOptionNew(libxlDomainDefParserConfig,
+ libxlDomainXMLPrivateDataCallbacks,
+ NULL);
+}
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 2dcd0b8..7a9a7d5 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -158,6 +158,8 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports,
 int
 libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev);
 
+virDomainXMLOptionPtr libxlCreateXMLConf(void);
+
 int
 libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
virDomainDefPtr def,
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index e00a3fb..00ff14f 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1100,6 +1100,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 #endif
 virHostdevManagerPtr hostdev_mgr = driver-hostdevMgr;
 
+libxl_domain_config_init(d_config);
+
 if (libxlDomainObjPrivateInitCtx(vm)  0)
 return ret;
 
@@ -1149,8 +1151,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 VIR_FREE(managed_save_path);
 }
 
-libxl_domain_config_init(d_config);
-
 if (libxlBuildDomainConfig(driver-reservedVNCPorts, vm-def,
priv-ctx, d_config)  0)
 goto endjob;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index df7d510..515d5c9 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -353,9 +353,7 @@ libxlStateInitialize(bool privileged,
 goto error;
 }
 
-if (!(libxl_driver-xmlopt = 
virDomainXMLOptionNew(libxlDomainDefParserConfig,
-   
libxlDomainXMLPrivateDataCallbacks,
-   NULL)))
+if (!(libxl_driver-xmlopt = libxlCreateXMLConf()))
 goto error;
 
 /* Load running domains first. */
-- 
1.9.3

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


[libvirt] [PATCH 4/5] Add more test suite mock helpers

2014-05-30 Thread Daniel P. Berrange
Rename the VIR_MOCK_IMPL* macros to VIR_MOCK_WRAP*
and add new VIR_MOCK_IMPL macros which let you directly
implement overrides in the preloaded source.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 tests/virfirewalltest.c |  4 ++--
 tests/virmock.h | 54 +
 tests/virsystemdtest.c  |  4 ++--
 3 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
index ba2e6ad..81c5557 100644
--- a/tests/virfirewalltest.c
+++ b/tests/virfirewalltest.c
@@ -67,7 +67,7 @@ static bool fwError;
 target prot opt source   destination\n
 
 # if WITH_DBUS
-VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
+VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
DBusMessage *,
DBusConnection *, connection,
DBusMessage *, message,
@@ -82,7 +82,7 @@ 
VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
 char **args = NULL;
 char *type = NULL;
 
-VIR_MOCK_IMPL_INIT_REAL(dbus_connection_send_with_reply_and_block);
+VIR_MOCK_REAL_INIT(dbus_connection_send_with_reply_and_block);
 
 if (STREQ(service, org.freedesktop.DBus) 
 STREQ(member, ListNames)) {
diff --git a/tests/virmock.h b/tests/virmock.h
index 0dd8bb5..8352e30 100644
--- a/tests/virmock.h
+++ b/tests/virmock.h
@@ -234,33 +234,61 @@
  */
 
 # define VIR_MOCK_IMPL_RET_ARGS(name, rettype, ...) \
+rettype name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));   \
+static rettype (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__));  \
+rettype name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
+
+# define VIR_MOCK_IMPL_RET_VOID(name, rettype)  \
+rettype name(void); \
+static rettype (*real_##name)(void);\
+rettype name(void)
+
+# define VIR_MOCK_IMPL_VOID_ARGS(name, ...) \
+void name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));  \
+static void (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__)); \
+void name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
+
+# define VIR_MOCK_IMPL_VOID_VOID(name)  \
+void name(void);\
+static void (*real_##name)(void);   \
+void name(void)
+
+/*
+ * The VIR_MOCK_WRAP_NNN_MMM() macros are intended for use in the
+ * individual test suites. The define a stub implementation of
+ * the wrapped method and insert the caller provided code snippet
+ * as the body of the method.
+ */
+
+# define VIR_MOCK_WRAP_RET_ARGS(name, rettype, ...) \
 rettype wrap_##name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));\
 static rettype (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__));  \
 rettype wrap_##name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
 
-# define VIR_MOCK_IMPL_INIT_REAL(name)  \
-do {\
-if (real_##name == NULL   \
-!(real_##name = dlsym(RTLD_NEXT,\
-  #name))) {\
-fprintf(stderr, Missing symbol ' #name '\n);\
-abort();\
-}   \
-} while (0)
-
-# define VIR_MOCK_IMPL_RET_VOID(name, rettype)  \
+# define VIR_MOCK_WRAP_RET_VOID(name, rettype)  \
 rettype wrap_##name(void);  \
 static rettype (*real_##name)(void);\
 rettype wrap_##name(void)
 
-# define VIR_MOCK_IMPL_VOID_ARGS(name, ...) \
+# define VIR_MOCK_WRAP_VOID_ARGS(name, ...) \
 void wrap_##name(VIR_MOCK_ARGTYPENAMES(__VA_ARGS__));   \
 static void (*real_##name)(VIR_MOCK_ARGTYPES(__VA_ARGS__)); \
 void wrap_##name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__))
 
-# define VIR_MOCK_IMPL_VOID_VOID(name)  \
+# define VIR_MOCK_WRAP_VOID_VOID(name)  \
 void wrap_##name(void); \
 static void (*real_##name)(void);   \
 void wrap_##name(void)
 
+
+# define VIR_MOCK_REAL_INIT(name)   \
+do {\
+if (real_##name == NULL   \
+!(real_##name = dlsym(RTLD_NEXT,   

[libvirt] [PATCH 5/5] Add a test suite for libxl option generator

2014-05-30 Thread Daniel P. Berrange
The libxl library allows a libxl_domain_config object to be
serialized to a JSON string. Use this to allow testing of
the XML - libxl_domain_config conversion process

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 tests/Makefile.am|  25 +++-
 tests/libxlxml2jsondata/minimal.json | 172 +++
 tests/libxlxml2jsondata/minimal.xml  |  36 ++
 tests/libxlxml2jsontest.c| 219 +++
 tests/virmocklibxl.c |  87 ++
 5 files changed, 538 insertions(+), 1 deletion(-)
 create mode 100644 tests/libxlxml2jsondata/minimal.json
 create mode 100644 tests/libxlxml2jsondata/minimal.xml
 create mode 100644 tests/libxlxml2jsontest.c
 create mode 100644 tests/virmocklibxl.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5ef8940..a988342 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -82,6 +82,7 @@ EXTRA_DIST =  \
domainsnapshotxml2xmlout \
fchostdata \
interfaceschemadata \
+   libxlxml2jsondata \
lxcconf2xmldata \
lxcxml2xmldata \
lxcxml2xmloutdata \
@@ -216,6 +217,9 @@ if WITH_XEN
 test_programs += xml2sexprtest sexpr2xmltest \
xmconfigtest xencapstest statstest reconnect
 endif WITH_XEN
+if WITH_LIBXL
+test_programs += libxlxml2jsontest
+endif WITH_LIBXL
 if WITH_QEMU
 test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \
qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \
@@ -378,7 +382,9 @@ test_libraries += libqemumonitortestutils.la \
qemuxml2argvmock.la \
$(NULL)
 endif WITH_QEMU
-
+if WITH_LIBXL
+test_libraries += virmocklibxl.la
+endif WITH_LIBXL
 if WITH_BHYVE
 test_libraries += bhyvexml2argvmock.la
 endif WITH_BHYVE
@@ -577,6 +583,23 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c 
qemuargv2xmltest.c \
$(QEMUMONITORTESTUTILS_SOURCES)
 endif ! WITH_QEMU
 
+if WITH_LIBXL
+libxl_LDADDS = ../src/libvirt_driver_libxl_impl.la
+libxl_LDADDS += $(LDADDS)
+
+libxlxml2jsontest_SOURCES = \
+   libxlxml2jsontest.c testutilsxen.c testutilsxen.h \
+   testutils.c testutils.h
+libxlxml2jsontest_LDADD = $(libxl_LDADDS) $(LIBXML_LIBS)
+
+virmocklibxl_la_SOURCES = \
+   virmocklibxl.c
+virmocklibxl_la_CFLAGS = $(AM_CFLAGS)
+virmocklibxl_la_LDFLAGS = -module -avoid-version \
+-rpath /evil/libtool/hack/to/force/shared/lib/creation
+
+endif WITH_LIBXL
+
 if WITH_LXC
 
 lxc_LDADDS = ../src/libvirt_driver_lxc_impl.la
diff --git a/tests/libxlxml2jsondata/minimal.json 
b/tests/libxlxml2jsondata/minimal.json
new file mode 100644
index 000..930e1d8
--- /dev/null
+++ b/tests/libxlxml2jsondata/minimal.json
@@ -0,0 +1,172 @@
+{
+c_info: {
+type: pv,
+hap: default,
+oos: default,
+ssidref: 0,
+name: rhel5pv,
+uuid: 8f07fe28-753f-2729-d76d-bdbd892f949a,
+xsdata: {
+
+},
+platformdata: {
+
+},
+poolid: 0,
+run_hotplug_scripts: default
+},
+b_info: {
+max_vcpus: 4,
+avail_vcpus: [
+0,
+1,
+2,
+3
+],
+cpumap: [
+
+],
+nodemap: [
+
+],
+numa_placement: default,
+tsc_mode: default,
+max_memkb: 256,
+target_memkb: 307200,
+video_memkb: -1,
+shadow_memkb: -1,
+rtc_timeoffset: 0,
+exec_ssidref: 0,
+localtime: default,
+disable_migrate: default,
+cpuid: [
+
+],
+blkdev_start: null,
+device_model_version: null,
+device_model_stubdomain: default,
+device_model: null,
+device_model_ssidref: 0,
+extra: [
+
+],
+extra_pv: [
+
+],
+extra_hvm: [
+
+],
+sched_params: {
+sched: unknown,
+weight: 1000,
+cap: -1,
+period: -1,
+slice: -1,
+latency: -1,
+extratime: -1
+},
+ioports: [
+
+],
+irqs: [
+
+],
+iomem: [
+
+],
+claim_mode: default,
+u: {
+kernel: null,
+slack_memkb: -1,
+bootloader: /usr/bin/pygrub,
+bootloader_args: [
+
+],
+cmdline: null,
+ramdisk: null,
+e820_host: default
+}
+},
+disks: [
+{
+backend_domid: 0,
+backend_domname: null,
+pdev_path: /var/lib/xen/images/rhel5pv.img,
+vdev: xvda,
+backend: tap,
+format: raw,
+script: null,
+removable: 1,
+readwrite: 1,
+is_cdrom: 0
+},
+{
+backend_domid: 0,
+backend_domname: null,
+pdev_path: /root/qcow1-xen.img,
+vdev: xvdd,
+backend: 

Re: [libvirt] [libvirt-python PATCH] fix leak in memoryStats with older python

2014-05-30 Thread Michal Privoznik

On 27.05.2014 17:28, Martin Kletzander wrote:

libvirt_virDomainMemoryStats() function creates a dictionary without
any checks whether the additions were successful, whether the python
objects were created and, most importantly, without decrementing the
reference count on the objects added to the dictionary.  This is
somehow not an issue with current upstream versions, however with
python 2.6 this exposes a leak in our bindings.  The following patch
works on both old and new CPython versions and is already used in
other parts of the code, so it's also most straightforward.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
  libvirt-override.c | 68 +++---
  1 file changed, 44 insertions(+), 24 deletions(-)


ACK and safe for the freeze.

Michal

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


Re: [libvirt] [PATCH] Don't use AI_ADDRCONFIG when binding to wildcard addresses

2014-05-30 Thread Eric Blake
On 05/30/2014 12:21 AM, Ján Tomko wrote:

 +if (nodename 
 +!(virSocketAddrParse(tmp_addr, nodename, AF_UNSPEC)  0 
 +  virSocketAddrIsWildcard(tmp_addr)))
 +hints.ai_flags = AI_ADDRCONFIG;

 Shouldn't this be |= ?

 
 Functionally it's the same, AI_PASSIVE is ignored if nodename is non-NULL.
 
 But it should be |= if we were using other flags.

Okay.  ACK to the patch, and safe to include in 1.2.5.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] Libvrt Migrate an Offline VM

2014-05-30 Thread Eric Blake
On 05/30/2014 12:23 AM, Sijo Jose wrote:
 Hi,
 
 Is it possible to migrate an Offline VM using libvirt API ?

Yes.  Check out the --offline flag of 'virsh migrate' to see what
happens under the hood.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] docs: bhyve driver documentation improvements

2014-05-30 Thread Eric Blake
On 05/11/2014 01:30 AM, Roman Bogorodskiy wrote:
 - Document 'domxml-to-native' command
 - Mention that the nmdm console support needs an appropriate
   kernel module loaded
 ---
  docs/drvbhyve.html.in | 21 +
  1 file changed, 21 insertions(+)
 

Long lines; you may want to wrap to fit in 80 columns.

 +p
 +The codevirsh domxml-to-native/code command allows to preview the actual 
 codebhyve/code commands

s/allows to/can/

 +that will be executed for a given domain. It outputs two lines, the first 
 line is a codebhyveload/code
 +command and the second is a codebhyve/code command.
 +/p
 +
 +pPlease note that the codevirsh domxml-to-native/code doesn't do any 
 real actions but printing the command,

s/but/other than/

 +for example, it doesn't try to find a proper TAP interface and create it, 
 like it's done when starting a domain, and

s/it's/what is/
s/domain,/domain;/

 +always returns codetap0/code for the network interface. So if you're 
 going to run these commands manually, most likely
 +you might want to tweak them./p
 +
 +pre
 +# virsh -c bhyve:///system  domxml-to-native --format bhyve-argv --xml 
 /path/to/bhyve.xml
 +/usr/sbin/bhyveload -m 214 -d /home/user/vm1.img vm1
 +/usr/sbin/bhyve -c 2 -m 214 -A -I -H -P -s 0:0,hostbridge -s 
 3:0,virtio-net,tap0,mac=52:54:00:5d:74:e3 -s 
 2:0,virtio-blk,/home/user/vm1.img -s 1,lpc -l com1,/dev/nmdm0A vm1
 +/pre
  
/body
  /html
 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] libxl: add discard support to libxl_device_disk

2014-05-30 Thread Jim Fehlig
Olaf Hering wrote:
 Translate libvirt discard settings into libxl-4.5 discard settings.

 Signed-off-by: Olaf Hering o...@aepfle.de
 ---

 default means leave decision to libxl.
 Not sure if that is what default in libvirt terms really means.
   

It generally means hypervisor default, so deferring to libxl is the
right thing to do.


  src/libxl/libxl_conf.c | 28 
  1 file changed, 28 insertions(+)

 diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
 index b7fed7f..4cb062e 100644
 --- a/src/libxl/libxl_conf.c
 +++ b/src/libxl/libxl_conf.c
 @@ -713,6 +713,33 @@ libxlMakeDomBuildInfo(virDomainObjPtr vm, 
 libxl_domain_config *d_config)
  return -1;
  }
  
 +static void
 +libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
 +{
 +if (!x_disk-readwrite)
 +return;
 +#if defined(LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE)
 +switch (discard) {
 +case VIR_DOMAIN_DISK_DISCARD_DEFAULT:
   
case VIR_DOMAIN_DISK_DISCARD_LAST:
 +break;
 +case VIR_DOMAIN_DISK_DISCARD_UNMAP:
 +libxl_defbool_set(x_disk-discard_enable, true);
 +break;
 +default:
   

Then you can remove 'default'.

 +case VIR_DOMAIN_DISK_DISCARD_IGNORE:
 +libxl_defbool_set(x_disk-discard_enable, false);
 +break;
 +}
 +#else
 +if (discard == VIR_DOMAIN_DISK_DISCARD_DEFAULT)
 +return;
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   _(This version of libxenlight does not support 
 + discard= option passing));
   

An error would be reported here, but the overall libxlMakeDisk operation
would succeed right?  Shouldn't it fail if the user requests discard but
it is not supported?

Regards,
Jim

 +#endif
 +}
 +
 +
  int
  libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
  {
 @@ -827,6 +854,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, 
 libxl_device_disk *x_disk)
  x_disk-removable = 1;
  x_disk-readwrite = !l_disk-readonly;
  x_disk-is_cdrom = l_disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 
 0;
 +libxlDiskSetDiscard(x_disk, l_disk-discard);
  /* An empty CDROM must have the empty format, otherwise libxl fails. */
  if (x_disk-is_cdrom  !x_disk-pdev_path)
  x_disk-format = LIBXL_DISK_FORMAT_EMPTY;

   

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


Re: [libvirt] [PATCH] libxl: add discard support to libxl_device_disk

2014-05-30 Thread Eric Blake
On 05/30/2014 03:07 PM, Jim Fehlig wrote:
 Olaf Hering wrote:
 Translate libvirt discard settings into libxl-4.5 discard settings.

 Signed-off-by: Olaf Hering o...@aepfle.de
 ---


 +static void
 +libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
 +{
 +if (!x_disk-readwrite)
 +return;
 +#if defined(LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE)
 +switch (discard) {

This line needs a cast to the enum type, if you want the compiler to
enforce that you covered all enum values.

 +case VIR_DOMAIN_DISK_DISCARD_DEFAULT:
   
 case VIR_DOMAIN_DISK_DISCARD_LAST:
 +break;
 +case VIR_DOMAIN_DISK_DISCARD_UNMAP:
 +libxl_defbool_set(x_disk-discard_enable, true);
 +break;
 +default:
   
 
 Then you can remove 'default'.

Removing 'default' is also essential for the compiler-enforced full enum
coverage trick in a switch statement.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] libxl: add support for cache=directsync to to libxl_device_disk

2014-05-30 Thread Jim Fehlig
Olaf Hering wrote:
 Up to now xend and libxl did not have any way and need to specify a cache=
 mode for block devices. At least the libxl driver did just ignore the
 cachemode value.
   

Even though cache mode was ignored before, should we now error on
unsupported values?

 With xen-4.5 a new knob was added to libxl which instructs qemu to use
 threads=native cache=directsync in the qdisk driver. This mode was disabled
 early due to a pvops kernel bug. As a workaround cache=writeback was set as
 default. The kernel bug is long fixed. I assume the forward ported xenlinux
 kernel never had this bug.

 This change exposes the knob to libvirt. If cache=directsync is set qemu is
 instructed to use native AIO.

 This change also lays the ground work to set cache=unsafe at some point.
 It is currently under investigation whether libxl should expose such knob.

 Signed-off-by: Olaf Hering o...@aepfle.de
 ---
  src/libxl/libxl_conf.c | 15 +++
  1 file changed, 15 insertions(+)

 diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
 index 4cb062e..79f4a82 100644
 --- a/src/libxl/libxl_conf.c
 +++ b/src/libxl/libxl_conf.c
 @@ -739,6 +739,20 @@ libxlDiskSetDiscard(libxl_device_disk *x_disk, int 
 discard)
  #endif
  }
  
 +static void
 +libxlDiskSetCacheMode(libxl_device_disk *x_disk, int cachemode)
 +{
 +switch (cachemode) {
 +#if defined(LIBXL_HAVE_DEVICE_DISK_DIRECT_IO_SAFE)
 +case VIR_DOMAIN_DISK_CACHE_DIRECTSYNC:
 +x_disk-direct_io_safe = 1;
 +break;
 +#endif
 +default:
 +break;
 +}
   

If the answer to the above is yes, this should be changed to something like

switch (cachemode) {
case VIR_DOMAIN_DISK_CACHE_DEFAULT:
break;

case VIR_DOMAIN_DISK_CACHE_DISABLE:
case VIR_DOMAIN_DISK_CACHE_WRITETHRU:
case VIR_DOMAIN_DISK_CACHE_WRITEBACK:
case VIR_DOMAIN_DISK_CACHE_UNSAFE:
case VIR_DOMAIN_DISK_CACHE_LAST:
error...
break;

case VIR_DOMAIN_DISK_CACHE_DIRECTSYNC:
#if defined(LIBXL_HAVE_DEVICE_DISK_DIRECT_IO_SAFE)
x_disk-direct_io_safe = 1;
#else
error...
#endif
break;
}

Regards,
Jim

 +}
 +
  
  int
  libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
 @@ -855,6 +869,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, 
 libxl_device_disk *x_disk)
  x_disk-readwrite = !l_disk-readonly;
  x_disk-is_cdrom = l_disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 
 0;
  libxlDiskSetDiscard(x_disk, l_disk-discard);
 +libxlDiskSetCacheMode(x_disk, l_disk-cachemode);
  /* An empty CDROM must have the empty format, otherwise libxl fails. */
  if (x_disk-is_cdrom  !x_disk-pdev_path)
  x_disk-format = LIBXL_DISK_FORMAT_EMPTY;

   

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


Re: [libvirt] [PATCHv3 01/36] storage: backend: Add unique id retrieval API

2014-05-30 Thread Eric Blake
On 05/30/2014 02:37 AM, Peter Krempa wrote:
 Different protocols have different means to uniquely identify a storage
 file. This patch implements a storage driver API to retrieve a unique
 string describing a volume. The current implementation works for local
 storage only and returns the canonical path of the volume.
 
 To add caching support the local filesystem driver now has a private
 structure holding the cached string, which is created only when it's
 initially accessed.
 
 This patch provides the implementation for local files only for start.
 ---

ACK.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH 1/5] Don't pass virDomainObjPtr to libxlBuildDomainConfig

2014-05-30 Thread Jim Fehlig
Daniel P. Berrange wrote:
 To make it easier to unit test, change libxlBuildDomainConfig
 so that it takes 'virDomainDefPtr' and 'libxl_ctx *' objects
 as separate parameters.

 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/libxl/libxl_conf.c   | 19 +--
  src/libxl/libxl_conf.h   |  4 +++-
  src/libxl/libxl_domain.c |  3 ++-
  3 files changed, 14 insertions(+), 12 deletions(-)
   

ACK.

Regards,
Jim

 diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
 index b7fed7f..a98e27a 100644
 --- a/src/libxl/libxl_conf.c
 +++ b/src/libxl/libxl_conf.c
 @@ -567,10 +567,10 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
  }
  
  static int
 -libxlMakeDomBuildInfo(virDomainObjPtr vm, libxl_domain_config *d_config)
 +libxlMakeDomBuildInfo(virDomainDefPtr def,
 +  libxl_ctx *ctx,
 +  libxl_domain_config *d_config)
  {
 -virDomainDefPtr def = vm-def;
 -libxlDomainObjPrivatePtr priv = vm-privateData;
  libxl_domain_build_info *b_info = d_config-b_info;
  int hvm = STREQ(def-os.type, hvm);
  size_t i;
 @@ -583,7 +583,7 @@ libxlMakeDomBuildInfo(virDomainObjPtr vm, 
 libxl_domain_config *d_config)
  libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV);
  
  b_info-max_vcpus = def-maxvcpus;
 -if (libxl_cpu_bitmap_alloc(priv-ctx, b_info-avail_vcpus, 
 def-maxvcpus))
 +if (libxl_cpu_bitmap_alloc(ctx, b_info-avail_vcpus, def-maxvcpus))
  goto error;
  libxl_bitmap_set_none(b_info-avail_vcpus);
  for (i = 0; i  def-vcpus; i++)
 @@ -1306,17 +1306,16 @@ libxlMakeCapabilities(libxl_ctx *ctx)
  
  int
  libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
 -   virDomainObjPtr vm, libxl_domain_config *d_config)
 +   virDomainDefPtr def,
 +   libxl_ctx *ctx,
 +   libxl_domain_config *d_config)
  {
 -virDomainDefPtr def = vm-def;
 -libxlDomainObjPrivatePtr priv = vm-privateData;
 -
  libxl_domain_config_init(d_config);
  
 -if (libxlMakeDomCreateInfo(priv-ctx, def, d_config-c_info)  0)
 +if (libxlMakeDomCreateInfo(ctx, def, d_config-c_info)  0)
  return -1;
  
 -if (libxlMakeDomBuildInfo(vm, d_config)  0)
 +if (libxlMakeDomBuildInfo(def, ctx, d_config)  0)
  return -1;
  
  if (libxlMakeDiskList(def, d_config)  0)
 diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
 index 24e1720..1dd9de3 100644
 --- a/src/libxl/libxl_conf.h
 +++ b/src/libxl/libxl_conf.h
 @@ -160,7 +160,9 @@ libxlMakePCI(virDomainHostdevDefPtr hostdev, 
 libxl_device_pci *pcidev);
  
  int
  libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
 -   virDomainObjPtr vm, libxl_domain_config *d_config);
 +   virDomainDefPtr def,
 +   libxl_ctx *ctx,
 +   libxl_domain_config *d_config);
  
  static inline void
  libxlDriverLock(libxlDriverPrivatePtr driver)
 diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
 index 80d5280..f95f8ce 100644
 --- a/src/libxl/libxl_domain.c
 +++ b/src/libxl/libxl_domain.c
 @@ -1151,7 +1151,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
 virDomainObjPtr vm,
  
  libxl_domain_config_init(d_config);
  
 -if (libxlBuildDomainConfig(driver, vm, d_config)  0)
 +if (libxlBuildDomainConfig(driver, vm-def,
 +   priv-ctx, d_config)  0)
  goto endjob;
  
  if (cfg-autoballoon  libxlDomainFreeMem(priv, d_config)  0) {
   

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


Re: [libvirt] [PATCH 2/5] Don't pass libxlDriverPrivatePtr into libxlBuildDomainConfig

2014-05-30 Thread Jim Fehlig
Daniel P. Berrange wrote:
 To make it easier to test, change libxlBuildDomainConfig so
 that it takes a virPortAllocatorPtr instead of the larger
 libxlDriverPrivatePtr object.

 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/libxl/libxl_conf.c   | 12 ++--
  src/libxl/libxl_conf.h   |  4 ++--
  src/libxl/libxl_domain.c |  2 +-
  3 files changed, 9 insertions(+), 9 deletions(-)
   

ACK.

Regards,
Jim

 diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
 index a98e27a..f9e3a1b 100644
 --- a/src/libxl/libxl_conf.c
 +++ b/src/libxl/libxl_conf.c
 @@ -959,7 +959,7 @@ libxlMakeNicList(virDomainDefPtr def,  
 libxl_domain_config *d_config)
  }
  
  int
 -libxlMakeVfb(libxlDriverPrivatePtr driver,
 +libxlMakeVfb(virPortAllocatorPtr graphicsports,
   virDomainGraphicsDefPtr l_vfb,
   libxl_device_vfb *x_vfb)
  {
 @@ -982,7 +982,7 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
  libxl_defbool_set(x_vfb-vnc.findunused, 0);
  if (l_vfb-data.vnc.autoport) {
  
 -if (virPortAllocatorAcquire(driver-reservedVNCPorts, port) 
  0)
 +if (virPortAllocatorAcquire(graphicsports, port)  0)
  return -1;
  l_vfb-data.vnc.port = port;
  }
 @@ -1004,7 +1004,7 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
  }
  
  static int
 -libxlMakeVfbList(libxlDriverPrivatePtr driver,
 +libxlMakeVfbList(virPortAllocatorPtr graphicsports,
   virDomainDefPtr def,
   libxl_domain_config *d_config)
  {
 @@ -1027,7 +1027,7 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver,
  for (i = 0; i  nvfbs; i++) {
  libxl_device_vkb_init(x_vkbs[i]);
  
 -if (libxlMakeVfb(driver, l_vfbs[i], x_vfbs[i])  0)
 +if (libxlMakeVfb(graphicsports, l_vfbs[i], x_vfbs[i])  0)
  goto error;
  }
  
 @@ -1305,7 +1305,7 @@ libxlMakeCapabilities(libxl_ctx *ctx)
  }
  
  int
 -libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
 +libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
 virDomainDefPtr def,
 libxl_ctx *ctx,
 libxl_domain_config *d_config)
 @@ -1324,7 +1324,7 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
  if (libxlMakeNicList(def, d_config)  0)
  return -1;
  
 -if (libxlMakeVfbList(driver, def, d_config)  0)
 +if (libxlMakeVfbList(graphicsports, def, d_config)  0)
  return -1;
  
  if (libxlMakePCIList(def, d_config)  0)
 diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
 index 1dd9de3..2dcd0b8 100644
 --- a/src/libxl/libxl_conf.h
 +++ b/src/libxl/libxl_conf.h
 @@ -152,14 +152,14 @@ libxlMakeNic(virDomainDefPtr def,
   virDomainNetDefPtr l_nic,
   libxl_device_nic *x_nic);
  int
 -libxlMakeVfb(libxlDriverPrivatePtr driver,
 +libxlMakeVfb(virPortAllocatorPtr graphicsports,
   virDomainGraphicsDefPtr l_vfb, libxl_device_vfb *x_vfb);
  
  int
  libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev);
  
  int
 -libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
 +libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
 virDomainDefPtr def,
 libxl_ctx *ctx,
 libxl_domain_config *d_config);
 diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
 index f95f8ce..e00a3fb 100644
 --- a/src/libxl/libxl_domain.c
 +++ b/src/libxl/libxl_domain.c
 @@ -1151,7 +1151,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
 virDomainObjPtr vm,
  
  libxl_domain_config_init(d_config);
  
 -if (libxlBuildDomainConfig(driver, vm-def,
 +if (libxlBuildDomainConfig(driver-reservedVNCPorts, vm-def,
 priv-ctx, d_config)  0)
  goto endjob;
  
   

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


[libvirt] [PATCH] build: avoid compiler warning on 32-bit platform

2014-05-30 Thread Eric Blake
On a 32-bit platform:

virstringtest.c: In function 'mymain':
virstringtest.c:673: warning: this decimal constant is unsigned only in ISO C90

I already had a comment in the file about the 64-bit counterpart;
the easiest fix was to make both sites use the standardized macro
that is guaranteed to work.

* tests/virstringtest.c (mymain): Minimum signed integers are a pain.

Signed-off-by: Eric Blake ebl...@redhat.com
---

Pushing under the build-breaker rule.

 tests/virstringtest.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/virstringtest.c b/tests/virstringtest.c
index 1e330f9..96c3784 100644
--- a/tests/virstringtest.c
+++ b/tests/virstringtest.c
@@ -670,7 +670,7 @@ mymain(void)
 -1LL, 0, 18446744073709551615ULL, 0);
 TEST_STRTOL(-2147483647, NULL, -2147483647, 0, 2147483649U, 0,
 -2147483647LL, 0, 18446744071562067969ULL, 0);
-TEST_STRTOL(-2147483648, NULL, -2147483648, 0, 2147483648U, 0,
+TEST_STRTOL(-2147483648, NULL, INT32_MIN, 0, 2147483648U, 0,
 -2147483648LL, 0, 18446744071562067968ULL, 0);
 TEST_STRTOL(-2147483649, NULL, 0, -1, 2147483647U, 0,
 -2147483649LL, 0, 18446744071562067967ULL, 0);
@@ -680,10 +680,8 @@ mymain(void)
 -4294967296LL, 0, 18446744069414584320ULL, 0);
 TEST_STRTOL(-9223372036854775807, NULL, 0, -1, 0U, -1,
 -9223372036854775807LL, 0, 9223372036854775809ULL, 0);
-/* Bah, stupid gcc warning about -9223372036854775808LL being an
- * unrepresentable integer constant */
 TEST_STRTOL(-9223372036854775808, NULL, 0, -1, 0U, -1,
-0x8000LL, 0, 9223372036854775808ULL, 0);
+INT64_MIN, 0, 9223372036854775808ULL, 0);
 TEST_STRTOL(-9223372036854775809, NULL, 0, -1, 0U, -1,
 0LL, -1, 9223372036854775807ULL, 0);
 TEST_STRTOL(-18446744073709551615, NULL, 0, -1, 0U, -1,
-- 
1.9.3

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


Re: [libvirt] [PATCH 3/5] libxl: Move virDomainXMLOptionNew into libxlCreateXMLConf

2014-05-30 Thread Jim Fehlig
Daniel P. Berrange wrote:
 To allow the test suite to creat the XML option object,
 move the virDomainXMLOptionNew call into a libxlCreateXMLConf
 method.

 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/libxl/libxl_conf.c   | 7 +++
  src/libxl/libxl_conf.h   | 2 ++
  src/libxl/libxl_domain.c | 4 ++--
  src/libxl/libxl_driver.c | 4 +---
  4 files changed, 12 insertions(+), 5 deletions(-)

 diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
 index f9e3a1b..967759c 100644
 --- a/src/libxl/libxl_conf.c
 +++ b/src/libxl/libxl_conf.c
 @@ -1336,3 +1336,10 @@ libxlBuildDomainConfig(virPortAllocatorPtr 
 graphicsports,
  
  return 0;
  }
 +
 +virDomainXMLOptionPtr libxlCreateXMLConf(void)
   

Return type and function name on separate lines.

 +{
 +return virDomainXMLOptionNew(libxlDomainDefParserConfig,
 + libxlDomainXMLPrivateDataCallbacks,
 + NULL);
 +}
 diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
 index 2dcd0b8..7a9a7d5 100644
 --- a/src/libxl/libxl_conf.h
 +++ b/src/libxl/libxl_conf.h
 @@ -158,6 +158,8 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports,
  int
  libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev);
  
 +virDomainXMLOptionPtr libxlCreateXMLConf(void);
   

Same here.

 +
  int
  libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
 virDomainDefPtr def,
 diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
 index e00a3fb..00ff14f 100644
 --- a/src/libxl/libxl_domain.c
 +++ b/src/libxl/libxl_domain.c
 @@ -1100,6 +1100,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
 virDomainObjPtr vm,
  #endif
  virHostdevManagerPtr hostdev_mgr = driver-hostdevMgr;
  
 +libxl_domain_config_init(d_config);
 +
  if (libxlDomainObjPrivateInitCtx(vm)  0)
  return ret;
  
 @@ -1149,8 +1151,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
 virDomainObjPtr vm,
  VIR_FREE(managed_save_path);
  }
  
 -libxl_domain_config_init(d_config);
 -
  if (libxlBuildDomainConfig(driver-reservedVNCPorts, vm-def,
 priv-ctx, d_config)  0)
  goto endjob;
   

Are these two hunks fixing a bug you found? :-)

Regards,
Jim

 diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
 index df7d510..515d5c9 100644
 --- a/src/libxl/libxl_driver.c
 +++ b/src/libxl/libxl_driver.c
 @@ -353,9 +353,7 @@ libxlStateInitialize(bool privileged,
  goto error;
  }
  
 -if (!(libxl_driver-xmlopt = 
 virDomainXMLOptionNew(libxlDomainDefParserConfig,
 -   
 libxlDomainXMLPrivateDataCallbacks,
 -   NULL)))
 +if (!(libxl_driver-xmlopt = libxlCreateXMLConf()))
  goto error;
  
  /* Load running domains first. */
   

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


Re: [libvirt] [PATCHv3 10/36] storage: Add infrastructure to parse remote network backing names

2014-05-30 Thread Eric Blake
On 05/30/2014 02:37 AM, Peter Krempa wrote:
 Add parsers for relative and absolute backing names for local and remote
 storage files.
 
 This parser parses relative paths as relative to their parents and
 absolute paths according to the protocol or local access.
 
 For remote storage volumes, all URI based backing file names are
 supported and for the qemu colon syntax the NBD protocol is supported.
 ---
  src/libvirt_private.syms  |   1 +
  src/util/virstoragefile.c | 364 
 ++
  src/util/virstoragefile.h |   4 +
  3 files changed, 369 insertions(+)
 

Big patch, but no impact until later patches use it.  Looks reasonable,
so ACK (again, this series is post-release).  Lots of FIXMEs, so I
assume later patches in the series clean that up.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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