[libvirt] [PATCH 1/6] Move file format enum to libvirt_util
Rename virStorageVolFormatFileSystem to virStorageFileFormat and
move to src/util/storage_file.[ch]
* src/Makefile.am: add src/util/storage_file.[ch]
* src/conf/storage_conf.[ch]: move enum from here ...
* src/util/storage_file.[ch]: .. to here
* src/libvirt_private.syms: update To/FromString exports
* src/storage/storage_backend.c, src/storage/storage_backend_fs.c,
src/vbox/vbox_tmpl.c: update for above changes
---
src/Makefile.am |1 +
src/conf/storage_conf.c | 25
src/conf/storage_conf.h | 17 --
src/libvirt_private.syms |5 ++-
src/storage/storage_backend.c| 17 +++--
src/storage/storage_backend_fs.c | 33 ++-
src/util/storage_file.c | 31 +
src/util/storage_file.h | 46 ++
src/vbox/vbox_tmpl.c | 15 ++-
9 files changed, 125 insertions(+), 65 deletions(-)
create mode 100644 src/util/storage_file.c
create mode 100644 src/util/storage_file.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 9cbec47..47bc39b 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -49,6 +49,7 @@ UTIL_SOURCES =
\
util/hostusb.c util/hostusb.h \
util/qparams.c util/qparams.h \
util/stats_linux.c util/stats_linux.h \
+ util/storage_file.c util/storage_file.h \
util/threads.c util/threads.h \
util/threads-pthread.h \
util/threads-win32.h\
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index cb063cc..788de15 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -36,6 +36,7 @@
#include virterror_internal.h
#include datatypes.h
#include storage_conf.h
+#include storage_file.h
#include xml.h
#include uuid.h
@@ -82,12 +83,6 @@ VIR_ENUM_IMPL(virStorageVolFormatDisk,
linux-lvm, linux-raid,
extended)
-VIR_ENUM_IMPL(virStorageVolFormatFileSystem,
- VIR_STORAGE_VOL_FILE_LAST,
- raw, dir, bochs,
- cloop, cow, dmg, iso,
- qcow, qcow2, vmdk, vpc)
-
VIR_ENUM_IMPL(virStoragePartedFsType,
VIR_STORAGE_PARTED_FS_TYPE_LAST,
ext2, ext2, fat16,
@@ -150,9 +145,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
},
{ .poolType = VIR_STORAGE_POOL_DIR,
.volOptions = {
-.defaultFormat = VIR_STORAGE_VOL_FILE_RAW,
-.formatFromString = virStorageVolFormatFileSystemTypeFromString,
-.formatToString = virStorageVolFormatFileSystemTypeToString,
+.defaultFormat = VIR_STORAGE_FILE_RAW,
+.formatFromString = virStorageFileFormatTypeFromString,
+.formatToString = virStorageFileFormatTypeToString,
},
},
{ .poolType = VIR_STORAGE_POOL_FS,
@@ -162,9 +157,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
.formatToString = virStoragePoolFormatFileSystemTypeToString,
},
.volOptions = {
-.defaultFormat = VIR_STORAGE_VOL_FILE_RAW,
-.formatFromString = virStorageVolFormatFileSystemTypeFromString,
-.formatToString = virStorageVolFormatFileSystemTypeToString,
+.defaultFormat = VIR_STORAGE_FILE_RAW,
+.formatFromString = virStorageFileFormatTypeFromString,
+.formatToString = virStorageFileFormatTypeToString,
},
},
{ .poolType = VIR_STORAGE_POOL_NETFS,
@@ -176,9 +171,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
.formatToString = virStoragePoolFormatFileSystemNetTypeToString,
},
.volOptions = {
-.defaultFormat = VIR_STORAGE_VOL_FILE_RAW,
-.formatFromString = virStorageVolFormatFileSystemTypeFromString,
-.formatToString = virStorageVolFormatFileSystemTypeToString,
+.defaultFormat = VIR_STORAGE_FILE_RAW,
+.formatFromString = virStorageFileFormatTypeFromString,
+.formatToString = virStorageFileFormatTypeToString,
},
},
{ .poolType = VIR_STORAGE_POOL_ISCSI,
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 421d305..9fedb12 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -429,23 +429,6 @@ enum virStoragePoolFormatLogical {
};
VIR_ENUM_DECL(virStoragePoolFormatLogical)
-
-enum virStorageVolFormatFileSystem {
-VIR_STORAGE_VOL_FILE_RAW = 0,
-VIR_STORAGE_VOL_FILE_DIR,
-VIR_STORAGE_VOL_FILE_BOCHS,
-VIR_STORAGE_VOL_FILE_CLOOP,
-VIR_STORAGE_VOL_FILE_COW,
-VIR_STORAGE_VOL_FILE_DMG,
-VIR_STORAGE_VOL_FILE_ISO,
-VIR_STORAGE_VOL_FILE_QCOW,
-VIR_STORAGE_VOL_FILE_QCOW2,
-
[libvirt] [PATCH 5/6] Add virStorageFileGetMetadata() helper
* src/util/storage_file.c: add virStorageFileGetMetadata() so that
the caller does not need to open the file
---
src/libvirt_private.syms |1 +
src/util/storage_file.c | 20
src/util/storage_file.h |3 +++
3 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4890032..635c6b6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -393,6 +393,7 @@ virStorageGenerateQcowPassphrase;
# storage_file.h
virStorageFileFormatTypeToString;
virStorageFileFormatTypeFromString;
+virStorageFileGetMetadata;
virStorageFileGetMetadataFromFD;
# threads.h
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index e674713..44057d2 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -25,6 +25,7 @@
#include storage_file.h
#include unistd.h
+#include fcntl.h
#include memory.h
#include virterror_internal.h
@@ -402,3 +403,22 @@ virStorageFileGetMetadataFromFD(virConnectPtr conn,
return 0;
}
+
+int
+virStorageFileGetMetadata(virConnectPtr conn,
+ const char *path,
+ virStorageFileMetadata *meta)
+{
+int fd, ret;
+
+if ((fd = open(path, O_RDONLY)) 0) {
+virReportSystemError(conn, errno, _(cannot open file '%s'), path);
+return -1;
+}
+
+ret = virStorageFileGetMetadataFromFD(conn, path, fd, meta);
+
+close(fd);
+
+return ret;
+}
diff --git a/src/util/storage_file.h b/src/util/storage_file.h
index e34d749..b0abcaf 100644
--- a/src/util/storage_file.h
+++ b/src/util/storage_file.h
@@ -51,6 +51,9 @@ typedef struct _virStorageFileMetadata {
bool encrypted;
} virStorageFileMetadata;
+int virStorageFileGetMetadata(virConnectPtr conn,
+ const char *path,
+ virStorageFileMetadata *meta);
int virStorageFileGetMetadataFromFD(virConnectPtr conn,
const char *path,
int fd,
--
1.6.2.5
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] Move virStorageGetMetadataFromFD() to libvirt_util
Finally, we get to the point of all this.
Move virStorageGetMetadataFromFD() to virStorageFileGetMetadataFromFD()
and move to src/util/storage_file.[ch]
There's no functional changes in this patch, just code movement
* src/storage/storage_backend_fs.c: move code from here ...
* src/util/storage_file.[ch]: ... to here
* src/libvirt_private.syms: export virStorageFileGetMetadataFromFD()
---
src/libvirt_private.syms |1 +
src/storage/storage_backend_fs.c | 368 +-
src/util/storage_file.c | 373 ++
src/util/storage_file.h |5 +
4 files changed, 380 insertions(+), 367 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ddab21d..4890032 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -393,6 +393,7 @@ virStorageGenerateQcowPassphrase;
# storage_file.h
virStorageFileFormatTypeToString;
virStorageFileFormatTypeFromString;
+virStorageFileGetMetadataFromFD;
# threads.h
virMutexInit;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 7db73bf..6816da8 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -46,375 +46,9 @@
#include memory.h
#include xml.h
-enum lv_endian {
-LV_LITTLE_ENDIAN = 1, /* 1234 */
-LV_BIG_ENDIAN /* 4321 */
-};
-
-enum {
-BACKING_STORE_OK,
-BACKING_STORE_INVALID,
-BACKING_STORE_ERROR,
-};
-
-static int cowGetBackingStore(virConnectPtr, char **,
- const unsigned char *, size_t);
-static int qcowXGetBackingStore(virConnectPtr, char **,
-const unsigned char *, size_t);
-static int vmdk4GetBackingStore(virConnectPtr, char **,
-const unsigned char *, size_t);
-
-/* Either 'magic' or 'extension' *must* be provided */
-struct FileTypeInfo {
-int type; /* One of the constants above */
-const char *magic; /* Optional string of file magic
- * to check at head of file */
-const char *extension; /* Optional file extension to check */
-enum lv_endian endian; /* Endianness of file format */
-int versionOffset;/* Byte offset from start of file
- * where we find version number,
- * -1 to skip version test */
-int versionNumber;/* Version number to validate */
-int sizeOffset; /* Byte offset from start of file
- * where we find capacity info,
- * -1 to use st_size as capacity */
-int sizeBytes;/* Number of bytes for size field */
-int sizeMultiplier; /* A scaling factor if size is not in bytes */
- /* Store a COW base image path (possibly relative),
- * or NULL if there is no COW base image, to RES;
- * return BACKING_STORE_* */
-int qcowCryptOffset; /* Byte offset from start of file
- * where to find encryption mode,
- * -1 if encryption is not used */
-int (*getBackingStore)(virConnectPtr conn, char **res,
- const unsigned char *buf, size_t buf_size);
-};
-struct FileTypeInfo const fileTypeInfo[] = {
-/* Bochs */
-/* XXX Untested
-{ VIR_STORAGE_FILE_BOCHS, Bochs Virtual HD Image, NULL,
- LV_LITTLE_ENDIAN, 64, 0x2,
- 32+16+16+4+4+4+4+4, 8, 1, -1, NULL },*/
-/* CLoop */
-/* XXX Untested
-{ VIR_STORAGE_VOL_CLOOP, #!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0
mount -r -t iso9660 /dev/cloop $1\n, NULL,
- LV_LITTLE_ENDIAN, -1, 0,
- -1, 0, 0, -1, NULL }, */
-/* Cow */
-{ VIR_STORAGE_FILE_COW, OOOM, NULL,
- LV_BIG_ENDIAN, 4, 2,
- 4+4+1024+4, 8, 1, -1, cowGetBackingStore },
-/* DMG */
-/* XXX QEMU says there's no magic for dmg, but we should check... */
-{ VIR_STORAGE_FILE_DMG, NULL, .dmg,
- 0, -1, 0,
- -1, 0, 0, -1, NULL },
-/* XXX there's probably some magic for iso we can validate too... */
-{ VIR_STORAGE_FILE_ISO, NULL, .iso,
- 0, -1, 0,
- -1, 0, 0, -1, NULL },
-/* Parallels */
-/* XXX Untested
-{ VIR_STORAGE_FILE_PARALLELS, WithoutFreeSpace, NULL,
- LV_LITTLE_ENDIAN, 16, 2,
- 16+4+4+4+4, 4, 512, -1, NULL },
-*/
-/* QCow */
-{ VIR_STORAGE_FILE_QCOW, QFI, NULL,
- LV_BIG_ENDIAN, 4, 1,
- 4+4+8+4+4, 8, 1, 4+4+8+4+4+8+1+1+2, qcowXGetBackingStore },
-/* QCow 2 */
-{ VIR_STORAGE_FILE_QCOW2, QFI, NULL,
- LV_BIG_ENDIAN, 4, 2,
- 4+4+8+4+4, 8, 1, 4+4+8+4+4+8, qcowXGetBackingStore },
-/* VMDK 3 */
-/* XXX Untested
-{ VIR_STORAGE_FILE_VMDK, COWD, NULL,
- LV_LITTLE_ENDIAN, 4, 1,
- 4+4+4, 4, 512, -1, NULL },
-*/
-/* VMDK 4 */
-{ VIR_STORAGE_FILE_VMDK, KDMV, NULL,
- LV_LITTLE_ENDIAN, 4, 1,
-
[libvirt] [PATCH 6/6] Re-label image file backing stores
Use virStorageFileGetMetadata() to find any backing stores for images
and re-label them
Without this, qemu cannot access qcow2 backing files, see:
https://bugzilla.redhat.com/497131
* src/security/security_selinux.c: re-label backing store files in
SELinuxSetSecurityImageLabel()
---
src/security/security_selinux.c | 28
1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index b84de8f..670fcb2 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -27,6 +27,7 @@
#include logging.h
#include pci.h
#include hostusb.h
+#include storage_file.h
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -403,10 +404,37 @@ SELinuxSetSecurityImageLabel(virConnectPtr conn,
{
const virSecurityLabelDefPtr secdef = vm-def-seclabel;
+const char *path;
if (!disk-src)
return 0;
+path = disk-src;
+do {
+virStorageFileMetadata meta;
+int ret;
+
+memset(meta, 0, sizeof(meta));
+
+ret = virStorageFileGetMetadata(conn, path, meta);
+
+if (path != disk-src)
+VIR_FREE(path);
+path = NULL;
+
+if (ret 0)
+return -1;
+
+if (meta.backingStore != NULL
+SELinuxSetFilecon(conn, meta.backingStore,
+ default_content_context) 0) {
+VIR_FREE(meta.backingStore);
+return -1;
+}
+
+path = meta.backingStore;
+} while (path != NULL);
+
if (disk-shared) {
return SELinuxSetFilecon(conn, disk-src, default_image_context);
} else if (disk-readonly) {
--
1.6.2.5
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] Split virStorageGetMetadataFromFD() from virStorageBackendProbeTarget()
Prepare the code probing a file's format and associated metadata for
moving into libvirt_util.
* src/storage/storage_backend_fs.c: re-factor the format and metadata
probing code in preparation for moving it
---
src/storage/storage_backend_fs.c | 148 +++---
1 files changed, 91 insertions(+), 57 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index e7a3ca9..87c30cd 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -283,49 +283,37 @@ static char *absolutePathFromBaseFile(const char
*base_file, const char *path)
* Probe the header of a file to determine what type of disk image
* it is, and info about its capacity if available.
*/
-static int virStorageBackendProbeTarget(virConnectPtr conn,
-virStorageVolTargetPtr target,
-char **backingStore,
-unsigned long long *allocation,
-unsigned long long *capacity,
-virStorageEncryptionPtr *encryption) {
-int fd;
+static int
+virStorageGetMetadataFromFD(virConnectPtr conn,
+const char *path,
+int fd,
+int *format,
+bool *encrypted,
+char **backingStore,
+unsigned long long *capacity)
+{
unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */
-int len, i, ret;
+int len, i;
+if (format) /* If all else fails, call it a raw file */
+*format = VIR_STORAGE_FILE_RAW;
+if (encrypted)
+*encrypted = false;
if (backingStore)
*backingStore = NULL;
-if (encryption)
-*encryption = NULL;
-
-if ((fd = open(target-path, O_RDONLY)) 0) {
-virReportSystemError(conn, errno,
- _(cannot open volume '%s'),
- target-path);
-return -1;
-}
-
-if ((ret = virStorageBackendUpdateVolTargetInfoFD(conn, target, fd,
- allocation,
- capacity)) 0) {
-close(fd);
-return ret; /* Take care to propagate ret, it is not always -1 */
-}
+/* Do not overwrite capacity
+ * if (capacity)
+ * *capacity = 0;
+ */
if ((len = read(fd, head, sizeof(head))) 0) {
-virReportSystemError(conn, errno,
- _(cannot read header '%s'),
- target-path);
-close(fd);
+virReportSystemError(conn, errno, _(cannot read header '%s'), path);
return -1;
}
-close(fd);
-
/* First check file magic */
for (i = 0 ; i ARRAY_CARDINALITY(fileTypeInfo) ; i++) {
int mlen;
-bool encrypted_qcow = false;
if (fileTypeInfo[i].magic == NULL)
continue;
@@ -385,18 +373,19 @@ static int virStorageBackendProbeTarget(virConnectPtr
conn,
*capacity *= fileTypeInfo[i].sizeMultiplier;
}
-if (fileTypeInfo[i].qcowCryptOffset != -1) {
+if (fileTypeInfo[i].qcowCryptOffset != -1 encrypted) {
int crypt_format;
crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] 24) |
(head[fileTypeInfo[i].qcowCryptOffset+1] 16) |
(head[fileTypeInfo[i].qcowCryptOffset+2] 8) |
head[fileTypeInfo[i].qcowCryptOffset+3];
-encrypted_qcow = crypt_format != 0;
+*encrypted = crypt_format != 0;
}
/* Validation passed, we know the file format now */
-target-format = fileTypeInfo[i].type;
+if (format)
+*format = fileTypeInfo[i].type;
if (fileTypeInfo[i].getBackingStore != NULL backingStore) {
char *base;
@@ -411,8 +400,7 @@ static int virStorageBackendProbeTarget(virConnectPtr conn,
return -1;
}
if (base != NULL) {
-*backingStore
-= absolutePathFromBaseFile(target-path, base);
+*backingStore = absolutePathFromBaseFile(path, base);
VIR_FREE(base);
if (*backingStore == NULL) {
virReportOOMError(conn);
@@ -420,23 +408,6 @@ static int virStorageBackendProbeTarget(virConnectPtr conn,
}
}
}
-if (encryption != NULL encrypted_qcow) {
-virStorageEncryptionPtr enc;
-
-if (VIR_ALLOC(enc) 0) {
-virReportOOMError(conn);
-if (backingStore)
-VIR_FREE(*backingStore);
-return -1;
-}
-enc-format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW;
-
[libvirt] [PATCH 3/6] Introduce virStorageFileMetadata structure
Introduce a metadata structure and make virStorageGetMetadataFromFD()
fill it in.
* src/util/storage_file.h: add virStorageFileMetadata
* src/backend/storage_backend_fs.c: virStorageGetMetadataFromFD() now
fills in the virStorageFileMetadata structure
---
src/storage/storage_backend_fs.c | 65 ++---
src/util/storage_file.h |8 +
2 files changed, 40 insertions(+), 33 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 87c30cd..7db73bf 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -287,24 +287,13 @@ static int
virStorageGetMetadataFromFD(virConnectPtr conn,
const char *path,
int fd,
-int *format,
-bool *encrypted,
-char **backingStore,
-unsigned long long *capacity)
+virStorageFileMetadata *meta)
{
unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */
int len, i;
-if (format) /* If all else fails, call it a raw file */
-*format = VIR_STORAGE_FILE_RAW;
-if (encrypted)
-*encrypted = false;
-if (backingStore)
-*backingStore = NULL;
-/* Do not overwrite capacity
- * if (capacity)
- * *capacity = 0;
- */
+/* If all else fails, call it a raw file */
+meta-format = VIR_STORAGE_FILE_RAW;
if ((len = read(fd, head, sizeof(head))) 0) {
virReportSystemError(conn, errno, _(cannot read header '%s'), path);
@@ -345,9 +334,9 @@ virStorageGetMetadataFromFD(virConnectPtr conn,
}
/* Optionally extract capacity from file */
-if (fileTypeInfo[i].sizeOffset != -1 capacity) {
+if (fileTypeInfo[i].sizeOffset != -1) {
if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) {
-*capacity =
+meta-capacity =
((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]
56) |
((unsigned long long)head[fileTypeInfo[i].sizeOffset+6]
48) |
((unsigned long long)head[fileTypeInfo[i].sizeOffset+5]
40) |
@@ -357,7 +346,7 @@ virStorageGetMetadataFromFD(virConnectPtr conn,
((unsigned long long)head[fileTypeInfo[i].sizeOffset+1]
8) |
((unsigned long long)head[fileTypeInfo[i].sizeOffset]);
} else {
-*capacity =
+meta-capacity =
((unsigned long long)head[fileTypeInfo[i].sizeOffset]
56) |
((unsigned long long)head[fileTypeInfo[i].sizeOffset+1]
48) |
((unsigned long long)head[fileTypeInfo[i].sizeOffset+2]
40) |
@@ -368,25 +357,24 @@ virStorageGetMetadataFromFD(virConnectPtr conn,
((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]);
}
/* Avoid unlikely, but theoretically possible overflow */
-if (*capacity (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier))
+if (meta-capacity (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier))
continue;
-*capacity *= fileTypeInfo[i].sizeMultiplier;
+meta-capacity *= fileTypeInfo[i].sizeMultiplier;
}
-if (fileTypeInfo[i].qcowCryptOffset != -1 encrypted) {
+if (fileTypeInfo[i].qcowCryptOffset != -1) {
int crypt_format;
crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] 24) |
(head[fileTypeInfo[i].qcowCryptOffset+1] 16) |
(head[fileTypeInfo[i].qcowCryptOffset+2] 8) |
head[fileTypeInfo[i].qcowCryptOffset+3];
-*encrypted = crypt_format != 0;
+meta-encrypted = crypt_format != 0;
}
/* Validation passed, we know the file format now */
-if (format)
-*format = fileTypeInfo[i].type;
-if (fileTypeInfo[i].getBackingStore != NULL backingStore) {
+meta-format = fileTypeInfo[i].type;
+if (fileTypeInfo[i].getBackingStore != NULL) {
char *base;
switch (fileTypeInfo[i].getBackingStore(conn, base, head, len)) {
@@ -400,9 +388,9 @@ virStorageGetMetadataFromFD(virConnectPtr conn,
return -1;
}
if (base != NULL) {
-*backingStore = absolutePathFromBaseFile(path, base);
+meta-backingStore = absolutePathFromBaseFile(path, base);
VIR_FREE(base);
-if (*backingStore == NULL) {
+if (meta-backingStore == NULL) {
virReportOOMError(conn);
return -1;
}
@@ -419,8 +407,7 @@ virStorageGetMetadataFromFD(virConnectPtr conn,
if (!virFileHasSuffix(path, fileTypeInfo[i].extension))
Re: [libvirt] [PATCH] fix some XPath relative node resets
On Mon, Sep 28, 2009 at 06:42:14PM -0400, Laine Stump wrote: On 09/28/2009 06:39 PM, Laine Stump wrote: On 09/28/2009 11:30 AM, Daniel Veillard wrote: The virXPath... function take extra care to preserve the XPath context node (ctxt-node) but in the case of virXPathString and virXPathBoolean they forgot to do this on the error path. This patch fixes this and move all ctxt-node = relnode instuctions just after the xmlXPathEval() to make sure this doesn't happen if this code is modified. ACK to this. The failure I encountered was due to a similar problem somewhere else (patch upcoming), but these are definitely correcting a problem. I take that back - one of my two problems *was* fixed by this patch ;-) Okay, commited, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Silently ignored virDomainRestore failures
On Mon, Sep 28, 2009 at 09:03:37PM -0500, Charles Duffy wrote: Charles Duffy wrote: What I'm tempted to do is add a command which sends a sigil to stderr to the end of the exec: migration lines specified by libvirt, and wait for either that sigil or an error to show up in the log for that domain before issuing the cont; if my memory is at all correct, libvirt should have some helper functions useful for that purpose already available. Ugh, bad idea -- I was thinking only of the migrate-from-file case, and not of migration from any alternate source; any non-exec source doesn't allow for this hack. Anthony suggested dropping -S from the command line on incoming migrations, and using info status to poll for when the guest resumes itself; however, this doesn't work for cases when the VM was saved or migrated in a paused state and the end-user expectation is for it to remain paused. We can't drop -S, because we need to be able to run 'info cpus' and then do sched_setaffinity on each vCPU, before it ever starts executing, otherwise memory will end u pinned to the wrong NUMA nodes I'm tempted to add additional output to info migrate indicating whether an inbound migration is ongoing; however, this wouldn't help versions of qemu without the patch applied. Yeah, I've been thinking much the same this morning. I think we should consider what the optimal setup is for our needs long term and try and do whatever we can for that in QEMU now. I think it'd definitely be worthwhile to have an 'info migrate' impl for incoming migration, and even to make '-incoming' optional, and add a 'migrate-incoming' command to the monitor, which like 'migrate' could be run in either blocking or non-blocking mode. For existing QEMU, it might be sufficient to just put an arbitrary 'sleep(5)' before issuing 'cont', which would at least give it a reasonable chance of avoiding the race condition. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Maintain value of ctxt-node in virInterfaceDefParseDhcp.
On Mon, Sep 28, 2009 at 06:54:19PM -0400, Laine Stump wrote: This was causing subsequent calls to virXPathxxx() to fail, since ctxt-node was left pointing at the dhcp node, rather than the protocol node. Previously this had gone unnoticed, as the dhcp node was the only thing parsed under ip, if it was there. Makes sense, applied, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] Move file format enum to libvirt_util
+enum virStorageFileFormat {
+VIR_STORAGE_FILE_RAW = 0,
+VIR_STORAGE_FILE_DIR,
+VIR_STORAGE_FILE_BOCHS,
+VIR_STORAGE_FILE_CLOOP,
+VIR_STORAGE_FILE_COW,
+VIR_STORAGE_FILE_DMG,
+VIR_STORAGE_FILE_ISO,
+VIR_STORAGE_FILE_QCOW,
+VIR_STORAGE_FILE_QCOW2,
+VIR_STORAGE_FILE_VMDK,
+VIR_STORAGE_FILE_VPC,
+VIR_STORAGE_FILE_LAST,
+};
Any chance we could add
VIR_STORAGE_FILE_VDI (VirtualBox)
and VIR_STORAGE_FILE_HDD (Parallels)
here as well?
Regards,
Pritesh
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] Move file format enum to libvirt_util
On Tue, Sep 29, 2009 at 11:27:18AM +0200, Pritesh Kothari wrote:
+enum virStorageFileFormat {
+VIR_STORAGE_FILE_RAW = 0,
+VIR_STORAGE_FILE_DIR,
+VIR_STORAGE_FILE_BOCHS,
+VIR_STORAGE_FILE_CLOOP,
+VIR_STORAGE_FILE_COW,
+VIR_STORAGE_FILE_DMG,
+VIR_STORAGE_FILE_ISO,
+VIR_STORAGE_FILE_QCOW,
+VIR_STORAGE_FILE_QCOW2,
+VIR_STORAGE_FILE_VMDK,
+VIR_STORAGE_FILE_VPC,
+VIR_STORAGE_FILE_LAST,
+};
Any chance we could add
VIR_STORAGE_FILE_VDI (VirtualBox)
and VIR_STORAGE_FILE_HDD (Parallels)
here as well?
Of course - patches to add more formats are welcome. The formats we
have there so far are just the ones I was able to figure out from the
qemu-img source. At the minimum level you just need to figure out the
magic bytes to identify the format, and the offset into the file
where the logical capacity is stored.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Resubmission #2: [PATCH 0/3] sVirt AppArmor security driver
On Fri, Sep 25, 2009 at 05:44:35PM -0500, Jamie Strandboge wrote: This simplifies adding new files. Part of this process involved adding support for kernel, initrd, loader, serial, and console. I have commented out code for hostdev (and pci would be easy enough), but it requires a patch to to move the _usbDevice struct fully into hostusb.h. I figure once the AppArmor driver is in, I can submit a patch for that. No, the usbDevice struct is intended to be private because we want the ability to easily change it without impacting callers. The idea is that there is not neccessarily just 1 single path associated with a USB device and thus 'path' should not be exposed to callers. Instead if you need process the 1-or-more paths associated with a USB device you should invoke the usbDeviceFileIterate() method and the callback you supply will be executing once for each path. The same pattern is there for PCI devices which have between 1 and 4 paths that need dealing with typically. This iteration also properly works with attach/detach. While I use virDomainDefFormat() primarily, this does not work with attach because the definition is not updated with the information yet (neither def nor newDef), so I pass the disk file on the command line. For now this is ok since SetSecurityImageLabel currently only deals with a single file, but this will need to be adjusted depending on how the storage backend patch works out. I thought about creating a new def and inserting the disk into it and sending it off to virt-aa-helper, but I couldn't find an easy way to get an accurate virCapsPtr in security_apparmor.c. I had a similar problem with virt-aa-helper, but there I just created a fake virCapsPtr (which defaults to 'hvm', which was a compromise I was willing to make at this time since sVirt only supports qemu right now). The issues with virCapsPtr aside, the code is overall cleaner and much more easily extendable, so thanks for the excellent feedback. :) For virCapsPtr issues we should probably evaluate whether we can make the capabilities object passed to virDomainDefParse() optionally NULLable. The capabilities object is used to fill in missing pieces of metadata when getting an XML blob in from the client app. By the time we get into your virt-aa-helper, all this metadata ought to be present correct, so in theory there should be no need for a virCapsPtr object. Does the virt-aa-helper need the full domain XML when attaching or detaching a disk ? If not, then you could try an approach where you pass the full domain XML when initially starting the guest, and then only pass the domain UUID + the device XML when attaching or detaching a particular device on the fly. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/27] Refactor QEMU monitor command handling
On Thu, Sep 24, 2009 at 04:00:02PM +0100, Daniel P. Berrange wrote: In the QEMU driver source code the methods which talk to the QEMU monitor currently all just call qemudMonitorCommand() directly with the raw command string, and then parse the raw reply. In the not too distant future QEMU is introducing a new machine parsable monitor syntax. With the current way the code is structured supporting this new mode will be seriously unpleasant. This large series of patches, moves all the monitor command formatting and parsing code out into a separate source file src/qemu/qemu_monitor_text.c. There is one API in that file for each logical monitor command we wish to issue, accepting (mostly) strongly typed arguments. The exception is the NIC hotplug method which still takes the raw NIC string for now. The main qemu_driver.c file now directly calls the appropriate monitor command APIs, making logic there much cleaner. When we add support for the new QEMU monitor syntax we'll gain another file src/qemu/qemu_monitor_json.c which implements all the same APIs as src/qemu/qemu_monitor_text.c, but using the new JSON syntax instead of raw text strings This patch series is soo large, because I did it in many small steps, one command at a time. It is also now much easier to debug the monitor by just setting the env variables LIBVIRT_LOG_OUTPUTS=1:stderr LIBVIRT_LOG_FILTERS=1:qemu_monitor And you'll get the command reply of each monitor interaction printed I've tested basic handling of every new method with the exception of the migration ones, since I don't have a convenient target host when on my laptop. Overall we get a small increase in code size, but huge increase in readability ! ACK, I see that Mark made a detailed code review already, this all looks fine to me ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] in Vbox driver, OpenHardDisk is called with NULL instead of
This patch replaces the use of NULL with empty strings () in a call to
OpenHardDisk, which otherwise fails with E_INVALIDARG.
Florian
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index c02b18c..f5a8c97 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3206,9 +3206,9 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
hddfileUtf16,
AccessMode_ReadWrite,
0,
- NULL,
+ ,
0,
- NULL,
+ ,
hardDisk);
#endif
}
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
RE: [libvirt] [RFC] Multi-IQN proposal
-Original Message-
From: libvir-list-boun...@redhat.com [mailto:libvir-list-
boun...@redhat.com] On Behalf Of Daniel P. Berrange
Sent: Monday, September 28, 2009 4:47 PM
To: Iyer, Shyam
Cc: libvir-list@redhat.com; Bellad, Sudhir; Domsch, Matt; KM, Paniraja
Subject: Re: [libvirt] [RFC] Multi-IQN proposal
On Sat, Sep 26, 2009 at 12:45:03AM +0530, shyam_i...@dell.com wrote:
Would this proposal be acceptable ?
It is hard to say. Can you try and explain what it actually does,
and/or what you are trying to achieve ? Googling for more docs / info
about 'Multi-IQN' returns this mailing thread as the top hit which
doesn't really help understanding.
Example XML schema for an iSCSI storage pool created --
pool type=iscsi
namedell/name
uuidcf354733-b01b-8870-2040-94888640e24d/uuid
capacity0/capacity
allocation0/allocation
available0/available
- source
initiator iqnname = initiator IQN1
initiator iqnname = initiator IQN2
host name=iSCSI target hostname or Target IP address /
device path=iSCSI Target IQN name /
/source
- target
path/dev/disk/by-path/path
- permissions
mode0700/mode
owner0/owner
group0/group
/permissions
/target
/pool
Each initiator iqn name can be parsed to create the unique sessions.
This doesn't make a whole lot of sense - for each iSCSI storage pool
defined in libvirt there is exactly one iSCSI session. So I'm not
really following why we'd need multiple IQN names in a single pool
if that were to imply in multiple sessions.
Extending the pool concept to multiple sessions opens up different
possibilities. Already the pool enumerates the different LUNs behind the
Target IQN.
Extending the mesh at the initiator side just opens up new combinations
with added advantages.
Multi-iqn offers flexibility to split I/Os by unique sessions and thus
allows for better manageability.
Today the open-iscsi initiator can create multi-IQN iSCSI sessions by
using ifaces.
See example /etc/iscsi/iface configuration file.
iface.transport_name = tcp
iface.initiatorname = iqn.2003-04.com.fedora:test
#iscsiadm -m node -T target iqn name -I iface_conf -l
-This could be a backend command.
Extending this flexibility to the VMs offer per VM manageability.
Ultimately a storage pool in libvirt is all about listing volumes,
and optionally creating/deleting volumes. iSCSI pools can only
list volumes, where each volume corresponds to a LUN in the iSCSI
target configured for the pool. So what effect does Multi-IQN have
on the process of enumerating LUNs ?
Since Multi-IQN is enumerating a new session it is no different from a
multipathing concept. A new LUN will be enumerated for each unique
IQN-pair and will have a unique /dev/sdX path.
This should solve the following possibilities --
* possibility of multiple IQNs for a single Guest
* option for Guest's own BIOS initiator to use these IQNs (iSCSI
in
Guest)
* option for hypervisor's initiator to use these IQNs on behalf of
the
guest
(Multi-IQN)
Compile tested only. Needs beatification.
The patch doesn't seem to actually do anything beyond parsing the
extra XML data - what is it actually to be used for in the iSCSI
storage implementation in storage_backend_iscsi.c ?
Regards,
Daniel
--
Planning to propose something like this but I wanted a consensus on the
XML schema first.
diff --git a/src/storage_backend_iscsi.c b/src/storage_backend_iscsi.c
index b516add..222eb33 100644
--- a/src/storage_backend_iscsi.c
+++ b/src/storage_backend_iscsi.c
@@ -157,13 +157,25 @@ static int
virStorageBackendISCSIConnection(virConnectPtr conn,
virStoragePoolObjPtr pool,
const char *portal,
- const char *action)
+ const char *action, int ifaces)
{
+/* Somehow this logic has to get better
+ * Things to note:
+ * 1) How to pass ifaces are configured?
+ * 2) How to run loop to login to each of the iface provided initiator
iqn names
+ * */
+if (ifaces) {
+const char *const cmdargv[] = {
+ISCSIADM, --mode, node, --portal, portal,
+--targetname, pool-def-source.devices[0].path, -I,
pool-def-source.initiator[0].iqnname, action, NULL
+};
+}
+else {
const char *const cmdargv[] = {
ISCSIADM, --mode, node, --portal, portal,
--targetname, pool-def-source.devices[0].path, action, NULL
};
-
+}
if (virRun(conn, cmdargv, NULL) 0)
return -1;
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Screenshot
Hi, Got my git/hg builds of libvirtd/virt-manager running together with kvm. Took some time but finally think I'm getting the hang of it. I noticed two things going from my pre-packaged Debian packages. The older xml files I had seemed to generate a faulty kvm cmdline: disk type='file' device='cdrom' target dev='hdc' bus='ide'/ readonly/ /disk This seems to translate into a -drive file=,... The file argument for kvm may be empty but should then not be defined: If you don’t specify the file= argument, you define an empty drive. As a result the first vm I tried to start came back with a message saying it could not open the diskfile. Additionally, check below screenshot; the vm names appear to be missing. Here's the complete XML of one vm, did anything change coming from this one? domain type='kvm' namevm1/name uuidea14abdc-12a4-48cb-1424-46cdf15405f4/uuid memory524288/memory currentMemory524288/currentMemory vcpu1/vcpu os type arch='x86_64' machine='pc'hvm/type boot dev='hd'/ /os features acpi/ apic/ pae/ /features clock offset='utc'/ on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashrestart/on_crash devices emulator/usr/bin/kvm/emulator disk type='file' device='disk' source file='/kvmimages/vm1.qcow2'/ target dev='vda' bus='virtio'/ /disk interface type='bridge' mac address='54:52:00:0a:dc:10'/ source bridge='br0'/ model type='virtio'/ /interface serial type='pty' source path='/dev/pts/1'/ target port='0'/ /serial console type='pty' tty='/dev/pts/1' source path='/dev/pts/1'/ target port='0'/ /console input type='mouse' bus='ps2'/ graphics type='vnc' port='-1' autoport='yes' keymap='en-us'/ /devices /domain attachment: Screenshot-Virtual Machine Manager.png-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Silently ignored virDomainRestore failures
On Tue, Sep 29, 2009 at 4:16 AM, Daniel P. Berrange berra...@redhat.comwrote: Yeah, I've been thinking much the same this morning. I think we should consider what the optimal setup is for our needs long term and try and do whatever we can for that in QEMU now. I think it'd definitely be worthwhile to have an 'info migrate' impl for incoming migration, and even to make '-incoming' optional, and add a 'migrate-incoming' command to the monitor, which like 'migrate' could be run in either blocking or non-blocking mode. 'migrate-incoming' is heavier surgery than I'm comfortable doing myself, but I'll try my hand at the info command. The only thing that bugs me about this is that if the output is being added to the existing 'info migrate', an explicit negative output (Incoming: none) will be necessary to distinguish from the case where reporting incoming migration is simply unsupported; as such, the current behavior of reporting no output at all if no migration is ongoing would need to change. For existing QEMU, it might be sufficient to just put an arbitrary 'sleep(5)' before issuing 'cont', which would at least give it a reasonable chance of avoiding the race condition. Well -- I wasn't going to submit the patch I'm now using internally (using and waiting for a sigil on stderr when migrate_url is stdin), but I suppose that with an else clause doing a sleep it might actually be the closest available option to a Right Thing for preexisting qemu. I'll wait to hear back today from the contractors testing with it (they were hitting this race condition frequently) and post an amended copy here if it gets their thumbs-up. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Error messages with --debug
Oh, regarding my previous post, I ran virt-manager --debug so see what's going on (I'm missing much more than just the vm names). I'm thinking I either missed installing some bits or my gtk/glade setup is already obsolete (Debian Lenny). Any hints? (virt-manager.py:27842): libglade-WARNING **: unknown property `orientation' for class `GtkVBox' (virt-manager.py:27842): libglade-WARNING **: could not look up stock id 'Restore saved machine...' (virt-manager.py:27842): libglade-WARNING **: could not look up stock id 'Add Connection...' (virt-manager.py:27842): libglade-WARNING **: could not look up stock id 'Virtual Machine Details' (virt-manager.py:27842): libglade-WARNING **: could not look up stock id 'Delete' (virt-manager.py:27842): libglade-WARNING **: could not look up stock id '_Contents' (virt-manager.py:27842): libglade-WARNING **: unknown property `orientation' for class `GtkVBox' /usr/local/share/virt-manager/virtManager/manager.py:150: GtkWarning: gtk_toolbar_set_icon_size: assertion `icon_size != GTK_ICON_SIZE_INVALID' failed self.window = gtk.glade.XML(config.get_glade_dir() + /vmm-manager.glade, vmm-manager, domain=virt-manager) /usr/local/share/virt-manager/virtManager/manager.py:394: GtkWarning: Failed to set text from markup due to error parsing markup: Attribute 'font' is not allowed on the span tag on line 1 char 40 win.show_all() /usr/local/share/virt-manager/virt-manager.py:371: GtkWarning: Failed to set text from markup due to error parsing markup: Attribute 'font' is not allowed on the span tag on line 1 char 24 gtk.main() /usr/local/share/virt-manager/virt-manager.py:371: GtkWarning: Failed to set text from markup due to error parsing markup: Attribute 'font' is not allowed on the span tag on line 1 char 25 gtk.main() -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Error messages with --debug
On Tue, Sep 29, 2009 at 01:11:45PM +0200, Fred Leeflang wrote: Oh, regarding my previous post, I ran virt-manager --debug so see what's going on (I'm missing much more than just the vm names). I'm thinking I either missed installing some bits or my gtk/glade setup is already obsolete (Debian Lenny). Any hints? This isn't the right list for virt-manager questions, we just focus on libvirt here http://virt-manager.org/mailinglist.html Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
RE: [libvirt] [RFC] Multi-IQN proposal
-Original Message- From: Dave Allan [mailto:dal...@redhat.com] Sent: Monday, September 28, 2009 6:44 PM To: Iyer, Shyam Cc: libvir-list@redhat.com; Bellad, Sudhir; Domsch, Matt; KM, Paniraja Subject: Re: [libvirt] [RFC] Multi-IQN proposal shyam_i...@dell.com wrote: Thanks for the review. -Original Message- From: Dave Allan [mailto:dal...@redhat.com] Sent: Saturday, September 26, 2009 2:43 AM To: Iyer, Shyam Cc: libvir-list@redhat.com; Bellad, Sudhir; Domsch, Matt; KM, Paniraja Subject: Re: [libvirt] [RFC] Multi-IQN proposal shyam_i...@dell.com wrote: Would this proposal be acceptable ? In principle, I think what you're proposing is reasonable, and is certainly contemplated by the iSCSI specs. Example XML schema for an iSCSI storage pool created -- pool type=iscsi namedell/name uuidcf354733-b01b-8870-2040-94888640e24d/uuid capacity0/capacity allocation0/allocation available0/available - source initiator iqnname = initiator IQN1 initiator iqnname = initiator IQN2 host name=iSCSI target hostname or Target IP address / device path=iSCSI Target IQN name / /source - target path/dev/disk/by-path/path - permissions mode0700/mode owner0/owner group0/group /permissions /target /pool I think you have the initiator name specified in the right place in the XML. I would make the initiator iqn an element rather than an attribute, since your proposal contemplates adding additional initiator specific information later, and stylistically I think elements will be cleaner. That gives: initiator iqniqn.foo1.bar.baz/iqn iqniqn.foo2.bar.baz/iqn iqniqn.foo3.bar.baz/iqn /initiator Each initiator iqn name can be parsed to create the unique sessions. Fair enough. You should propose specifically how you see the sessions being set up. Each pool currently describes something that basically resembles a session, so your proposal modifies that paradigm a bit. Another possible way to implement what you describe would be to allow zero or one initiator tags within a pool. If no initiator tag is specified, the system will use the system default; if a tag is specified, the system will attempt to use the information contained in it. The more I think about it, the more I like that approach since it keeps the pool paradigm unmodified. Ok. This should solve the following possibilities -- * possibility of multiple IQNs for a single Guest * option for Guest's own BIOS initiator to use these IQNs (iSCSI in Guest) * option for hypervisor's initiator to use these IQNs on behalf of the guest (Multi-IQN) How is this different from the first possibility? The first possibility is usage 1 and 2(below) whereas the third possibility is usage 3 and 4(below) Compile tested only. Needs beatification. I didn't go over the code closely, but I didn't see anything that struck me as completely off base. I think it's more important to get the details of how this information will be used worked out at this point than to get the code finalized. Dave Example Usages: Usage 1: VM1 - Init iqn1 Target iqn1 Init iqn2 Target iqn1 Init iqn3 Target iqn1 Init iqn4 Target iqn1 Usage 2: VM1 - Init iqn1 Target iqn1 Init iqn2 Target iqn2 Init iqn3 Target iqn3 Init iqn4 Target iqn4 Usage 3: VM1 - Init iqn1 Target iqn1 VM2 - Init iqn2 Target iqn1 Usage 4: VM1 - Init iqn1 Target iqn1 VM2 - Init iqn2 Target iqn2 Ok, I see what you mean now. From libvirt's perspective, there's no difference between these cases; you would simply be starting a bunch of pools and assigning the volumes to the appropriate guest(s). I am concerned now that you are proposing something larger than simply providing support for libvirt to use more than one iqn when starting pools on a host. As Dan Berrange also requested, please explain exactly how you intend for this functionality to be used. Replied to Dan's request. Idea is to reduce iSCSI manageability windows. Virt-manager or any management app that uses libvirt should be used by user to just provide initiator iqns. Your statement about providing the iqn to the guest to be used by its BIOS is particularly unclear to me. I understand what you want to do, but how do you envision that process working? There would be no pool started on the host in such a case. Libvirt currently has no
Re: [libvirt] [PATCH 00/27] Refactor QEMU monitor command handling
On Tue, Sep 29, 2009 at 12:12:23PM +0200, Daniel Veillard wrote: On Thu, Sep 24, 2009 at 04:00:02PM +0100, Daniel P. Berrange wrote: In the QEMU driver source code the methods which talk to the QEMU monitor currently all just call qemudMonitorCommand() directly with the raw command string, and then parse the raw reply. In the not too distant future QEMU is introducing a new machine parsable monitor syntax. With the current way the code is structured supporting this new mode will be seriously unpleasant. This large series of patches, moves all the monitor command formatting and parsing code out into a separate source file src/qemu/qemu_monitor_text.c. There is one API in that file for each logical monitor command we wish to issue, accepting (mostly) strongly typed arguments. The exception is the NIC hotplug method which still takes the raw NIC string for now. The main qemu_driver.c file now directly calls the appropriate monitor command APIs, making logic there much cleaner. When we add support for the new QEMU monitor syntax we'll gain another file src/qemu/qemu_monitor_json.c which implements all the same APIs as src/qemu/qemu_monitor_text.c, but using the new JSON syntax instead of raw text strings This patch series is soo large, because I did it in many small steps, one command at a time. It is also now much easier to debug the monitor by just setting the env variables LIBVIRT_LOG_OUTPUTS=1:stderr LIBVIRT_LOG_FILTERS=1:qemu_monitor And you'll get the command reply of each monitor interaction printed I've tested basic handling of every new method with the exception of the migration ones, since I don't have a convenient target host when on my laptop. Overall we get a small increase in code size, but huge increase in readability ! ACK, I see that Mark made a detailed code review already, this all looks fine to me ! Ok, I've pushed all this stuff Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH] Stop double free
On Mon, Sep 28, 2009 at 04:30:06PM -0600, Jim Fehlig wrote: Chris Lalancette wrote: While your patch would fix the problem, I'm not sure it's a good long-term solution. Other differences might come up in the future, and trying to worry about vshDeinit being re-entrant is probably not worth the effort. (Indeed, it looks like there were earlier attempts to avoid this, but things have changed since then, breaking the workaround). I think we should make it so that vshDeinit() does not try to re-enter itself. At the moment I don't have a patch, but I would look at either splitting vshError() into vshPrintError() and vshError(), or just doing a couple of fprintf()'s directly in vshDeinit() and not calling vshError() at all (with a comment explaining why). I'm seeing this problem in 0.7.1 as well when explicitly providing a connection URI, e.g. 'virsh -c ...'. I cooked up a patch based on your latter suggestion but changed it to the attached after chatting with danpb on IRC. This approach removes the doexit parameter and vshDeinit() call from vshError() altogether, requiring callers of vshError() to exit if they so desire. Looks fine, better simplify this, applied, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] in Vbox driver, OpenHardDisk is called with NULL instead of
On Tuesday 29 September 2009 12:27:49 Florian Vichot wrote:
This patch replaces the use of NULL with empty strings () in a call to
OpenHardDisk, which otherwise fails with E_INVALIDARG.
The patch fix's it but the more appropriate fix would be as below.
Regards,
Pritesh
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 07515fe..0c9fff0 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3199,7 +3199,8 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
if (def-disks[i]-type == VIR_DOMAIN_DISK_TYPE_FILE) {
IHardDisk *hardDisk = NULL;
PRUnichar *hddfileUtf16 = NULL;
-vboxIID *hdduuid= NULL;
+vboxIID *hdduuid = NULL;
+PRUnichar *hddEmpty = NULL;
/* Current Limitation: Harddisk can't be connected to
* Secondary Master as Secondary Master is always used
* for CD/DVD Drive, so don't connect the harddisk if it
@@ -3207,6 +3208,7 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
*/
data-pFuncs-pfnUtf8ToUtf16(def-disks[i]-src, hddfileUtf16);
+data-pFuncs-pfnUtf8ToUtf16(, hddEmpty);
data-vboxObj-vtbl-FindHardDisk(data-vboxObj, hddfileUtf16, hardDisk);
@@ -3221,9 +3223,9 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
hddfileUtf16,
AccessMode_ReadWrite,
0,
- NULL,
+ hddEmpty,
0,
- NULL,
+ hddEmpty,
hardDisk);
#endif
}
@@ -3286,6 +3288,7 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
hardDisk-vtbl-imedium.nsisupports.Release((nsISupports *)hardDisk);
}
vboxIIDUnalloc(hdduuid);
+data-pFuncs-pfnUtf16Free(hddEmpty);
data-pFuncs-pfnUtf16Free(hddfileUtf16);
} else if (def-disks[i]-type == VIR_DOMAIN_DISK_TYPE_BLOCK) {
}
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Resubmission #2: [PATCH 0/3] sVirt AppArmor security driver
On Tue, 29 Sep 2009, Daniel P. Berrange wrote: On Fri, Sep 25, 2009 at 05:44:35PM -0500, Jamie Strandboge wrote: This simplifies adding new files. Part of this process involved adding support for kernel, initrd, loader, serial, and console. I have commented out code for hostdev (and pci would be easy enough), but it requires a patch to to move the _usbDevice struct fully into hostusb.h. I figure once the AppArmor driver is in, I can submit a patch for that. No, the usbDevice struct is intended to be private because we want the ability to easily change it without impacting callers. The idea is that there is not neccessarily just 1 single path associated with a USB device and thus 'path' should not be exposed to callers. Instead if you need process the 1-or-more paths associated with a USB device you should invoke the usbDeviceFileIterate() method and the callback you supply will be executing once for each path. The same pattern is there for PCI devices which have between 1 and 4 paths that need dealing with typically. Ok, I'll look at this once the patch lands. For virCapsPtr issues we should probably evaluate whether we can make the capabilities object passed to virDomainDefParse() optionally NULLable. The capabilities object is used to fill in missing pieces of metadata when getting an XML blob in from the client app. By the time we get into your virt-aa-helper, all this metadata ought to be present correct, so in theory there should be no need for a virCapsPtr object. I was hoping I could pass NULL, but as you said, as it's written, the caps is needed to fill in a lot of blanks. I didn't want to try to change that myself because I didn't have the overall picture and couldn't forsee all the side-effects, especially (but certainly not limited to ;) non-qemu domains. Does the virt-aa-helper need the full domain XML when attaching or detaching a disk ? If not, then you could try an approach where you pass the full domain XML when initially starting the guest, and then only pass the domain UUID + the device XML when attaching or detaching a particular device on the fly. virt-aa-helper will be called on startup via AppArmorGenSecurityLabel and then also when things change via AppArmorRestoreSecurityImageLabel and AppArmorSetSecurityImageLabel. For just attaching and detaching a disk, I could probably do as you suggest (I had already considered this), but the required virDomainDiskDefParseXML() is internal and thus not available to virt-aa-helper. I then thought it would probably be safest anyway to pass the complete XML since: a) the helper can always be sure it is in sync b) it makes it easier down the line if def or newDef is updated for some action. All I'd need to do is put a load_profile() call in the right place in security_apparmor.c rather than having to rewrite virt-aa-helper again to deal with a different XML situation. I also wasn't sure if it was a design decision that the def or newDef wasn't updated when attaching/detaching. I really wanted to just insert the disk definition using virDomainDiskInsert, which would have solved this issue completely, but I couldn't find a clean way to obtain a copy of a domain definition that I could modify, send off to virt-aa-helper, then discard. Thanks for your feedback! :) Jamie -- Jamie Strandboge | http://www.canonical.com signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] in Vbox driver, OpenHardDisk is called with NULL instead of
On Tue, Sep 29, 2009 at 02:18:41PM +0200, Pritesh Kothari wrote: On Tuesday 29 September 2009 12:27:49 Florian Vichot wrote: This patch replaces the use of NULL with empty strings () in a call to OpenHardDisk, which otherwise fails with E_INVALIDARG. The patch fix's it but the more appropriate fix would be as below. Okay, I applied the UTF-16 correct version :-) thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Libvirtd on Non Linux?
Hi Experts, Just wondering is it possible now to run libvirt deamon on non-linux platforms (mainly windows and mac)? If yes, are there instructions on how to build it. If no, what was the roadblock? I'm trying to find a tool that can control the VMM for desktop/laptop users under win/mac/linux. Mainly for automation of deployment of some appliances. The best tool I've heard is libvirt, but couldn't find any doc on using it for other platforms. Any help is greatly appreciated. -Yushu -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirtd on Non Linux?
On Tue, Sep 29, 2009 at 05:50:34AM -0600, Yushu Yao wrote: Hi Experts, Just wondering is it possible now to run libvirt deamon on non-linux platforms (mainly windows and mac)? If yes, are there instructions on how to build it. If no, what was the roadblock? I'm trying to find a tool that can control the VMM for desktop/laptop users under win/mac/linux. Mainly for automation of deployment of some appliances. The best tool I've heard is libvirt, but couldn't find any doc on using it for other platforms. You could probably make libvirtd / QEMU work on Mac OS-X, since its is just BSD underneath and thus close enough to POSIX. It wouldn't magically give you control of the native Mac virtualizaation software - that would requirea new driver. Windows is a much harder task due to the crazy platform API it has, that looks nothing like POSIX. The lack of fork/exec is a huge problem that even gnulib can't magically solve for us. I'm not saying it is impossible, but it will involve alot of work Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Tentative schedule for next release
I'm suggesting to try to push 0.7.2 around Fri 16 Oct, we already have a number of cleanup patches in and serious improvements for ESX driver. If we go with that plan I think patches and big changes should be pushed in the tree by Fri 9, this includes among other things the AppArmor security driver. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Tentative schedule for next release
On Tue, Sep 29, 2009 at 04:24:16PM +0200, Daniel Veillard wrote: I'm suggesting to try to push 0.7.2 around Fri 16 Oct, we already have a number of cleanup patches in and serious improvements for ESX driver. If we go with that plan I think patches and big changes should be pushed in the tree by Fri 9, this includes among other things the AppArmor security driver. I think it would be nice to get back on an end-of-the-month release schedule. It was nice to have predictability that the date was approx the 30/31st of the month. If we did two 5 week cycles instead of 4 week cycles we'd be trivially aligned again, Fri 23 Oct and then Fri 27 Nov, etc Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/11] Generic data stream handling
FYI, the data streams patches are now committed Daniel On Fri, Sep 25, 2009 at 09:58:51AM +0200, Daniel Veillard wrote: On Mon, Aug 24, 2009 at 09:51:03PM +0100, Daniel P. Berrange wrote: The following series of patches introduce support for generic data streams in the libvirt API, the remote protocol, client daemon. The public API takes the form of a new object virStreamPtr and methods to read/write/close it The remote protocol was the main hard bit. Since the protocol allows multiple concurrent API calls on a single connection, this needed to also allow concurrent data streams. It is also not acceptable for a large data stream to block other traffic while it is transferring. Thus, we introduce a new protocol message type 'REMOTE_STREAM' to handle transfer for the stream data. A method involving a data streams starts off in the normal way with a REMOTE_CALL to the server, and a REMOTE_REPLY response message. If this was successful, there now follows the data stream traffic. For outgoing streams (data from client to server), the client will send zero or more REMOTE_STREAM packets containing the data with status == REMOTE_CONTINUE. These are asynchronous and not acknowledged by the server. At any time the server may send an async message with a type of REMOTE_STREAM and status of REMOTE_ERROR. This indicates to the client that the transfer is aborting at server request. If the client wishes to abort, it can send the server a REMOTE_STREAM+REMOTE_ERROR message. If the client finishes its data transfer, it will send a final REMOTE_STREAM+REMOTE_OK message, and the server will respond with the same. This full roundtrip handshake ensures any async error messages are guarenteed to be flushed For incoming data streams (data from server to client), the server sends zero or more REMOTE_STREAM packets containing the data with status == REMOTE_CONTINUE. These are asynchronous and not acknowledged by the client. At any time the client may send an async message with a type of REMOTE_STREAM and status of REMOTE_ERROR. This indicates to the server that the transfer is aborting at client request. If the server wishes to abort, it can send the server a REMOTE_STREAM+REMOTE_ERROR message. When the server finishes its data transfer, it will send a final REMOTE_STREAM+REMOTE_CONTINUE message ewith a data length of zero (ie EOF). The client will then send a REMOTE_STREAM+REMOTE_OK packet and the server will respond with the same. This full roundtrip handshake ensures any async error messages are guarenteed to be flushed This all ensures that multiple data streams can be active in parallel, and with a maximum data packet size of 256 KB, no single stream can cause too much latency on the connection for other API calls/streams. Okay, this is very similar in principle with HTTP pipelining with IMHO the same benefits and the same potential drawbacks. A couple of things to check might be: - the maximum amount of concurrent active streams allowed, for example suppose you want to migrate in emergency all the domains out of a failing machine, some level of serialization may be better than say attempting to migrate all 100 domains at the same time. 10 parallel stream might be better, but we need to make sure the API allows to report such condition. - the maximum chunking size, but with 256k I think this is covered. - synchronization internally between threads to avoid deadlocks or poor performances, that can be very hard to debug, so I guess an effort should be provided to explain how things are designed internally. But this sounds fine in general. The only thing it does not allow for is one API method to use two or more streams. These may be famous last words, but I don't think that use case will be neccessary for any of our APIs... as long as the limitation is documented especially in the parts of teh code where the assumption is made, sounds fine. The last 5 patches with a subject of [DEMO] are *NOT* intended to be committed to the repository. They merely demonstrate the use of data streams for a couple of hypothetical file upload and download APIs. Actually they were mostly to allow me to test the code streams code without messing around with the QEMU migration code. The immediate use case for this data stream code is Chris' QEMU migration patchset. The next use case is to allow serial console access to be tunnelled over libvirtd, eg to make 'virsh console GUEST' work remotely. This use case is why I included the support for non-blocking data streams and event loop integration (not required for Chris' migration use case) Okay, next to individual patches reviews, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com |
[libvirt] [PATCH] bug in vboxDomainDestroy in vbox driver
Hi All,
There is bug in vboxDomainDestroy() where in the while powerdown the method
doesn't wait till its completion.
Fix in the attached patch.
Regards,
Pritesh
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 07515fe..65e3449 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -1333,8 +1333,10 @@ static int vboxDomainDestroy(virDomainPtr dom) {
#else
IProgress *progress;
console-vtbl-PowerDown(console, progress);
-if (progress)
+if (progress) {
+progress-vtbl-WaitForCompletion(progress, -1);
progress-vtbl-nsisupports.Release((nsISupports *)progress);
+}
#endif
console-vtbl-nsisupports.Release((nsISupports *)console);
ret = 0;
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Tentative schedule for next release
On Tue, Sep 29, 2009 at 03:29:49PM +0100, Daniel P. Berrange wrote: On Tue, Sep 29, 2009 at 04:24:16PM +0200, Daniel Veillard wrote: I'm suggesting to try to push 0.7.2 around Fri 16 Oct, we already have a number of cleanup patches in and serious improvements for ESX driver. If we go with that plan I think patches and big changes should be pushed in the tree by Fri 9, this includes among other things the AppArmor security driver. I think it would be nice to get back on an end-of-the-month release schedule. It was nice to have predictability that the date was approx the 30/31st of the month. If we did two 5 week cycles instead of 4 week cycles we'd be trivially aligned again, Fri 23 Oct and then Fri 27 Nov, etc Hum, understood but I would prefer 2 releases than one big one, so what about 9/10 code freeze for 0.7.2 16/10 release of 0.7.2 23/10 code freeze for 0.7.3 30/10 release of 0.7.3 this mean a bit more churn on the releases push but I prefer than than waiting 6 weeks, I didn that the last time and IMHO that was too long. And that should not block development too much, if by the 9 some feature is not ready keep it warm until after 0.7.2, unless there is really a set of dependant changes, the intermediate week of bug fixes should not really affect productivity, right ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virtio network problems with kvm guests with 2.6.26 kernel
Daniel P. Berrange wrote: On Tue, Sep 29, 2009 at 02:39:59AM -0400, Jim Paris wrote: Hi, After upgrading libvirt on one system, my kvm guests running a 2.6.26 kernel with virtio networking could no longer communicate with the network. It seems that the problem is caused by newer libvirt versions unconditionally enabling GSO support by setting IFF_VNET_HDR [1]. However, support for this feature is apparently broken in 2.6.26. Ubuntu seems to have also discovered this problem [2] and they currently work around it by removing GSO support from kvm [3]. Currently, I'm running with a patched libvirt that never sets IFF_VNET_HDR, and it's working fine once again. While this is not a libvirt problem per se, it certainly violated the principle of least surprise given that libvirt was the only thing that changed! But I don't know if there's much that libvirt can do to avoid this problem, as it's really something that needs to get fixed in kvm, or the guest needs to be upgraded, or switched away from virtio. libvirt probes to see if IFF_VNET_HDR is supported by the kernel and if so, enables it. If its broken in the kernel, the kernel should have been patch, or had the flag disabled as its not really practical for libvirt to special case this thing further Libvirt probes support in the host kernel. The problem lies with the guest kernel, which fails to communicate with the host when the host has IFF_VNET_HDR enabled. -jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Support for SPICE graphics
This series of patches adds minimal support for SPICE graphics which is newly introduced in RHEL-5.4's fork of KVM. Since this is not yet merged in upstream QEMU/KVM, I'm not proposing to merge all these patches. The two XML schema patches are straightforward to merge. The two implementation ones are RHEL5 specific and I want to avoid declaring them supported until we have a sign of what the upstream merge will look like, since previous attempst to support KVM args prior to QEMU acceptance have been very painful long term. Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Add a QXL graphics card type to domain XML schema
* src/qemu_conf.c: Add dummy entry in enumeration
* docs/schemas/domain.rng: Add 'qxl' as a type for the video tag
* src/domain_conf.c, src/domain_conf.h: Add QXL to video type
enumerations
---
docs/schemas/domain.rng |7 +++
src/conf/domain_conf.c |3 ++-
src/conf/domain_conf.h |1 +
src/qemu/qemu_conf.c|3 ++-
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 70e98a7..f9d6d7e 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -805,10 +805,8 @@
/element
/define
!--
- A graphic description, currently in Xen only 2 types are supported:
- - sdl with optional display, xauth and fullscreen
- - vnc with a required port and optional listen IP address, password
- and keymap
+ A video adapter description, allowing configuration of device
+ model, number of virtual heads, and video ram size
--
define name=video
element name=video
@@ -821,6 +819,7 @@
valuevmvga/value
valuexen/value
valuevbox/value
+ valueqxl/value
/choice
/attribute
optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5e37d96..4930c38 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -149,7 +149,8 @@ VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
cirrus,
vmvga,
xen,
- vbox)
+ vbox,
+ qxl)
VIR_ENUM_IMPL(virDomainInput, VIR_DOMAIN_INPUT_TYPE_LAST,
mouse,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 7c918a7..7ea1152 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -305,6 +305,7 @@ enum virDomainVideoType {
VIR_DOMAIN_VIDEO_TYPE_VMVGA,
VIR_DOMAIN_VIDEO_TYPE_XEN,
VIR_DOMAIN_VIDEO_TYPE_VBOX,
+VIR_DOMAIN_VIDEO_TYPE_QXL,
VIR_DOMAIN_VIDEO_TYPE_LAST
};
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 2db38df..7ba0ac2 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -86,7 +86,8 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
cirrus,
vmware,
NULL, /* no arg needed for xen */
- NULL /* don't support vbox */);
+ NULL, /* don't support vbox */
+ NULL, /* Not implemented yet */);
#define PROC_MOUNT_BUF_LEN 255
--
1.6.2.5
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] Add a graphics type for SPICE protocol
This adds an element
graphics type='spice' port='5903' tlsPort='5904' listen='127.0.0.1'/
This is the bare minimum that should be exposed in the guest
config for SPICE. Other parameters are better handled as per
host level configuration tunables
* docs/schemas/domain.rng: Define the SPICE graphics schema
* src/domain_conf.h, src/domain_conf.c: Add parsing and formatting
for SPICE graphics config
* src/qemu_conf.c: Complain about unsupported graphics types
---
docs/schemas/domain.rng | 30 +
src/conf/domain_conf.c | 65 ++-
src/conf/domain_conf.h |8 ++
src/qemu/qemu_conf.c|9 ++
4 files changed, 111 insertions(+), 1 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index f9d6d7e..8aa2079 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -746,6 +746,36 @@
/group
group
attribute name=type
+valuespice/value
+ /attribute
+ optional
+attribute name=port
+ ref name=PortNumber/
+/attribute
+ /optional
+ optional
+attribute name=tlsPort
+ ref name=PortNumber/
+/attribute
+ /optional
+ optional
+attribute name=listen
+ ref name=addrIP/
+/attribute
+ /optional
+ optional
+attribute name=passwd
+ text/
+/attribute
+ /optional
+ optional
+attribute name=keymap
+ text/
+/attribute
+ /optional
+/group
+group
+ attribute name=type
valuerdp/value
/attribute
optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4930c38..ca5dfc3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -165,7 +165,8 @@ VIR_ENUM_IMPL(virDomainGraphics,
VIR_DOMAIN_GRAPHICS_TYPE_LAST,
sdl,
vnc,
rdp,
- desktop)
+ desktop,
+ spice)
VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
subsystem,
@@ -267,6 +268,12 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
VIR_FREE(def-data.desktop.display);
break;
+
+case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+VIR_FREE(def-data.spice.listenAddr);
+VIR_FREE(def-data.spice.keymap);
+VIR_FREE(def-data.spice.passwd);
+break;
}
VIR_FREE(def);
@@ -1692,6 +1699,38 @@ virDomainGraphicsDefParseXML(virConnectPtr conn,
def-data.desktop.fullscreen = 0;
def-data.desktop.display = virXMLPropString(node, display);
+} else if (def-type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+char *port = virXMLPropString(node, port);
+char *tlsPort;
+
+if (port) {
+if (virStrToLong_i(port, NULL, 10, def-data.spice.port) 0) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot parse spice port %s), port);
+VIR_FREE(port);
+goto error;
+}
+VIR_FREE(port);
+} else {
+def-data.spice.port = 5900;
+}
+
+tlsPort = virXMLPropString(node, tlsPort);
+if (tlsPort) {
+if (virStrToLong_i(tlsPort, NULL, 10, def-data.spice.tlsPort)
0) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot parse spice tlsPort %s),
tlsPort);
+VIR_FREE(tlsPort);
+goto error;
+}
+VIR_FREE(tlsPort);
+} else {
+def-data.spice.tlsPort = 0;
+}
+
+def-data.spice.listenAddr = virXMLPropString(node, listen);
+def-data.spice.passwd = virXMLPropString(node, passwd);
+def-data.spice.keymap = virXMLPropString(node, keymap);
}
cleanup:
@@ -4100,6 +4139,30 @@ virDomainGraphicsDefFormat(virConnectPtr conn,
break;
+case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+if (def-data.spice.port)
+virBufferVSprintf(buf, port='%d',
+ def-data.spice.port);
+
+if (def-data.spice.tlsPort)
+virBufferVSprintf(buf, tlsPort='%d',
+ def-data.spice.tlsPort);
+
+if (def-data.spice.listenAddr)
+virBufferVSprintf(buf, listen='%s',
+ def-data.spice.listenAddr);
+
+if (def-data.spice.keymap)
+virBufferEscapeString(buf, keymap='%s',
+ def-data.spice.keymap);
+
+if (def-data.spice.passwd
+(flags VIR_DOMAIN_XML_SECURE))
+virBufferEscapeString(buf, passwd='%s',
+
[libvirt] [PATCH 3/4] Implement RHEL-5.4 KVM QXL support in QEMU driver
This supports the -qxl argument in RHEL-5's fork of KVM
which has SPICE support. QXL is a graphics card, but
inexplicably doesn't use the standard -vga syntax for
generic configuration. Also -qxl is rather useless unless
you also supply -spice (coming in next patch)
* src/qemu_conf.c: Probe for -qxl arg in QEMU help. Format a
-qxl arg for launching VMs
* src/qemu_conf.h: Add flag for -qxl arg availability
* tests/qemuhelpdata/kvm-83-rhel, tests/qemuhelptest.c: test
for -qxl arg help parsing
* tests/qemuxml2argvtest.c, tests/qemuxml2xmltest.c,
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args,
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml: add
tests for -qxl graphics XML to ARGV handling
---
src/qemu/qemu_conf.c | 41 +-
src/qemu/qemu_conf.h |2 +-
tests/qemuhelpdata/kvm-83-rhel | 151
tests/qemuhelptest.c | 16 ++
.../qemuxml2argv-graphics-spice.args |1 +
.../qemuxml2argv-graphics-spice.xml| 25
tests/qemuxml2argvtest.c |3 +
tests/qemuxml2xmltest.c|1 +
8 files changed, 235 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuhelpdata/kvm-83-rhel
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ae171bc..695ee7c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -87,7 +87,7 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
vmware,
NULL, /* no arg needed for xen */
NULL, /* don't support vbox */
- NULL, /* Not implemented yet */);
+ NULL, /* qxl is a bizarre special case */);
#define PROC_MOUNT_BUF_LEN 255
@@ -865,6 +865,8 @@ static unsigned int qemudComputeCmdFlags(const char *help,
}
if (strstr(help, -vga) !strstr(help, -std-vga))
flags |= QEMUD_CMD_FLAG_VGA;
+if (strstr(help, -qxl))
+flags |= QEMUD_CMD_FLAG_QXL;
if (strstr(help, boot=on))
flags |= QEMUD_CMD_FLAG_DRIVE_BOOT;
if (strstr(help, serial=s))
@@ -2174,9 +2176,15 @@ int qemudBuildCommandLine(virConnectPtr conn,
}
if (qemuCmdFlags QEMUD_CMD_FLAG_VGA) {
-if (def-videos[0]-type == VIR_DOMAIN_VIDEO_TYPE_XEN) {
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
/* nothing - vga has no effect on Xen pvfb */
-} else {
+break;
+case VIR_DOMAIN_VIDEO_TYPE_QXL:
+/* handle later */
+break;
+default:
+{
const char *vgastr =
qemuVideoTypeToString(def-videos[0]-type);
if (!vgastr) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -2187,9 +2195,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
ADD_ARG_LIT(-vga);
ADD_ARG_LIT(vgastr);
+break;
+}
}
} else {
-
switch (def-videos[0]-type) {
case VIR_DOMAIN_VIDEO_TYPE_VGA:
ADD_ARG_LIT(-std-vga);
@@ -2204,6 +2213,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
/* No special args - this is the default */
break;
+case VIR_DOMAIN_VIDEO_TYPE_QXL:
+/* handle later */
+break;
+
default:
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_(video type %s is not supported with QEMU),
@@ -2211,6 +2224,26 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto error;
}
}
+
+if (def-videos[0]-type == VIR_DOMAIN_VIDEO_TYPE_QXL) {
+
+if (qemuCmdFlags QEMUD_CMD_FLAG_QXL) {
+char *optstr;
+
+if (virAsprintf(optstr, %u,ram=%u,
+def-videos[0]-heads,
+(def-videos[0]-vram /1024)) 0)
+goto no_memory;
+
+ADD_ARG_LIT(-qxl);
+ADD_ARG(optstr);
+} else {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
%s,
+ _(qxl graphics are not supported with this
QEMU));
+goto error;
+}
+}
+
}
/* Add sound hardware */
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 96b7c0c..c8e3276 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -60,7 +60,6 @@ enum qemud_cmd_flags {
QEMUD_CMD_FLAG_KVM = (1 13), /* Whether KVM is compiled
in */
QEMUD_CMD_FLAG_DRIVE_FORMAT = (1
[libvirt] [PATCH 4/4] Implement RHEL-5.4 KVM support for SPICE graphics
This supports the -spice argument in RHEL-5's fork of KVM
which has SPICE support. There are many more options for
-spice than need to be added - at very least the x509
cert paths should be pulled out of /etc/libvirt/qemu.conf
* src/qemu_conf.c, src/qemu_conf.h: Add SPICE flag. Check for
-spice availability. Format -spice arg for command line
* qemuhelptest.c: Add SPICE flag
* qemuxml2argvdata/qemuxml2argv-graphics-spice.args: Add graphics
for spice
* qemuxml2argvdata/qemuxml2argv-graphics-spice.xml: Add -spice arg
* qemuxml2argvtest.c: Add SPICE flag
---
src/qemu/qemu_conf.c | 38
src/qemu/qemu_conf.h |1 +
tests/qemuhelptest.c |3 +-
.../qemuxml2argv-graphics-spice.args |2 +-
.../qemuxml2argv-graphics-spice.xml|2 +
tests/qemuxml2argvtest.c |2 +-
6 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 695ee7c..fea0747 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -865,6 +865,8 @@ static unsigned int qemudComputeCmdFlags(const char *help,
}
if (strstr(help, -vga) !strstr(help, -std-vga))
flags |= QEMUD_CMD_FLAG_VGA;
+if (strstr(help, -spice))
+flags |= QEMUD_CMD_FLAG_SPICE;
if (strstr(help, -qxl))
flags |= QEMUD_CMD_FLAG_QXL;
if (strstr(help, boot=on))
@@ -2163,6 +2165,42 @@ int qemudBuildCommandLine(virConnectPtr conn,
*/
ADD_ENV_COPY(QEMU_AUDIO_DRV);
ADD_ENV_COPY(SDL_AUDIODRIVER);
+} else if ((def-ngraphics == 1)
+ def-graphics[0]-type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+virBuffer opt = VIR_BUFFER_INITIALIZER;
+char *optstr;
+
+if (!(qemuCmdFlags QEMUD_CMD_FLAG_SPICE)) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, %s,
+ _(spice graphics are not supported with this
QEMU));
+goto error;
+}
+
+virBufferVSprintf(opt, port=%u, def-graphics[0]-data.spice.port);
+
+if (def-graphics[0]-data.spice.tlsPort)
+virBufferVSprintf(opt, ,sport=%u,
def-graphics[0]-data.spice.tlsPort);
+
+if (def-graphics[0]-data.spice.listenAddr)
+virBufferVSprintf(opt, ,host=%s,
def-graphics[0]-data.spice.listenAddr);
+
+if (virBufferError(opt))
+goto no_memory;
+
+optstr = virBufferContentAndReset(opt);
+
+ADD_ARG_LIT(-spice);
+ADD_ARG(optstr);
+if (def-graphics[0]-data.spice.keymap) {
+ADD_ARG_LIT(-k);
+ADD_ARG_LIT(def-graphics[0]-data.spice.keymap);
+}
+/* SPICE includes native support for tunnelling audio, so we
+ * set the audio backend to none, to prevent it opening the
+ * host OS audio devices since that causes security issues
+ * and is non-sensical when using SPICE.
+ */
+ADD_ENV_LIT(QEMU_AUDIO_DRV=none);
} else if (def-ngraphics) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
%s, _(unsupported graphics output requested));
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index c8e3276..3866f0e 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -70,6 +70,7 @@ enum qemud_cmd_flags {
QEMUD_CMD_FLAG_DRIVE_SERIAL = (1 19), /* -driver serial= available */
QEMUD_CMD_FLAG_XEN_DOMID = (1 20), /* -xen-domid (new style xen
integration) */
QEMUD_CMD_FLAG_QXL = (1 21), /* Is -qxl avail (RHEL-5/6
custom) */
+QEMUD_CMD_FLAG_SPICE = (1 22), /* Is -spice avail (RHEL-5/6
custom) */
};
/* Main driver state */
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 7fd9e92..980d8bd 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -193,7 +193,8 @@ mymain(int argc, char **argv)
QEMUD_CMD_FLAG_KVM |
QEMUD_CMD_FLAG_DRIVE_FORMAT |
QEMUD_CMD_FLAG_VGA |
-QEMUD_CMD_FLAG_QXL,
+QEMUD_CMD_FLAG_QXL |
+QEMUD_CMD_FLAG_SPICE,
9001, 1, 0);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args
b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args
index d401b85..43516da 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args
@@ -1 +1 @@
-LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M
pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait
-no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel
none -usb -qxl 3,ram=64
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M
pc -m 214 -smp 1 -monitor unix:/tmp/test-monitor,server,nowait
Re: [libvirt] virtio network problems with kvm guests with 2.6.26 kernel
Jim Paris wrote: Daniel P. Berrange wrote: On Tue, Sep 29, 2009 at 02:39:59AM -0400, Jim Paris wrote: Hi, After upgrading libvirt on one system, my kvm guests running a 2.6.26 kernel with virtio networking could no longer communicate with the network. It seems that the problem is caused by newer libvirt versions unconditionally enabling GSO support by setting IFF_VNET_HDR [1]. However, support for this feature is apparently broken in 2.6.26. Ubuntu seems to have also discovered this problem [2] and they currently work around it by removing GSO support from kvm [3]. Currently, I'm running with a patched libvirt that never sets IFF_VNET_HDR, and it's working fine once again. While this is not a libvirt problem per se, it certainly violated the principle of least surprise given that libvirt was the only thing that changed! But I don't know if there's much that libvirt can do to avoid this problem, as it's really something that needs to get fixed in kvm, or the guest needs to be upgraded, or switched away from virtio. libvirt probes to see if IFF_VNET_HDR is supported by the kernel and if so, enables it. If its broken in the kernel, the kernel should have been patch, or had the flag disabled as its not really practical for libvirt to special case this thing further Libvirt probes support in the host kernel. The problem lies with the guest kernel, which fails to communicate with the host when the host has IFF_VNET_HDR enabled. Nevermind that, my conclusions were bogus. Things still didn't quite add up, so I tracked down what's really going on here: - Libvirt 0.7.1 (as packaged by Debian) has IFF_VNET_HDR support. - Qemu-kvm 0.11.0 (as built by myself) did NOT include IFF_VNET_HDR support, because it was built against the older headers on my system. Libvirt assumes that if it can support IFF_VNET_HDR, and qemu is new enough, then qemu must support IFF_VNET_HDR too. This assumption was wrong in my case, but it seems there's no easy way for libvirt to figure out the correct thing to do. Anyway, after updating the linux-libc-dev package and rebuilding qemu-kvm, things work now. -jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Minor comment changes - OOPS! Wrong patchfile!
This wasn't the file I meant to send! On 09/29/2009 03:58 PM, Laine Stump wrote: Fix some minor grammer (and one other) nits in comments that end up in generated API reference documentation. No functional/binary differences. [...] -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Support reporting live interface IP/netmask.
From: root r...@vlap.laine.org
This patch adds the flag VIR_INTERFACE_XML_INACTIVE to
virInterfaceGetXMLDesc's flags. When it is *not* set (the default),
the live interface info will be returned in the XML. in particular,
the IP address(es) and netmask(s) will be retrieved by querying the
device directly, rather than just reporting what's in the config
file. The backend of this is in netcf's new ncf_if_xml_state()
function.
Any live interface ip address info in the xml will have the property
source set to device, eg:
ip address='10.24.0.1' prefix='24' source='device'/
Also, if an interface is currently inactive, no ip addresses will be
returned, since an inactive interface device can't be queried for IP
addresses (effectively it has none).
A difference in the XML from previously - it is now acceptable to have
both a dhcp *and* an ip node (or neither) within the protocol
node. Before you had to have one or the other, but couldn't have both.
Note that you need at least netcf 0.1.2 for this to build and work,
and an upcoming release (patches submitted today) for it to work
exactly as described above.
---
include/libvirt/libvirt.h.in |4 +++
src/conf/interface_conf.c| 62 ++---
src/conf/interface_conf.h|1 +
src/interface/netcf_driver.c |8 -
src/libvirt.c| 15 +++---
tools/virsh.c| 10 +-
6 files changed, 63 insertions(+), 37 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 4e63e48..fd0c90b 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -922,6 +922,10 @@ virInterfacePtr virInterfaceLookupByMACString
(virConnectPtr conn,
const char* virInterfaceGetName (virInterfacePtr iface);
const char* virInterfaceGetMACString (virInterfacePtr iface);
+typedef enum {
+VIR_INTERFACE_XML_INACTIVE = 1 /* dump inactive interface information */
+} virInterfaceXMLFlags;
+
char * virInterfaceGetXMLDesc(virInterfacePtr iface,
unsigned int flags);
virInterfacePtr virInterfaceDefineXML (virConnectPtr conn,
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index e646351..b422464 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -92,6 +92,7 @@ void virInterfaceDefFree(virInterfaceDefPtr def)
VIR_FREE(def-proto.family);
VIR_FREE(def-proto.address);
VIR_FREE(def-proto.gateway);
+VIR_FREE(def-proto.source);
VIR_FREE(def);
}
@@ -272,6 +273,8 @@ virInterfaceDefParseIp(virConnectPtr conn,
virInterfaceDefPtr def,
%s, _(Invalid ip address prefix value));
return(-1);
}
+tmp = virXPathString(conn, string(./ip[1]/@source), ctxt);
+def-proto.source = tmp;
}
tmp = virXPathString(conn, string(./route[1]/@gateway), ctxt);
def-proto.gateway = tmp;
@@ -282,22 +285,19 @@ virInterfaceDefParseIp(virConnectPtr conn,
virInterfaceDefPtr def,
static int
virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceDefPtr def,
xmlXPathContextPtr ctxt) {
-xmlNodePtr cur;
-int ret;
+xmlNodePtr dhcp, ip;
+int ret = 0;
-cur = virXPathNode(conn, ./dhcp, ctxt);
-if (cur != NULL)
-ret = virInterfaceDefParseDhcp(conn, def, cur, ctxt);
-else {
-cur = virXPathNode(conn, ./ip, ctxt);
-if (cur != NULL)
-ret = virInterfaceDefParseIp(conn, def, cur, ctxt);
-else {
-virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
-%s, _(interface miss dhcp or ip
adressing));
-ret = -1;
-}
-}
+dhcp = virXPathNode(conn, ./dhcp, ctxt);
+if (dhcp != NULL)
+ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
+
+if (ret != 0)
+return(ret);
+
+ip = virXPathNode(conn, ./ip, ctxt);
+if (ip != NULL)
+ret = virInterfaceDefParseIp(conn, def, ip, ctxt);
return(ret);
}
@@ -1005,6 +1005,7 @@ virInterfaceVlanDefFormat(virConnectPtr conn,
virBufferPtr buf,
static int
virInterfaceProtocolDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED,
virBufferPtr buf, const virInterfaceDefPtr def) {
+char prefixStr[16];
if (def-proto.family == NULL)
return(0);
virBufferVSprintf(buf, protocol family='%s'\n, def-proto.family);
@@ -1015,20 +1016,23 @@ virInterfaceProtocolDefFormat(virConnectPtr conn
ATTRIBUTE_UNUSED,
virBufferAddLit(buf, dhcp peerdns='yes'/\n);
else
virBufferAddLit(buf, dhcp/\n);
-} else {
-/* theorically if we don't have dhcp we should have an address */
-if (def-proto.address != NULL) {
-if (def-proto.prefix != 0)
-virBufferVSprintf(buf, ip
Re: [libvirt] [PATCH] Support reporting live interface IP/netmask.
On 09/29/2009 04:02 PM, Laine Stump wrote:
From: rootr...@vlap.laine.org
Note that I accidentally did the local commit as root, and didn't notice
it until now. If this patch gets committed, please change the From: first.
This patch adds the flag VIR_INTERFACE_XML_INACTIVE to
virInterfaceGetXMLDesc's flags. When it is *not* set (the default),
the live interface info will be returned in the XML. in particular,
the IP address(es) and netmask(s) will be retrieved by querying the
device directly, rather than just reporting what's in the config
file. The backend of this is in netcf's new ncf_if_xml_state()
function.
Any live interface ip address info in the xml will have the property
source set to device, eg:
ip address='10.24.0.1' prefix='24' source='device'/
Also, if an interface is currently inactive, no ip addresses will be
returned, since an inactive interface device can't be queried for IP
addresses (effectively it has none).
A difference in the XML from previously - it is now acceptable to have
both a dhcp *and* an ip node (or neither) within the protocol
node. Before you had to have one or the other, but couldn't have both.
Note that you need at least netcf 0.1.2 for this to build and work,
and an upcoming release (patches submitted today) for it to work
exactly as described above.
---
include/libvirt/libvirt.h.in |4 +++
src/conf/interface_conf.c| 62 ++---
src/conf/interface_conf.h|1 +
src/interface/netcf_driver.c |8 -
src/libvirt.c| 15 +++---
tools/virsh.c| 10 +-
6 files changed, 63 insertions(+), 37 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 4e63e48..fd0c90b 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -922,6 +922,10 @@ virInterfacePtr virInterfaceLookupByMACString
(virConnectPtr conn,
const char* virInterfaceGetName (virInterfacePtr iface);
const char* virInterfaceGetMACString (virInterfacePtr iface);
+typedef enum {
+VIR_INTERFACE_XML_INACTIVE = 1 /* dump inactive interface information */
+} virInterfaceXMLFlags;
+
char * virInterfaceGetXMLDesc(virInterfacePtr iface,
unsigned int flags);
virInterfacePtr virInterfaceDefineXML (virConnectPtr conn,
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index e646351..b422464 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -92,6 +92,7 @@ void virInterfaceDefFree(virInterfaceDefPtr def)
VIR_FREE(def-proto.family);
VIR_FREE(def-proto.address);
VIR_FREE(def-proto.gateway);
+VIR_FREE(def-proto.source);
VIR_FREE(def);
}
@@ -272,6 +273,8 @@ virInterfaceDefParseIp(virConnectPtr conn,
virInterfaceDefPtr def,
%s, _(Invalid ip address prefix value));
return(-1);
}
+tmp = virXPathString(conn, string(./ip[1]/@source), ctxt);
+def-proto.source = tmp;
}
tmp = virXPathString(conn, string(./route[1]/@gateway), ctxt);
def-proto.gateway = tmp;
@@ -282,22 +285,19 @@ virInterfaceDefParseIp(virConnectPtr conn,
virInterfaceDefPtr def,
static int
virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceDefPtr def,
xmlXPathContextPtr ctxt) {
-xmlNodePtr cur;
-int ret;
+xmlNodePtr dhcp, ip;
+int ret = 0;
-cur = virXPathNode(conn, ./dhcp, ctxt);
-if (cur != NULL)
-ret = virInterfaceDefParseDhcp(conn, def, cur, ctxt);
-else {
-cur = virXPathNode(conn, ./ip, ctxt);
-if (cur != NULL)
-ret = virInterfaceDefParseIp(conn, def, cur, ctxt);
-else {
-virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
-%s, _(interface miss dhcp or ip
adressing));
-ret = -1;
-}
-}
+dhcp = virXPathNode(conn, ./dhcp, ctxt);
+if (dhcp != NULL)
+ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
+
+if (ret != 0)
+return(ret);
+
+ip = virXPathNode(conn, ./ip, ctxt);
+if (ip != NULL)
+ret = virInterfaceDefParseIp(conn, def, ip, ctxt);
return(ret);
}
@@ -1005,6 +1005,7 @@ virInterfaceVlanDefFormat(virConnectPtr conn,
virBufferPtr buf,
static int
virInterfaceProtocolDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED,
virBufferPtr buf, const virInterfaceDefPtr def)
{
+char prefixStr[16];
if (def-proto.family == NULL)
return(0);
virBufferVSprintf(buf, protocol family='%s'\n, def-proto.family);
@@ -1015,20 +1016,23 @@ virInterfaceProtocolDefFormat(virConnectPtr conn
ATTRIBUTE_UNUSED,
virBufferAddLit(buf, dhcp peerdns='yes'/\n);
else
virBufferAddLit(buf, dhcp/\n);
Re: [libvirt] virtio network problems with kvm guests with 2.6.26 kernel
On Tue, 2009-09-29 at 15:20 -0400, Jim Paris wrote: Nevermind that, my conclusions were bogus. Things still didn't quite add up, so I tracked down what's really going on here: - Libvirt 0.7.1 (as packaged by Debian) has IFF_VNET_HDR support. - Qemu-kvm 0.11.0 (as built by myself) did NOT include IFF_VNET_HDR support, because it was built against the older headers on my system. Libvirt assumes that if it can support IFF_VNET_HDR, and qemu is new enough, then qemu must support IFF_VNET_HDR too. This assumption was wrong in my case, but it seems there's no easy way for libvirt to figure out the correct thing to do. Anyway, after updating the linux-libc-dev package and rebuilding qemu-kvm, things work now. Yeah, that makes some sense - we have no way of knowing whether qemu was compiled with IFF_VNET_HDR support, all we can do is check that that version of qemu would have support if compiled against headers with IFF_VNET_HDR. Thanks, Mark. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Support reporting live interface IP/netmask.
On 09/29/2009 10:02 PM, Laine Stump wrote: From: rootr...@vlap.laine.org This patch adds the flag VIR_INTERFACE_XML_INACTIVE to virInterfaceGetXMLDesc's flags. What about calling it instead VIR_INTERFACE_XML_CONFIG and adding a dummy (0) VIR_INTERFACE_XML_DEVICE, and corresponding --config/--device flags to the virsh command? This would match the source attribute. I'll let others override me happily, though. Paolo -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Support reporting live interface IP/netmask.
On Tue, Sep 29, 2009 at 11:13:56PM +0200, Paolo Bonzini wrote: On 09/29/2009 10:02 PM, Laine Stump wrote: From: rootr...@vlap.laine.org This patch adds the flag VIR_INTERFACE_XML_INACTIVE to virInterfaceGetXMLDesc's flags. What about calling it instead VIR_INTERFACE_XML_CONFIG and adding a dummy (0) VIR_INTERFACE_XML_DEVICE, and corresponding --config/--device flags to the virsh command? This would match the source attribute. I'll let others override me happily, though. Well it's to keep coherent naming with the APIs for other kind of objects, for example there is an VIR_DOMAIN_XML_INACTIVE in libvirt.h to dump the inactive domain informations (when a domain is running/active). Similary 'virsh dumpxml --inactive' is about dumping the XML for the domain as it was defined. W.r.t. providing accessors for each informations we provide as part of an xml dump in virsh, I could try to add something generic, for example as a post processing step based on XPath if people really feel disarmed by grabbing something from XML output. But was that really the sense of your query ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Fix memleaks in libvirtd's message processing
2009/9/29 Daniel P. Berrange berra...@redhat.com: On Sat, Sep 26, 2009 at 02:24:33AM +0200, Matthias Bolte wrote: I tracked down a memleak in libvirtd's message processing. The leak was introduced in commit 47cab734995fa9521b1df05d37e9978eedd8d3a2 Split out code for handling incoming method call messages. This commit changed the way qemud_client_message objects were reused. Before this commit: - qemudWorker() removes a message from dispatch queue and passes it to remoteDispatchClientRequest() - remoteDispatchClientRequest() handles the message and reuses the same message for the response. If an error occurs the same message is used to report it (rpc_error label). If another error occurs while trying to report the first error remoteDispatchClientRequest() would return -1 and the message will be freed in qemudWorker() After this commit: - qemudWorker() removes a message from dispatch queue and passes it to remoteDispatchClientRequest() - remoteDispatchClientRequest() calls remoteDispatchClientCall() if the message it is an remote call or otherwise respond with a new error message by calling remoteSerializeReplyError() and the original message leaks - remoteDispatchClientCall() handles the message and reuses the same message for the response. If an error occurs it calls remoteSerializeReplyError() and the original message leaks. If a fatal error occurs remoteDispatchClientCall() returns -1 and the original message will be freed in qemudWorker() To fix this leak the original message has to be freed if remoteSerializeReplyError() succeeds. If remoteSerializeReplyError() fails the original message will be freed in qemudWorker(). In addition I came across another leak in remoteSerializeError(). If an error occurs while serializing the initial error the message leaks. To fix this leak the message has to be freed in case of an XDR error. ACK to your patch - this is all my fault caught up in the many times I re-wrote the streams support. I really hate the contract of the remoteDispatchClientRequest() method, it should always be responsible for free'ing the 'msg' regardless of return status. I had actually refactored things todo that, but then I rewrote it again and forget that I had changed the error reporting functions thus leading to this leak. Daniel Okay, I've committed the patch. Matthias -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
