[libvirt] [PATCHv6 1/5] virstring.h/c: Util method for finding regexp patterns in some strings
--- po/POTFILES.in |1 + src/libvirt_private.syms |1 + src/util/virstring.c | 97 ++ src/util/virstring.h |3 ++ 4 files changed, 102 insertions(+) diff --git a/po/POTFILES.in b/po/POTFILES.in index 0359b2f..738cfb1 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -188,6 +188,7 @@ src/util/virscsi.c src/util/virsocketaddr.c src/util/virstatslinux.c src/util/virstoragefile.c +src/util/virstring.c src/util/virsysinfo.c src/util/virerror.c src/util/virerror.h diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d1a58f9..68ca39d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1763,6 +1763,7 @@ virStorageFileResize; # util/virstring.h virArgvToString; virAsprintfInternal; +virSearchRegex; virSkipSpaces; virSkipSpacesAndBackslash; virSkipSpacesBackwards; diff --git a/src/util/virstring.c b/src/util/virstring.c index 8d0ca70..3c93450 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -23,6 +23,7 @@ #include stdlib.h #include stdio.h +#include regex.h #include c-ctype.h #include virstring.h @@ -645,3 +646,99 @@ int virStringSortRevCompare(const void *a, const void *b) return strcmp(*sb, *sa); } + +/** + * virSearchRegex: + * Allows you to get the nth occurrence of a substring in sourceString which matches + * a POSIX Extended regular expression pattern. + * If there is no substring, result is not modified. + * return -1 on error, 0 if not found and 1 if found. + * + * @sourceString: String to parse + * @occurrence: return occurrence 'n' (starting from 0) of a sub-string that + * matches the pattern. + * @regexp: POSIX Extended regular expression pattern used for matching + * @result: nth occurrence substring matching the @regexp pattern + * @code +char *source = 6853a496-1c10-472e-867a-8244937bd6f0 +773ab075-4cd7-4fc2-8b6e-21c84e9cb391 +bbb3c75c-d60f-43b0-b802-fd56b84a4222 +60c04aa1-0375-4654-8d9f-e149d9885273 +4548d465-9891-4c34-a184-3b1c34a26aa8; +char *ret1=NULL; +char *ret2=NULL; +char *ret3=NULL; +virSearchRegex(source, + 4, + ([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}), + ret1); +//ret1 = 4548d465-9891-4c34-a184-3b1c34a26aa8 +virSearchRegex(source, + 0, + ([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}), + ret2); +//ret2 = 6853a496-1c10-472e-867a-8244937bd6f0 +virSearchRegex(source, + 1, + ([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}), + ret3); +//ret3 = 773ab075-4cd7-4fc2-8b6e-21c84e9cb391 + * @endcode + */ + +int +virSearchRegex(const char *sourceString, + unsigned int occurrence, + const char *regexp, + char **result) +{ +regex_t pregUuidBracket; +size_t i = 0; +size_t nmatch = 0; +regmatch_t *pmatch = NULL; +int ret = -1; +int regError = -1; + +regError = regcomp(pregUuidBracket, regexp, REG_EXTENDED); +if (regError != 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Error while compiling regular expression: %d), + regError); +goto cleanup; +} +nmatch = pregUuidBracket.re_nsub; +if (VIR_ALLOC_N(pmatch, nmatch) 0) +goto cleanup; + +while (i (occurrence+1)) { +if (regexec(pregUuidBracket, sourceString, nmatch, pmatch, 0) == 0) { +regoff_t start = pmatch[0].rm_so; +regoff_t end = pmatch[0].rm_eo; +if (i == occurrence || +(occurrence i regexec(pregUuidBracket, sourceString[end], + nmatch, pmatch, 0) != 0)) { +/* We copy only if i == position (so that it is the uuid we're looking for), + * or position i AND there is no matches left in the rest of the string + * (this is the case where we give a biggest @occurence than the + * number of matches and we want to return the last one) + */ +if (VIR_STRNDUP(*result, sourceString + start, end - start) 0) +goto cleanup; + +ret = 1; +goto cleanup; +} +sourceString = sourceString[end]; +} else { +break; +ret = 0; +goto cleanup; +} +++i; +} + +cleanup: +regfree(pregUuidBracket); +VIR_FREE(pmatch); +return ret; +} diff --git a/src/util/virstring.h b/src/util/virstring.h index 13a6e5a..fe05403 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -226,4 +226,7 @@ size_t
[libvirt] [PATCHv6 0/5] Handling of undefine and redefine snapshots with VirtualBox 4.2
Hi, This is a serie of patches in order to support undefining and redefining snapshots with VirtualBox 4.2. The serie of patches is rather big, and adds among other things some utility functions unrelated to VirtualBox in patches 1 2. The code review could be done in several parts: e.g. patches 1 2 separately to validate the utility functions. The VirtualBox API provides only high level operations to manipulate snapshots, so it not possible to support flags like VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE and VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY with only API calls. Following an IRC talk with Eric Blake, the decision was taken to emulate these behaviours by manipulating directly the .vbox XML files. The first two patches are some util methods for handling regexp and strings that will be used after. The third patch brings more details in the snapshot XML returned by libvirt. We will need those modifications in order to redefine the snapshots. The fourth patch brings the support of the VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE and VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flags in virDomainSnapshotCreateXML. The fifth and last patch brings the support of the VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY flag in virDomainSnapshotDelete. The patches are only tested with Virtualbox 4.2 but the code is compliant with Virtualbox 4.3 API. Regards, Manuel VIVES v6: * Rebased because of a massive change in vbox_tmpl.c due to changes in the handling of different versions of VirtualBox v5: * The patches are modified according to a first review by Laine Stump: * renamed virSearchUuid to virSearchRegex and moved it from viruuid.{c,h} to virstring.{c,h}. * Various fixes. V4: * The code is compliant with Virtualbox 4.3 API * Some minor modifications in order to satisfy make syntax-check V3: * Use of STREQ_NULLABLE instead of STREQ in one case * Fix the method for finding uuids according to Ján Tomko review V2: * Fix a licence problem with the method for string replacement Manuel VIVES (5): virstring.h/c: Util method for finding regexp patterns in some strings virstring.h/c: Util method for making some find and replace in strings vbox_tmpl.c: Better XML description for snapshots vbox_tmpl.c: Patch for redefining snapshots vbox_tmpl.c: Add methods for undefining snapshots po/POTFILES.in |1 + src/libvirt_private.syms |2 + src/util/virstring.c | 163 +++- src/util/virstring.h |4 + src/vbox/vbox_tmpl.c | 2346 ++ 5 files changed, 2346 insertions(+), 170 deletions(-) -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv6 3/5] vbox_tmpl.c: Better XML description for snapshots
It will be needed for the future patches because we will redefine snapshots --- src/vbox/vbox_tmpl.c | 511 +- 1 file changed, 503 insertions(+), 8 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 1be4dc4..0d7809a 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -38,6 +38,7 @@ #include sys/types.h #include sys/stat.h #include fcntl.h +#include libxml/xmlwriter.h #include internal.h #include datatypes.h @@ -58,6 +59,8 @@ #include fdstream.h #include viruri.h #include virstring.h +#include virtime.h +#include virutil.h /* This one changes from version to version. */ #if VBOX_API_VERSION == 2002000 @@ -289,6 +292,13 @@ static void vboxDriverUnlock(vboxGlobalData *data) { virMutexUnlock(data-lock); } +typedef enum { +VBOX_STORAGE_DELETE_FLAG = 0, +#if VBOX_API_VERSION = 4002000 +VBOX_STORAGE_CLOSE_FLAG = 1, +#endif +} vboxStorageDeleteOrCloseFlags; + #if VBOX_API_VERSION == 2002000 static void nsIDtoChar(unsigned char *uuid, const nsID *iid) { @@ -5972,7 +5982,9 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL); if (!(def = virDomainSnapshotDefParseString(xmlDesc, data-caps, -data-xmlopt, 0, 0))) +data-xmlopt, -1, + VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | + VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE))) goto cleanup; if (def-ndisks) { @@ -6063,6 +6075,436 @@ cleanup: return ret; } +#if VBOX_API_VERSION =4002000 +static +int vboxSnapshotGetReadWriteDisks(virDomainSnapshotDefPtr def, +virDomainSnapshotPtr snapshot) +{ +virDomainPtr dom = snapshot-domain; +VBOX_OBJECT_CHECK(dom-conn, int, -1); +vboxIID domiid = VBOX_IID_INITIALIZER; +IMachine *machine = NULL; +ISnapshot *snap = NULL; +IMachine *snapMachine = NULL; +vboxArray mediumAttachments = VBOX_ARRAY_INITIALIZER; +PRUint32 maxPortPerInst[StorageBus_Floppy + 1] = {}; +PRUint32 maxSlotPerPort[StorageBus_Floppy + 1] = {}; +int diskCount = 0; +nsresult rc; +vboxIID snapIid = VBOX_IID_INITIALIZER; +char *snapshotUuidStr = NULL; +size_t i = 0; + +vboxIIDFromUUID(domiid, dom-uuid); +rc = VBOX_OBJECT_GET_MACHINE(domiid.value, machine); +if (NS_FAILED(rc)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(no domain with matching UUID)); +goto cleanup; +} +if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot-name))) +goto cleanup; + +rc = snap-vtbl-GetId(snap, snapIid.value); +if (NS_FAILED(rc)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Could not get snapshot id)); +goto cleanup; +} + +VBOX_UTF16_TO_UTF8(snapIid.value, snapshotUuidStr); +rc = snap-vtbl-GetMachine(snap, snapMachine); +if (NS_FAILED(rc)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(could not get machine)); +goto cleanup; +} +def-ndisks = 0; +rc = vboxArrayGet(mediumAttachments, snapMachine, snapMachine-vtbl-GetMediumAttachments); +if (NS_FAILED(rc)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(no medium attachments)); +goto cleanup; +} +/* get the number of attachments */ +for (i = 0; i mediumAttachments.count; i++) { +IMediumAttachment *imediumattach = mediumAttachments.items[i]; +if (imediumattach) { +IMedium *medium = NULL; + +rc = imediumattach-vtbl-GetMedium(imediumattach, medium); +if (NS_FAILED(rc)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(cannot get medium)); +goto cleanup; +} +if (medium) { +def-ndisks++; +VBOX_RELEASE(medium); +} +} +} +/* Allocate mem, if fails return error */ +if (VIR_ALLOC_N(def-disks, def-ndisks) 0) +goto cleanup; + +if (!vboxGetMaxPortSlotValues(data-vboxObj, maxPortPerInst, maxSlotPerPort)) +goto cleanup; + +/* get the attachment details here */ +for (i = 0; i mediumAttachments.count diskCount def-ndisks; i++) { +IStorageController *storageController = NULL; +PRUnichar *storageControllerName = NULL; +PRUint32 deviceType = DeviceType_Null; +PRUint32 storageBus = StorageBus_Null; +IMedium *disk = NULL; +PRUnichar *childLocUtf16 = NULL; +char *childLocUtf8 = NULL; +PRUint32 deviceInst = 0; +PRInt32devicePort = 0; +PRInt32deviceSlot = 0; +vboxArray
[libvirt] [PATCHv6 5/5] vbox_tmpl.c: Add methods for undefining snapshots
All the informations concerning snapshots (and snapshot disks) will be deleted from the vbox xml. But the differencing disks will be kept so you will be able to redefine the snapshots. --- src/vbox/vbox_tmpl.c | 448 +- 1 file changed, 447 insertions(+), 1 deletion(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 50b541d..6171a79 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -6176,6 +6176,68 @@ vboxSnapshotXmlAppendDiskToMediaRegistry(xmlNodePtr *inMediaRegistry, } } + +static void +vboxRemoveAllDisksExceptParentFromMediaRegistry(xmlNodePtr mediaRegistryNode){ +xmlNodePtr cur_node = NULL; +for (cur_node = mediaRegistryNode; cur_node; cur_node = cur_node-next) { +if (cur_node) { +if (cur_node-type == XML_ELEMENT_NODE + !xmlStrcmp(cur_node-name, (const xmlChar *) HardDisk)) { +xmlNodePtr child = NULL; +for (child = cur_node-children; child; child = child-next) { +/*We look over all the children + *If there is a node element, we delete it + */ +if (child-type == XML_ELEMENT_NODE) { +xmlUnlinkNode(child); +xmlFreeNode(child); +} +} +} +} +if ((cur_node-children)) + vboxRemoveAllDisksExceptParentFromMediaRegistry((cur_node-children)); +} +} + +static void +vboxRemoveDiskFromMediaRegistryIfNoChildren(xmlNodePtr mediaRegistryNode, +char *diskLocation) +{ +/* + *This function will remove a disk from the media registry only if it doesn't + *have any children + */ +xmlNodePtr cur_node = NULL; +for (cur_node = mediaRegistryNode; cur_node; cur_node = cur_node-next) { +if (cur_node) { +if (cur_node-type == XML_ELEMENT_NODE + !xmlStrcmp(cur_node-name, (const xmlChar *) HardDisk) + xmlHasProp(cur_node, BAD_CAST location) != NULL + strstr(diskLocation, (char *)xmlHasProp(cur_node, BAD_CAST location)-children-content) != NULL) { + +xmlNodePtr child = NULL; +bool deleteNode = true; +for (child = cur_node-children; child; child = child-next) { +/*We look over all the children + *If there is a node element, we don't delete it + */ +if (child-type == XML_ELEMENT_NODE) +deleteNode = false; +} +if (deleteNode) { +xmlUnlinkNode(cur_node); +xmlFreeNode(cur_node); +} +return; +} +} +if ((cur_node-children)) +vboxRemoveDiskFromMediaRegistryIfNoChildren((cur_node-children), diskLocation); +} +} + static int vboxSnapshotGenerateVboxXML(xmlNodePtr rootElementVboxXML, char *storageControllerString, @@ -8412,8 +8474,377 @@ cleanup: vboxArrayRelease(children); return ret; } +#if VBOX_API_VERSION = 4002000 +static int +vboxCloseDisk(virDomainPtr dom, + IMedium *baseDisk) { +VBOX_OBJECT_CHECK(dom-conn, int, -1); +nsresult rc; +vboxArray childrenDiskArray = VBOX_ARRAY_INITIALIZER; +size_t i = 0; +if (!baseDisk) +goto cleanup; + +rc = vboxArrayGet(childrenDiskArray, baseDisk, baseDisk-vtbl-GetChildren); +if (NS_FAILED(rc)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(could not get children disks)); +goto cleanup; +} +for (i = 0; i childrenDiskArray.count; ++i) +vboxCloseDisk(dom, childrenDiskArray.items[i]); + +baseDisk-vtbl-Close(baseDisk); +ret = 0; +cleanup: +vboxArrayRelease(childrenDiskArray); +return ret; +} static int +vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) +{ +/*This function will remove the node in the vbox xml corresponding to + *the snapshot. It is usually called by vboxDomainSnapshotDelete() with + *the flag VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY. + *If you want to use it anywhere else, be careful, if the snapshot you want to delete has children, + *the result is not granted, they will probably will be deleted in the xml, but you may have + *a problem with hard drives + * + *If the snapshot which is being deleted is the current, we will set the current snapshot of the machine to + *its parent. + * + *Before the writing of the modified xml file, we undefine the machine from vbox + *After the modification, we redefine the machine + */ + +virDomainPtr dom = snapshot-domain; +VBOX_OBJECT_CHECK(dom-conn, int, -1); +IMachine *machine = NULL; +IMachine
[libvirt] [PATCHv6 2/5] virstring.h/c: Util method for making some find and replace in strings
--- src/libvirt_private.syms |1 + src/util/virstring.c | 66 +- src/util/virstring.h |1 + 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 68ca39d..5103919 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1778,6 +1778,7 @@ virStringSortRevCompare; virStringSplit; virStrncpy; virStrndup; +virStrReplace; virStrToDouble; virStrToLong_i; virStrToLong_l; diff --git a/src/util/virstring.c b/src/util/virstring.c index 3c93450..b6f6192 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -618,7 +618,6 @@ size_t virStringListLength(char **strings) return i; } - /** * virStringSortCompare: * @@ -742,3 +741,68 @@ cleanup: VIR_FREE(pmatch); return ret; } + +/* + virStrReplace(haystack, oldneedle, newneedle) -- + Search haystack and replace all occurences of oldneedle with newneedle. + Return a string with all the replacements in case of success, NULL in case + of failure +*/ +char * +virStrReplace(char *haystack, + const char *oldneedle, const char *newneedle) +{ +char *ret = NULL; +size_t i = strlen(haystack); +size_t count = 0; +size_t newneedle_len = strlen(newneedle); +size_t oldneedle_len = strlen(oldneedle); +int diff_len = newneedle_len - oldneedle_len; +size_t totalLength = 0; +size_t numberOfElementsToAllocate = 0; +if (!haystack || !oldneedle || !newneedle) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(null value for haystack, oldneedle or newneedle)); +goto cleanup; +} +if (oldneedle_len == 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(oldneedle cannot be empty)); +goto cleanup; +} +if (diff_len == 0 memcmp(oldneedle, newneedle, newneedle_len) == 0) { +ignore_value(VIR_STRDUP(ret, haystack)); +goto cleanup; +} + +for (i = 0; haystack[i] != '\0'; i++) { +if (memcmp(haystack[i], oldneedle, oldneedle_len) == 0) { +count ++; +i += oldneedle_len - 1; +} +} +numberOfElementsToAllocate = (i + count * (newneedle_len - oldneedle_len)); + +if (VIR_ALLOC_N(ret, numberOfElementsToAllocate) 0) +goto cleanup; + +totalLength = sizeof(char) * numberOfElementsToAllocate; +i = 0; +while (*haystack) { +if (memcmp(haystack, oldneedle, oldneedle_len) == 0) { +if (virStrcpy(ret[i], newneedle, totalLength) == NULL) { +VIR_FREE(ret); +goto cleanup; +} +totalLength -= newneedle_len; +i += newneedle_len; +haystack += oldneedle_len; +} else { +ret[i++] = *haystack++; +totalLength --; +} +} +ret[i] = '\0'; +cleanup: +return ret; +} diff --git a/src/util/virstring.h b/src/util/virstring.h index fe05403..2df0695 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -227,6 +227,7 @@ int virStringSortCompare(const void *a, const void *b); int virStringSortRevCompare(const void *a, const void *b); int virSearchRegex(const char *sourceString, unsigned int occurrence, const char *regexp, char **result); +char * virStrReplace(char *haystack, const char *oldneedle, const char *newneedle); #endif /* __VIR_STRING_H__ */ -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv6 4/5] vbox_tmpl.c: Patch for redefining snapshots
The snapshots are saved in xml files, and then can be redefined --- src/vbox/vbox_tmpl.c | 1083 +- 1 file changed, 1075 insertions(+), 8 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 0d7809a..50b541d 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -61,6 +61,7 @@ #include virstring.h #include virtime.h #include virutil.h +#include dirname.h /* This one changes from version to version. */ #if VBOX_API_VERSION == 2002000 @@ -280,9 +281,19 @@ static vboxGlobalData *g_pVBoxGlobalData = NULL; #endif /* VBOX_API_VERSION = 400 */ +/*This error is a bit specific + *In the VBOX API it is named E_ACCESSDENIED + *It is returned when the called object is not ready. In + *particular when we do any call on a disk which has been closed +*/ +#define VBOX_E_ACCESSDENIED 0x80070005 static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml); static int vboxDomainCreate(virDomainPtr dom); static int vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags); +static virStorageVolPtr vboxStorageVolLookupByPath(virConnectPtr conn, const char *path); +static int vboxStorageDeleteOrClose(virStorageVolPtr vol, +unsigned int flags, +unsigned int flagDeleteOrClose); static void vboxDriverLock(vboxGlobalData *data) { virMutexLock(data-lock); @@ -5956,6 +5967,1055 @@ cleanup: return snapshot; } +#if VBOX_API_VERSION =4002000 +static void +vboxSnapshotXmlRetrieveSnapshotNodeByName(xmlNodePtr a_node, + const char *name, + xmlNodePtr *snap_node) +{ +xmlNodePtr cur_node = NULL; +char *xmlName; +for (cur_node = a_node; cur_node; cur_node = cur_node-next) { +if (cur_node-type == XML_ELEMENT_NODE) { +if (!xmlStrcmp(cur_node-name, (const xmlChar *) Snapshot)) { +if ((xmlName = virXMLPropString(cur_node, name))) { +if (STREQ(xmlName, name)) { +VIR_FREE(xmlName); +*snap_node = cur_node; +return; +} +VIR_FREE(xmlName); +} +} +} +if (cur_node-children) +vboxSnapshotXmlRetrieveSnapshotNodeByName(cur_node-children, name, + snap_node); +} +} + +static int +vboxDetachAndCloseDisks(virDomainPtr dom, +IMedium *disk) +{ +VBOX_OBJECT_CHECK(dom-conn, int, -1); +nsresult rc; +PRUnichar *location = NULL; +vboxArray childrenDiskArray = VBOX_ARRAY_INITIALIZER; +virStorageVolPtr volPtr = NULL; +char *location_utf8 = NULL; +PRUint32 dummyState = 0; +size_t i = 0; +if (disk == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(null pointer to disk)); +goto cleanup; +} +rc = disk-vtbl-GetLocation(disk, location); +if (rc == VBOX_E_ACCESSDENIED) { +ret = 0; +VIR_DEBUG(Disk already closed); +goto cleanup; +} +if (NS_FAILED(rc)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(cannot get disk location)); +goto cleanup; +} +rc = vboxArrayGet(childrenDiskArray, disk, disk-vtbl-GetChildren); +if (NS_FAILED(rc)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(cannot get children disks)); +goto cleanup; +} +for (i = 0; i childrenDiskArray.count; ++i) { +IMedium *childDisk = childrenDiskArray.items[i]; +if (childDisk) { +vboxDetachAndCloseDisks(dom, childDisk); +} +} +rc = disk-vtbl-RefreshState(disk, dummyState); +if (NS_FAILED(rc)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(cannot refresh state)); +goto cleanup; +} +VBOX_UTF16_TO_UTF8(location, location_utf8); +volPtr = vboxStorageVolLookupByPath(dom-conn, location_utf8); + +if (volPtr) { +VIR_DEBUG(Closing %s, location_utf8); +if (vboxStorageDeleteOrClose(volPtr, 0, VBOX_STORAGE_CLOSE_FLAG) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(error while closing disk)); +goto cleanup; +} +ret = 0; +} +cleanup: +VBOX_UTF8_FREE(location_utf8); +VBOX_UTF16_FREE(location); +vboxArrayRelease(childrenDiskArray); +return ret; +} + +static void +vboxSnapshotXmlAddChild(xmlNodePtr parent, +xmlNodePtr child) +{ +/*Used in order to add child without writing the stuff concerning xml namespaces*/ +xmlBufferPtr tmpBuf = xmlBufferCreate(); +char *tmpString = NULL; +xmlNodePtr tmpNode = NULL; +xmlNodeDump(tmpBuf, parent-doc, child, 0, 0); +if (VIR_STRDUP(tmpString, (char
Re: [libvirt] [PATCH 1/2] storage: Fix autostart of pool with fc_host type adapter
On 22/01/14 21:36, John Ferlan wrote: On 01/07/2014 06:07 AM, Osier Yang wrote: On 07/01/14 01:21, John Ferlan wrote: On 01/06/2014 05:19 AM, Osier Yang wrote: The checkPool is a bit different for pool with fc_host type source adapter, since the vHBA it's based on might be not created yet (it's created by startPool, which is involked after checkPool in storageDriverAutostart). So it should not fail, otherwise the autostart of the pool will fail either. The problem is easy to reproduce: * Enable autostart for the pool * Restart libvirtd service * Check the pool's state --- src/storage/storage_backend_scsi.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) Not sure this is the right thing to do. With this change it doesn't matter what getAdapterName() returns for fc_host's... It matters with if *isActive is true or false in the end. We don't need to try to start the pool if it's already active. Fair enough; however, let's consider the failure case. On failure, a message is reported and name == NULL. Do we need to clear that error now? I think my original thoughts were along the lines of having getAdapterName do more of the heavy lifting. Have both check and start call it. It would call the createVport which would be mostly unchanged, except for the virFileWaitForDevices() which could move to start... Found your method *might* break one logic. Assuming the pool is not marked as autostart, and the vHBA was not created yet. Since with your method checkPool will create the vHBA now, then it's possible for checkPool to return *isActive as true, as long as the device path for the created vHBA can show up in host very quickly (like what we talked a lot in PATCH 2/2, how long it will show up is also depended, the time can be long, but it also can be short), i.e. during the checkPool process. And thus the pool will be marked as Active. As the result, the problem on one side of the coin is fixed, but it introduces a similar problem on another side. :-) That's why I said previously I don't want to do any creation work in checkPool, it should only check something. So, I'm going forward to push my patch, with the last error reset in checkPool. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3] Add util virCgroupGetBlkioIo*Serviced methods.
On 01/20/2014 08:15 PM, Thorsten Behrens wrote: This reads blkio stats from blkio.throttle.io_service_bytes and blkio.throttle.io_serviced. --- Note on v3: - rebased to current master, sadly the virCgroupSetBlkioDeviceReadBps etc conflicted src/libvirt_private.syms | 2 + src/util/vircgroup.c | 242 +++ src/util/vircgroup.h | 12 +++ 3 files changed, 256 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3ede3d5..1e44ed8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1002,6 +1002,8 @@ virCgroupDenyDevice; virCgroupDenyDeviceMajor; virCgroupDenyDevicePath; virCgroupFree; +virCgroupGetBlkioIoDeviceServiced; +virCgroupGetBlkioIoServiced; virCgroupGetBlkioWeight; virCgroupGetCpuacctPercpuUsage; virCgroupGetCpuacctStat; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index a6d60c5..2b70bcb 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1786,6 +1786,221 @@ virCgroupPathOfController(virCgroupPtr group, /** + * virCgroupGetBlkioIoServiced: + * + * @group: The cgroup to get throughput for + * @bytes_read: Pointer to returned bytes read + * @bytes_write: Pointer to returned bytes written + * @requests_read: Pointer to returned read io ops + * @requests_write: Pointer to returned write io ops + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupGetBlkioIoServiced(virCgroupPtr group, +long long *bytes_read, +long long *bytes_write, +long long *requests_read, +long long *requests_write) +{ +long long stats_val; +char *str1 = NULL, *str2 = NULL, *p1, *p2; +size_t i; +int ret = -1; + +const char *value_names[] = { +Read , +Write +}; +long long *bytes_ptrs[] = { +bytes_read, +bytes_write +}; +long long *requests_ptrs[] = { +requests_read, +requests_write +}; + +*bytes_read = 0; +*bytes_write = 0; +*requests_read = 0; +*requests_write = 0; + +if (virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + blkio.throttle.io_service_bytes, str1) 0) +goto cleanup; + +if (virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + blkio.throttle.io_serviced, str2) 0) +goto cleanup; + +/* sum up all entries of the same kind, from all devices */ +for (i = 0; i ARRAY_CARDINALITY(value_names); i++) { +p1 = str1; +p2 = str2; + +while ((p1 = strstr(p1, value_names[i]))) { +p1 += strlen(value_names[i]); +if (virStrToLong_ll(p1, p1, 10, stats_val) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Cannot parse byte %sstat '%s'), + value_names[i], + p1); +goto cleanup; +} + +if (stats_val 0 || +(stats_val 0 *bytes_ptrs[i] (LLONG_MAX - stats_val))) +{ +virReportError(VIR_ERR_OVERFLOW, + _(Sum of byte %sstat overflows), + value_names[i]); +goto cleanup; +} +*bytes_ptrs[i] += stats_val; +} + +while ((p2 = strstr(p2, value_names[i]))) { +p2 += strlen(value_names[i]); +if (virStrToLong_ll(p2, p2, 10, stats_val) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Cannot parse %srequest stat '%s'), + value_names[i], + p2); +goto cleanup; +} + +if (stats_val 0 || +(stats_val 0 *requests_ptrs[i] (LLONG_MAX - stats_val))) +{ +virReportError(VIR_ERR_OVERFLOW, + _(Sum of %srequest stat overflows), + value_names[i]); +goto cleanup; +} +*requests_ptrs[i] += stats_val; +} +} + +ret = 0; + +cleanup: +VIR_FREE(str2); +VIR_FREE(str1); +return ret; +} + + +/** + * virCgroupGetBlkioIoDeviceServiced: + * + * @group: The cgroup to get throughput for + * @path: The device to get throughput for + * @bytes_read: Pointer to returned bytes read + * @bytes_write: Pointer to returned bytes written + * @requests_read: Pointer to returned read io ops + * @requests_write: Pointer to returned write io ops + * + * Returns: 0 on success, -1 on error + */ +int
Re: [libvirt] [PATCH v3] virsh nodecpustats returns incorrect stats of cpu on Linux when the number of online cpu exceed 9.
On 01/21/2014 06:21 AM, m...@linux.vnet.ibm.com wrote: From: Bing Bu Cao m...@linux.vnet.ibm.com To retrieve node cpu statistics on Linux system, the linuxNodeGetCPUstats function simply uses STRPREFIX() to match the cpuid with the cpuid read from /proc/stat, it will cause obvious error. For example: 'virsh nodecpustats 1' will display stats of cpu1* if the latter is online and cpu1 is offline. This patch fixes this bug. Signed-off-by: Bing Bu Cao m...@linux.vnet.ibm.com --- src/nodeinfo.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 05bc038..cf6d29b 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -691,7 +691,7 @@ linuxNodeGetCPUStats(FILE *procstat, char line[1024]; unsigned long long usr, ni, sys, idle, iowait; unsigned long long irq, softirq, steal, guest, guest_nice; -char cpu_header[3 + INT_BUFSIZE_BOUND(cpuNum)]; +char cpu_header[4 + INT_BUFSIZE_BOUND(cpuNum)]; if ((*nparams) == 0) { /* Current number of cpu stats supported by linux */ I have pushed this hunk, Michal pushed the rest from version 2 of the patch. Could you please take a look at the test I've posted here: https://www.redhat.com/archives/libvir-list/2014-January/msg01022.html and extend it with the /proc/stat file from the machine where you saw the failure? Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util: Add shareable field for virSCSIDevice struct
On 16/01/14 08:51, John Ferlan wrote: On 01/08/2014 09:51 AM, Osier Yang wrote: ... diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 751eaf0..3998c3a 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -58,6 +58,7 @@ struct _virSCSIDevice { const char *used_by; /* name of the domain using this dev */ bool readonly; +bool shareable; }; struct _virSCSIDeviceList { @@ -185,7 +186,8 @@ virSCSIDeviceNew(const char *adapter, unsigned int bus, unsigned int target, unsigned int unit, - bool readonly) + bool readonly, + bool shareable) { virSCSIDevicePtr dev, ret = NULL; char *sg = NULL; @@ -201,6 +203,7 @@ virSCSIDeviceNew(const char *adapter, dev-target = target; dev-unit = unit; dev-readonly = readonly; +dev-shareable= shareable; You still didn't add the space here before the = ACK if you do. I don't believe this is 1.2.1 material. This patch is standalone. Pushed with the indention fixed. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/6] Implement domainMemoryStats API slot for LXC driver.
On 01/20/2014 07:12 PM, Thorsten Behrens wrote: --- Notes on v2: - check if domain is running, fixed ret val calculation - api slot comment is now referencing 1.2.2 src/lxc/lxc_driver.c | 50 ++ 1 file changed, 50 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 5ae4b65..8cf8e48 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4556,6 +4556,55 @@ lxcNodeGetInfo(virConnectPtr conn, static int +lxcDomainMemoryStats(virDomainPtr dom, + struct _virDomainMemoryStat *stats, + unsigned int nr_stats, + unsigned int flags) +{ +virDomainObjPtr vm; +int ret = -1; +virLXCDomainObjPrivatePtr priv; + +virCheckFlags(0, -1); + +if (!(vm = lxcDomObjFromDomain(dom))) +goto cleanup; + +priv = vm-privateData; + +if (virDomainMemoryStatsEnsureACL(dom-conn, vm-def) 0) +goto cleanup; + +ret = 0; +if (!virDomainObjIsActive(vm)) +goto cleanup; + +if (ret nr_stats) { +stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON; +stats[ret].val = vm-def-mem.cur_balloon; +ret++; +} +if (ret nr_stats) { +stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_IN; +virCgroupGetMemSwapUsage(priv-cgroup, stats[ret].val); +ret++; +} +if (ret nr_stats) { +unsigned long kb; +stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS; +virCgroupGetMemoryUsage(priv-cgroup, kb); +stats[ret].val = kb; +ret++; +} + +cleanup: +if (vm) +virObjectUnlock(vm); +return ret; +} + + +static int lxcNodeGetCPUStats(virConnectPtr conn, int cpuNum, virNodeCPUStatsPtr params, @@ -4783,6 +4832,7 @@ static virDriver lxcDriver = { .domainSetSchedulerParameters = lxcDomainSetSchedulerParameters, /* 0.5.0 */ .domainSetSchedulerParametersFlags = lxcDomainSetSchedulerParametersFlags, /* 0.9.2 */ .domainInterfaceStats = lxcDomainInterfaceStats, /* 0.7.3 */ +.domainMemoryStats = lxcDomainMemoryStats, /* 1.2.2 */ .nodeGetCPUStats = lxcNodeGetCPUStats, /* 0.9.3 */ .nodeGetMemoryStats = lxcNodeGetMemoryStats, /* 0.9.3 */ .nodeGetCellsFreeMemory = lxcNodeGetCellsFreeMemory, /* 0.6.5 */ looks good to me ACK thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/6] Make qemuGetDomainTotalCPUStats a virCgroup function.
On 01/20/2014 07:12 PM, Thorsten Behrens wrote: To reuse this from other drivers, like lxc. --- Notes on v2: - renamed to proper camel case: virCgroupGetDomainTotalCpuStats src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 54 ++-- src/util/vircgroup.c | 53 +++ src/util/vircgroup.h | 5 + 4 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e4815b3..97198d1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1013,6 +1013,7 @@ virCgroupGetCpuCfsQuota; virCgroupGetCpusetCpus; virCgroupGetCpusetMems; virCgroupGetCpuShares; +virCgroupGetDomainTotalCpuStats; virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ebb77dc..a1e45c3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -105,7 +105,6 @@ #define QEMU_NB_NUMA_PARAM 2 -#define QEMU_NB_TOTAL_CPU_STAT_PARAM 3 #define QEMU_NB_PER_CPU_STAT_PARAM 2 #define QEMU_SCHED_MIN_PERIOD 1000LL @@ -15304,56 +15303,6 @@ cleanup: return ret; } -/* qemuDomainGetCPUStats() with start_cpu == -1 */ -static int -qemuDomainGetTotalcpuStats(virDomainObjPtr vm, - virTypedParameterPtr params, - int nparams) -{ -unsigned long long cpu_time; -int ret; -qemuDomainObjPrivatePtr priv = vm-privateData; - -if (nparams == 0) /* return supported number of params */ -return QEMU_NB_TOTAL_CPU_STAT_PARAM; -/* entry 0 is cputime */ -ret = virCgroupGetCpuacctUsage(priv-cgroup, cpu_time); -if (ret 0) { -virReportSystemError(-ret, %s, _(unable to get cpu account)); -return -1; -} - -if (virTypedParameterAssign(params[0], VIR_DOMAIN_CPU_STATS_CPUTIME, -VIR_TYPED_PARAM_ULLONG, cpu_time) 0) -return -1; - -if (nparams 1) { -unsigned long long user; -unsigned long long sys; - -ret = virCgroupGetCpuacctStat(priv-cgroup, user, sys); -if (ret 0) { -virReportSystemError(-ret, %s, _(unable to get cpu account)); -return -1; -} - -if (virTypedParameterAssign(params[1], -VIR_DOMAIN_CPU_STATS_USERTIME, -VIR_TYPED_PARAM_ULLONG, user) 0) -return -1; -if (nparams 2 -virTypedParameterAssign(params[2], -VIR_DOMAIN_CPU_STATS_SYSTEMTIME, -VIR_TYPED_PARAM_ULLONG, sys) 0) -return -1; - -if (nparams QEMU_NB_TOTAL_CPU_STAT_PARAM) -nparams = QEMU_NB_TOTAL_CPU_STAT_PARAM; -} - -return nparams; -} - /* This function gets the sums of cpu time consumed by all vcpus. * For example, if there are 4 physical cpus, and 2 vcpus in a domain, * then for each vcpu, the cpuacct.usage_percpu looks like this: @@ -15552,7 +15501,8 @@ qemuDomainGetCPUStats(virDomainPtr domain, } if (start_cpu == -1) -ret = qemuDomainGetTotalcpuStats(vm, params, nparams); +ret = virCgroupGetDomainTotalCpuStats(priv-cgroup, + params, nparams); else ret = qemuDomainGetPercpuStats(vm, params, nparams, start_cpu, ncpus); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 83fcefc..66045fb 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -51,11 +51,14 @@ #include virhashcode.h #include virstring.h #include virsystemd.h +#include virtypedparam.h #define CGROUP_MAX_VAL 512 #define VIR_FROM_THIS VIR_FROM_CGROUP +#define CGROUP_NB_TOTAL_CPU_STAT_PARAM 3 + #if defined(__linux__) defined(HAVE_GETMNTENT_R) \ defined(_DIRENT_HAVE_D_TYPE) defined(_SC_CLK_TCK) # define VIR_CGROUP_SUPPORTED @@ -2629,6 +2632,56 @@ virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int perms) } + +int +virCgroupGetDomainTotalCpuStats(virCgroupPtr group, +virTypedParameterPtr params, +int nparams) +{ +unsigned long long cpu_time; +int ret; + +if (nparams == 0) /* return supported number of params */ +return CGROUP_NB_TOTAL_CPU_STAT_PARAM; +/* entry 0 is cputime */ +ret = virCgroupGetCpuacctUsage(group, cpu_time); +if (ret 0) { +virReportSystemError(-ret, %s, _(unable to get cpu account)); +return -1; +} + +if (virTypedParameterAssign(params[0], VIR_DOMAIN_CPU_STATS_CPUTIME, +VIR_TYPED_PARAM_ULLONG, cpu_time)
[libvirt] [PATCH] qemu: remove memset params array to zero in qemuDomainGetPercpuStats
the array params is allocated by VIR_ALLOC_N in remoteDispatchDomainGetCPUStats. it had been set to zero. No need to reset it to zero again, and this reset here is incorrect too, nparams * ncpus is the array length not the size of params array. Signed-off-by: Gao feng gaof...@cn.fujitsu.com --- src/qemu/qemu_driver.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index df4f5b5..1e54164 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15818,7 +15818,6 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm, if (virCgroupGetCpuacctPercpuUsage(priv-cgroup, buf)) goto cleanup; pos = buf; -memset(params, 0, nparams * ncpus); /* return percpu cputime in index 0 */ param_idx = 0; -- 1.8.4.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Build failed in Jenkins: libvirt-syntax-check #1898
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/1898/ -- Started by upstream project libvirt-build build number 2132 Started by upstream project libvirt-build build number 2133 Building on master in workspace http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/ws/ [workspace] $ /bin/sh -xe /tmp/hudson9098701041827209295.sh + make syntax-check GENbracket-spacing-check src/util/virscsi.c:206: dev-shareable= shareable; maint.mk: incorrect whitespace, see HACKING for rules make: *** [bracket-spacing-check] Error 1 Build step 'Execute shell' marked build as failure -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Build failed in Jenkins: libvirt-syntax-check #1898
On 23/01/14 18:11, Jenkins CI wrote: See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/1898/ -- Started by upstream project libvirt-build build number 2132 Started by upstream project libvirt-build build number 2133 Building on master in workspace http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/ws/ [workspace] $ /bin/sh -xe /tmp/hudson9098701041827209295.sh + make syntax-check GENbracket-spacing-check src/util/virscsi.c:206: dev-shareable= shareable; Oh, no, I actually missed push the change. Pushing it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 4/6] Implement domainGetCPUStats for lxc driver.
On 01/20/2014 07:12 PM, Thorsten Behrens wrote: --- Notes on v2: - elided extra memset and leftover loop var n - api slot comment references 1.2.2 now src/lxc/lxc_driver.c | 128 +++ 1 file changed, 128 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8cf8e48..19426f5 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -75,6 +75,8 @@ #define LXC_NB_MEM_PARAM 3 +#define LXC_NB_PER_CPU_STAT_PARAM 1 + static int lxcStateInitialize(bool privileged, virStateInhibitCallback callback, @@ -4775,6 +4777,131 @@ cleanup: } +static int +lxcDomainGetPercpuStats(virDomainObjPtr vm, +virTypedParameterPtr params, +unsigned int nparams, +int start_cpu, +unsigned int ncpus) +{ +int rv = -1; +size_t i; +int id, max_id; +char *pos; +char *buf = NULL; +virLXCDomainObjPrivatePtr priv = vm-privateData; +virTypedParameterPtr ent; +int param_idx; +unsigned long long cpu_time; + +/* TODO: share api contract code with other drivers here */ + +/* return the number of supported params */ +if (nparams == 0 ncpus != 0) +return LXC_NB_PER_CPU_STAT_PARAM; + +/* To parse account file, we need to know how many cpus are present. */ +max_id = nodeGetCPUCount(); +if (max_id 0) +return rv; + +if (ncpus == 0) { /* returns max cpu ID */ +rv = max_id; +goto cleanup; +} + +if (start_cpu max_id) { +virReportError(VIR_ERR_INVALID_ARG, + _(start_cpu %d larger than maximum of %d), + start_cpu, max_id); +goto cleanup; +} + +/* we get percpu cputime accounting info. */ +if (virCgroupGetCpuacctPercpuUsage(priv-cgroup, buf)) +goto cleanup; +pos = buf; + +/* return percpu cputime in index 0 */ +param_idx = 0; + +/* number of cpus to compute */ +if (start_cpu = max_id - ncpus) +id = max_id - 1; +else +id = start_cpu + ncpus - 1; + +for (i = 0; i = id; i++) { +if (virStrToLong_ull(pos, pos, 10, cpu_time) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(cpuacct parse error)); +goto cleanup; +} +if (i start_cpu) +continue; +ent = params[(i - start_cpu) * nparams + param_idx]; +if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME, +VIR_TYPED_PARAM_ULLONG, cpu_time) 0) +goto cleanup; +} + +rv = nparams; + +cleanup: +VIR_FREE(buf); +return rv; +} + + +static int +lxcDomainGetCPUStats(virDomainPtr dom, + virTypedParameterPtr params, + unsigned int nparams, + int start_cpu, + unsigned int ncpus, + unsigned int flags) +{ +virDomainObjPtr vm = NULL; +int ret = -1; +bool isActive; +virLXCDomainObjPrivatePtr priv; + +virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); + +if (!(vm = lxcDomObjFromDomain(dom))) +return ret; + +priv = vm-privateData; + +if (virDomainGetCPUStatsEnsureACL(dom-conn, vm-def) 0) +goto cleanup; + +isActive = virDomainObjIsActive(vm); +if (!isActive) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(domain is not running)); +goto cleanup; +} + +if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) { +virReportError(VIR_ERR_OPERATION_INVALID, + %s, _(cgroup CPUACCT controller is not mounted)); +goto cleanup; +} + +if (start_cpu == -1) +ret = virCgroupGetDomainTotalCpuStats(priv-cgroup, + params, nparams); +else +ret = lxcDomainGetPercpuStats(vm, params, nparams, + start_cpu, ncpus); +cleanup: +if (vm) +virObjectUnlock(vm); +return ret; +} + + /* Function Tables */ static virDriver lxcDriver = { .no = VIR_DRV_LXC, @@ -4852,6 +4979,7 @@ static virDriver lxcDriver = { .nodeSuspendForDuration = lxcNodeSuspendForDuration, /* 0.9.8 */ .domainSetMetadata = lxcDomainSetMetadata, /* 1.1.3 */ .domainGetMetadata = lxcDomainGetMetadata, /* 1.1.3 */ +.domainGetCPUStats = lxcDomainGetCPUStats, /* 1.2.2 */ .nodeGetMemoryParameters = lxcNodeGetMemoryParameters, /* 0.10.2 */ .nodeSetMemoryParameters = lxcNodeSetMemoryParameters, /* 0.10.2 */ .domainSendProcessSignal = lxcDomainSendProcessSignal, /* 1.0.1 */ Looks good to me ACK thanks! --
Re: [libvirt] [PATCHv2 5/6] Implemet lxcDomainBlockStats for lxc driver
On 01/20/2014 07:12 PM, Thorsten Behrens wrote: --- Notes on v2: - works as-is, will send lxcDomainBlockStatsFlags patch separately src/lxc/lxc_driver.c | 51 +++ 1 file changed, 51 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 19426f5..bf6fd5c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2023,6 +2023,56 @@ lxcDomainGetSchedulerParameters(virDomainPtr domain, static int +lxcDomainBlockStats(virDomainPtr dom, +const char *path, +struct _virDomainBlockStats *stats) +{ +int ret = -1, idx; +virDomainObjPtr vm; +virDomainDiskDefPtr disk = NULL; +virLXCDomainObjPrivatePtr priv; + +if (!(vm = lxcDomObjFromDomain(dom))) +return ret; + +priv = vm-privateData; + +if (virDomainBlockStatsEnsureACL(dom-conn, vm-def) 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + %s, _(domain is not running)); +goto cleanup; +} + +if ((idx = virDomainDiskIndexByName(vm-def, path, false)) 0) { +virReportError(VIR_ERR_INVALID_ARG, + _(invalid path: %s), path); +goto cleanup; +} +disk = vm-def-disks[idx]; + +if (!disk-info.alias) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(missing disk device alias name for %s), disk-dst); +goto cleanup; +} + +ret = virCgroupGetBlkioIoDeviceServiced(priv-cgroup, +disk-info.alias, +stats-rd_bytes, +stats-wr_bytes, +stats-rd_req, +stats-wr_req); +cleanup: +if (vm) +virObjectUnlock(vm); +return ret; +} + + +static int lxcDomainSetBlkioParameters(virDomainPtr dom, virTypedParameterPtr params, int nparams, @@ -4958,6 +5008,7 @@ static virDriver lxcDriver = { .domainGetSchedulerParametersFlags = lxcDomainGetSchedulerParametersFlags, /* 0.9.2 */ .domainSetSchedulerParameters = lxcDomainSetSchedulerParameters, /* 0.5.0 */ .domainSetSchedulerParametersFlags = lxcDomainSetSchedulerParametersFlags, /* 0.9.2 */ +.domainBlockStats = lxcDomainBlockStats, /* 0.4.1 */ this should be 1.2.2 .domainInterfaceStats = lxcDomainInterfaceStats, /* 0.7.3 */ .domainMemoryStats = lxcDomainMemoryStats, /* 1.2.2 */ .nodeGetCPUStats = lxcNodeGetCPUStats, /* 0.9.3 */ ACK thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] daemon: Introduce max_anonymous_clients
On Mon, Dec 09, 2013 at 03:35:53PM +0100, Michal Privoznik wrote: https://bugzilla.redhat.com/show_bug.cgi?id=981729 This config tunable allows users to determine the maximum number of accepted but yet not authenticated users. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- daemon/libvirtd-config.c| 1 + daemon/libvirtd-config.h| 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 1 + daemon/libvirtd.conf| 3 +++ daemon/test_libvirtd.aug.in | 1 + src/locking/lock_daemon.c | 4 ++-- src/lxc/lxc_controller.c| 2 +- src/rpc/virnetserver.c | 37 +++-- src/rpc/virnetserver.h | 1 + 10 files changed, 47 insertions(+), 5 deletions(-) diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c index c816fda..04482c5 100644 --- a/daemon/libvirtd-config.c +++ b/daemon/libvirtd-config.c @@ -415,6 +415,7 @@ daemonConfigLoadOptions(struct daemonConfig *data, GET_CONF_INT(conf, filename, max_workers); GET_CONF_INT(conf, filename, max_clients); GET_CONF_INT(conf, filename, max_queued_clients); +GET_CONF_INT(conf, filename, max_anonymous_clients); GET_CONF_INT(conf, filename, prio_workers); diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h index a24d5d2..66dc80b 100644 --- a/daemon/libvirtd-config.h +++ b/daemon/libvirtd-config.h @@ -64,6 +64,7 @@ struct daemonConfig { int max_workers; int max_clients; int max_queued_clients; +int max_anonymous_clients; int prio_workers; diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug index 70fce5c..5a0807c 100644 --- a/daemon/libvirtd.aug +++ b/daemon/libvirtd.aug @@ -57,6 +57,7 @@ module Libvirtd = | int_entry max_workers | int_entry max_clients | int_entry max_queued_clients +| int_entry max_anonymous_clients | int_entry max_requests | int_entry max_client_requests | int_entry prio_workers diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 49c42ad..61c706c 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1367,6 +1367,7 @@ int main(int argc, char **argv) { config-max_workers, config-prio_workers, config-max_clients, +config-max_anonymous_clients, config-keepalive_interval, config-keepalive_count, !!config-keepalive_required, diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index 5353927..28d3735 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -263,6 +263,9 @@ # connection succeeds. #max_queued_clients = 1000 +# The maximum length of queue of accepted but not yet not +# authenticated clients. +#max_anonymous_clients = 20 Please mention what's the default here. # The minimum limit sets the number of workers to start up # initially. If the number of active clients exceeds this, diff --git a/daemon/test_libvirtd.aug.in b/daemon/test_libvirtd.aug.in index a7e8515..b03451c 100644 --- a/daemon/test_libvirtd.aug.in +++ b/daemon/test_libvirtd.aug.in @@ -36,6 +36,7 @@ module Test_libvirtd = } { max_clients = 20 } { max_queued_clients = 1000 } +{ max_anonymous_clients = 20 } { min_workers = 5 } { max_workers = 20 } { prio_workers = 5 } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 35ccb4e..c97bc61 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -143,8 +143,8 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) } if (!(lockd-srv = virNetServerNew(1, 1, 0, config-max_clients, - -1, 0, - false, NULL, + config-max_clients, + -1, 0, false, NULL, Since the default (when there's nothing in the config) is 0, it should mean the feature is disabled). If that's what you've intended, wouldn't it make more sense to use 0 so it's visible that the feature is not in effect? virLockDaemonClientNew, virLockDaemonClientPreExecRestart, virLockDaemonClientFree, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index c013147..578a135 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -736,7 +736,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) LXC_STATE_DIR, ctrl-name) 0) return -1; -if
[libvirt] [libvirt-java] state of affairs
Hi. It seems the Java wrapper is nearly dead. It has fallen way behind libvirt development. [in my local tree, there're still 120 libvirt API functions missing from the Java interface which /probably/ are worth to be added] I have send a few patches to the list, but no one is willing / able to review them. Some of those patches date almost a year back. Slowly I'm getting a bit frustrated and maybe also a tad impatient... Currently, I'm having +60 commits in my local git tree. As you might imagine, I'd really like to get these off my back. That's why I'm asking myself whether the ACKing / NACKing of patches is the right model for libvirt-java, given that there is, apparently, very little interest and at the same time next to nobody with good Java expertise on the list. Additionally, there are a few glitches in the API which are a thorn in my side ever since I began using the libvirt Java wrapper. It's obvious that the wrapper was written without much thinking about the Java environment and API. Some functions have only been wrapped just because it was possible or perhaps to just have a full coverage of libvirt version x.y.z, without any real use for a Java programmer. Do we really have to live with the failures of the past? I'd really like to fix these even if that means *breaking* the API. IMO, this would not be that bad in the Java world. It's not like that you suddenly happen to have an updated dynamic library on the system that's missing some symbols or has it's ABI changed which makes your program crash. Java libraries come with the API bundled and you get an error at the earliest time possible - at compilation time. Even if you upgrade a jar without recompiling your program you'll get a nice RuntimeException instead of undefined behavior. So, I'd say just bump the major or minor version up to the next number and send out a big SORRY, we messed up to everyone and be done with it. Alternatively, change the package and the artifact name to libvirt2 effectively creating some sort of fork?! But, given that there's not much review on list, failures are likely to happen again and the same situation would arise anew. Thank you. Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:http://www.av-test.org Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage: Fix the memory leak
The return value of virGetFCHostNameByWWN is a strdup'ed string. Also add comments to declare that the caller should take care of freeing it. --- src/storage/storage_backend_scsi.c | 15 ++- src/util/virutil.c | 5 - 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index fce2bae..b38e530 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -668,6 +668,8 @@ static int deleteVport(virStoragePoolSourceAdapter adapter) { unsigned int parent_host; +char *name = NULL; +int ret = -1; if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) return 0; @@ -680,18 +682,21 @@ deleteVport(virStoragePoolSourceAdapter adapter) if (!adapter.data.fchost.parent) return 0; -if (!(virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn, -adapter.data.fchost.wwpn))) +if (!(name = virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn, + adapter.data.fchost.wwpn))) return -1; if (getHostNumber(adapter.data.fchost.parent, parent_host) 0) -return -1; +goto cleanup; if (virManageVport(parent_host, adapter.data.fchost.wwpn, adapter.data.fchost.wwnn, VPORT_DELETE) 0) -return -1; +goto cleanup; -return 0; +ret = 0; +cleanup: +VIR_FREE(name); +return ret; } diff --git a/src/util/virutil.c b/src/util/virutil.c index 87cc2e7..7a2fbb0 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1820,7 +1820,10 @@ cleanup: /* virGetHostNameByWWN: * * Iterate over the sysfs tree to get FC host name (e.g. host5) - * by wwnn,wwpn pair. + * by the provided wwnn,wwpn pair. + * + * Returns the FC host name which must be freed by the caller, + * or NULL on failure. */ char * virGetFCHostNameByWWN(const char *sysfs_prefix, -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 6/6] Widening API change - accept empty path for virDomainBlockStats
On 01/20/2014 07:12 PM, Thorsten Behrens wrote: And provide domain summary stat in that case, for lxc backend. Use case is a container inheriting all devices from the host, e.g. when doing application containerization. --- Notes on v2: - adapted virDomainBlockStats docs - adapted virsh domblkstat docs - made virsh actually accept empty disk argument - pedaling back a bit on the API change - accepting a NULL ptr for the disk arg would need changing the remote protocol, so better just take the empty string. Makes this less invasive even. src/libvirt.c| 8 ++-- src/lxc/lxc_driver.c | 10 ++ tools/virsh-domain-monitor.c | 11 --- tools/virsh.pod | 5 +++-- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 87a4d46..ead0813 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -7781,7 +7781,9 @@ error: * an unambiguous source name of the block device (the source * file='...'/ sub-element, such as /path/to/image). Valid names * can be found by calling virDomainGetXMLDesc() and inspecting - * elements within //domain/devices/disk. + * elements within //domain/devices/disk. Some drivers might also + * accept the empty string for the @disk parameter, and then yield + * summary stats for the entire domain. * * Domains may have more than one block device. To get stats for * each you should make multiple calls to this function. @@ -7847,7 +7849,9 @@ error: * an unambiguous source name of the block device (the source * file='...'/ sub-element, such as /path/to/image). Valid names * can be found by calling virDomainGetXMLDesc() and inspecting - * elements within //domain/devices/disk. + * elements within //domain/devices/disk. Some drivers might also + * accept the empty string for the @disk parameter, and then yield + * summary stats for the entire domain. * * Domains may have more than one block device. To get stats for * each you should make multiple calls to this function. diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index bf6fd5c..31f1625 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2046,6 +2046,16 @@ lxcDomainBlockStats(virDomainPtr dom, goto cleanup; } +if (!*path) { +/* empty path - return entire domain blkstats instead */ +ret = virCgroupGetBlkioIoServiced(priv-cgroup, + stats-rd_bytes, + stats-wr_bytes, + stats-rd_req, + stats-wr_req); +goto cleanup; +} + if ((idx = virDomainDiskIndexByName(vm-def, path, false)) 0) { virReportError(VIR_ERR_INVALID_ARG, _(invalid path: %s), path); diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index b29b82a..6be253f 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -880,7 +880,7 @@ static const vshCmdOptDef opts_domblkstat[] = { }, {.name = device, .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ, + .flags = VSH_OFLAG_EMPTY_OK, .help = N_(block device) }, {.name = human, @@ -946,8 +946,13 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, name))) return false; -if (vshCommandOptStringReq(ctl, cmd, device, device) 0) -goto cleanup; +/* device argument is optional now. if it's missing, supply empty + string to denote 'all devices'. A NULL device arg would violate + API contract. + */ +rc = vshCommandOptStringReq(ctl, cmd, device, device); /* and ignore rc */ +if (!device) +device = ; rc = virDomainBlockStatsFlags(dom, device, NULL, nparams, 0); diff --git a/tools/virsh.pod b/tools/virsh.pod index 3534b54..c3ca016 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -616,12 +616,13 @@ If I--graceful is specified, don't resort to extreme measures (e.g. SIGKILL) when the guest doesn't stop after a reasonable timeout; return an error instead. -=item Bdomblkstat Idomain Iblock-device [I--human] +=item Bdomblkstat Idomain [Iblock-device] [I--human] Get device block stats for a running domain. A Iblock-device corresponds to a unique target name (target dev='name'/) or source file (source file='name'/) for one of the disk devices attached to Idomain (see -also Bdomblklist for listing these names). +also Bdomblklist for listing these names). On a lxc domain, omitting the +Iblock-device yields device block stats summarily for the entire domain. Use I--human for a more human readable output. Looks good to me ACK thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] closing old maint branches [was: [v0.9.12-maint 0/8] Backport changes for CVE-2013-6458 to v0.9.12-maint]
On Wed, Jan 22, 2014 at 12:13:48PM -0700, Eric Blake wrote: On 01/15/2014 01:43 PM, Eric Blake wrote: Is anyone still using v0.9.11-maint? The CVE extends back to 0.9.8, so we could argue that we should either fix the 0.9.11 branch, or add another commit to the branch that explicitly marks it as end-of-life because no one appears to be relying on it. Fedora 18 is now end-of-life, so from Fedora's perspective, I only care about 0.10.2 (RHEL and CentOS 6), 1.0.5 (F19), 1.1.3 (F20) and soon 1.2.1 (rawhide), although I didn't mind touching all the intermediate branches on my way down to 0.10.2. RHEL 5 is also vulnerable to CVE-2013-6458, but as we don't have an upstream v0.8.2-maint branch (thank goodness!), that's something for Red Hat to worry about. I've gone ahead and marked v0.8.3-maint and v0.9.11-maint as closed (I'm not posting the actual patch here, but it was done by 'git rm -f \*' followed by recreating .gitignore and a placeholder README that mentions the death of the branch). FYI for openstack I examined the current libvirt versions in some major distros: https://wiki.openstack.org/wiki/LibvirtDistroSupportMatrix Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: remove memset params array to zero in qemuDomainGetPercpuStats
On Thu, Jan 23, 2014 at 06:00:18PM +0800, Gao feng wrote: the array params is allocated by VIR_ALLOC_N in remoteDispatchDomainGetCPUStats. it had been set to zero. No need to reset it to zero again, and this reset here is incorrect too, nparams * ncpus is the array length not the size of params array. Signed-off-by: Gao feng gaof...@cn.fujitsu.com --- src/qemu/qemu_driver.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index df4f5b5..1e54164 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15818,7 +15818,6 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm, if (virCgroupGetCpuacctPercpuUsage(priv-cgroup, buf)) goto cleanup; pos = buf; -memset(params, 0, nparams * ncpus); /* return percpu cputime in index 0 */ param_idx = 0; ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Jenkins build is back to normal : libvirt-syntax-check #1899
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/1899/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-java] state of affairs
On Thu, Jan 23, 2014 at 11:20:46AM +0100, Claudio Bley wrote: Hi. It seems the Java wrapper is nearly dead. It has fallen way behind libvirt development. [in my local tree, there're still 120 libvirt API functions missing from the Java interface which /probably/ are worth to be added] I have send a few patches to the list, but no one is willing / able to review them. Some of those patches date almost a year back. Slowly I'm getting a bit frustrated and maybe also a tad impatient... Currently, I'm having +60 commits in my local git tree. As you might imagine, I'd really like to get these off my back. That's why I'm asking myself whether the ACKing / NACKing of patches is the right model for libvirt-java, given that there is, apparently, very little interest and at the same time next to nobody with good Java expertise on the list. I think that there's a strong case to be made, that since you have so many useful patches and no one is able to ack them, you have defacto become the new libvirt java primary maintainer. Congratulations ! I'd suggest that you just spam the list with all of your pending patches in one big series. Leave it a week for anyone to appear and review them, then just push them all to the master git repository. Also update the AUTHORS file in libvirt-java to explicitly say Primary maintainer: Claudio Bley cb...@av-test.de just before the list of patches. Additionally, there are a few glitches in the API which are a thorn in my side ever since I began using the libvirt Java wrapper. It's obvious that the wrapper was written without much thinking about the Java environment and API. Some functions have only been wrapped just because it was possible or perhaps to just have a full coverage of libvirt version x.y.z, without any real use for a Java programmer. Do we really have to live with the failures of the past? I'd really like to fix these even if that means *breaking* the API. Obviously libvirt C library aims to be permanently compatible at the API level. We've not explicitly stated the aim of the language bindings but there is an implicit suggestion that the language bindings would remain API stable too. That said if you can make a good case for why the Java language binding really must break API, then it is at least worth discussing it because I don't think we need to force language bindings to 100% follow libvirt's API stability policy. We wouldn't want API breakage to become a frequent thing, but a one-off breakage if done for the right reasons/benefits could be OK. IMO, this would not be that bad in the Java world. It's not like that you suddenly happen to have an updated dynamic library on the system that's missing some symbols or has it's ABI changed which makes your program crash. Java libraries come with the API bundled and you get an error at the earliest time possible - at compilation time. Even if you upgrade a jar without recompiling your program you'll get a nice RuntimeException instead of undefined behavior. So, I'd say just bump the major or minor version up to the next number and send out a big SORRY, we messed up to everyone and be done with it. I don't know about the scope of the changes you'd plan to make to the API, so my answer would probably depend on exactly what you propose to break and thus how much pain it might cause to downstream users. I think the most important thing would be to raise the possibility with our main downstream user of libvirt-java which I believe is CloudStack. Send a mail to this list, and copy their dev list, indicating what you'd like to change with the ABI and ask for their feedback as to whether it would a) help them in the long term, b) be acceptable level of pain. Alternatively, change the package and the artifact name to libvirt2 effectively creating some sort of fork?! But, given that there's not much review on list, failures are likely to happen again and the same situation would arise anew. I'd be against renaming / forking the package to change the API, since that would still leave existing users screwed as the old code would no doubt be left in a dead unmaintained state. IMHO we just need to decide if the API change is acceptable or not given what it will achieve and if we decide its positive just do the change and bump the major version number of the bindings. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Fix the memory leak
On Thu, Jan 23, 2014 at 06:23:32PM +0800, Osier Yang wrote: The return value of virGetFCHostNameByWWN is a strdup'ed string. Also add comments to declare that the caller should take care of freeing it. --- src/storage/storage_backend_scsi.c | 15 ++- src/util/virutil.c | 5 - 2 files changed, 14 insertions(+), 6 deletions(-) ACK, Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-java] state of affairs
At Thu, 23 Jan 2014 10:49:18 +, Daniel P. Berrange wrote: On Thu, Jan 23, 2014 at 11:20:46AM +0100, Claudio Bley wrote: Hi. It seems the Java wrapper is nearly dead. It has fallen way behind libvirt development. [in my local tree, there're still 120 libvirt API functions missing from the Java interface which /probably/ are worth to be added] I have send a few patches to the list, but no one is willing / able to review them. Some of those patches date almost a year back. Slowly I'm getting a bit frustrated and maybe also a tad impatient... Currently, I'm having +60 commits in my local git tree. As you might imagine, I'd really like to get these off my back. That's why I'm asking myself whether the ACKing / NACKing of patches is the right model for libvirt-java, given that there is, apparently, very little interest and at the same time next to nobody with good Java expertise on the list. I think that there's a strong case to be made, that since you have so many useful patches and no one is able to ack them, you have defacto become the new libvirt java primary maintainer. Congratulations ! Awesome! But give me a moment to let this sink in... I'd suggest that you just spam the list with all of your pending patches in one big series. Leave it a week for anyone to appear and review them, then just push them all to the master git repository. Also update the AUTHORS file in libvirt-java to explicitly say Primary maintainer: Claudio Bley cb...@av-test.de just before the list of patches. Alright, I'll do. Additionally, there are a few glitches in the API which are a thorn in my side ever since I began using the libvirt Java wrapper. It's obvious that the wrapper was written without much thinking about the Java environment and API. Some functions have only been wrapped just because it was possible or perhaps to just have a full coverage of libvirt version x.y.z, without any real use for a Java programmer. Do we really have to live with the failures of the past? I'd really like to fix these even if that means *breaking* the API. Obviously libvirt C library aims to be permanently compatible at the API level. We've not explicitly stated the aim of the language bindings but there is an implicit suggestion that the language bindings would remain API stable too. That said if you can make a good case for why the Java language binding really must break API, then it is at least worth discussing it because I don't think we need to force language bindings to 100% follow libvirt's API stability policy. We wouldn't want API breakage to become a frequent thing, but a one-off breakage if done for the right reasons/benefits could be OK. For one, it's for my sane state of mind.. (does that count?), another reason is consistency, making the API more Java-like so that it doesn't feel like an alien element. It depends from case to case, of course. IMO, this would not be that bad in the Java world. It's not like that you suddenly happen to have an updated dynamic library on the system that's missing some symbols or has it's ABI changed which makes your program crash. Java libraries come with the API bundled and you get an error at the earliest time possible - at compilation time. Even if you upgrade a jar without recompiling your program you'll get a nice RuntimeException instead of undefined behavior. So, I'd say just bump the major or minor version up to the next number and send out a big SORRY, we messed up to everyone and be done with it. I don't know about the scope of the changes you'd plan to make to the API, so my answer would probably depend on exactly what you propose to break and thus how much pain it might cause to downstream users. I think the most important thing would be to raise the possibility with our main downstream user of libvirt-java which I believe is CloudStack. That's what I guessed, too, skimming through the the list archives of the past few months. Happens to be no problem currently, since - out of interest - I already compiled cloudstack's hypervisor/kvm plugin against an updated jar of my local branch. Send a mail to this list, and copy their dev list, indicating what you'd like to change with the ABI and ask for their feedback as to whether it would a) help them in the long term, b) be acceptable level of pain. OK. Regards, Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:http://www.av-test.org Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Fix deadlock in nwfilter code
Since we introduced fine grained locking into the QEMU driver so that VM start can run in parallel, we appear to have caused a race with the nwfilter code. In particular since we no longer hold the global QEMU driver lock for the duration of VM startup we have a lock ordering flaw. This results in deadlock when nwfilter operations happen in parallel with VM startup. This also affects the LXC driver. This patch series attempts to address the problem https://bugzilla.redhat.com/show_bug.cgi?id=929412 the removal of the windows thread impl isn't strictly required, I just didn't want to waste time creating a read/write lock impl for Windows threads. Daniel P. Berrange (5): Add a read/write lock implementation Remove windows thread implementation in favour of pthreads Push nwfilter update locking up to top level Add a mutex to serialize updates to firewall Turn nwfilter conf update mutex into a read/write lock configure.ac | 15 +- src/Makefile.am| 4 - src/conf/nwfilter_conf.c | 23 +- src/conf/nwfilter_conf.h | 3 +- src/libvirt_private.syms | 8 +- src/lxc/lxc_driver.c | 6 + src/nwfilter/nwfilter_driver.c | 12 +- src/nwfilter/nwfilter_gentech_driver.c | 21 +- src/nwfilter/nwfilter_gentech_driver.h | 2 +- src/qemu/qemu_driver.c | 6 + src/uml/uml_driver.c | 4 + src/util/virthread.c | 291 +++- src/util/virthread.h | 51 - src/util/virthreadpthread.c| 278 --- src/util/virthreadpthread.h| 49 src/util/virthreadwin32.c | 396 - src/util/virthreadwin32.h | 53 - 17 files changed, 395 insertions(+), 827 deletions(-) delete mode 100644 src/util/virthreadpthread.c delete mode 100644 src/util/virthreadpthread.h delete mode 100644 src/util/virthreadwin32.c delete mode 100644 src/util/virthreadwin32.h -- 1.8.4.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] Turn nwfilter conf update mutex into a read/write lock
The nwfilter conf update mutex is held whenever a virt driver is starting a VM, or when an nwfilter define/undefine operation is taking place. As such this serializes startup of VMs which is highly undesirable. The VM startup process doesn't make changes to the nwfilter configuration, so it can be relaxed to only hold a read lock. Only the define/undefine operations need write locks. Thus using a read/write lock removes contention when starting guests, unless concurrent nwfilter updates are taking place. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/conf/nwfilter_conf.c | 17 +++-- src/conf/nwfilter_conf.h | 3 ++- src/libvirt_private.syms | 3 ++- src/lxc/lxc_driver.c | 4 ++-- src/nwfilter/nwfilter_driver.c | 4 ++-- src/qemu/qemu_driver.c | 4 ++-- src/uml/uml_driver.c | 4 ++-- 7 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index e712ca5..52e1c06 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -143,17 +143,22 @@ static const struct int_map chain_priorities[] = { /* * only one filter update allowed */ -static virMutex updateMutex; +static virRWLock updateLock; static bool initialized = false; void -virNWFilterLockFilterUpdates(void) { -virMutexLock(updateMutex); +virNWFilterReadLockFilterUpdates(void) { +virRWLockRead(updateLock); +} + +void +virNWFilterWriteLockFilterUpdates(void) { +virRWLockWrite(updateLock); } void virNWFilterUnlockFilterUpdates(void) { -virMutexUnlock(updateMutex); +virRWLockUnlock(updateLock); } @@ -3477,7 +3482,7 @@ int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, initialized = true; -if (virMutexInitRecursive(updateMutex) 0) +if (virRWLockInit(updateLock) 0) return -1; return 0; @@ -3489,7 +3494,7 @@ void virNWFilterConfLayerShutdown(void) if (!initialized) return; -virMutexDestroy(updateMutex); +virRWLockDestroy(updateLock); initialized = false; virNWFilterDomainFWUpdateOpaque = NULL; diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index 6b8b515..0d09b6a 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -716,7 +716,8 @@ virNWFilterDefPtr virNWFilterDefParseFile(const char *filename); void virNWFilterObjLock(virNWFilterObjPtr obj); void virNWFilterObjUnlock(virNWFilterObjPtr obj); -void virNWFilterLockFilterUpdates(void); +void virNWFilterWriteLockFilterUpdates(void); +void virNWFilterReadLockFilterUpdates(void); void virNWFilterUnlockFilterUpdates(void); int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, void *opaque); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index eb91693..a4b1c14 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -575,7 +575,6 @@ virNWFilterDefParseString; virNWFilterInstFiltersOnAllVMs; virNWFilterJumpTargetTypeToString; virNWFilterLoadAllConfigs; -virNWFilterLockFilterUpdates; virNWFilterObjAssignDef; virNWFilterObjDeleteDef; virNWFilterObjFindByName; @@ -587,6 +586,7 @@ virNWFilterObjSaveDef; virNWFilterObjUnlock; virNWFilterPrintStateMatchFlags; virNWFilterPrintTCPFlags; +virNWFilterReadLockFilterUpdates; virNWFilterRegisterCallbackDriver; virNWFilterRuleActionTypeToString; virNWFilterRuleDirectionTypeToString; @@ -594,6 +594,7 @@ virNWFilterRuleProtocolTypeToString; virNWFilterTestUnassignDef; virNWFilterUnlockFilterUpdates; virNWFilterUnRegisterCallbackDriver; +virNWFilterWriteLockFilterUpdates; # conf/nwfilter_ipaddrmap.h diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b1f8a89..aeaa2da 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1015,7 +1015,7 @@ static int lxcDomainCreateWithFiles(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, -1); -virNWFilterLockFilterUpdates(); +virNWFilterReadLockFilterUpdates(); if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; @@ -1112,7 +1112,7 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, NULL); -virNWFilterLockFilterUpdates(); +virNWFilterReadLockFilterUpdates(); if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto cleanup; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 8518e90..f8952c9 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -556,7 +556,7 @@ nwfilterDefineXML(virConnectPtr conn, virNWFilterPtr ret = NULL; nwfilterDriverLock(driver); -virNWFilterLockFilterUpdates(); +virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); if (!(def = virNWFilterDefParseString(xml))) @@ -596,7 +596,7 @@ nwfilterUndefine(virNWFilterPtr obj) { int ret = -1; nwfilterDriverLock(driver); -
[libvirt] [PATCH 3/5] Push nwfilter update locking up to top level
The NWFilter code has as a deadlock race condition between the virNWFilter{Define,Undefine} APIs and starting of guest VMs due to mis-matched lock ordering. In the virNWFilter{Define,Undefine} codepaths the lock ordering is 1. nwfilter driver lock 2. virt driver lock 3. nwfilter update lock 4. domain object lock In the VM guest startup paths the lock ordering is 1. virt driver lock 2. domain object lock 3. nwfilter update lock As can be seen the domain object and nwfilter update locks are not acquired in a consistent order. The fix used is to push the nwfilter update lock upto the top level resulting in a lock ordering for virNWFilter{Define,Undefine} of 1. nwfilter driver lock 2. nwfilter update lock 3. virt driver lock 4. domain object lock and VM start using 1. nwfilter update lock 2. virt driver lock 3. domain object lock This has the effect of serializing VM startup once again, even if no nwfilters are applied to the guest. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/conf/nwfilter_conf.c | 6 -- src/lxc/lxc_driver.c | 6 ++ src/nwfilter/nwfilter_driver.c | 8 src/nwfilter/nwfilter_gentech_driver.c | 6 -- src/qemu/qemu_driver.c | 6 ++ src/uml/uml_driver.c | 4 6 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 6db8ea9..e712ca5 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2990,14 +2990,12 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, return NULL; } -virNWFilterLockFilterUpdates(); if ((nwfilter = virNWFilterObjFindByName(nwfilters, def-name))) { if (virNWFilterDefEqual(def, nwfilter-def, false)) { virNWFilterDefFree(nwfilter-def); nwfilter-def = def; -virNWFilterUnlockFilterUpdates(); return nwfilter; } @@ -3005,7 +3003,6 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, /* trigger the update on VMs referencing the filter */ if (virNWFilterTriggerVMFilterRebuild()) { nwfilter-newDef = NULL; -virNWFilterUnlockFilterUpdates(); virNWFilterObjUnlock(nwfilter); return NULL; } @@ -3013,12 +3010,9 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, virNWFilterDefFree(nwfilter-def); nwfilter-def = def; nwfilter-newDef = NULL; -virNWFilterUnlockFilterUpdates(); return nwfilter; } -virNWFilterUnlockFilterUpdates(); - if (VIR_ALLOC(nwfilter) 0) return NULL; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e319234..b1f8a89 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1015,6 +1015,8 @@ static int lxcDomainCreateWithFiles(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, -1); +virNWFilterLockFilterUpdates(); + if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; @@ -1053,6 +1055,7 @@ cleanup: if (event) virObjectEventStateQueue(driver-domainEventState, event); virObjectUnref(cfg); +virNWFilterUnlockFilterUpdates(); return ret; } @@ -1109,6 +1112,8 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, NULL); +virNWFilterLockFilterUpdates(); + if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto cleanup; @@ -1164,6 +1169,7 @@ cleanup: virObjectEventStateQueue(driver-domainEventState, event); virObjectUnref(caps); virObjectUnref(cfg); +virNWFilterUnlockFilterUpdates(); return dom; } diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index d21dd82..2972731 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -554,6 +554,7 @@ nwfilterDefineXML(virConnectPtr conn, virNWFilterPtr ret = NULL; nwfilterDriverLock(driver); +virNWFilterLockFilterUpdates(); virNWFilterCallbackDriversLock(); if (!(def = virNWFilterDefParseString(xml))) @@ -580,6 +581,7 @@ cleanup: virNWFilterObjUnlock(nwfilter); virNWFilterCallbackDriversUnlock(); +virNWFilterUnlockFilterUpdates(); nwfilterDriverUnlock(driver); return ret; } @@ -592,9 +594,8 @@ nwfilterUndefine(virNWFilterPtr obj) { int ret = -1; nwfilterDriverLock(driver); -virNWFilterCallbackDriversLock(); - virNWFilterLockFilterUpdates(); +virNWFilterCallbackDriversLock(); nwfilter = virNWFilterObjFindByUUID(driver-nwfilters, obj-uuid); if (!nwfilter) { @@ -626,9 +627,8 @@ cleanup: if (nwfilter) virNWFilterObjUnlock(nwfilter); -virNWFilterUnlockFilterUpdates(); - virNWFilterCallbackDriversUnlock(); +virNWFilterUnlockFilterUpdates(); nwfilterDriverUnlock(driver);
[libvirt] [PATCH 2/5] Remove windows thread implementation in favour of pthreads
There are a number of pthreads impls available on Win32 these days, in particular the mingw64 project has a good impl. Delete the native windows thread implementation and rely on using pthreads everywhere. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- configure.ac| 15 +- src/Makefile.am | 4 - src/util/virthread.c| 291 +++- src/util/virthread.h| 41 - src/util/virthreadpthread.c | 309 -- src/util/virthreadpthread.h | 53 -- src/util/virthreadwin32.c | 396 src/util/virthreadwin32.h | 53 -- 8 files changed, 329 insertions(+), 833 deletions(-) delete mode 100644 src/util/virthreadpthread.c delete mode 100644 src/util/virthreadpthread.h delete mode 100644 src/util/virthreadwin32.c delete mode 100644 src/util/virthreadwin32.h diff --git a/configure.ac b/configure.ac index 3a70375..168eb27 100644 --- a/configure.ac +++ b/configure.ac @@ -270,12 +270,21 @@ AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \ posix_memalign prlimit regexec sched_getaffinity setgroups setns \ setrlimit symlink sysctlbyname]) -dnl Availability of pthread functions (if missing, win32 threading is -dnl assumed). Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE. -dnl LIB_PTHREAD and LIBMULTITHREAD were set during gl_INIT by gnulib. +dnl Availability of pthread functions. Because of $LIB_PTHREAD, we +dnl cannot use AC_CHECK_FUNCS_ONCE. LIB_PTHREAD and LIBMULTITHREAD +dnl were set during gl_INIT by gnulib. old_LIBS=$LIBS LIBS=$LIBS $LIB_PTHREAD $LIBMULTITHREAD + +pthread_found=yes AC_CHECK_FUNCS([pthread_mutexattr_init]) +AC_CHECK_HEADER([pthread.h],,[pthread_found=no]) + +if test $ac_cv_func_pthread_mutexattr_init:$pthread_found != yes:yes +then + AC_MSG_ERROR([A pthreads impl is required for building libvirt]) +fi + dnl At least mingw64-winpthreads #defines pthread_sigmask to 0, dnl which in turn causes compilation to complain about unused variables. dnl Expose this broken implementation, so we can work around it. diff --git a/src/Makefile.am b/src/Makefile.am index 8f77658..2298fdb 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -141,8 +141,6 @@ UTIL_SOURCES = \ util/virsysinfo.c util/virsysinfo.h \ util/virsystemd.c util/virsystemd.h \ util/virthread.c util/virthread.h \ - util/virthreadpthread.h \ - util/virthreadwin32.h \ util/virthreadpool.c util/virthreadpool.h \ util/virtime.h util/virtime.c \ util/virtpm.h util/virtpm.c \ @@ -165,8 +163,6 @@ util/virkeymaps.h: $(srcdir)/util/keymaps.csv \ $(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py \ $(srcdir)/util/keymaps.csv $(srcdir)/util/virkeymaps.h -EXTRA_DIST += util/virthreadpthread.c util/virthreadwin32.c - # Internal generic driver infrastructure NODE_INFO_SOURCES = nodeinfo.h nodeinfo.c DATATYPES_SOURCES = datatypes.h datatypes.c diff --git a/src/util/virthread.c b/src/util/virthread.c index dd1768e..b60fb4a 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -23,12 +23,289 @@ #include virthread.h -/* On mingw, we prefer native threading over the sometimes-broken - * pthreads-win32 library wrapper. */ -#ifdef WIN32 -# include virthreadwin32.c -#elif defined HAVE_PTHREAD_MUTEXATTR_INIT -# include virthreadpthread.c +#include unistd.h +#include inttypes.h +#if HAVE_SYS_SYSCALL_H +# include sys/syscall.h +#endif + +#include viralloc.h + + +/* Nothing special required for pthreads */ +int virThreadInitialize(void) +{ +return 0; +} + +void virThreadOnExit(void) +{ +} + +int virOnce(virOnceControlPtr once, virOnceFunc init) +{ +return pthread_once(once-once, init); +} + + +int virMutexInit(virMutexPtr m) +{ +int ret; +pthread_mutexattr_t attr; +pthread_mutexattr_init(attr); +pthread_mutexattr_settype(attr, PTHREAD_MUTEX_NORMAL); +ret = pthread_mutex_init(m-lock, attr); +pthread_mutexattr_destroy(attr); +if (ret != 0) { +errno = ret; +return -1; +} +return 0; +} + +int virMutexInitRecursive(virMutexPtr m) +{ +int ret; +pthread_mutexattr_t attr; +pthread_mutexattr_init(attr); +pthread_mutexattr_settype(attr, PTHREAD_MUTEX_RECURSIVE); +ret = pthread_mutex_init(m-lock, attr); +pthread_mutexattr_destroy(attr); +if (ret != 0) { +errno = ret; +return -1; +} +return 0; +} + +void virMutexDestroy(virMutexPtr m) +{ +pthread_mutex_destroy(m-lock); +} + +void virMutexLock(virMutexPtr m){ +pthread_mutex_lock(m-lock); +} + +void virMutexUnlock(virMutexPtr m) +{ +
[libvirt] [PATCH 4/5] Add a mutex to serialize updates to firewall
The nwfilter conf update mutex previously serialized updates to the internal data structures for firewall rules, and updates to the firewall itself. Since the former is going to be turned into a read/write lock instead of a mutex, a new lock is required to serialize access to the firewall itself. With this new lock, the lock ordering rules will be for virNWFilter{Define,Undefine} 1. nwfilter driver lock 2. nwfilter update lock 3. virt driver lock 4. domain object lock 5. gentech driver lock and VM start 1. nwfilter update lock 2. virt driver lock 3. domain object lock 4. gentech driver lock Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/nwfilter/nwfilter_driver.c | 4 +++- src/nwfilter/nwfilter_gentech_driver.c | 15 --- src/nwfilter/nwfilter_gentech_driver.h | 2 +- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 2972731..8518e90 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -200,7 +200,8 @@ nwfilterStateInitialize(bool privileged, if (virNWFilterDHCPSnoopInit() 0) goto err_exit_learnshutdown; -virNWFilterTechDriversInit(privileged); +if (virNWFilterTechDriversInit(privileged) 0) +goto err_dhcpsnoop_shutdown; if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB, driverState) 0) @@ -251,6 +252,7 @@ error: err_techdrivers_shutdown: virNWFilterTechDriversShutdown(); +err_dhcpsnoop_shutdown: virNWFilterDHCPSnoopShutdown(); err_exit_learnshutdown: virNWFilterLearnShutdown(); diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index f0e78ed..af7505e 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -55,15 +55,21 @@ static virNWFilterTechDriverPtr filter_tech_drivers[] = { NULL }; +/* Serializes execution of iptables/ip6tables/ebtables calls */ +static virMutex updateMutex; -void virNWFilterTechDriversInit(bool privileged) { +int virNWFilterTechDriversInit(bool privileged) { size_t i = 0; VIR_DEBUG(Initializing NWFilter technology drivers); +if (virMutexInitRecursive(updateMutex) 0) +return -1; + while (filter_tech_drivers[i]) { if (!(filter_tech_drivers[i]-flags TECHDRV_FLAG_INITIALIZED)) filter_tech_drivers[i]-init(privileged); i++; } +return 0; } @@ -74,6 +80,7 @@ void virNWFilterTechDriversShutdown(void) { filter_tech_drivers[i]-shutdown(); i++; } +virMutexDestroy(updateMutex); } @@ -935,7 +942,7 @@ _virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, int ifindex; int rc; - +virMutexLock(updateMutex); /* after grabbing the filter update lock check for the interface; if it's not there anymore its filters will be or are being removed (while holding the lock) and we don't want to build new ones */ @@ -963,7 +970,7 @@ _virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, foundNewFilter); cleanup: - +virMutexUnlock(updateMutex); return rc; } @@ -982,6 +989,7 @@ virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver, int rc; bool foundNewFilter = false; +virMutexLock(updateMutex); rc = __virNWFilterInstantiateFilter(driver, vmuuid, true, @@ -1005,6 +1013,7 @@ virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver, } } +virMutexUnlock(updateMutex); return rc; } diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h index f4789e1..d72e040 100644 --- a/src/nwfilter/nwfilter_gentech_driver.h +++ b/src/nwfilter/nwfilter_gentech_driver.h @@ -31,7 +31,7 @@ virNWFilterTechDriverPtr virNWFilterTechDriverForName(const char *name); int virNWFilterRuleInstAddData(virNWFilterRuleInstPtr res, void *data); -void virNWFilterTechDriversInit(bool privileged); +int virNWFilterTechDriversInit(bool privileged); void virNWFilterTechDriversShutdown(void); enum instCase { -- 1.8.4.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] Add a read/write lock implementation
Add virRWLock backed up by a POSIX rwlock primitive Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/libvirt_private.syms| 5 + src/util/virthread.h| 10 ++ src/util/virthreadpthread.c | 31 +++ src/util/virthreadpthread.h | 4 4 files changed, 50 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d1a58f9..eb91693 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1816,6 +1816,11 @@ virMutexInitRecursive; virMutexLock; virMutexUnlock; virOnce; +virRWLockInit; +virRWLockDestroy; +virRWLockRead; +virRWLockWrite; +virRWLockUnlock; virThreadCancel; virThreadCreate; virThreadID; diff --git a/src/util/virthread.h b/src/util/virthread.h index 84d3bdc..7015d60 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -28,6 +28,9 @@ typedef struct virMutex virMutex; typedef virMutex *virMutexPtr; +typedef struct virRWLock virRWLock; +typedef virRWLock *virRWLockPtr; + typedef struct virCond virCond; typedef virCond *virCondPtr; @@ -89,6 +92,13 @@ void virMutexLock(virMutexPtr m); void virMutexUnlock(virMutexPtr m); +int virRWLockInit(virRWLockPtr m) ATTRIBUTE_RETURN_CHECK; +void virRWLockDestroy(virRWLockPtr m); + +void virRWLockRead(virRWLockPtr m); +void virRWLockWrite(virRWLockPtr m); +void virRWLockUnlock(virRWLockPtr m); + int virCondInit(virCondPtr c) ATTRIBUTE_RETURN_CHECK; int virCondDestroy(virCondPtr c); diff --git a/src/util/virthreadpthread.c b/src/util/virthreadpthread.c index ca841e4..2efb4c1 100644 --- a/src/util/virthreadpthread.c +++ b/src/util/virthreadpthread.c @@ -91,6 +91,37 @@ void virMutexUnlock(virMutexPtr m) } +int virRWLockInit(virRWLockPtr m) +{ +if (pthread_rwlock_init(m-lock, NULL) != 0) { +errno = EINVAL; +return -1; +} +return 0; +} + +void virRWLockDestroy(virRWLockPtr m) +{ +pthread_rwlock_destroy(m-lock); +} + + +void virRWLockRead(virRWLockPtr m) +{ +pthread_rwlock_rdlock(m-lock); +} + +void virRWLockWrite(virRWLockPtr m) +{ +pthread_rwlock_wrlock(m-lock); +} + + +void virRWLockUnlock(virRWLockPtr m) +{ +pthread_rwlock_unlock(m-lock); +} + int virCondInit(virCondPtr c) { int ret; diff --git a/src/util/virthreadpthread.h b/src/util/virthreadpthread.h index b9f1319..cb607d0 100644 --- a/src/util/virthreadpthread.h +++ b/src/util/virthreadpthread.h @@ -27,6 +27,10 @@ struct virMutex { pthread_mutex_t lock; }; +struct virRWLock { +pthread_rwlock_t lock; +}; + struct virCond { pthread_cond_t cond; }; -- 1.8.4.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] closing old maint branches [was: [v0.9.12-maint 0/8] Backport changes for CVE-2013-6458 to v0.9.12-maint]
On 01/23/2014 12:26 PM, Daniel P. Berrange wrote: On Wed, Jan 22, 2014 at 12:13:48PM -0700, Eric Blake wrote: On 01/15/2014 01:43 PM, Eric Blake wrote: Is anyone still using v0.9.11-maint? The CVE extends back to 0.9.8, so we could argue that we should either fix the 0.9.11 branch, or add another commit to the branch that explicitly marks it as end-of-life because no one appears to be relying on it. Fedora 18 is now end-of-life, so from Fedora's perspective, I only care about 0.10.2 (RHEL and CentOS 6), 1.0.5 (F19), 1.1.3 (F20) and soon 1.2.1 (rawhide), although I didn't mind touching all the intermediate branches on my way down to 0.10.2. RHEL 5 is also vulnerable to CVE-2013-6458, but as we don't have an upstream v0.8.2-maint branch (thank goodness!), that's something for Red Hat to worry about. I've gone ahead and marked v0.8.3-maint and v0.9.11-maint as closed (I'm not posting the actual patch here, but it was done by 'git rm -f \*' followed by recreating .gitignore and a placeholder README that mentions the death of the branch). FYI for openstack I examined the current libvirt versions in some major distros: https://wiki.openstack.org/wiki/LibvirtDistroSupportMatrix After seeing that list, I thought an end of life column could be interesing, but then realized the only bit I was interested in was how long we will need to put of with the oldest version on the list. As far as I can tell, Ubuntu 12.04 LTS is scheduled for EOL in April 2017 (date from here: https://wiki.ubuntu.com/Releases ), so I guess *somebody* has to care about libvirt-0.9.8 until 2017 (of course we don't have a v0.9.8-maint branch anyway, so that's not likely going to happen within upstream infrastructure) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] Add a read/write lock implementation
On 01/23/2014 02:37 PM, Daniel P. Berrange wrote: Add virRWLock backed up by a POSIX rwlock primitive Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/libvirt_private.syms| 5 + src/util/virthread.h| 10 ++ src/util/virthreadpthread.c | 31 +++ src/util/virthreadpthread.h | 4 4 files changed, 50 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d1a58f9..eb91693 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1816,6 +1816,11 @@ virMutexInitRecursive; virMutexLock; virMutexUnlock; virOnce; +virRWLockInit; +virRWLockDestroy; +virRWLockRead; +virRWLockWrite; +virRWLockUnlock; These are out of order. virThreadCancel; virThreadCreate; virThreadID; diff --git a/src/util/virthread.h b/src/util/virthread.h index 84d3bdc..7015d60 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -28,6 +28,9 @@ typedef struct virMutex virMutex; typedef virMutex *virMutexPtr; +typedef struct virRWLock virRWLock; +typedef virRWLock *virRWLockPtr; + typedef struct virCond virCond; typedef virCond *virCondPtr; @@ -89,6 +92,13 @@ void virMutexLock(virMutexPtr m); void virMutexUnlock(virMutexPtr m); +int virRWLockInit(virRWLockPtr m) ATTRIBUTE_RETURN_CHECK; +void virRWLockDestroy(virRWLockPtr m); + +void virRWLockRead(virRWLockPtr m); +void virRWLockWrite(virRWLockPtr m); +void virRWLockUnlock(virRWLockPtr m); + I was about to predict a build failure on Windows since you haven't defined these functions in virthreadwin32.c, but then realized you aren't yet calling them from anywhere, so the build will complete just fine. It would reduce code motion a bit to swap this patch with patch 2, but functionally there's no difference. As far as the implementation of the functions - it can't get any more striaghtforward that this! ACK. int virCondInit(virCondPtr c) ATTRIBUTE_RETURN_CHECK; int virCondDestroy(virCondPtr c); diff --git a/src/util/virthreadpthread.c b/src/util/virthreadpthread.c index ca841e4..2efb4c1 100644 --- a/src/util/virthreadpthread.c +++ b/src/util/virthreadpthread.c @@ -91,6 +91,37 @@ void virMutexUnlock(virMutexPtr m) } +int virRWLockInit(virRWLockPtr m) +{ +if (pthread_rwlock_init(m-lock, NULL) != 0) { +errno = EINVAL; +return -1; +} +return 0; +} + +void virRWLockDestroy(virRWLockPtr m) +{ +pthread_rwlock_destroy(m-lock); +} + + +void virRWLockRead(virRWLockPtr m) +{ +pthread_rwlock_rdlock(m-lock); +} + +void virRWLockWrite(virRWLockPtr m) +{ +pthread_rwlock_wrlock(m-lock); +} + + +void virRWLockUnlock(virRWLockPtr m) +{ +pthread_rwlock_unlock(m-lock); +} + int virCondInit(virCondPtr c) { int ret; diff --git a/src/util/virthreadpthread.h b/src/util/virthreadpthread.h index b9f1319..cb607d0 100644 --- a/src/util/virthreadpthread.h +++ b/src/util/virthreadpthread.h @@ -27,6 +27,10 @@ struct virMutex { pthread_mutex_t lock; }; +struct virRWLock { +pthread_rwlock_t lock; +}; + struct virCond { pthread_cond_t cond; }; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] storage: Fix autostart of pool with fc_host type adapter
On 01/23/2014 04:45 AM, Osier Yang wrote: On 22/01/14 21:36, John Ferlan wrote: On 01/07/2014 06:07 AM, Osier Yang wrote: On 07/01/14 01:21, John Ferlan wrote: On 01/06/2014 05:19 AM, Osier Yang wrote: The checkPool is a bit different for pool with fc_host type source adapter, since the vHBA it's based on might be not created yet (it's created by startPool, which is involked after checkPool in storageDriverAutostart). So it should not fail, otherwise the autostart of the pool will fail either. The problem is easy to reproduce: * Enable autostart for the pool * Restart libvirtd service * Check the pool's state --- src/storage/storage_backend_scsi.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) Not sure this is the right thing to do. With this change it doesn't matter what getAdapterName() returns for fc_host's... It matters with if *isActive is true or false in the end. We don't need to try to start the pool if it's already active. Fair enough; however, let's consider the failure case. On failure, a message is reported and name == NULL. Do we need to clear that error now? I think my original thoughts were along the lines of having getAdapterName do more of the heavy lifting. Have both check and start call it. It would call the createVport which would be mostly unchanged, except for the virFileWaitForDevices() which could move to start... Found your method *might* break one logic. Assuming the pool is not marked as autostart, and the vHBA was not created yet. Since with your method checkPool will create the vHBA now, then it's possible for checkPool to return *isActive as true, as long as the device path for the created vHBA can show up in host very quickly (like what we talked a lot in PATCH 2/2, how long it will show up is also depended, the time can be long, but it also can be short), i.e. during the checkPool process. And thus the pool will be marked as Active. As the result, the problem on one side of the coin is fixed, but it introduces a similar problem on another side. :-) Huh? Not sure I see what problem you're driving at. Look back at the caller - isActive from _scsi translates to started in the caller. The 'started' is used to decide whether to call 'startPool' and 'refreshPool'. If autostart is disabled, then 'startPool' won't be called. Truly all that's not done prior to refreshPool being called is the 'virFileWaitForDevices();'... So move that into createVport(). Or am I missing something? If the pool isn't autostarted, then the next start will call getAdapterName() which determines the pool is started (or not) and be happy. John That's why I said previously I don't want to do any creation work in checkPool, it should only check something. So, I'm going forward to push my patch, with the last error reset in checkPool. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Fix the memory leak
On 23/01/14 19:06, Martin Kletzander wrote: On Thu, Jan 23, 2014 at 06:23:32PM +0800, Osier Yang wrote: The return value of virGetFCHostNameByWWN is a strdup'ed string. Also add comments to declare that the caller should take care of freeing it. --- src/storage/storage_backend_scsi.c | 15 ++- src/util/virutil.c | 5 - 2 files changed, 14 insertions(+), 6 deletions(-) ACK, Thanks, pushed. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] Remove windows thread implementation in favour of pthreads
On 01/23/2014 02:37 PM, Daniel P. Berrange wrote: There are a number of pthreads impls available on Win32 these days, in particular the mingw64 project has a good impl. Delete the native windows thread implementation and rely on using pthreads everywhere. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- configure.ac| 15 +- src/Makefile.am | 4 - src/util/virthread.c| 291 +++- src/util/virthread.h| 41 - src/util/virthreadpthread.c | 309 -- src/util/virthreadpthread.h | 53 -- src/util/virthreadwin32.c | 396 src/util/virthreadwin32.h | 53 -- 8 files changed, 329 insertions(+), 833 deletions(-) delete mode 100644 src/util/virthreadpthread.c delete mode 100644 src/util/virthreadpthread.h delete mode 100644 src/util/virthreadwin32.c delete mode 100644 src/util/virthreadwin32.h diff --git a/configure.ac b/configure.ac index 3a70375..168eb27 100644 --- a/configure.ac +++ b/configure.ac @@ -270,12 +270,21 @@ AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \ posix_memalign prlimit regexec sched_getaffinity setgroups setns \ setrlimit symlink sysctlbyname]) -dnl Availability of pthread functions (if missing, win32 threading is -dnl assumed). Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE. -dnl LIB_PTHREAD and LIBMULTITHREAD were set during gl_INIT by gnulib. +dnl Availability of pthread functions. Because of $LIB_PTHREAD, we +dnl cannot use AC_CHECK_FUNCS_ONCE. LIB_PTHREAD and LIBMULTITHREAD +dnl were set during gl_INIT by gnulib. old_LIBS=$LIBS LIBS=$LIBS $LIB_PTHREAD $LIBMULTITHREAD + +pthread_found=yes AC_CHECK_FUNCS([pthread_mutexattr_init]) +AC_CHECK_HEADER([pthread.h],,[pthread_found=no]) + +if test $ac_cv_func_pthread_mutexattr_init:$pthread_found != yes:yes +then + AC_MSG_ERROR([A pthreads impl is required for building libvirt]) +fi + dnl At least mingw64-winpthreads #defines pthread_sigmask to 0, dnl which in turn causes compilation to complain about unused variables. dnl Expose this broken implementation, so we can work around it. diff --git a/src/Makefile.am b/src/Makefile.am index 8f77658..2298fdb 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -141,8 +141,6 @@ UTIL_SOURCES = \ util/virsysinfo.c util/virsysinfo.h \ util/virsystemd.c util/virsystemd.h \ util/virthread.c util/virthread.h \ - util/virthreadpthread.h \ - util/virthreadwin32.h \ util/virthreadpool.c util/virthreadpool.h \ util/virtime.h util/virtime.c \ util/virtpm.h util/virtpm.c \ @@ -165,8 +163,6 @@ util/virkeymaps.h: $(srcdir)/util/keymaps.csv \ $(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py \ $(srcdir)/util/keymaps.csv $(srcdir)/util/virkeymaps.h -EXTRA_DIST += util/virthreadpthread.c util/virthreadwin32.c - # Internal generic driver infrastructure NODE_INFO_SOURCES = nodeinfo.h nodeinfo.c DATATYPES_SOURCES = datatypes.h datatypes.c diff --git a/src/util/virthread.c b/src/util/virthread.c index dd1768e..b60fb4a 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -23,12 +23,289 @@ #include virthread.h -/* On mingw, we prefer native threading over the sometimes-broken - * pthreads-win32 library wrapper. */ -#ifdef WIN32 -# include virthreadwin32.c -#elif defined HAVE_PTHREAD_MUTEXATTR_INIT -# include virthreadpthread.c +#include unistd.h +#include inttypes.h +#if HAVE_SYS_SYSCALL_H +# include sys/syscall.h +#endif + +#include viralloc.h + + +/* Nothing special required for pthreads */ +int virThreadInitialize(void) +{ +return 0; +} + +void virThreadOnExit(void) +{ +} + +int virOnce(virOnceControlPtr once, virOnceFunc init) +{ +return pthread_once(once-once, init); +} + + +int virMutexInit(virMutexPtr m) +{ +int ret; +pthread_mutexattr_t attr; +pthread_mutexattr_init(attr); +pthread_mutexattr_settype(attr, PTHREAD_MUTEX_NORMAL); +ret = pthread_mutex_init(m-lock, attr); +pthread_mutexattr_destroy(attr); +if (ret != 0) { +errno = ret; +return -1; +} +return 0; +} You just moved the contents of virthreadpthread.c into virthread.c, right? Is there maybe some way to convince git to make a shorter diff? (I did something similar in netcf - moved 90% of one file into another, and renamed the original, and it properly figured out to rename the original into the new 90%, and create the one I thought I was renaming from scratch). Not terribly important though, since we all know what
[libvirt] [PATCH 0/4] Improve QoS and unit-test it
*** BLURB HERE *** Michal Privoznik (4): networkAllocateActualDevice: Set QoS for bridgeless networks too tests: Introduce virnetdevbandwidthtest virnetdevbandwidthtest: Introduce mocking virnetdevbandwidthtest: Introduce testVirNetDevBandwidthSet src/libvirt_private.syms | 1 + src/network/bridge_driver.c| 25 - src/util/virnetdevbandwidth.c | 88 +++ src/util/virnetdevbandwidth.h | 6 + tests/Makefile.am | 14 +++ tests/virnetdevbandwidthmock.c | 106 ++ tests/virnetdevbandwidthtest.c | 244 + 7 files changed, 482 insertions(+), 2 deletions(-) create mode 100644 tests/virnetdevbandwidthmock.c create mode 100644 tests/virnetdevbandwidthtest.c -- 1.8.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] virnetdevbandwidthtest: Introduce mocking
The mocking will be used in later commits to mock all calls to the virCommandRun(). This is easier to do than cutting off the command creation and run into two separate pieces. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/Makefile.am | 9 tests/virnetdevbandwidthmock.c | 106 + tests/virnetdevbandwidthtest.c | 21 +++- 3 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 tests/virnetdevbandwidthmock.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 0930073..f914244 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -319,6 +319,7 @@ test_libraries = libshunload.la \ virnetserverclientmock.la \ vircgroupmock.la \ virpcimock.la \ + virnetdevbandwidthmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la @@ -655,6 +656,14 @@ virnetdevbandwidthtest_SOURCES = \ virnetdevbandwidthtest.c testutils.h testutils.c virnetdevbandwidthtest_LDADD = $(LDADDS) +virnetdevbandwidthmock_la_SOURCES = \ + virnetdevbandwidthmock.c +virnetdevbandwidthmock_la_CFLAGS = $(AM_CFLAGS) +virnetdevbandwidthmock_la_LIBADD = $(GNULIB_LIBS) \ + ../src/libvirt.la +virnetdevbandwidthmock_la_LDFLAGS = -module -avoid-version \ +-rpath /evil/libtool/hack/to/force/shared/lib/creation + if WITH_LIBVIRTD libvirtdconftest_SOURCES = \ libvirtdconftest.c testutils.h testutils.c \ diff --git a/tests/virnetdevbandwidthmock.c b/tests/virnetdevbandwidthmock.c new file mode 100644 index 000..8bb590b --- /dev/null +++ b/tests/virnetdevbandwidthmock.c @@ -0,0 +1,106 @@ +/* + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Author: Michal Privoznik mpriv...@redhat.com + */ + +#include config.h + +#include dlfcn.h +#include fcntl.h +#include stdlib.h +#include sys/stat.h +#include sys/types.h +#include viralloc.h +#include vircommand.h +#include virfile.h + +static int (*realvirCommandRun)(virCommandPtr cmd, int *exitstatus); + +/* Don't make static, since it causes problems with clang + * when passed as an arg to virAsprintf() + * vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an inline function with external linkage [-Werror,-Wstatic-in-inline] + */ +char *outfile; + +#define STDERR(...) \ +fprintf(stderr, %s %zu: , __FUNCTION__, (size_t) __LINE__); \ +fprintf(stderr, __VA_ARGS__); \ +fprintf(stderr, \n); \ + +#define ABORT(...) \ +do {\ +STDERR(__VA_ARGS__);\ +abort();\ +} while (0) + +static void +init(void) +{ +if (outfile) +return; + +if (!(outfile = getenv(LIBVIRT_OUTFILE))) +ABORT(Missing LIBVIRT_OUTDIR env variable\n); + +#define LOAD_SYM(name) \ +do {\ +if (!(real ## name = dlsym(RTLD_NEXT, #name))) \ +ABORT(Cannot find real '%s' symbol\n, #name); \ +} while (0) + +LOAD_SYM(virCommandRun); +} + +/* Here we don't really run the command, but instead get its string + * representation which is then written to a temporary file referenced + * by @outfile. + */ +int +virCommandRun(virCommandPtr cmd, int *exitstatus) +{ +int ret = -1; +int fd = 0; +char *buf = NULL; + +if (!outfile) +init(); + +if (!(buf = virCommandToString(cmd))) { +*exitstatus = EXIT_FAILURE; +return 0; +} + +if ((fd = open(outfile, O_WRONLY | O_APPEND | O_CREAT, S_IRUSR | S_IWUSR)) == -1) { +STDERR(unable to open file: %s %d, outfile, errno); +goto cleanup; +} + +if (safewrite(fd, buf, strlen(buf)) 0 || +safewrite(fd, \n, 1) 0) { +STDERR(unable to write to file: %s %d, outfile, errno); +goto cleanup; +} + +ret
[libvirt] [PATCH 4/4] virnetdevbandwidthtest: Introduce testVirNetDevBandwidthSet
The test tries to set some QoS limits and check if the commands that are actually executed are the expected ones. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/virnetdevbandwidthtest.c | 70 ++ 1 file changed, 70 insertions(+) diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 09cc2ec..b30e0fc 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -32,6 +32,14 @@ struct testMinimalStruct { const char *band2; }; +struct testSetStruct { +const char *outfile; +const char *band; +const char *exp_cmd; +const char *iface; +const bool hierarchical_class; +}; + #define PARSE(xml, var) \ do {\ xmlDocPtr doc; \ @@ -103,6 +111,42 @@ cleanup: return ret; } +static int +testVirNetDevBandwidthSet(const void *data) +{ +int ret = -1; +const struct testSetStruct *info = data; +const char *iface = info-iface; +virNetDevBandwidthPtr band = NULL; +char *actual_cmd = NULL; + +PARSE(info-band, band); + +if (!iface) +iface = eth0; + +/* Clear the temporary file between runs */ +truncate(info-outfile, 0); + +if (virNetDevBandwidthSet(iface, band, info-hierarchical_class) 0) +goto cleanup; + +if (virFileReadAll(info-outfile, 2048, actual_cmd) 0) +goto cleanup; + +if (STRNEQ(info-exp_cmd, actual_cmd)) { +virtTestDifference(stderr, info-exp_cmd, actual_cmd); +goto cleanup; +} + +ret = 0; +cleanup: +virNetDevBandwidthFree(band); +VIR_FREE(actual_cmd); +return ret; +} + + #define OUTFILETEMPLATE abs_builddir /virnetdevbandwidthtest-XX static int @@ -132,6 +176,18 @@ mymain(void) ret = -1; \ } while (0) +#define DO_TEST_SET(Band, Exp_cmd, ...) \ +do {\ +struct testSetStruct data = {.outfile = outfile,\ + .band = Band, \ + .exp_cmd = Exp_cmd,\ + __VA_ARGS__}; \ +if (virtTestRun(virNetDevBandwidthSet,\ +testVirNetDevBandwidthSet, \ +data) 0) \ +ret = -1; \ +} while (0) + DO_TEST_MINIMAL(NULL, NULL, NULL); @@ -165,6 +221,20 @@ mymain(void) outbound average='5' peak='6'/ /bandwidth); +DO_TEST_SET(bandwidth + inbound average='1' peak='2' floor='3' burst='4'/ + outbound average='5' peak='6' burst='7'/ +/bandwidth, +/sbin/tc qdisc del dev eth0 root\n +/sbin/tc qdisc del dev eth0 ingress\n +/sbin/tc qdisc add dev eth0 root handle 1: htb default 1\n +/sbin/tc class add dev eth0 parent 1: classid 1:1 htb rate 1kbps ceil 2kbps burst 4kb\n +/sbin/tc qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n +/sbin/tc filter add dev eth0 parent 1:0 protocol ip handle 1 fw flowid 1\n +/sbin/tc qdisc add dev eth0 ingress\n +/sbin/tc filter add dev eth0 parent : protocol ip u32 match ip src 0.0.0.0/0 +police rate 5kbps burst 7kb mtu 64kb drop flowid :1\n); + if (getenv(LIBVIRT_SKIP_CLEANUP) == NULL) unlink(outfile); -- 1.8.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] tests: Introduce virnetdevbandwidthtest
The only API tested so far would be virNetDevBandwidthMinimal. But there's more to come! Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/Makefile.am | 5 ++ tests/virnetdevbandwidthtest.c | 155 + 2 files changed, 160 insertions(+) create mode 100644 tests/virnetdevbandwidthtest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 568b7a0..0930073 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -141,6 +141,7 @@ test_programs = virshtest sockettest \ virportallocatortest \ sysinfotest \ virstoragetest \ + virnetdevbandwidthtest \ $(NULL) if WITH_REMOTE @@ -650,6 +651,10 @@ commandhelper_SOURCES = \ commandhelper_LDADD = $(LDADDS) commandhelper_LDFLAGS = -static +virnetdevbandwidthtest_SOURCES = \ + virnetdevbandwidthtest.c testutils.h testutils.c +virnetdevbandwidthtest_LDADD = $(LDADDS) + if WITH_LIBVIRTD libvirtdconftest_SOURCES = \ libvirtdconftest.c testutils.h testutils.c \ diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c new file mode 100644 index 000..989018e --- /dev/null +++ b/tests/virnetdevbandwidthtest.c @@ -0,0 +1,155 @@ +/* + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Author: Michal Privoznik mpriv...@redhat.com + */ + +#include config.h + +#include testutils.h +#include virnetdevbandwidth.h +#include netdev_bandwidth_conf.c + +#define VIR_FROM_THIS VIR_FROM_NONE + +struct testMinimalStruct { +const char *expected_result; +const char *band1; +const char *band2; +}; + +#define PARSE(xml, var) \ +do {\ +xmlDocPtr doc; \ +xmlXPathContextPtr ctxt = NULL; \ +\ +if (!xml) \ +break; \ +\ +if (!(doc = virXMLParseStringCtxt((xml),\ + bandwidth definition, \ + ctxt))) \ +goto cleanup; \ +\ +(var) = virNetDevBandwidthParse(ctxt-node, \ +VIR_DOMAIN_NET_TYPE_NETWORK); \ +xmlFreeDoc(doc);\ +xmlXPathFreeContext(ctxt); \ +if (!(var)) \ +goto cleanup; \ +} while (0) + +static int +testVirNetDevBandwidthMinimal(const void *data) +{ +int ret = -1; +const struct testMinimalStruct *info = data; +virNetDevBandwidthPtr expected_result = NULL, result = NULL, + band1 = NULL, band2 = NULL; + + +/* Parse given XMLs */ +PARSE(info-expected_result, expected_result); +PARSE(info-band1, band1); +PARSE(info-band2, band2); + +if (virNetDevBandwidthMinimal(result, band1, band2) 0) +goto cleanup; + +if (!virNetDevBandwidthEqual(expected_result, result)) { +virBuffer exp_buf = VIR_BUFFER_INITIALIZER, + res_buf = VIR_BUFFER_INITIALIZER; +char *exp = NULL, *res = NULL; + +fprintf(stderr, expected_result != result); + +if (virNetDevBandwidthFormat(expected_result, exp_buf) 0 || +virNetDevBandwidthFormat(result, res_buf) 0 || +!(exp = virBufferContentAndReset(exp_buf)) || +!(res = virBufferContentAndReset(res_buf))) { +fprintf(stderr, Failed to fail); +virBufferFreeAndReset(exp_buf); +virBufferFreeAndReset(res_buf); +VIR_FREE(exp); +VIR_FREE(res); +goto cleanup; +} + +virtTestDifference(stderr, exp,
Re: [libvirt] [PATCH 1/5] Add a read/write lock implementation
On Thu, Jan 23, 2014 at 03:22:53PM +0200, Laine Stump wrote: On 01/23/2014 02:37 PM, Daniel P. Berrange wrote: Add virRWLock backed up by a POSIX rwlock primitive Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/libvirt_private.syms| 5 + src/util/virthread.h| 10 ++ src/util/virthreadpthread.c | 31 +++ src/util/virthreadpthread.h | 4 4 files changed, 50 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d1a58f9..eb91693 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1816,6 +1816,11 @@ virMutexInitRecursive; virMutexLock; virMutexUnlock; virOnce; +virRWLockInit; +virRWLockDestroy; +virRWLockRead; +virRWLockWrite; +virRWLockUnlock; These are out of order. virThreadCancel; virThreadCreate; virThreadID; diff --git a/src/util/virthread.h b/src/util/virthread.h index 84d3bdc..7015d60 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -28,6 +28,9 @@ typedef struct virMutex virMutex; typedef virMutex *virMutexPtr; +typedef struct virRWLock virRWLock; +typedef virRWLock *virRWLockPtr; + typedef struct virCond virCond; typedef virCond *virCondPtr; @@ -89,6 +92,13 @@ void virMutexLock(virMutexPtr m); void virMutexUnlock(virMutexPtr m); +int virRWLockInit(virRWLockPtr m) ATTRIBUTE_RETURN_CHECK; +void virRWLockDestroy(virRWLockPtr m); + +void virRWLockRead(virRWLockPtr m); +void virRWLockWrite(virRWLockPtr m); +void virRWLockUnlock(virRWLockPtr m); + I was about to predict a build failure on Windows since you haven't defined these functions in virthreadwin32.c, but then realized you aren't yet calling them from anywhere, so the build will complete just fine. It would reduce code motion a bit to swap this patch with patch 2, but functionally there's no difference. FYI the reason I did it in this order, is to make it easier to backport to the stable branches. eg on the stable branches I'd want to skip patch 2, since that's going to require a gnulib update too. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] storage: Fix autostart of pool with fc_host type adapter
On 23/01/14 21:35, John Ferlan wrote: On 01/23/2014 04:45 AM, Osier Yang wrote: On 22/01/14 21:36, John Ferlan wrote: On 01/07/2014 06:07 AM, Osier Yang wrote: On 07/01/14 01:21, John Ferlan wrote: On 01/06/2014 05:19 AM, Osier Yang wrote: The checkPool is a bit different for pool with fc_host type source adapter, since the vHBA it's based on might be not created yet (it's created by startPool, which is involked after checkPool in storageDriverAutostart). So it should not fail, otherwise the autostart of the pool will fail either. The problem is easy to reproduce: * Enable autostart for the pool * Restart libvirtd service * Check the pool's state --- src/storage/storage_backend_scsi.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) Not sure this is the right thing to do. With this change it doesn't matter what getAdapterName() returns for fc_host's... It matters with if *isActive is true or false in the end. We don't need to try to start the pool if it's already active. Fair enough; however, let's consider the failure case. On failure, a message is reported and name == NULL. Do we need to clear that error now? I think my original thoughts were along the lines of having getAdapterName do more of the heavy lifting. Have both check and start call it. It would call the createVport which would be mostly unchanged, except for the virFileWaitForDevices() which could move to start... Found your method *might* break one logic. Assuming the pool is not marked as autostart, and the vHBA was not created yet. Since with your method checkPool will create the vHBA now, then it's possible for checkPool to return *isActive as true, as long as the device path for the created vHBA can show up in host very quickly (like what we talked a lot in PATCH 2/2, how long it will show up is also depended, the time can be long, but it also can be short), i.e. during the checkPool process. And thus the pool will be marked as Active. As the result, the problem on one side of the coin is fixed, but it introduces a similar problem on another side. :-) Huh? Not sure I see what problem you're driving at. Look back at the caller - isActive from _scsi translates to started in the caller. The 'started' is used to decide whether to call 'startPool' and 'refreshPool'. If autostart is disabled, then 'startPool' won't be called. Assuming pool-autostart if false, and started is happened to be true. Calling refreshPool is likely to cause pool-active == 1. ... if (started) { if (backend-refreshPool(conn, pool) 0) { virErrorPtr err = virGetLastError(); if (backend-stopPool) backend-stopPool(conn, pool); VIR_ERROR(_(Failed to autostart storage pool '%s': %s), pool-def-name, err ? err-message : _(no error message found)); virStoragePoolObjUnlock(pool); continue; } pool-active = 1; } /... Since the vHBA was already created in checkPool, then I see not much reason why refreshPool can not be success. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] Push nwfilter update locking up to top level
On 01/23/2014 02:37 PM, Daniel P. Berrange wrote: The NWFilter code has as a deadlock race condition between the virNWFilter{Define,Undefine} APIs and starting of guest VMs due to mis-matched lock ordering. In the virNWFilter{Define,Undefine} codepaths the lock ordering is 1. nwfilter driver lock 2. virt driver lock 3. nwfilter update lock 4. domain object lock In the VM guest startup paths the lock ordering is 1. virt driver lock 2. domain object lock 3. nwfilter update lock and previously, virNWFilterLockUpdates was called not directly from the hypervisor driver, but inside a function in the nwfilter driver that was called from the hypervisor driver's code indirectly via some sort of callback scheme. As can be seen the domain object and nwfilter update locks are not acquired in a consistent order. The fix used is to push the nwfilter update lock upto the top level resulting in a lock ordering for virNWFilter{Define,Undefine} of 1. nwfilter driver lock 2. nwfilter update lock 3. virt driver lock 4. domain object lock and VM start using 1. nwfilter update lock 2. virt driver lock 3. domain object lock This has the effect of serializing VM startup once again, even if no nwfilters are applied to the guest. Maybe this comment should be made more prominent, so that nobody makes the mistake of applying this patch to fix the crash, but then *not* applying the two following patches that parallelize guest startups again. (Possibly even mention the followup patches by name as an extra clue). Aside from this, agreeing with your analysis, and nothing that make syntax-check and make passed for me on F20, I don't feel qualified to do a deeper review including potential unseen consequences of these changes, so hopefully Stefan will be able to do that. (I should also say in advance that I plan to review/build 4/5 and 5/5, but only to see if I can pick out some problems - for now I'll leave the ACKing up to (hopefully) Stefan.) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH python] Fix calling of virStreamSend method
Change d40861 removed the 'len' argument from the virStreamSend C level wrapper, but forgot to remove it from the python level wrapper. Reported-by: Robie Basak robie.ba...@canonical.com Signed-off-by: Daniel P. Berrange berra...@redhat.com --- libvirt-override-virStream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-override-virStream.py b/libvirt-override-virStream.py index cd44314..ce82da6 100644 --- a/libvirt-override-virStream.py +++ b/libvirt-override-virStream.py @@ -122,6 +122,6 @@ with the call, but may instead be delayed until a subsequent call. -ret = libvirtmod.virStreamSend(self._o, data, len(data)) +ret = libvirtmod.virStreamSend(self._o, data) if ret == -1: raise libvirtError ('virStreamSend() failed') return ret -- 1.8.4.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Add support for changing timeout value to open unix monitor socket
I agree, there is no harm in adding an option of configuration, different setup configurations require different timeout values. my setup was 8 servers booted with PXE boot and running on nfs rootfs, with 8 vms on each. when I tried to start all of them together the bottle neck was the network and it takes about 5 minutes till they all start. I think a good solution will include changes in qemu's behavior, but the solution that I suggested is much better than the current state. we would be very happy not to manage our own version of libvirt. Silence gives consent? On Thu, Jan 16, 2014 at 5:49 PM, Martin Kletzander mklet...@redhat.comwrote: Ping? Can we at least change the default [2/2] or should I send a v2 for that one? Martin On Fri, Jan 10, 2014 at 04:27:40PM +0100, Martin Kletzander wrote: On Fri, Jan 10, 2014 at 02:18:37PM +, Daniel P. Berrange wrote: On Thu, Jan 09, 2014 at 09:22:05AM +0100, Martin Kletzander wrote: From: Pavel Fux pa...@stratoscale.com Adding an option to change monitor socket opening timeout the current default is 3 seconds and in some cases it's not enough Signed-off-by: Pavel Fux pa...@stratoscale.com Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: I modified the description in the config file, made the use of the opaque argument in qemuMonitorOpen and rebased it on current master. I also added the config options in augeas test to make the 'make check' pass. IMHO we shouldn't add this config parameter. This kind of parameter is basically saying our code doesn't work by default, set this to fix it which is just horrible behaviour. Further more an admin won't even find out about this until the worst possible time. Just increase the default timeout if we need to. Even better would be to figure out how we can properly fix this to avoid any need for timeout at all. The same can be said about e.g. audio-related options in the config file or (when going to extremes) debug logs. Yes, there might be problems and this is a way how admins/users can check where a particular problem might be. And the very fact that we need to change this variable now does in fact proves that this might need another change in the future. Having this particular value configurable is merely a _option_ for admins/users and I see no drawback at all in it. As Rich suggested (and Cole copied), check out the number of hits for: https://www.google.co.uk/search?q=monitor+socket+did+not+show+up; Many of them are related to the domains having managed-save, but since nobody looked for a root cause of it (as I know of), this might be related to this very problem. Does this mean ACK to [2/2] and NACK to [1/2] then? Martin P.S.: I also forgot to mention that this might most probably resolve these bugs: https://bugzilla.redhat.com/show_bug.cgi?id=892273 https://bugzilla.redhat.com/show_bug.cgi?id=895901 https://bugzilla.redhat.com/show_bug.cgi?id=987088 https://bugzilla.redhat.com/show_bug.cgi?id=1051364 Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Add support for changing timeout value to open unix monitor socket
On Thu, Jan 23, 2014 at 04:40:40PM +0200, Pavel Fux wrote: I agree, there is no harm in adding an option of configuration, different setup configurations require different timeout values. my setup was 8 servers booted with PXE boot and running on nfs rootfs, with 8 vms on each. when I tried to start all of them together the bottle neck was the network and it takes about 5 minutes till they all start. That doesn't make any sense. The waiting code here is about the QEMU process' initial startup sequence - ie the gap between exec'ing the QEMU binary, and it listening on the monitor socket. PXE / nfsroot doesn't get involved there at all. Even if it were involve, if you're seeing 5 minute delays with only 8 vms on the host, something is seriously screwed with your host. This isn't a compelling reason to add this config option to libvirt. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH python] Fix calling of virStreamSend method
On 23/01/14 22:25, Daniel P. Berrange wrote: About the subject prefix, [libvirt-python] should be better, like what we did for Perl binding. Change d40861 removed the 'len' argument from the virStreamSend C level wrapper, but forgot to remove it from the python level wrapper. Reported-by: Robie Basak robie.ba...@canonical.com Signed-off-by: Daniel P. Berrange berra...@redhat.com --- libvirt-override-virStream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-override-virStream.py b/libvirt-override-virStream.py index cd44314..ce82da6 100644 --- a/libvirt-override-virStream.py +++ b/libvirt-override-virStream.py @@ -122,6 +122,6 @@ with the call, but may instead be delayed until a subsequent call. -ret = libvirtmod.virStreamSend(self._o, data, len(data)) +ret = libvirtmod.virStreamSend(self._o, data) if ret == -1: raise libvirtError ('virStreamSend() failed') return ret ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] allow OVMF users to disable qemu's -boot strict=on
On Wed, Jan 22, 2014 at 01:40:26PM +0100, Laszlo Ersek wrote: On 01/22/14 12:45, Laine Stump wrote: On 01/22/2014 12:45 PM, Daniel P. Berrange wrote: On Wed, Jan 22, 2014 at 01:33:18AM +0100, Laszlo Ersek wrote: Recently, commit 96fddee322c7d39a57cfdc5e7be71326d597d30a Author: Laine Stump la...@laine.org Date: Mon Dec 2 14:07:12 2013 +0200 qemu: add -boot strict to commandline whenever possible introduced a regression for OVMF guests. The symptoms and causes are described in patch 3/4, and in https://bugzilla.redhat.com/show_bug.cgi?id=1056258 Let's allow users to opt-out of -boot strict=on while preserving it as default. I don't really get from that bug description why this can't be made to work as desired in OVMF. It seems like its is just a bug in the OVMF impl that it doesn't work. I was on the verge of making that same comment in question form. From the information in the patches and the BZ, it sounds like either --boot strict is implemented incorrectly for OVMF, or OVMF doesn't do the proper thing with the HALT. What does OVMF do with bootable devices that aren't given a specific boot order? For seabios, those devices are all on the boot list following those with specific orders; this is what necessitates --boot strict. The behavior of the option should be consistent regardless of BIOS choice. Here's again how OVMF works, in detail. First, the list of openfirmware device paths is downloaded from fw_cfg. Then they are translated to UEFI device path *prefixes*. This translation (even just for the prefixes) is inexact (best effort), because no complete mapping exists. Also it can only cover UEFI device path prefixes because the OpenFirmware device paths don't extend into file paths. In UEFI you can have two separate boot options that boot two separate files from the exact same device (including partition), and you can't distinguish these in OpenFirmware device paths. Certainly not on the qemu command line. OK, so now you have two lists, the list of UEFI boot options (pre-set by the user in the firmware, or auto-generated by the firmware, doesn't matter), and the translated prefix list from qemu/fw_cfg. OVMF then iterates over the fw_cfg list, looks up the first prefix match from the UEFI boot option list that matches the current translated fw_cfg entry. If it is found, then this UEFI boot option is appended to the output list, and the UEFI boot option is also marked as having been added to the output list When the outer loop completes, you have a third list (the output list) which describes the user's boot preference. You also have some boot options that are unmarked (left unmatched by any translated fw_cfg entry). The question is what you do with these. Originally, I simply dropped these. This is precisely the -boot strict=on behavior. And it was wrong. Users wanted to keep at least *some* of these entries at the end of the list. My first question was ok why don't you just specify those in fw_cfg? And the answer is that those options *cannot* be specified. Ok, so this is the crux of the matter it seems to me. There are a bunch of boot options which cannot be expressed on the QEMU command line, or libvirt XML. Users are trying to deal with this problem by going behind QEMU/libvirt's back and hacking the firmware image to encode these options. I place this in the same category as people who replace the QEMU binary with a shell wrapper script to add a bunch of extra cli args. ie totally unsupportable. The goal we have is that the XML description should fully describe the configuration of a VM. Having a situation where the XML config only partially describes the setup, and the rest is delegated to a config embedded in the firmware image is not something we wish to support. IMHO what we need to tackle here is the inability to properly configure the firmware boot order from QEMU / libvirt. ie make it possible for users to fully control it via libvirt XML. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] qemu: Don't fail if the SCSI host device is shareable between domains
It doesn't make sense to fail if the SCSI host device is specified as shareable explicitly between domains (NB, it works if and only if the device is specified as shareable for *all* domains, otherwise it fails). To fix the problem, this patch introduces an array for virSCSIDevice struct, which records all the names of domain which are using the device (note that the recorded domains must specifiy the device as shareable). And the change on the data struct brings on many subsequent changes in the code. Since prior to this patch, the shareable tag didn't work as expected, it actually work like non-shareable. If there was a live domain using the SCSI device with shareable specified, then after upgrading (assuming the live domain keep running during upgrading) the older libvirt (without this patch) to newer libvirt (with this patch), it may cause some confusion when user tries to start later domains as shareable. So this patch also added notes in formatdomain.html to declare the fact. * src/util/virscsi.h: - Remove virSCSIDeviceGetUsedBy - Change definition of virSCSIDeviceGetUsedBy and virSCSIDeviceListDel - Add virSCSIDeviceIsAvailable * src/util/virscsi.c: - struct virSCSIDevice: Change used_by to be an array; Add n_used_by as the array count - virSCSIDeviceGetUsedBy: Removed - virSCSIDeviceFree: frees the used_by array - virSCSIDeviceSetUsedBy: Copy the domain name to avoid potential memory corruption - virSCSIDeviceIsAvailable: New - virSCSIDeviceListDel: Change the logic, for device which is already in the list, just remove the corresponding entry in used_by. And since it's only used in one place, we can safely removing the code to find out the dev in the list first. - Copyright updating * src/libvirt_private.sys: - virSCSIDeviceGetUsedBy: Remove - virSCSIDeviceIsAvailable: New * src/qemu/qemu_hostdev.c: - qemuUpdateActiveScsiHostdevs: Check if the device existing before adding it to the list; - qemuPrepareHostdevSCSIDevices: Error out if the not all domains use the device as shareable; Also don't try to add the device to the activeScsiHostdevs list if it already there; And make more sensible error w.r.t the current shareable value in driver-activeScsiHostdevs. - qemuDomainReAttachHostScsiDevices: Change the logic according to the changes on helpers. --- docs/formatdomain.html.in | 6 src/libvirt_private.syms | 2 +- src/qemu/qemu_hostdev.c | 77 ++- src/util/virscsi.c| 45 +-- src/util/virscsi.h| 7 +++-- 5 files changed, 90 insertions(+), 47 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ff50214..18bfad1 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2798,6 +2798,12 @@ between domains (assuming the hypervisor and OS support this). Only supported by SCSI host device. span class=sinceSince 1.0.6/span +p + Note: though codeshareable/shareable was introduced + span class=sincesince 1.0.6/span, but it doesn't work as + as expected (actually works like non-shareable) until + span class=since1.2.2/span. +/p /dd /dl diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 13caf93..0f30292 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1686,7 +1686,7 @@ virSCSIDeviceGetSgName; virSCSIDeviceGetShareable; virSCSIDeviceGetTarget; virSCSIDeviceGetUnit; -virSCSIDeviceGetUsedBy; +virSCSIDeviceIsAvailable; virSCSIDeviceListAdd; virSCSIDeviceListCount; virSCSIDeviceListDel; diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 86a463a..2b9d274 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -250,13 +250,14 @@ qemuUpdateActiveScsiHostdevs(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev = NULL; size_t i; int ret = -1; +virSCSIDevicePtr scsi = NULL; +virSCSIDevicePtr tmp = NULL; if (!def-nhostdevs) return 0; virObjectLock(driver-activeScsiHostdevs); for (i = 0; i def-nhostdevs; i++) { -virSCSIDevicePtr scsi = NULL; hostdev = def-hostdevs[i]; if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || @@ -271,11 +272,18 @@ qemuUpdateActiveScsiHostdevs(virQEMUDriverPtr driver, hostdev-shareable))) goto cleanup; -virSCSIDeviceSetUsedBy(scsi, def-name); - -if (virSCSIDeviceListAdd(driver-activeScsiHostdevs, scsi) 0) { +if ((tmp = virSCSIDeviceListFind(driver-activeScsiHostdevs, scsi))) { +if (virSCSIDeviceSetUsedBy(tmp, def-name) 0) { +virSCSIDeviceFree(scsi); +goto cleanup; +} virSCSIDeviceFree(scsi); -goto cleanup; +} else { +if
Re: [libvirt] [PATCH v3] qemu: Don't fail if the SCSI host device is shareable between domains
On 23/01/14 23:04, Osier Yang wrote: It doesn't make sense to fail if the SCSI host device is specified as shareable explicitly between domains (NB, it works if and only if the device is specified as shareable for *all* domains, otherwise it fails). To fix the problem, this patch introduces an array for virSCSIDevice struct, which records all the names of domain which are using the device (note that the recorded domains must specifiy the device as shareable). And the change on the data struct brings on many subsequent changes in the code. Since prior to this patch, the shareable tag didn't work as expected, it actually work like non-shareable. If there was a live domain using the SCSI device with shareable specified, then after upgrading (assuming the live domain keep running during upgrading) the older libvirt (without this patch) to newer libvirt (with this patch), it may cause some confusion when user tries to start later domains as shareable. So this patch also added notes in formatdomain.html to declare the fact. * src/util/virscsi.h: - Remove virSCSIDeviceGetUsedBy - Change definition of virSCSIDeviceGetUsedBy and virSCSIDeviceListDel - Add virSCSIDeviceIsAvailable * src/util/virscsi.c: - struct virSCSIDevice: Change used_by to be an array; Add n_used_by as the array count - virSCSIDeviceGetUsedBy: Removed - virSCSIDeviceFree: frees the used_by array - virSCSIDeviceSetUsedBy: Copy the domain name to avoid potential memory corruption - virSCSIDeviceIsAvailable: New - virSCSIDeviceListDel: Change the logic, for device which is already in the list, just remove the corresponding entry in used_by. And since it's only used in one place, we can safely removing the code to find out the dev in the list first. - Copyright updating * src/libvirt_private.sys: - virSCSIDeviceGetUsedBy: Remove - virSCSIDeviceIsAvailable: New * src/qemu/qemu_hostdev.c: - qemuUpdateActiveScsiHostdevs: Check if the device existing before adding it to the list; - qemuPrepareHostdevSCSIDevices: Error out if the not all domains use the device as shareable; Also don't try to add the device to the activeScsiHostdevs list if it already there; And make more sensible error w.r.t the current shareable value in driver-activeScsiHostdevs. - qemuDomainReAttachHostScsiDevices: Change the logic according to the changes on helpers. --- docs/formatdomain.html.in | 6 src/libvirt_private.syms | 2 +- src/qemu/qemu_hostdev.c | 77 ++- src/util/virscsi.c| 45 +-- src/util/virscsi.h| 7 +++-- 5 files changed, 90 insertions(+), 47 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ff50214..18bfad1 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2798,6 +2798,12 @@ between domains (assuming the hypervisor and OS support this). Only supported by SCSI host device. span class=sinceSince 1.0.6/span +p + Note: though codeshareable/shareable was introduced s/shareable/code/, :( -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add nocow feature option to vol-create
2014/1/15 Michal Privoznik mpriv...@redhat.com On 24.12.2013 09:56, Chunyan Liu wrote: Btrfs has terrible performance when hosting VM images, even more when the guest in those VM are also using btrfs as file system. One way to mitigate this bad performance is to turn off COW attributes on VM files (since having copy on write for this kind of data is not useful). According to 'chattr' manpage, NOCOW could be set to new or empty file only on btrfs, so this patch tries to add nocow feature option in volume xml and handle it in vol-create, so that users could have a chance to set NOCOW to a new volume if that happens on a btrfs like file system. Signed-off-by: Chunyan Liu cy...@suse.com --- This is a revised version to: http://www.redhat.com/archives/libvir-list/2013-December/msg00303.html Changes: * fix Daniel's comments --- docs/formatstorage.html.in | 12 +--- docs/schemas/storagefilefeatures.rng |3 ++ src/conf/storage_conf.c |9 -- src/storage/storage_backend.c|4 +- src/storage/storage_backend_fs.c | 48 ++ src/util/virstoragefile.c|1 + src/util/virstoragefile.h|1 + 7 files changed, 69 insertions(+), 9 deletions(-) Can you add a testcase that (at least) checks the RNG scheme? The more your test tests the better. Michal, thanks for your comment. Sorry for late response. I'm on vacation these days. Sure, I can add some testcase for that. diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index a089a31..3de1a2b 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -385,6 +385,7 @@ lt;compatgt;1.1lt;/compatgt; lt;featuresgt; lt;lazy_refcounts/gt; +lt;nocow/gt; lt;/featuresgt; lt;/targetgt;/pre @@ -423,11 +424,14 @@ span class=sinceSince 1.1.0/span /dd dtcodefeatures/code/dt - ddFormat-specific features. Only used for codeqcow2/code now. -Valid sub-elements are: -ul + ddFormat-specific features. Valid sub-elements are: + ul licodelt;lazy_refcounts/gt;/code - allow delayed reference - counter updates. span class=sinceSince 1.1.0/span/li + counter updates. Only used for codeqcow2/code now. + span class=sinceSince 1.1.0/span/li + licodelt;nocow/gt;/code - turn off copy-on-write. Only valid + to volume on codebtrfs/code, can improve performance. + span class=sinceSince 1.2.2/span/li /ul /dd /dl diff --git a/docs/schemas/storagefilefeatures.rng b/docs/schemas/storagefilefeatures.rng index 424b4e2..0cf3513 100644 --- a/docs/schemas/storagefilefeatures.rng +++ b/docs/schemas/storagefilefeatures.rng @@ -17,6 +17,9 @@ element name='lazy_refcounts' empty/ /element + element name='nocow' +empty/ + /element /optional /interleave /element diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 22e38c1..b6409a6 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1398,9 +1398,6 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if ((n = virXPathNodeSet(./target/features/*, ctxt, nodes)) 0) goto error; -if (!ret-target.compat VIR_STRDUP(ret-target.compat, 1.1) 0) -goto error; - if (!(ret-target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) goto error; @@ -1412,6 +1409,12 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, (const char*)nodes[i]-name); goto error; } + +if (f == VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS) { +if (!ret-target.compat VIR_STRDUP(ret-target.compat, 1.1) 0) +goto error; +} + ignore_value(virBitmapSetBit(ret-target.features, f)); } VIR_FREE(nodes); diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index b08d646..b4ab866 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -423,7 +423,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, operation_flags |= VIR_FILE_OPEN_FORK; if ((fd = virFileOpenAs(vol-target.path, -O_RDWR | O_CREAT | O_EXCL, +O_RDWR | O_CREAT, I don't think this is safe. I see why are you doing this [2], but what if user hadn't specified nocow feature? Then we might overwrite an existing file. And we want to do that. I'm aware of the overwrite issue. But in fact, if there is an
Re: [libvirt] [PATCH python] Fix calling of virStreamSend method
On 01/23/2014 07:47 AM, Osier Yang wrote: On 23/01/14 22:25, Daniel P. Berrange wrote: About the subject prefix, [libvirt-python] should be better, like what we did for Perl binding. Not possible. Anything sent to the list gets '[libvirt]' prepended, and then 'git format-patch --subject-prefix=PATCH python' prepends [PATCH python]. Your idea of setting libvirt-python in the git portion of the prefix would be redundant with the libvirt in the list portion of the prefix. I'm fine with the approach Daniel used. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Enable 'host-passthrough' cpu mode for aarch64
This patch allows libvirt user to specify 'host-passthrough' cpu mode while using qemu/kvm backend on aarch64. It uses 'host' as a CPU model name instead of some other stub (correct CPU detection is not implemented yet) to allow libvirt user to specify 'host-model' cpu mode as well. Signed-off-by: Oleg Strikov oleg.stri...@canonical.com --- src/cpu/cpu_aarch64.c | 30 ++ 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/cpu/cpu_aarch64.c b/src/cpu/cpu_aarch64.c index 3959deb..4afe9db 100644 --- a/src/cpu/cpu_aarch64.c +++ b/src/cpu/cpu_aarch64.c @@ -25,6 +25,7 @@ #include viralloc.h #include cpu.h +#include virstring.h #define VIR_FROM_THIS VIR_FROM_CPU @@ -44,16 +45,19 @@ AArch64NodeData(virArch arch) } static int -AArch64Decode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, +AArch64Decode(virCPUDefPtr cpu, const virCPUData *data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, const char *preferred ATTRIBUTE_UNUSED, unsigned int flags) { - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); +if (cpu-model == NULL +VIR_STRDUP(cpu-model, host) 0) +return -1; + return 0; } @@ -63,6 +67,24 @@ AArch64DataFree(virCPUDataPtr data) VIR_FREE(data); } +static int +AArch64Update(virCPUDefPtr guest, + const virCPUDef *host) +{ +guest-match = VIR_CPU_MATCH_EXACT; +virCPUDefFreeModel(guest); +return virCPUDefCopyModel(guest, host, true); +} + +static virCPUCompareResult +AArch64GuestData(virCPUDefPtr host ATTRIBUTE_UNUSED, + virCPUDefPtr guest ATTRIBUTE_UNUSED, + virCPUDataPtr *data ATTRIBUTE_UNUSED, + char **message ATTRIBUTE_UNUSED) +{ +return VIR_CPU_COMPARE_IDENTICAL; +} + struct cpuArchDriver cpuDriverAARCH64 = { .name = aarch64, .arch = archs, @@ -72,8 +94,8 @@ struct cpuArchDriver cpuDriverAARCH64 = { .encode = NULL, .free = AArch64DataFree, .nodeData = AArch64NodeData, -.guestData = NULL, +.guestData = AArch64GuestData, .baseline = NULL, -.update = NULL, +.update = AArch64Update, .hasFeature = NULL, }; -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] Add a mutex to serialize updates to firewall
On 01/23/2014 02:37 PM, Daniel P. Berrange wrote: The nwfilter conf update mutex previously serialized updates to the internal data structures for firewall rules, and updates to the firewall itself. Since the former is going to be turned into a read/write lock instead of a mutex, a new lock is required to serialize access to the firewall itself. Again, I agree with the analysis and theory of the change, but don't think I'm qualified to ACK. I did do a make and make syntax-check successfully (after fixing the few problems in the prior patches in the series, of course) Same comment goes for 5/5. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] allow OVMF users to disable qemu's -boot strict=on
On 01/23/14 15:58, Daniel P. Berrange wrote: On Wed, Jan 22, 2014 at 01:40:26PM +0100, Laszlo Ersek wrote: On 01/22/14 12:45, Laine Stump wrote: On 01/22/2014 12:45 PM, Daniel P. Berrange wrote: On Wed, Jan 22, 2014 at 01:33:18AM +0100, Laszlo Ersek wrote: Recently, commit 96fddee322c7d39a57cfdc5e7be71326d597d30a Author: Laine Stump la...@laine.org Date: Mon Dec 2 14:07:12 2013 +0200 qemu: add -boot strict to commandline whenever possible introduced a regression for OVMF guests. The symptoms and causes are described in patch 3/4, and in https://bugzilla.redhat.com/show_bug.cgi?id=1056258 Let's allow users to opt-out of -boot strict=on while preserving it as default. I don't really get from that bug description why this can't be made to work as desired in OVMF. It seems like its is just a bug in the OVMF impl that it doesn't work. I was on the verge of making that same comment in question form. From the information in the patches and the BZ, it sounds like either --boot strict is implemented incorrectly for OVMF, or OVMF doesn't do the proper thing with the HALT. What does OVMF do with bootable devices that aren't given a specific boot order? For seabios, those devices are all on the boot list following those with specific orders; this is what necessitates --boot strict. The behavior of the option should be consistent regardless of BIOS choice. Here's again how OVMF works, in detail. First, the list of openfirmware device paths is downloaded from fw_cfg. Then they are translated to UEFI device path *prefixes*. This translation (even just for the prefixes) is inexact (best effort), because no complete mapping exists. Also it can only cover UEFI device path prefixes because the OpenFirmware device paths don't extend into file paths. In UEFI you can have two separate boot options that boot two separate files from the exact same device (including partition), and you can't distinguish these in OpenFirmware device paths. Certainly not on the qemu command line. OK, so now you have two lists, the list of UEFI boot options (pre-set by the user in the firmware, or auto-generated by the firmware, doesn't matter), and the translated prefix list from qemu/fw_cfg. OVMF then iterates over the fw_cfg list, looks up the first prefix match from the UEFI boot option list that matches the current translated fw_cfg entry. If it is found, then this UEFI boot option is appended to the output list, and the UEFI boot option is also marked as having been added to the output list When the outer loop completes, you have a third list (the output list) which describes the user's boot preference. You also have some boot options that are unmarked (left unmatched by any translated fw_cfg entry). The question is what you do with these. Originally, I simply dropped these. This is precisely the -boot strict=on behavior. And it was wrong. Users wanted to keep at least *some* of these entries at the end of the list. My first question was ok why don't you just specify those in fw_cfg? And the answer is that those options *cannot* be specified. Ok, so this is the crux of the matter it seems to me. There are a bunch of boot options which cannot be expressed on the QEMU command line, or libvirt XML. Users are trying to deal with this problem by going behind QEMU/libvirt's back and hacking the firmware image to encode these options. You are technically right. I only have a problem with the word hacking. Setting UEFI boot options (which also includes saving them in flash memory, which indeed can be considered part of the firmware), is *valid*. It's not going behind anyone's back. It's a perfectly valid thing to do in a UEFI guest (even at runtime, with the efibootmgr Linux utility for example). I place this in the same category as people who replace the QEMU binary with a shell wrapper script to add a bunch of extra cli args. ie totally unsupportable. I agree that wrapper scripts are unsupportable. I disagree that users setting their persistent boot variables form inside the guest fall in the same category. That feature is an unalienable part of UEFI. The goal we have is that the XML description should fully describe the configuration of a VM. Having a situation where the XML config only partially describes the setup, and the rest is delegated to a config embedded in the firmware image is not something we wish to support. I understand. IMHO what we need to tackle here is the inability to properly configure the firmware boot order from QEMU / libvirt. ie make it possible for users to fully control it via libvirt XML. We'd face two hurdles towards this goal. - The first is that you'd need to get a basically free-form string through. Technically it wouldn't be very hard, but it's completely foreign from the current bootindex concept in libvirt/qemu. In UEFI, bootable device can refer to something that's just a chunk of guest RAM for qemu.
[libvirt] [v11 5/6] add hostdev pci backend type for xen
Add VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN. For legacy xen, it will use pciback as stub driver. Signed-off-by: Chunyan Liu cy...@suse.com --- docs/schemas/domaincommon.rng |1 + src/conf/domain_conf.c|3 ++- src/conf/domain_conf.h|1 + src/libxl/libxl_domain.c |9 + src/qemu/qemu_command.c |3 +-- src/qemu/qemu_hotplug.c |4 +--- src/util/virhostdev.c | 12 7 files changed, 27 insertions(+), 6 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7f55f24..9f5c47d 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3376,6 +3376,7 @@ choice valuekvm/value valuevfio/value + valuexen/value /choice /attribute empty/ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 28e24f9..69b10c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -623,7 +623,8 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysPciBackend, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST, default, kvm, - vfio) + vfio, + xen) VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST, storage, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d8f2e49..7a00cad 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -414,6 +414,7 @@ typedef enum { VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, /* detect automaticaly, prefer VFIO */ VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM,/* force legacy kvm style */ VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO, /* force vfio */ +VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN,/* force legacy xen style, use pciback */ VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST } virDomainHostdevSubsysPciBackendType; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index e72c483..36dd2f5 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -392,6 +392,15 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, STRNEQ(def-os.type, hvm)) dev-data.chr-targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; +if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV) { +virDomainHostdevDefPtr hostdev = dev-data.hostdev; + +if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS +hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI +hostdev-source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) +hostdev-source.subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; +} + return 0; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c6d075f..3809a1c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5537,8 +5537,7 @@ qemuBuildPCIHostdevDevStr(virDomainDefPtr def, virBufferAddLit(buf, vfio-pci); break; -case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: -case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: +default: virReportError(VIR_ERR_INTERNAL_ERROR, _(invalid PCI passthrough type '%s'), virDomainHostdevSubsysPciBackendTypeToString(backend)); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index badf7c7..8a69727 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1194,9 +1194,7 @@ qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver, virProcessSetMaxMemLock(vm-pid, memKB * 1024); break; -case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: -case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: -case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: +default: break; } diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index fa178e2..85d9e8d 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -373,6 +373,13 @@ virHostdevGetPciHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) } break; +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN : +if (virPCIDeviceSetStubDriver(dev, pciback) 0) { +virObjectUnref(list); +return NULL; +} +break; + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM : case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: if (virPCIDeviceSetStubDriver(dev, pci-stub) 0) { @@ -1192,6 +1199,11 @@ virHostdevUpdateActivePciHostdevs(virHostdevManagerPtr mgr, goto cleanup; break; +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN : +if (virPCIDeviceSetStubDriver(dev, pciback) 0) +goto cleanup; +break; + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM : case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: if (virPCIDeviceSetStubDriver(dev, pci-stub) 0) -- 1.6.0.2 -- libvir-list mailing list
[libvirt] [v11 0/6] Write separate module for hostdev passthrough
These patches implements a separate module for hostdev passthrough so that it could be shared by different drivers and can maintain a global state of a host device. patch 1/6: extract hostdev passthrough function from qemu_hostdev.c and make it reusable by multiple drivers. patch 2/6: add a unit test for hostdev common library. patch 3/6: switch qemu driver to use the common library instead of its own hostdev passthrough APIs. patch 4/6: switch lxc driver to use the common library instead of its own hostdev passthrough APIs. patch 5/6: add a hostdev pci backend type for xen usage. patch 6/6: add pci passthrough to libxl driver. --- Changes * rebase to lastest changes in src/util/virscsi.c, src/qemu/qemu_process.c, tests/virpcimock.c, etc. Chunyan Liu (6): add hostdev passthrough common library add unit test for hostdev common library change qemu driver to use hostdev common library change lxc driver to use hostdev common library add hostdev pci backend type for xen add pci passthrough to libxl driver docs/schemas/domaincommon.rng |1 + po/POTFILES.in |3 +- src/Makefile.am|3 +- src/conf/domain_conf.c |3 +- src/conf/domain_conf.h |1 + src/libvirt_private.syms | 21 + src/libxl/libxl_conf.c | 63 + src/libxl/libxl_conf.h |4 + src/libxl/libxl_domain.c |9 + src/libxl/libxl_driver.c | 448 +- src/lxc/lxc_conf.h |4 - src/lxc/lxc_driver.c | 47 +- src/lxc/lxc_hostdev.c | 413 - src/lxc/lxc_hostdev.h | 43 - src/lxc/lxc_process.c | 24 +- src/qemu/qemu_command.c|4 +- src/qemu/qemu_conf.h |9 +- src/qemu/qemu_domain.c | 22 + src/qemu/qemu_driver.c | 81 +- src/qemu/qemu_hostdev.c| 1457 - src/qemu/qemu_hostdev.h| 76 - src/qemu/qemu_hotplug.c| 136 +- src/qemu/qemu_process.c| 40 +- src/util/virhostdev.c | 1706 src/util/virhostdev.h | 134 ++ src/util/virpci.c | 30 +- src/util/virpci.h |9 +- src/util/virscsi.c | 28 +- src/util/virscsi.h |8 +- src/util/virusb.c | 29 +- src/util/virusb.h |8 +- tests/Makefile.am |5 + .../qemuxml2argv-hostdev-pci-address.xml |1 + .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml |1 + tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml|2 + tests/virhostdevtest.c | 473 ++ 36 files changed, 3132 insertions(+), 2214 deletions(-) delete mode 100644 src/lxc/lxc_hostdev.c delete mode 100644 src/lxc/lxc_hostdev.h delete mode 100644 src/qemu/qemu_hostdev.c delete mode 100644 src/qemu/qemu_hostdev.h create mode 100644 src/util/virhostdev.c create mode 100644 src/util/virhostdev.h create mode 100644 tests/virhostdevtest.c -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [v11 6/6] add pci passthrough to libxl driver
Add pci passthrough to libxl driver, support attach-device, detach-device and start a vm with pci hostdev specified. Signed-off-by: Chunyan Liu cy...@suse.com --- src/libxl/libxl_conf.c | 63 +++ src/libxl/libxl_conf.h |4 + src/libxl/libxl_driver.c | 448 +- 3 files changed, 514 insertions(+), 1 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4cefadf..54f9f9c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1141,6 +1141,66 @@ libxlDriverConfigGet(libxlDriverPrivatePtr driver) return cfg; } +int +libxlMakePci(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev) +{ +if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) +return -1; +if (hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) +return -1; + +pcidev-domain = hostdev-source.subsys.u.pci.addr.domain; +pcidev-bus = hostdev-source.subsys.u.pci.addr.bus; +pcidev-dev = hostdev-source.subsys.u.pci.addr.slot; +pcidev-func = hostdev-source.subsys.u.pci.addr.function; + +return 0; +} + +static int +libxlMakePciList(virDomainDefPtr def, libxl_domain_config *d_config) +{ +virDomainHostdevDefPtr *l_hostdevs = def-hostdevs; +size_t nhostdevs = def-nhostdevs; +size_t npcidevs = 0; +libxl_device_pci *x_pcidevs; +size_t i, j; + +if (nhostdevs == 0) +return 0; + +if (VIR_ALLOC_N(x_pcidevs, nhostdevs) 0) +return -1; + +for (i = 0, j = 0; i nhostdevs; i++) { +if (l_hostdevs[i]-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) +continue; +if (l_hostdevs[i]-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) +continue; + +libxl_device_pci_init(x_pcidevs[j]); + +if (libxlMakePci(l_hostdevs[i], x_pcidevs[j]) 0) +goto error; + +npcidevs++; +j++; +} + +VIR_SHRINK_N(x_pcidevs, nhostdevs, nhostdevs - npcidevs); +d_config-pcidevs = x_pcidevs; +d_config-num_pcidevs = npcidevs; + +return 0; + +error: +for (i = 0; i npcidevs; i++) +libxl_device_pci_dispose(x_pcidevs[i]); + +VIR_FREE(x_pcidevs); +return -1; +} + virCapsPtr libxlMakeCapabilities(libxl_ctx *ctx) { @@ -1189,6 +1249,9 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver, if (libxlMakeVfbList(driver, def, d_config) 0) return -1; +if (libxlMakePciList(def, d_config) 0) +return -1; + d_config-on_reboot = def-onReboot; d_config-on_poweroff = def-onPoweroff; d_config-on_crash = def-onCrash; diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index f743541..65c3553 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -39,6 +39,7 @@ # include virchrdev.h +# define LIBXL_DRIVER_NAME xenlight # define LIBXL_VNC_PORT_MIN 5900 # define LIBXL_VNC_PORT_MAX 65535 @@ -151,6 +152,9 @@ libxlMakeVfb(libxlDriverPrivatePtr driver, virDomainGraphicsDefPtr l_vfb, libxl_device_vfb *x_vfb); int +libxlMakePci(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev); + +int libxlBuildDomainConfig(libxlDriverPrivatePtr driver, virDomainObjPtr vm, libxl_domain_config *d_config); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index fc0efa2..4f156be 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -52,6 +52,7 @@ #include virsysinfo.h #include viraccessapicheck.h #include viratomic.h +#include virhostdev.h #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -328,6 +329,7 @@ libxlVmReap(libxlDriverPrivatePtr driver, virDomainShutoffReason reason) { libxlDomainObjPrivatePtr priv = vm-privateData; +virHostdevManagerPtr hostdev_mgr; if (libxl_domain_destroy(priv-ctx, vm-def-id, NULL) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -335,6 +337,11 @@ libxlVmReap(libxlDriverPrivatePtr driver, return -1; } +hostdev_mgr = virHostdevManagerGetDefault(); +if (hostdev_mgr != NULL) +virHostdevReAttachDomainHostdevs(hostdev_mgr, LIBXL_DRIVER_NAME, + vm-def, VIR_SP_PCI_HOSTDEV); + libxlVmCleanup(driver, vm, reason); return 0; } @@ -558,6 +565,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, #ifdef LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS libxl_domain_restore_params params; #endif +virHostdevManagerPtr hostdev_mgr; if (libxlDomainObjPrivateInitCtx(vm) 0) goto error; @@ -616,6 +624,13 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto error; } +VIR_DEBUG(Preparing host PCI devices); +hostdev_mgr = virHostdevManagerGetDefault(); +if (hostdev_mgr == NULL || +virHostdevPrepareDomainHostdevs(hostdev_mgr, LIBXL_DRIVER_NAME, +vm-def, VIR_SP_PCI_HOSTDEV) 0) +goto error; + /* use as
[libvirt] [v11 4/6] change lxc driver to use hostdev common library
Change lxc driver to use hostdev common library instead of APIs in lxc_hostdev.[ch] Signed-off-by: Chunyan Liu cy...@suse.com --- po/POTFILES.in|1 - src/Makefile.am |1 - src/lxc/lxc_conf.h|4 - src/lxc/lxc_driver.c | 47 --- src/lxc/lxc_hostdev.c | 416 - src/lxc/lxc_hostdev.h | 43 - src/lxc/lxc_process.c | 24 +++- 7 files changed, 48 insertions(+), 488 deletions(-) delete mode 100644 src/lxc/lxc_hostdev.c delete mode 100644 src/lxc/lxc_hostdev.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 9e71db3..94017c6 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -61,7 +61,6 @@ src/locking/lock_manager.c src/locking/sanlock_helper.c src/lxc/lxc_cgroup.c src/lxc/lxc_fuse.c -src/lxc/lxc_hostdev.c src/lxc/lxc_container.c src/lxc/lxc_conf.c src/lxc/lxc_controller.c diff --git a/src/Makefile.am b/src/Makefile.am index e8b6fc4..20ceec2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -619,7 +619,6 @@ LXC_DRIVER_SOURCES = \ lxc/lxc_container.c lxc/lxc_container.h \ lxc/lxc_cgroup.c lxc/lxc_cgroup.h \ lxc/lxc_domain.c lxc/lxc_domain.h \ - lxc/lxc_hostdev.c lxc/lxc_hostdev.h \ lxc/lxc_monitor.c lxc/lxc_monitor.h \ lxc/lxc_process.c lxc/lxc_process.h \ lxc/lxc_fuse.c lxc/lxc_fuse.h \ diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index e04dcdd..5be159b 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -93,10 +93,6 @@ struct _virLXCDriver { /* Immutable pointer, self-locking APIs */ virDomainObjListPtr domains; -/* Immutable pointer. Requires lock to be held before - * calling APIs. */ -virUSBDeviceListPtr activeUsbHostdevs; - /* Immutable pointer, self-locking APIs */ virObjectEventStatePtr domainEventState; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 982f3fc..3a62638 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -70,6 +70,7 @@ #include virstring.h #include viraccessapicheck.h #include viraccessapichecklxc.h +#include virhostdev.h #define VIR_FROM_THIS VIR_FROM_LXC @@ -1526,9 +1527,6 @@ static int lxcStateInitialize(bool privileged, if (!(lxc_driver-securityManager = lxcSecurityInit(cfg))) goto cleanup; -if ((lxc_driver-activeUsbHostdevs = virUSBDeviceListNew()) == NULL) -goto cleanup; - if ((virLXCDriverGetCapabilities(lxc_driver, true)) == NULL) goto cleanup; @@ -1643,7 +1641,6 @@ static int lxcStateCleanup(void) virSysinfoDefFree(lxc_driver-hostsysinfo); -virObjectUnref(lxc_driver-activeUsbHostdevs); virObjectUnref(lxc_driver-caps); virObjectUnref(lxc_driver-securityManager); virObjectUnref(lxc_driver-xmlopt); @@ -3903,6 +3900,7 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver, mode_t mode; bool created = false; virUSBDevicePtr usb = NULL; +virHostdevManagerPtr hostdev_mgr; if (virDomainHostdevFind(vm-def, def, NULL) = 0) { virReportError(VIR_ERR_OPERATION_FAILED, %s, @@ -3910,6 +3908,14 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver, return -1; } +hostdev_mgr = virHostdevManagerGetDefault(); +if (hostdev_mgr == NULL || +virHostdevPrepareUsbHostdevs(hostdev_mgr, + LXC_DRIVER_NAME, + vm-def-name, + def, 1, 0) 0) +return -1; + if (virAsprintf(vroot, /proc/%llu/root, (unsigned long long)priv-initpid) 0) goto cleanup; @@ -3985,6 +3991,11 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver, ret = 0; cleanup: +virHostdevReAttachUsbHostdevs(hostdev_mgr, + LXC_DRIVER_NAME, + vm-def-name, + def, + 1); virDomainAuditHostdev(vm, def, attach, ret == 0); if (ret 0 created) unlink(dstfile); @@ -4447,8 +4458,7 @@ cleanup: static int -lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, -virDomainObjPtr vm, +lxcDomainDetachDeviceHostdevUSBLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) { virLXCDomainObjPrivatePtr priv = vm-privateData; @@ -4457,6 +4467,7 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, char *dst = NULL; char *vroot = NULL; virUSBDevicePtr usb = NULL; +virHostdevManagerPtr hostdev_mgr; if ((idx = virDomainHostdevFind(vm-def, dev-data.hostdev, @@ -4501,9 +4512,10 @@
[libvirt] [v11 2/6] add unit test for hostdev common library
Add unit test for hostdev common library. Current tests are based on virpcimock. Signed-off-by: Chunyan Liu cy...@suse.com --- tests/Makefile.am |5 + tests/virhostdevtest.c | 473 2 files changed, 478 insertions(+), 0 deletions(-) create mode 100644 tests/virhostdevtest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 568b7a0..3e66d8c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -141,6 +141,7 @@ test_programs = virshtest sockettest \ virportallocatortest \ sysinfotest \ virstoragetest \ + virhostdevtest \ $(NULL) if WITH_REMOTE @@ -754,6 +755,10 @@ vircgroupmock_la_CFLAGS = $(AM_CFLAGS) vircgroupmock_la_LDFLAGS = -module -avoid-version \ -rpath /evil/libtool/hack/to/force/shared/lib/creation +virhostdevtest_SOURCES = \ + virhostdevtest.c testutils.h testutils.c +virhostdevtest_LDADD = $(LDADDS) + virpcitest_SOURCES = \ virpcitest.c testutils.h testutils.c virpcitest_LDADD = $(LDADDS) diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c new file mode 100644 index 000..2dd9370 --- /dev/null +++ b/tests/virhostdevtest.c @@ -0,0 +1,473 @@ +/* + * Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Author: Chunyan Liu cy...@suse.com + */ + +#include config.h + +#include testutils.h + +#ifdef __linux__ + +# include stdlib.h +# include stdio.h +# include sys/types.h +# include sys/stat.h +# include fcntl.h +# include virlog.h +# include virhostdev.h + +# define VIR_FROM_THIS VIR_FROM_NONE + +# define CHECK_LIST_COUNT(list, cnt)\ +if ((count = virPCIDeviceListCount(list)) != cnt) { \ +virReportError(VIR_ERR_INTERNAL_ERROR, \ + Unexpected count of items in #list : %d,\ + expecting %zu, count, (size_t) cnt); \ +goto cleanup; \ +} + +# define TEST_STATE_DIR abs_builddir /hostdevmgr +static const char *drv_name = test_driver; +static const char *dom_name = test_domain; +static const unsigned char *uuid = +(unsigned char *)(f92360b0-2541-8791-fb32-d1f838811541); +static int nhostdevs = 3; +static virDomainHostdevDefPtr hostdevs[] = {NULL, NULL, NULL}; +static virPCIDevicePtr dev[] = {NULL, NULL, NULL}; +static virHostdevManagerPtr mgr = NULL; + +static void +myCleanup(void) +{ +size_t i; +for (i = 0; i nhostdevs; i++) { + virPCIDeviceFree(dev[i]); + virDomainHostdevDefFree(hostdevs[i]); +} + +if (mgr) { +virObjectUnref(mgr-activePciHostdevs); +virObjectUnref(mgr-inactivePciHostdevs); +virObjectUnref(mgr-activeUsbHostdevs); +VIR_FREE(mgr-stateDir); +VIR_FREE(mgr); +} +} + +static int +myInit(void) +{ +size_t i; + +for (i = 0; i nhostdevs; i++) { +virDomainHostdevSubsys subsys; +hostdevs[i] = virDomainHostdevDefAlloc(); +if (!hostdevs[i]) +goto cleanup; +hostdevs[i]-mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; +subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; +subsys.u.pci.addr.domain = 0; +subsys.u.pci.addr.bus = 0; +subsys.u.pci.addr.slot = i + 1; +subsys.u.pci.addr.function = 0; +subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM; +hostdevs[i]-source.subsys = subsys; +} + +for (i = 0; i nhostdevs; i++) { +if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)) || +virPCIDeviceSetStubDriver(dev[i], pci-stub) 0) +goto cleanup; +} + +if (VIR_ALLOC(mgr) 0) +goto cleanup; +if ((mgr-activePciHostdevs = virPCIDeviceListNew()) == NULL) +goto cleanup; +if ((mgr-activeUsbHostdevs = virUSBDeviceListNew()) == NULL) +goto cleanup; +if ((mgr-inactivePciHostdevs = virPCIDeviceListNew()) == NULL) +goto cleanup; +if ((mgr-activeScsiHostdevs = virSCSIDeviceListNew()) == NULL) +goto cleanup; +if (VIR_STRDUP(mgr-stateDir, TEST_STATE_DIR) 0) +goto cleanup; +if (virFileMakePath(mgr-stateDir) 0) +goto cleanup; + +return 0; + +cleanup: +
Re: [libvirt] [PATCH 1/5] Add a read/write lock implementation
On 01/23/2014 03:46 PM, Daniel P. Berrange wrote: On Thu, Jan 23, 2014 at 03:22:53PM +0200, Laine Stump wrote: On 01/23/2014 02:37 PM, Daniel P. Berrange wrote: Add virRWLock backed up by a POSIX rwlock primitive Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/libvirt_private.syms| 5 + src/util/virthread.h| 10 ++ src/util/virthreadpthread.c | 31 +++ src/util/virthreadpthread.h | 4 4 files changed, 50 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d1a58f9..eb91693 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1816,6 +1816,11 @@ virMutexInitRecursive; virMutexLock; virMutexUnlock; virOnce; +virRWLockInit; +virRWLockDestroy; +virRWLockRead; +virRWLockWrite; +virRWLockUnlock; These are out of order. virThreadCancel; virThreadCreate; virThreadID; diff --git a/src/util/virthread.h b/src/util/virthread.h index 84d3bdc..7015d60 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -28,6 +28,9 @@ typedef struct virMutex virMutex; typedef virMutex *virMutexPtr; +typedef struct virRWLock virRWLock; +typedef virRWLock *virRWLockPtr; + typedef struct virCond virCond; typedef virCond *virCondPtr; @@ -89,6 +92,13 @@ void virMutexLock(virMutexPtr m); void virMutexUnlock(virMutexPtr m); +int virRWLockInit(virRWLockPtr m) ATTRIBUTE_RETURN_CHECK; +void virRWLockDestroy(virRWLockPtr m); + +void virRWLockRead(virRWLockPtr m); +void virRWLockWrite(virRWLockPtr m); +void virRWLockUnlock(virRWLockPtr m); + I was about to predict a build failure on Windows since you haven't defined these functions in virthreadwin32.c, but then realized you aren't yet calling them from anywhere, so the build will complete just fine. It would reduce code motion a bit to swap this patch with patch 2, but functionally there's no difference. FYI the reason I did it in this order, is to make it easier to backport to the stable branches. eg on the stable branches I'd want to skip patch 2, since that's going to require a gnulib update too. Aha. Totally worth it then! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] [PATCH]Correct syntax-check Using qemu with sheepdog pool
From: Joel SIMOES joel.sim...@laposte.net --- src/qemu/qemu_conf.c | 16 +--- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index dfafcdc..954542a 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1215,17 +1215,13 @@ qemuAddSheepPoolSourceHost(virDomainDiskDefPtr def, 7000) 0) goto cleanup; - - - - /* Storage pool have not supported these 2 attributes yet, * use the defaults. */ def-hosts[0].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; def-hosts[0].socket = NULL; - + def-protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; ret = 0; @@ -1402,15 +1398,14 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, def-srcpool-pooltype = pooldef-type; def-srcpool-voltype = info.type; - -if (def-srcpool-mode pooldef-type != VIR_STORAGE_POOL_ISCSI !(def-srcpool-mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT pooldef-type == VIR_STORAGE_POOL_SHEEPDOG) ) { - + +if (def-srcpool-mode pooldef-type != VIR_STORAGE_POOL_ISCSI !(def-srcpool-mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT pooldef-type == VIR_STORAGE_POOL_SHEEPDOG)) { virReportError(VIR_ERR_XML_ERROR, %s, _(disk source mode is only valid when storage pool is of iscsi type or only direct for sheepdog )); goto cleanup; } - + VIR_FREE(def-src); virDomainDiskHostDefFree(def-nhosts, def-hosts); virDomainDiskAuthClear(def); @@ -1496,11 +1491,10 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, def-srcpool-mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT; def-protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; def-src = virStorageVolGetPath(vol); - qemuAddSheepPoolSourceHost(def, pooldef); break; case VIR_STORAGE_POOL_MPATH: -case VIR_STORAGE_POOL_RBD: +case VIR_STORAGE_POOL_RBD: case VIR_STORAGE_POOL_GLUSTER: case VIR_STORAGE_POOL_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Add support for changing timeout value to open unix monitor socket
there are 8 servers with 8 vms on each server. all the qcow images are on the nfs share on the same external server. we are starting all 64 vms at the same time. each vm is 2.5GB X 64vms = 160GB = 1280Gb to read all of the data on a 1Gbe interface will take 1280sec = 21.3min not all of the image is being read on boot so it takes only 5min anyway, one timeout won't solve all problems, why not 31? or 60? On Thu, Jan 23, 2014 at 4:44 PM, Daniel P. Berrange berra...@redhat.comwrote: On Thu, Jan 23, 2014 at 04:40:40PM +0200, Pavel Fux wrote: I agree, there is no harm in adding an option of configuration, different setup configurations require different timeout values. my setup was 8 servers booted with PXE boot and running on nfs rootfs, with 8 vms on each. when I tried to start all of them together the bottle neck was the network and it takes about 5 minutes till they all start. That doesn't make any sense. The waiting code here is about the QEMU process' initial startup sequence - ie the gap between exec'ing the QEMU binary, and it listening on the monitor socket. PXE / nfsroot doesn't get involved there at all. Even if it were involve, if you're seeing 5 minute delays with only 8 vms on the host, something is seriously screwed with your host. This isn't a compelling reason to add this config option to libvirt. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/:| |: http://libvirt.org -o- http://virt-manager.org:| |: http://autobuild.org -o- http://search.cpan.org/~danberr/:| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc:| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] What is the robust/recommended way to retrieve the PID of a VM's init process ?
So I did give this interesting lxc-enter-namespace feature a try Thank you. This is great, it solves my initial problem quite nicely, and I have given up on using my own tool :-) So my comments are, - first that defining some even minimal PATH would help; PATH relative to the container? Yes, that might be a nice addition; would you like to propose the means of passing that information through to the child? Maybe I can describe my needs a little bit more Typically I need to run ‘make’ in the guest as part of its ‘preparation’ before I hand it over to users Unfortunately not all guests have the ‘make’ binary at the same location - esp. with the recent usrmove business… Actually what I am doing right now is to let bash handle that with virsh lxc-enter-namespace container /bin/bash -c “make -C /build” which fits my needs to not have to decide where make might sit This seems to work fine, although I am not sure to have covered all the corner cases yet --- This being said, this is fine as long as I use that in scripts, but having to do this in everyday life would be a hassle I am not sure that I see any good reason not to just call ‘execvp’ instead of ‘execv’ in cmdLxcEnterNamespace It is true that the PATH as set in the host might not be the best setting for the guest, but it could serve as a decent approximation for a daily usage - and second that in this first form, it would be great if virsh could write some error on stderr instead of being almost totally silent, it took me some time to figure that it kind of worked Does this mean we would lose any message sent on stderr ? No, stderr is preserved. But I have patches awaiting review that make the process nicer, including informing you of any non-zero exit status of the executed command: https://www.redhat.com/archives/libvir-list/2013-December/msg01247.html This change would have been welcome during my very first attempt :-) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] bhyve: add a basic driver
At this point it has a limited functionality and is highly experimental. Supported domain operations are: * define * start * destroy * dumpxml * dominfo It's only possible to have only one disk device and only one network, which should be of type bridge. --- configure.ac| 11 + daemon/libvirtd.c | 9 + include/libvirt/virterror.h | 1 + m4/virt-driver-bhyve.m4 | 52 src/Makefile.am | 38 +++ src/bhyve/bhyve_command.c | 269 + src/bhyve/bhyve_command.h | 41 src/bhyve/bhyve_driver.c| 566 src/bhyve/bhyve_driver.h| 28 +++ src/bhyve/bhyve_process.c | 205 src/bhyve/bhyve_process.h | 36 +++ src/bhyve/bhyve_utils.h | 48 src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + src/driver.h| 1 + src/libvirt.c | 3 + src/util/virerror.c | 1 + 17 files changed, 1312 insertions(+), 1 deletion(-) create mode 100644 m4/virt-driver-bhyve.m4 create mode 100644 src/bhyve/bhyve_command.c create mode 100644 src/bhyve/bhyve_command.h create mode 100644 src/bhyve/bhyve_driver.c create mode 100644 src/bhyve/bhyve_driver.h create mode 100644 src/bhyve/bhyve_process.c create mode 100644 src/bhyve/bhyve_process.h create mode 100644 src/bhyve/bhyve_utils.h diff --git a/configure.ac b/configure.ac index 3a70375..bfbd3a8 100644 --- a/configure.ac +++ b/configure.ac @@ -524,6 +524,10 @@ AC_ARG_WITH([parallels], [AS_HELP_STRING([--with-parallels], [add Parallels Cloud Server support @:@default=check@:@])]) m4_divert_text([DEFAULTS], [with_parallels=check]) +AC_ARG_WITH([bhyve], + [AS_HELP_STRING([--with-bhyve], +[add BHyVe support @:@default=check@:@])]) +m4_divert_text([DEFAULTS], [with_bhyve=check]) AC_ARG_WITH([test], [AS_HELP_STRING([--with-test], [add test driver support @:@default=yes@:@])]) @@ -1031,6 +1035,12 @@ fi AM_CONDITIONAL([WITH_PARALLELS], [test $with_parallels = yes]) dnl +dnl Checks for bhyve driver +dnl + +LIBVIRT_DRIVER_CHECK_BHYVE + +dnl dnl check for shell that understands redirection without truncation, dnl needed by src/qemu/qemu_monitor_{text,json}.c. dnl @@ -2677,6 +2687,7 @@ AC_MSG_NOTICE([ PHYP: $with_phyp]) AC_MSG_NOTICE([ ESX: $with_esx]) AC_MSG_NOTICE([ Hyper-V: $with_hyperv]) AC_MSG_NOTICE([Parallels: $with_parallels]) +LIBIVRT_DRIVER_RESULT_BHYVE AC_MSG_NOTICE([ Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) AC_MSG_NOTICE([ Network: $with_network]) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 49c42ad..b27c6fd 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -77,6 +77,9 @@ # ifdef WITH_VBOX # include vbox/vbox_driver.h # endif +# ifdef WITH_BHYVE +# include bhyve/bhyve_driver.h +# endif # ifdef WITH_NETWORK # include network/bridge_driver.h # endif @@ -405,6 +408,9 @@ static void daemonInitialize(void) # ifdef WITH_VBOX virDriverLoadModule(vbox); # endif +# ifdef WITH_BHYVE +virDriverLoadModule(bhyve); +# endif #else # ifdef WITH_NETWORK networkRegister(); @@ -442,6 +448,9 @@ static void daemonInitialize(void) # ifdef WITH_VBOX vboxRegister(); # endif +# ifdef WITH_BHYVE +bhyveRegister(); +# endif #endif } diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index e31e9c4..7915bbb 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -121,6 +121,7 @@ typedef enum { VIR_FROM_ACCESS = 55, /* Error from access control manager */ VIR_FROM_SYSTEMD = 56, /* Error from systemd code */ +VIR_FROM_BHYVE = 57,/* Error from bhyve driver */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST # endif diff --git a/m4/virt-driver-bhyve.m4 b/m4/virt-driver-bhyve.m4 new file mode 100644 index 000..7b13722 --- /dev/null +++ b/m4/virt-driver-bhyve.m4 @@ -0,0 +1,52 @@ +dnl The bhyve driver +dnl +dnl Copyright (C) 2014 Roman Bogorodskiy +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl http://www.gnu.org/licenses/. +dnl + +AC_DEFUN([LIBVIRT_DRIVER_CHECK_BHYVE],[ +if test $with_bhyve != no; then +AC_PATH_PROG([BHYVE], [bhyve], [], [$PATH:/usr/sbin]) +AC_PATH_PROG([BHYVECTL], [bhyvectl], [$PATH:/usr/sbin]) +
[libvirt] [PATCH v3] bhyve: add a basic driver
Changes from the previos version, based on Daniel's feedback: * Moved autoconf driver detection to m4/ * Added driver sources to STATEFUL_DRIVER_SOURCE_FILES * Dropped unneeded locking from DefineXML * Initial implemtation of dumpxml/dominfo, stub of state * Moved command generation to bhyve_command.c * Attempt to unload VM on errors Roman Bogorodskiy (1): bhyve: add a basic driver configure.ac| 11 + daemon/libvirtd.c | 9 + include/libvirt/virterror.h | 1 + m4/virt-driver-bhyve.m4 | 52 src/Makefile.am | 38 +++ src/bhyve/bhyve_command.c | 269 + src/bhyve/bhyve_command.h | 41 src/bhyve/bhyve_driver.c| 566 src/bhyve/bhyve_driver.h| 28 +++ src/bhyve/bhyve_process.c | 205 src/bhyve/bhyve_process.h | 36 +++ src/bhyve/bhyve_utils.h | 48 src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + src/driver.h| 1 + src/libvirt.c | 3 + src/util/virerror.c | 1 + 17 files changed, 1312 insertions(+), 1 deletion(-) create mode 100644 m4/virt-driver-bhyve.m4 create mode 100644 src/bhyve/bhyve_command.c create mode 100644 src/bhyve/bhyve_command.h create mode 100644 src/bhyve/bhyve_driver.c create mode 100644 src/bhyve/bhyve_driver.h create mode 100644 src/bhyve/bhyve_process.c create mode 100644 src/bhyve/bhyve_process.h create mode 100644 src/bhyve/bhyve_utils.h -- 1.8.4.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv7 0/2] Implement RBD storage pool support
Here is a re-based and fixed up re-submission of my patches to implement RBD storage pool support for QEMU domains. Nothing in it has changed much, other than it has been rebased against the lastest libvirt and two of my patches have been squashed in order to pass a make, make check, and make syntax-check after each individual patch. The code here still works well for me, but I only have the one host to test this on. If possible, I would like to try and get this merged for the 1.2.2 release cycle, so that other users can benefit from the added storage pool support. As always, if you find any problems with my patches, please let me know, and I will fix them up as soon as time permits. Adam Walters (2): qemu: conf Implement domain RBD storage pool support domain: conf: Fix secret type checking for network volumes src/conf/domain_conf.c | 6 +++-- src/qemu/qemu_conf.c | 62 +- 2 files changed, 65 insertions(+), 3 deletions(-) -- 1.8.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv7 1/2] qemu: conf Implement domain RBD storage pool support
This patch adds a helper function, qemuAddRBDPoolSourceHost, and implements the usage of this function to allow RBD storage pools in QEMU domain XML. The new function grabs RBD monitor hosts from the storage pool definition and applies them to the domain's disk definition at runtime. This function is used by my modifications to qemuTranslateDiskSourcePool similar to the function used by the iSCSI code. My modifications to qemuTranslateDiskSourcePool is based heavily on the existing iSCSI code, but modified to support RBD install. It will place all relevant information into the domain's disk definition at runtime to allow access to a RBD storage pool. Signed-off-by: Adam Walters a...@pandorasboxen.com --- src/qemu/qemu_conf.c | 62 +++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ac53f6d..b1a6bfe 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1247,6 +1247,45 @@ cleanup: } static int +qemuAddRBDPoolSourceHost(virDomainDiskDefPtr def, + virStoragePoolDefPtr pooldef) +{ +int ret = -1; +size_t i = 0; +char **tokens = NULL; + +def-nhosts = pooldef-source.nhost; + +if (VIR_ALLOC_N(def-hosts, def-nhosts) 0) +goto cleanup; + +for (i = 0; i def-nhosts; i++) { +if (VIR_STRDUP(def-hosts[i].name, pooldef-source.hosts[i].name) 0) +goto cleanup; + +if (virAsprintf(def-hosts[i].port, %d, +pooldef-source.hosts[i].port ? +pooldef-source.hosts[i].port : +6789) 0) +goto cleanup; + +/* Storage pools have not supported these 2 attributes yet, + * use the defaults. + */ +def-hosts[i].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; +def-hosts[i].socket = NULL; +} + +def-protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; + +ret = 0; + +cleanup: +virStringFreeList(tokens); +return ret; +} + +static int qemuTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def, virStoragePoolDefPtr pooldef) { @@ -1439,8 +1478,29 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, } break; -case VIR_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_RBD: +if (def-startupPolicy) { +virReportError(VIR_ERR_XML_ERROR, %s, + _('startupPolicy' is only valid for + 'file' file volume)); +goto cleanup; +} + +def-srcpool-actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK; +def-protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; + +if (!(def-src = virStorageVolGetPath(vol))) +goto cleanup; + +if (qemuTranslateDiskSourcePoolAuth(def, pooldef) 0) +goto cleanup; + +if (qemuAddRBDPoolSourceHost(def, pooldef) 0) +goto cleanup; + +break; + +case VIR_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_SHEEPDOG: case VIR_STORAGE_POOL_GLUSTER: case VIR_STORAGE_POOL_LAST: -- 1.8.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv7 2/2] domain: conf: Fix secret type checking for network volumes
This patch fixes the secret type checking done in the virDomainDiskDefParseXML function. Previously, it would not allow any volumes that utilized a secret. This patch is a simple bypass of the checking code for volumes. Signed-off-by: Adam Walters a...@pandorasboxen.com --- src/conf/domain_conf.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 28e24f9..773dc26 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5418,7 +5418,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, cur = cur-next; } -if (auth_secret_usage != -1 auth_secret_usage != expected_secret_usage) { +if (auth_secret_usage != -1 auth_secret_usage != expected_secret_usage +def-type != VIR_DOMAIN_DISK_TYPE_VOLUME) { virReportError(VIR_ERR_INTERNAL_ERROR, _(invalid secret type '%s'), virSecretUsageTypeTypeToString(auth_secret_usage)); @@ -18214,7 +18215,8 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, if (!disk-src || disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK || (disk-type == VIR_DOMAIN_DISK_TYPE_VOLUME disk-srcpool - disk-srcpool-mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT)) + (disk-srcpool-mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT || + disk-srcpool-mode == VIR_DOMAIN_DISK_TYPE_NETWORK))) return 0; if (iter(disk, disk-src, 0, opaque) 0) -- 1.8.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] Turn nwfilter conf update mutex into a read/write lock
On 01/23/2014 07:37 AM, Daniel P. Berrange wrote: @@ -3477,7 +3482,7 @@ int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, initialized = true; -if (virMutexInitRecursive(updateMutex) 0) +if (virRWLockInit(updateLock) 0) return -1; Obviously it was a recursive lock before. Now pthread_rwlock_wrlock() must not be called twice by the same thread. I am fine with the conversion below per se. The functions below are quite low-level now (close to virsh) that they are not called recursively. So from this perspective for this patch: ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCHv2 1/2] driver: Implement new state driver field
This implements a new field in the virStateDrvier struct. In order to prevent possible compilation issues, this patch also implements the new field in all of the existing drivers. Additionally, a change was needed to check-driverimpls.pl to prevent a make check problem. Other than those two items, all other changes are a single line addition to the driver source files to define this field. Signed-off-by: Adam Walters a...@pandorasboxen.com --- src/check-driverimpls.pl| 1 + src/driver.h| 7 +++ src/interface/interface_backend_netcf.c | 1 + src/libxl/libxl_driver.c| 1 + src/lxc/lxc_driver.c| 1 + src/network/bridge_driver.c | 1 + src/node_device/node_device_hal.c | 1 + src/node_device/node_device_udev.c | 1 + src/nwfilter/nwfilter_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/secret/secret_driver.c | 1 + src/storage/storage_driver.c| 1 + src/uml/uml_driver.c| 1 + src/xen/xen_driver.c| 1 + 15 files changed, 21 insertions(+) diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl index 17e2b48..f726403 100755 --- a/src/check-driverimpls.pl +++ b/src/check-driverimpls.pl @@ -37,6 +37,7 @@ while () { next if $api eq no; next if $api eq name; +next if $api eq stateDrvType; my $suffix = $impl; my $prefix = $impl; diff --git a/src/driver.h b/src/driver.h index 5f4cd8d..943e1b1 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1834,6 +1834,12 @@ typedef int typedef int (*virDrvStateStop)(void); +typedef enum { +VIR_DRV_STATE_DRV_LIBVIRT = 1, +VIR_DRV_STATE_DRV_HYPERVISOR = 2, +VIR_DRV_STATE_DRV_LAST, +} virDrvStateDrvType; + typedef struct _virStateDriver virStateDriver; typedef virStateDriver *virStateDriverPtr; @@ -1844,6 +1850,7 @@ struct _virStateDriver { virDrvStateCleanup stateCleanup; virDrvStateReload stateReload; virDrvStateStop stateStop; +virDrvStateDrvType stateDrvType; }; # endif diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index c525ca9..7c80fbe 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -1186,6 +1186,7 @@ static virStateDriver interfaceStateDriver = { .stateInitialize = netcfStateInitialize, .stateCleanup = netcfStateCleanup, .stateReload = netcfStateReload, +.stateDrvType = VIR_DRV_STATE_DRV_LIBVIRT, }; int netcfIfaceRegister(void) { diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index fc0efa2..826fd81 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4420,6 +4420,7 @@ static virStateDriver libxlStateDriver = { .stateAutoStart = libxlStateAutoStart, .stateCleanup = libxlStateCleanup, .stateReload = libxlStateReload, +.stateDrvType = VIR_DRV_STATE_DRV_HYPERVISOR, }; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 982f3fc..6fb3cf8 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5440,6 +5440,7 @@ static virStateDriver lxcStateDriver = { .stateAutoStart = lxcStateAutoStart, .stateCleanup = lxcStateCleanup, .stateReload = lxcStateReload, +.stateDrvType = VIR_DRV_STATE_DRV_HYPERVISOR, }; int lxcRegister(void) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0b43a67..e437fde 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3134,6 +3134,7 @@ static virStateDriver networkStateDriver = { .stateAutoStart = networkStateAutoStart, .stateCleanup = networkStateCleanup, .stateReload = networkStateReload, +.stateDrvType = VIR_DRV_STATE_DRV_LIBVIRT, }; int networkRegister(void) { diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index fafd520..15cdecb 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -827,6 +827,7 @@ static virStateDriver halStateDriver = { .stateInitialize = nodeStateInitialize, /* 0.5.0 */ .stateCleanup = nodeStateCleanup, /* 0.5.0 */ .stateReload = nodeStateReload, /* 0.5.0 */ +.stateDrvType = VIR_DRV_STATE_DRV_LIBVIRT, }; int diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 5d49968..91f8e9d 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1816,6 +1816,7 @@ static virStateDriver udevStateDriver = { .stateInitialize = nodeStateInitialize, /* 0.7.3 */ .stateCleanup = nodeStateCleanup, /* 0.7.3 */ .stateReload = nodeStateReload, /* 0.7.3 */ +.stateDrvType = VIR_DRV_STATE_DRV_LIBVIRT, }; int udevNodeRegister(void) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index
[libvirt] [RFC PATCHv2 0/2] Implement tiered driver loading
This patchset implements a tiered driver loading system. I split the hypervisor drivers out into their own tier, which is loaded after the other drivers. This has the net effect of ensuring that things like secrets, networks, etc., are initialized and auto-started before any hypervisors, such as QEMU, LXC, etc. This resolves the race condition currently present when starting libvirtd while domains are running, which happens when restarting libvirtd after having started at least one domain. This patch will work without my config driver patchset, which is about to be submitted, as well. Without the config driver patchset, however, RBD storage pools using CephX authentication can not be auto-started due to a circular dependency between the QEMU and storage drivers. This may also affect other storage backends, but I currently only have the capacity to test with RBD and file backed storage pools. The reason this interferes with RBD storage pools is that currently, the storage driver has a hard-coded connection to QEMU in order to look up secrets. After this patchset, the QEMU driver will not be loaded until after the storage driver has completed its initialization and auto-start routines, which causes issues looking up secrets. Any pool type that does not use or need data from outside of the base storage pool definition should continue to auto-start along with no longer being affected by the current race condition. I have verified that file-based storage pools definitely auto-start fine after this patchset, and no longer have any issue with the current race condition. For anyone who is not familiar with the race condition I mention above, the basic description is that upon restarting libvirtd, any running QEMU domains using storage volumes are killed randomly due to their associated storage pool not yet being online. This is due to storage pool auto-start not having completed prior to QEMU initialization. In my prior testing, I found that this race condition affected at least one domain approximately 40% of the time. I sent this information to the mailing list back on 06DEC2013, if anyone is interested in going back and re-reading my description. I would appreciate any comments and suggestions about this patchset. It works for me on 4 machines running three different distros of Linux (Archlinux, Gentoo, and CentOS), so I would imagine that it should work most anywhere. Adam Walters (2): driver: Implement new state driver field libvirt: Implement tiered driver loading src/check-driverimpls.pl| 1 + src/driver.h| 7 + src/interface/interface_backend_netcf.c | 1 + src/libvirt.c | 45 - src/libxl/libxl_driver.c| 1 + src/lxc/lxc_driver.c| 1 + src/network/bridge_driver.c | 1 + src/node_device/node_device_hal.c | 1 + src/node_device/node_device_udev.c | 1 + src/nwfilter/nwfilter_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/secret/secret_driver.c | 1 + src/storage/storage_driver.c| 1 + src/uml/uml_driver.c| 1 + src/xen/xen_driver.c| 1 + 16 files changed, 48 insertions(+), 18 deletions(-) -- 1.8.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCHv2 2/2] libvirt: Implement tiered driver loading
This utilizes the new state driver field to implement a tiered driver loading system. The two currently defined classes of driver are libvirt and hypervisor drivers. Hypervisor drivers are fairly self-explanatory, they provide domain services. Libvirt drivers are more like backend drivers, like the secret and storage drivers. The way the tiered loading is implemented makes it simple to add additional tiers as needed. The tiers are loaded in numeric order, starting with the lowered numbered tier. These tiers are defined in driver.h. Each tier's drivers are all initialized, then all auto-started before moving on to the next tier. This allows some interdependency between drivers within a tier, similar to the current free-for-all loading, while ensuring that auto-start is complete for a tier before moving on. By loading drivers in this manner, there is little-to-no need to hard-code driver load ordering or create a fully dynamic dependency-based loading algorithm. We still gain the benefits of a more orderly driver load order, which helps minimize the possibility of a race condition during startup. If a race condition is discovered in the future, the number of tiers can be changed to mitigate it without requiring too much effort. Signed-off-by: Adam Walters a...@pandorasboxen.com --- src/libvirt.c | 45 +++-- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index c15e29a..d8d1723 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -729,31 +729,40 @@ virStateInitialize(bool privileged, void *opaque) { size_t i; +size_t stateDriverTier; if (virInitialize() 0) return -1; -for (i = 0; i virStateDriverTabCount; i++) { -if (virStateDriverTab[i]-stateInitialize) { -VIR_DEBUG(Running global init for %s state driver, - virStateDriverTab[i]-name); -if (virStateDriverTab[i]-stateInitialize(privileged, - callback, - opaque) 0) { -virErrorPtr err = virGetLastError(); -VIR_ERROR(_(Initialization of %s state driver failed: %s), - virStateDriverTab[i]-name, - err err-message ? err-message : _(Unknown problem)); -return -1; +for (stateDriverTier = 0; stateDriverTier VIR_DRV_STATE_DRV_LAST; stateDriverTier++) { +for (i = 0; i virStateDriverTabCount; i++) { +if (virStateDriverTab[i]-stateDrvType != stateDriverTier) +continue; + +if (virStateDriverTab[i]-stateInitialize) { +VIR_DEBUG(Running global init for %s state driver, + virStateDriverTab[i]-name); +if (virStateDriverTab[i]-stateInitialize(privileged, + callback, + opaque) 0) { +virErrorPtr err = virGetLastError(); +VIR_ERROR(_(Initialization of %s state driver failed: %s), + virStateDriverTab[i]-name, + err err-message ? err-message : _(Unknown problem)); +return -1; +} } } -} -for (i = 0; i virStateDriverTabCount; i++) { -if (virStateDriverTab[i]-stateAutoStart) { -VIR_DEBUG(Running global auto start for %s state driver, - virStateDriverTab[i]-name); -virStateDriverTab[i]-stateAutoStart(); +for (i = 0; i virStateDriverTabCount; i++) { +if (virStateDriverTab[i]-stateDrvType != stateDriverTier) +continue; + +if (virStateDriverTab[i]-stateAutoStart) { +VIR_DEBUG(Running global auto start for %s state driver, + virStateDriverTab[i]-name); +virStateDriverTab[i]-stateAutoStart(); +} } } return 0; -- 1.8.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCHv2 0/4] Adding 'config' driver
This patchset adds a driver named 'config' which provides access to configuration data, such as secret and storage definitions, without requiring a connection to a hypervisor. This complements my previous patchset, which resolves the race condition on libvirtd startup. The rationale behind this idea is that there exist circumstances under which a driver may require access to things such as secrets during a time at which there is no active connection to a hypervisor. Currently, that data can not be accessed. An example of this in libvirt today is that the storage driver requires a connection in order to access secret data to auto-start an RBD storage pool that uses CephX authentication. My previous patchset breaks the ability to auto-start that type of storage pool, and this patchset fixes it again. This driver is technically what one may call a hypervisor driver, but it does not implement any domain operations. It simply exists to handle requests by drivers for access to information that would otherwise by unattainable. The URI provided by this driver is 'config:///' and has been tested working on four different machines of mine, running three different Linux distributions (Archlinux, Gentoo, and CentOS). Being a very simple driver, I would expect it to work pretty much anywhere. I welcome comments and suggestions related to this driver. At the very least, this patchset combined with my previous patchset resolves the current race condition present on startup of libvirtd without any loss of functionality. Adam Walters (4): config: Adding config driver configure: Implement config driver libvirtd: Reorder load of secrets driver storage: Change hardcoded QEMU connection to use config driver configure.ac | 11 ++ daemon/libvirtd.c| 21 ++-- include/libvirt/virterror.h | 2 + po/POTFILES.in | 1 + src/Makefile.am | 25 + src/config/config_driver.c | 238 +++ src/config/config_driver.h | 44 src/storage/storage_driver.c | 13 +-- src/util/virerror.c | 2 + 9 files changed, 345 insertions(+), 12 deletions(-) create mode 100644 src/config/config_driver.c create mode 100644 src/config/config_driver.h -- 1.8.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCHv2 1/4] config: Adding config driver
This patch adds the source and header for a new driver, named config, which provides the 'config:///' connection URI. This driver provides access to non-domain-related configuration information, such as secret or network definitions. This helps prevent circular dependencies between drivers, including the current case between the QEMU and storage drivers. Signed-off-by: Adam Walters a...@pandorasboxen.com --- include/libvirt/virterror.h | 2 + src/config/config_driver.c | 238 src/config/config_driver.h | 44 src/util/virerror.c | 2 + 4 files changed, 286 insertions(+) create mode 100644 src/config/config_driver.c create mode 100644 src/config/config_driver.h diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index e31e9c4..0f25a65 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -121,6 +121,8 @@ typedef enum { VIR_FROM_ACCESS = 55, /* Error from access control manager */ VIR_FROM_SYSTEMD = 56, /* Error from systemd code */ +VIR_FROM_CONFIG = 57, /* Error from config driver */ + # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST # endif diff --git a/src/config/config_driver.c b/src/config/config_driver.c new file mode 100644 index 000..c64b532 --- /dev/null +++ b/src/config/config_driver.c @@ -0,0 +1,238 @@ +/* + * config_driver.c: core driver methods for accessing configs + * + * Copyright (C) 2013 Adam Walters + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Author: Adam Walters a...@pandorasboxen.com + */ +#include config.h +#include string.h + +#include internal.h +#include datatypes.h +#include driver.h +#include virlog.h +#include viralloc.h +#include virerror.h +#include virstring.h +#include viraccessapicheck.h +#include nodeinfo.h +#include config_driver.h + +#define VIR_FROM_THIS VIR_FROM_CONFIG + +virConfigDriverPtr config_driver = NULL; + +static int configStateCleanup(void) { +if (config_driver == NULL) +return -1; + +virObjectUnref(config_driver-closeCallbacks); + +virSysinfoDefFree(config_driver-hostsysinfo); + +virMutexDestroy(config_driver-lock); +VIR_FREE(config_driver); + +return 0; +} + +static int configStateInitialize(bool privileged, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ +if (!privileged) { +VIR_INFO(Not running privileged, disabling driver); +return 0; +} + +if (VIR_ALLOC(config_driver) 0) +return -1; + +if (virMutexInit(config_driver-lock) 0) { +VIR_ERROR(_(cannot initialize mutex)); +VIR_FREE(config_driver); +return -1; +} + +config_driver-hostsysinfo = virSysinfoRead(); + +if (!(config_driver-closeCallbacks = virCloseCallbacksNew())) +goto cleanup; + +return 0; + +cleanup: +configStateCleanup(); +return -1; +} + +static int configStateReload(void) { +return 0; +} + +static virDrvOpenStatus configConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags) +{ +virDrvOpenStatus ret = VIR_DRV_OPEN_ERROR; +virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + +if (conn-uri != NULL) { +/* If URI isn't 'config', decline the connection */ +if (conn-uri-scheme == NULL || +STRNEQ(conn-uri-scheme, config)) { +ret = VIR_DRV_OPEN_DECLINED; +goto cleanup; +} + +/* Allow remote driver to deal with URIs with hostname set */ +if (conn-uri-server != NULL) { +ret = VIR_DRV_OPEN_DECLINED; +goto cleanup; +} + +if (config_driver == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(config state driver is not active)); +goto cleanup; +} +} else { +ret = VIR_DRV_OPEN_DECLINED; +goto cleanup; +} + +if (virConnectOpenEnsureACL(conn) 0) +goto cleanup; + +conn-privateData = config_driver; + +ret = VIR_DRV_OPEN_SUCCESS; +cleanup: +return ret; +} + +static int configConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{
[libvirt] [RFC PATCHv2 2/4] configure: Implement config driver
This patch completes the addition of the config driver by adding it to the configure script, Makefile, and loading the driver in libvirtd. Additionally, to prevent a make syntax-check error, I also added src/config/config_driver.c to po/POTFILES.in. Signed-off-by: Adam Walters a...@pandorasboxen.com --- configure.ac | 11 +++ daemon/libvirtd.c | 9 + po/POTFILES.in| 1 + src/Makefile.am | 25 + 4 files changed, 46 insertions(+) diff --git a/configure.ac b/configure.ac index 3a70375..f05b0e0 100644 --- a/configure.ac +++ b/configure.ac @@ -1635,6 +1635,16 @@ if test $with_secrets = yes ; then fi AM_CONDITIONAL([WITH_SECRETS], [test $with_secrets = yes]) +if test $with_libvirtd = no ; then + with_config=no +else + with_config=yes +fi +if test $with_config = yes ; then + AC_DEFINE_UNQUOTED([WITH_CONFIG], 1, [whether local config driver is enabled]) +fi +AM_CONDITIONAL([WITH_CONFIG], [test $with_config = yes]) + AC_ARG_WITH([storage-dir], [AS_HELP_STRING([--with-storage-dir], @@ -2684,6 +2694,7 @@ AC_MSG_NOTICE([ Libvirtd: $with_libvirtd]) AC_MSG_NOTICE([Interface: $with_interface]) AC_MSG_NOTICE([ macvtap: $with_macvtap]) AC_MSG_NOTICE([ virtport: $with_virtualport]) +AC_MSG_NOTICE([ config: $with_config]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Storage Drivers]) AC_MSG_NOTICE([]) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 49c42ad..1e76f5a 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -95,6 +95,9 @@ # ifdef WITH_NWFILTER # include nwfilter/nwfilter_driver.h # endif +# ifdef WITH_CONFIG +# include config/config_driver.h +# endif #endif #include configmake.h @@ -369,6 +372,9 @@ static void daemonInitialize(void) * If they try to open a connection for a module that * is not loaded they'll get a suitable error at that point */ +# ifdef WITH_CONFIG +virDriverLoadModule(config); +# endif # ifdef WITH_NETWORK virDriverLoadModule(network); # endif @@ -406,6 +412,9 @@ static void daemonInitialize(void) virDriverLoadModule(vbox); # endif #else +# ifdef WITH_CONFIG +configRegister(); +# endif # ifdef WITH_NETWORK networkRegister(); # endif diff --git a/po/POTFILES.in b/po/POTFILES.in index 0359b2f..e14d04d 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -27,6 +27,7 @@ src/conf/snapshot_conf.c src/conf/storage_conf.c src/conf/storage_encryption_conf.c src/conf/virchrdev.c +src/config/config_driver.c src/cpu/cpu.c src/cpu/cpu_generic.c src/cpu/cpu_map.c diff --git a/src/Makefile.am b/src/Makefile.am index 7844efa..6c60aae 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -504,6 +504,7 @@ DRIVER_SOURCE_FILES = \ $(QEMU_DRIVER_SOURCES) \ $(REMOTE_DRIVER_SOURCES) \ $(SECRET_DRIVER_SOURCES) \ + $(CONFIG_DRIVER_SOURCES) \ $(STORAGE_DRIVER_SOURCES) \ $(TEST_DRIVER_SOURCES) \ $(UML_DRIVER_SOURCES) \ @@ -523,6 +524,7 @@ STATEFUL_DRIVER_SOURCE_FILES = \ $(NWFILTER_DRIVER_SOURCES) \ $(QEMU_DRIVER_SOURCES) \ $(SECRET_DRIVER_SOURCES) \ + $(CONFIG_DRIVER_SOURCES) \ $(STORAGE_DRIVER_SOURCES) \ $(UML_DRIVER_SOURCES) \ $(XEN_DRIVER_SOURCES) \ @@ -801,6 +803,9 @@ endif WITH_INTERFACE SECRET_DRIVER_SOURCES =\ secret/secret_driver.h secret/secret_driver.c +CONFIG_DRIVER_SOURCES =\ + config/config_driver.h config/config_driver.c + # Storage backend specific impls STORAGE_DRIVER_SOURCES = \ storage/storage_driver.h storage/storage_driver.c \ @@ -1388,6 +1393,25 @@ endif WITH_DRIVER_MODULES libvirt_driver_secret_la_SOURCES = $(SECRET_DRIVER_SOURCES) endif WITH_SECRETS +if WITH_CONFIG +if WITH_DRIVER_MODULES +mod_LTLIBRARIES += libvirt_driver_config.la +else ! WITH_DRIVER_MODULES +noinst_LTLIBRARIES += libvirt_driver_config.la +# Stateful, so linked to daemon instead +#libvirt_la_BUILD_LIBADD += libvirt_driver_config.la +endif ! WITH_DRIVER_MODULES +libvirt_driver_config_la_CFLAGS = \ + -I$(top_srcdir)/src/access \ + -I$(top_srcdir)/src/conf \ + $(AM_CFLAGS) +if WITH_DRIVER_MODULES +libvirt_driver_config_la_LIBADD = ../gnulib/lib/libgnu.la +libvirt_driver_config_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +endif WITH_DRIVER_MODULES +libvirt_driver_config_la_SOURCES = $(CONFIG_DRIVER_SOURCES) +endif WITH_CONFIG + # Needed to keep automake quiet about conditionals libvirt_driver_storage_impl_la_SOURCES = libvirt_driver_storage_impl_la_CFLAGS = \ @@ -1663,6 +1687,7 @@ EXTRA_DIST += \ $(SECURITY_DRIVER_SELINUX_SOURCES) \ $(SECURITY_DRIVER_APPARMOR_SOURCES) \
[libvirt] [RFC PATCHv2 3/4] libvirtd: Reorder load of secrets driver
This patch changes the order in which the drivers are loaded in libvirtd.c. Per the comment at the top of the daemonInitialize function, order is important. Since the storage driver may depend on the secrets driver, but the secrets driver can't depend on the storage driver, I moved the loading of the secrets driver to just above the storage driver. Signed-off-by: Adam Walters a...@pandorasboxen.com --- daemon/libvirtd.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 1e76f5a..251c2f4 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -378,15 +378,15 @@ static void daemonInitialize(void) # ifdef WITH_NETWORK virDriverLoadModule(network); # endif +# ifdef WITH_SECRETS +virDriverLoadModule(secret); +# endif # ifdef WITH_STORAGE virDriverLoadModule(storage); # endif # ifdef WITH_NODE_DEVICES virDriverLoadModule(nodedev); # endif -# ifdef WITH_SECRETS -virDriverLoadModule(secret); -# endif # ifdef WITH_NWFILTER virDriverLoadModule(nwfilter); # endif @@ -421,15 +421,15 @@ static void daemonInitialize(void) # ifdef WITH_INTERFACE interfaceRegister(); # endif +# ifdef WITH_SECRETS +secretRegister(); +# endif # ifdef WITH_STORAGE storageRegister(); # endif # ifdef WITH_NODE_DEVICES nodedevRegister(); # endif -# ifdef WITH_SECRETS -secretRegister(); -# endif # ifdef WITH_NWFILTER nwfilterRegister(); # endif -- 1.8.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCHv2 4/4] storage: Change hardcoded QEMU connection to use config driver
This utilizes the new config driver I am submitting to resolve the hardcoded QEMU connection string in the storage driver. With this, the storage driver no longer has a circular dependency with QEMU. Without this patch, when libvirtd is restarted, QEMU requires storage (when domains are using storage pool backings), and storage requires QEMU (for access to secrets). This causes issues during libvirtd startup. Signed-off-by: Adam Walters a...@pandorasboxen.com --- src/storage/storage_driver.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index d753e34..a86d564 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -70,12 +70,13 @@ storageDriverAutostart(virStorageDriverStatePtr driver) { size_t i; virConnectPtr conn = NULL; -/* XXX Remove hardcoding of QEMU URI */ -if (driverState-privileged) -conn = virConnectOpen(qemu:///system); -else -conn = virConnectOpen(qemu:///session); -/* Ignoring NULL conn - let backends decide */ +conn = virConnectOpen(config:///); +/* Ignoring NULL conn - let backends decide. + * As long as the config driver is built, and + * it should be, since it is default on and + * not configurable, a NULL conn should never + * happen. + */ for (i = 0; i driver-pools.count; i++) { virStoragePoolObjPtr pool = driver-pools.objs[i]; -- 1.8.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] closing old maint branches [was: [v0.9.12-maint 0/8] Backport changes for CVE-2013-6458 to v0.9.12-maint]
On Thu, Jan 23, 2014 at 10:26:14AM +, Daniel P. Berrange wrote: On Wed, Jan 22, 2014 at 12:13:48PM -0700, Eric Blake wrote: On 01/15/2014 01:43 PM, Eric Blake wrote: Is anyone still using v0.9.11-maint? The CVE extends back to 0.9.8, so we could argue that we should either fix the 0.9.11 branch, or add another commit to the branch that explicitly marks it as end-of-life because no one appears to be relying on it. Fedora 18 is now end-of-life, so from Fedora's perspective, I only care about 0.10.2 (RHEL and CentOS 6), 1.0.5 (F19), 1.1.3 (F20) and soon 1.2.1 (rawhide), although I didn't mind touching all the intermediate branches on my way down to 0.10.2. RHEL 5 is also vulnerable to CVE-2013-6458, but as we don't have an upstream v0.8.2-maint branch (thank goodness!), that's something for Red Hat to worry about. I've gone ahead and marked v0.8.3-maint and v0.9.11-maint as closed (I'm not posting the actual patch here, but it was done by 'git rm -f \*' followed by recreating .gitignore and a placeholder README that mentions the death of the branch). FYI for openstack I examined the current libvirt versions in some major distros: https://wiki.openstack.org/wiki/LibvirtDistroSupportMatrix Great. I took the libverty to add wheezy-backports as well since we ship updated versions from there and it's nowadays integrated into the main Debian archive. Cheers, -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] Push nwfilter update locking up to top level
On 01/23/2014 07:37 AM, Daniel P. Berrange wrote: The NWFilter code has as a deadlock race condition between the virNWFilter{Define,Undefine} APIs and starting of guest VMs due to mis-matched lock ordering. In the virNWFilter{Define,Undefine} codepaths the lock ordering is 1. nwfilter driver lock 2. virt driver lock 3. nwfilter update lock 4. domain object lock In the VM guest startup paths the lock ordering is 1. virt driver lock 2. domain object lock 3. nwfilter update lock As can be seen the domain object and nwfilter update locks are not acquired in a consistent order. The fix used is to push the nwfilter update lock upto the top level resulting in a lock ordering for virNWFilter{Define,Undefine} of 1. nwfilter driver lock 2. nwfilter update lock 3. virt driver lock 4. domain object lock and VM start using 1. nwfilter update lock 2. virt driver lock 3. domain object lock Makes sense... there are other paths as well: - SIGHUP or restart of libvirt that recreates all filters - late instantiation of filters following detection of VM's IP address This has the effect of serializing VM startup once again, even if no nwfilters are applied to the guest. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/conf/nwfilter_conf.c | 6 -- src/lxc/lxc_driver.c | 6 ++ src/nwfilter/nwfilter_driver.c | 8 src/nwfilter/nwfilter_gentech_driver.c | 6 -- src/qemu/qemu_driver.c | 6 ++ src/uml/uml_driver.c | 4 6 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 6db8ea9..e712ca5 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2990,14 +2990,12 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, return NULL; } -virNWFilterLockFilterUpdates(); if ((nwfilter = virNWFilterObjFindByName(nwfilters, def-name))) { if (virNWFilterDefEqual(def, nwfilter-def, false)) { virNWFilterDefFree(nwfilter-def); nwfilter-def = def; -virNWFilterUnlockFilterUpdates(); return nwfilter; } I think you should add a comment to this function that it must be called with this lock held. With the removal of the locking from this function here you have to do the locking in the callers. Callers of virNWFilterObjAssignDef are: src/conf/nwfilter_conf.c::virNWFilterObjLoad : I don't see this function grabbing the lock. I think this is missing. src/conf/nwfilter_driver.c::nwfilterDefineXML : ok, found it below diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e319234..b1f8a89 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1015,6 +1015,8 @@ static int lxcDomainCreateWithFiles(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, -1); +virNWFilterLockFilterUpdates(); + if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; @@ -1053,6 +1055,7 @@ cleanup: if (event) virObjectEventStateQueue(driver-domainEventState, event); virObjectUnref(cfg); +virNWFilterUnlockFilterUpdates(); return ret; } @@ -1109,6 +1112,8 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, NULL); +virNWFilterLockFilterUpdates(); + if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto cleanup; @@ -1164,6 +1169,7 @@ cleanup: virObjectEventStateQueue(driver-domainEventState, event); virObjectUnref(caps); virObjectUnref(cfg); +virNWFilterUnlockFilterUpdates(); return dom; } Probably these calls are correctly placed since all other creation functions funnel into them.Same for UML and QEMU drivers. diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 89913cf8..f0e78ed 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -935,7 +935,6 @@ _virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, int ifindex; int rc; -virNWFilterLockFilterUpdates(); /* after grabbing the filter update lock check for the interface; if it's not there anymore its filters will be or are being removed @@ -964,7 +963,6 @@ _virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, foundNewFilter); cleanup: -virNWFilterUnlockFilterUpdates(); return rc; } This function is called by virNWFilterInstantiateFilter and virNWFilterUpdateInstantiateFilter. So, in the former case this is called when a VM is started, in the latter case if VMs' filters are updated while the VM is running. I think you are covering the VM creation case with the above calls for lxc and further below with the changes to the qemu driver and the
[libvirt] [PATCH] maint: update to latest gnulib, for mingw improvements
On Fedora 20, mingw-headers has switched over to winpthreads as the provider for its pthread.h. winpthreads is notorious for providing a less-than-stellar header, and needs several workarounds before it can be used in a project assuming POSIX semantics. While we still use Windows primitives rather than pthread when compiling for mingw, this update will make it possible to switch to mingw pthreads. * .gnulib: Update to latest, for mingw fixes. Signed-off-by: Eric Blake ebl...@redhat.com --- We're in between releases, so I'm pushing this under the gnulib/trivial rule. * .gnulib d18d1b8...d5fec6c (15): pthread: work around winpthread header pollution on mingw qacl: check for fchmod Fix typo in ChangeLog. fdopen-tests: port to Tru64 stdalign: port to HP-UX compilers strtoimax: port to platforms lacking 'long long' update from texinfo relocatable-maint.texi: escape braces maint: remove duplicate ChangeLog entry maint.mk: adapt openat.h-include-without-use test relocatable-perl: like relocatable-script, but for Perl scripts relocatable-shell: Update suggested usage in maintainer documentation. tests: fix export bug in previous patch tests: simplify porting to Solaris 10 /bin/sh update from texinfo .gnulib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gnulib b/.gnulib index d18d1b8..d5fec6c 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit d18d1b802380bb8f0a079c7312a1adffdce0 +Subproject commit d5fec6c22f03c6a73d62260c9ce091c10c0a9cbd -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [question]Is there a good way to get the mac of guest interface after I attached it to a guest
Now I working on a new KVM management tool base on html5 and libvirt we will support a REST API for detach and attach guest interface. we want to use mac as the identifier of the network interface. That meas we should get the mac after the user create a interface. also other people may be doing the exact same thing at the exact same time to create a interface. Here are the demo codes: dom = self._get_vm(vm) xml = interface type='network' source network='default'/ /interface dom.attachDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) # now I want to get the mac, how to get it? I have check libvirt use this code to generate a mac address. does libvirt can gurantee that the MAC address generated is unique? void virMacAddrGenerate(const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN], virMacAddrPtr addr) { addr-addr[0] = prefix[0]; addr-addr[1] = prefix[1]; addr-addr[2] = prefix[2]; addr-addr[3] = virRandomBits(8); addr-addr[4] = virRandomBits(8); addr-addr[5] = virRandomBits(8); } libvirt also use the KVM prefix as default as it's in the privately administered range: 52:54:00:XX:XX:XX -- Thanks and best regards! Sheldon Feng(冯少合)shao...@linux.vnet.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Block info query: Add check for transient domain
Currently the qemuDomainGetBlockInfo will return allocation == physical for most backing stores. For a qcow2 block backed device it's possible to return the highest lv extent allocated from qemu for an active guest. That is a value where allocation != physical and one would hope be less. However, if the guest is not running, then the code falls back to returning allocation == physical. This turns out to be problematic for rhev which monitors the size of the backing store. During a migration, before the VM has been started on the target and while it is deemed inactive on the source, there's a small window of time where the allocation is returned as physical triggering the code to extend the file unnecessarily. Since rhev uses transient domains and this is edge condition for a transient domain, rather than returning good status and allocation == physical when this window of opportunity exists, this patch will check for a transient (or non persistent) domain and return a failure to the caller rather than returning the defaults. For a persistent domain, the defaults will be returned. The description for the virDomainGetBlockInfo has been updated to describe the phenomena. Signed-off-by: John Ferlan jfer...@redhat.com --- The v1 of this patch is at: https://www.redhat.com/archives/libvir-list/2013-December/msg00984.html There is a bz associated as well: https://bugzilla.redhat.com/show_bug.cgi?id=1040507 I've read, reread, changed the text for the function so many times. I'm open to any suggestions for adjustments! It's not easy to describe the issues without getting into too much detail. include/libvirt/libvirt.h.in | 8 --- src/libvirt.c| 53 src/qemu/qemu_driver.c | 29 +++- 3 files changed, 82 insertions(+), 8 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 47a896b..b3ce000 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2140,7 +2140,8 @@ int virDomainBlockResize (virDomainPtr dom, /** virDomainBlockInfo: * - * This struct provides information about the size of a block device backing store + * This struct provides information about the size of a block device + * backing store * * Examples: * @@ -2153,11 +2154,12 @@ int virDomainBlockResize (virDomainPtr dom, * * - qcow2 file in filesystem * * capacity: logical size from qcow2 header - * * allocation, physical: logical size of the file / highest qcow extent (identical) + * * allocation, physical: logical size of the file / + * highest qcow extent (identical) * * - qcow2 file in a block device * * capacity: logical size from qcow2 header - * * allocation: highest qcow extent written + * * allocation: highest qcow extent written for an active domain * * physical: size of the block device container */ typedef struct _virDomainBlockInfo virDomainBlockInfo; diff --git a/src/libvirt.c b/src/libvirt.c index c15e29a..9cc5b1c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8403,6 +8403,59 @@ error: * can be found by calling virDomainGetXMLDesc() and inspecting * elements within //domain/devices/disk. * + * For QEMU domains, the allocation and physical virDomainBlockInfo + * values returned will generally be the same, except when using a + * non raw, block backing device, such as qcow2 for an active domain. + * When the persistent domain is not active, QEMU will return the + * default which is the same value for allocation and physical. + * + * Active QEMU domains can return an allocation value which is more + * representative of the currently used blocks by the device compared + * to the physical size of the device. Applications can use/monitor + * the allocation value with the understanding that if the domain + * becomes inactive during an attempt to get the value, the default + * values will be returned. Thus, the application should check + * after the call for the domain being inactive if the values are + * the same. Optionally, the application could be watching for a + * shutdown event and then ignore any values received afterwards. + * This can be an issue when a domain is being migrated and the + * exact timing of the domain being made inactive and check of + * the allocation value results the default being returned. For + * a transient domain in the similar situation, this call will return + * -1 and an error message indicating the domain is not running. + * + * The following is some pseudo code illustrating the call sequence: + * + * ... + * virDomainPtr dom; + * virDomainBlockInfo info; + * char *device; + * ... + * // Either get a list of all domains or a specific domain + * // via a virDomainLookupBy*() call. + * // + * // It's also required to fill in the device pointer, but that's + * // specific to the
[libvirt] [RFC PATCH] Cannot set persistent config mem/vcpu above live maxmem/maxvcpu
I submitted bug https://bugzilla.redhat.com/show_bug.cgi?id=1038363 for being unable to raise the persistent mem/vcpu values above a live domain’s maxmem/maxvcpu values (rather than the persistent maxmem/maxvcpu values). I was asked to submit my patch here for a wider review. For memory, check the newmem value against mem.max_baloon of the live def for non-persistent changes, or mem.max_baloon of the persistent def for persistent changes. Same theory for setting vcpus/maxvcpus. --- a/libvirt0/libvirt-1.1.1/src/qemu/qemu_driver.c +++ b/libvirt0/libvirt-1.1.1/src/qemu/qemu_driver.c @@ -2236,13 +2236,13 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } else { /* resize the current memory */ -if (newmem vm-def-mem.max_balloon) { -virReportError(VIR_ERR_INVALID_ARG, %s, - _(cannot set memory higher than max memory)); -goto endjob; -} - if (flags VIR_DOMAIN_AFFECT_LIVE) { +if (newmem vm-def-mem.max_balloon) { +virReportError(VIR_ERR_INVALID_ARG, %s, +_(cannot set memory higher than max memory)); +goto endjob; +} + priv = vm-privateData; qemuDomainObjEnterMonitor(driver, vm); r = qemuMonitorSetBalloon(priv-mon, newmem); @@ -2262,6 +2262,12 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (newmem persistentDef-mem.max_balloon) { +virReportError(VIR_ERR_INVALID_ARG, %s, +_(cannot set memory higher than max memory)); +goto endjob; +} + sa_assert(persistentDef); persistentDef-mem.cur_balloon = newmem; ret = virDomainSaveConfig(cfg-configDir, persistentDef); @@ -4160,6 +4166,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, virDomainDefPtr persistentDef; int ret = -1; bool maximum; +unsigned int maxvcpus; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; qemuAgentCPUInfoPtr cpuinfo = NULL; @@ -4207,11 +4214,12 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } -if (!maximum nvcpus vm-def-maxvcpus) { +maxvcpus = (flags VIR_DOMAIN_AFFECT_LIVE) ? vm-def-maxvcpus : persistentDef-maxvcpus; +if (!maximum nvcpus maxvcpus) { virReportError(VIR_ERR_INVALID_ARG, _(requested vcpus is greater than max allowable vcpus for the domain: %d %d), - nvcpus, vm-def-maxvcpus); + nvcpus, maxvcpus); goto endjob; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v11 0/6] Write separate module for hostdev passthrough
Chunyan Liu wrote: These patches implements a separate module for hostdev passthrough so that it could be shared by different drivers and can maintain a global state of a host device. patch 1/6: extract hostdev passthrough function from qemu_hostdev.c and make it reusable by multiple drivers. patch 2/6: add a unit test for hostdev common library. patch 3/6: switch qemu driver to use the common library instead of its own hostdev passthrough APIs. patch 4/6: switch lxc driver to use the common library instead of its own hostdev passthrough APIs. patch 5/6: add a hostdev pci backend type for xen usage. patch 6/6: add pci passthrough to libxl driver. Thanks for addressing my comments from V10. Any objections to applying this series now? hostdev passthrough is long overdue in the libxl driver :). Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [question]Is there a good way to get the mac of guest interface after I attached it to a guest
- Original Message - Now I working on a new KVM management tool base on html5 and libvirt we will support a REST API for detach and attach guest interface. we want to use mac as the identifier of the network interface. That meas we should get the mac after the user create a interface. also other people may be doing the exact same thing at the exact same time to create a interface. Here are the demo codes: dom = self._get_vm(vm) xml = interface type='network' source network='default'/ /interface This interface xml is partial, and it is not recommended according to 'man virsh': Note: using of partial device definition XML files may lead to unexpected results as some fields may be autogenerated and thus match devices other than expected. And there is a way to find out the mac address specified by libvirt, we can check /var/lib/libvirt/dnsmasq/default.leases, newer one is on top # cat /var/lib/libvirt/dnsmasq/default.leases 1390549166 52:54:00:ea:9f:58 192.168.122.106 * * 1390547957 52:54:00:9d:43:38 192.168.122.134 * * 1390547828 52:54:00:0b:2f:ec 192.168.122.76 localhost * But seems there is some latency, :( dom.attachDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) # now I want to get the mac, how to get it? I have check libvirt use this code to generate a mac address. does libvirt can gurantee that the MAC address generated is unique? yes, I also have this question. void virMacAddrGenerate(const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN], virMacAddrPtr addr) { addr-addr[0] = prefix[0]; addr-addr[1] = prefix[1]; addr-addr[2] = prefix[2]; addr-addr[3] = virRandomBits(8); addr-addr[4] = virRandomBits(8); addr-addr[5] = virRandomBits(8); } libvirt also use the KVM prefix as default as it's in the privately administered range: 52:54:00:XX:XX:XX -- Thanks and best regards! Sheldon Feng(冯少合)shao...@linux.vnet.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [question]Is there a good way to get the mac of guest interface after I attached it to a guest
On 01/24/2014 02:40 PM, Jincheng Miao wrote: - Original Message - Now I working on a new KVM management tool base on html5 and libvirt we will support a REST API for detach and attach guest interface. we want to use mac as the identifier of the network interface. That meas we should get the mac after the user create a interface. also other people may be doing the exact same thing at the exact same time to create a interface. Here are the demo codes: dom = self._get_vm(vm) xml = interface type='network' source network='default'/ /interface This interface xml is partial, and it is not recommended according to 'man virsh': Note: using of partial device definition XML files may lead to unexpected results as some fields may be autogenerated and thus match devices other than expected. And there is a way to find out the mac address specified by libvirt, we can check /var/lib/libvirt/dnsmasq/default.leases, newer one is on top # cat /var/lib/libvirt/dnsmasq/default.leases 1390549166 52:54:00:ea:9f:58 192.168.122.106 * * 1390547957 52:54:00:9d:43:38 192.168.122.134 * * 1390547828 52:54:00:0b:2f:ec 192.168.122.76 localhost * But seems there is some latency, :( yes, but only the vm is running, we can get the mac and IP. dom.attachDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) # now I want to get the mac, how to get it? I have check libvirt use this code to generate a mac address. does libvirt can gurantee that the MAC address generated is unique? yes, I also have this question. void virMacAddrGenerate(const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN], virMacAddrPtr addr) { addr-addr[0] = prefix[0]; addr-addr[1] = prefix[1]; addr-addr[2] = prefix[2]; addr-addr[3] = virRandomBits(8); addr-addr[4] = virRandomBits(8); addr-addr[5] = virRandomBits(8); } libvirt also use the KVM prefix as default as it's in the privately administered range: 52:54:00:XX:XX:XX -- Thanks and best regards! Sheldon Feng(冯少合)shao...@linux.vnet.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Thanks and best regards! Sheldon Feng(冯少合)shao...@linux.vnet.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [question]Is there a good way to get the mac of guest interface after I attached it to a guest
On Fri, Jan 24, 2014 at 10:37:20AM +0800, Sheldon wrote: Now I working on a new KVM management tool base on html5 and libvirt we will support a REST API for detach and attach guest interface. we want to use mac as the identifier of the network interface. That meas we should get the mac after the user create a interface. also other people may be doing the exact same thing at the exact same time to create a interface. Here are the demo codes: dom = self._get_vm(vm) xml = interface type='network' source network='default'/ /interface dom.attachDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) # now I want to get the mac, how to get it? You have to parse the XML back and find it. I have check libvirt use this code to generate a mac address. does libvirt can gurantee that the MAC address generated is unique? No, as mentioned in the second subthread, the XML you provided is incomplete and can cause address clashing. So te best answer to your question in $SUBJ would be generate it yourself and add it to the XML before attaching. Martin void virMacAddrGenerate(const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN], virMacAddrPtr addr) { addr-addr[0] = prefix[0]; addr-addr[1] = prefix[1]; addr-addr[2] = prefix[2]; addr-addr[3] = virRandomBits(8); addr-addr[4] = virRandomBits(8); addr-addr[5] = virRandomBits(8); } libvirt also use the KVM prefix as default as it's in the privately administered range: 52:54:00:XX:XX:XX -- Thanks and best regards! Sheldon Feng(冯少合)shao...@linux.vnet.ibm.com IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list