[libvirt] [PATCH] qemu: cgroup: Fix priorities when setting emulatorpin
Use the custom emulator pin setting with the highest priority same as with vcpupin. --- src/qemu/qemu_cgroup.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index e83342d..bf0621f 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1110,10 +1110,10 @@ qemuSetupCgroupForEmulator(virDomainObjPtr vm) if (virCgroupMoveTask(priv-cgroup, cgroup_emulator) 0) goto cleanup; -if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) -cpumask = priv-autoCpuset; -else if (def-cputune.emulatorpin) +if (def-cputune.emulatorpin) cpumask = def-cputune.emulatorpin-cpumask; +else if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) +cpumask = priv-autoCpuset; else if (def-cpumask) cpumask = def-cpumask; -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: avoid obsolete index()
Commit 2a530a3e5 is not portable to mingw, which intentionally avoids declaring the obsolete index(). See also: https://bugzilla.redhat.com/show_bug.cgi?id=1214605 * src/util/virstring.c (virStringStripControlChars): Use strchr. Signed-off-by: Eric Blake ebl...@redhat.com --- Pushing under the build-breaker rule. src/util/virstring.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index 1cd4987..7b0cad7 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2012-2014 Red Hat, Inc. + * Copyright (C) 2012-2015 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 @@ -1003,7 +1003,7 @@ virStringStripControlChars(char *str) len = strlen(str); for (i = 0, j = 0; i len; i++) { -if (index(control_chars, str[i])) +if (strchr(control_chars, str[i])) continue; str[j++] = str[i]; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 8/9] qemu: Add support to Add/Delete IOThreads
On 04/23/2015 09:22 AM, Peter Krempa wrote: On Tue, Apr 21, 2015 at 19:31:29 -0400, John Ferlan wrote: Add qemuDomainAddIOThread and qemuDomainDelIOThread in order to add or remove an IOThread to/from the host either for live or config optoins The implementation for the 'live' option will use the iothreadpids list in order to make decision, while the 'config' option will use the iothreadids list. Additionally, for deletion each may have to adjust the iothreadpin list. IOThreads are implemented by qmp objects, the code makes use of the existing qemuMonitorAddObject or qemuMonitorDelObject APIs. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_audit.c | 9 ++ src/conf/domain_audit.h | 6 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 401 +++ 4 files changed, 417 insertions(+) ... diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ee13d08..c34abc9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6137,6 +6137,405 @@ qemuDomainPinIOThread(virDomainPtr dom, return ret; } +static int +qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int iothread_id) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +char *alias = NULL; +size_t idx; +int rc = -1; +int ret = -1; +unsigned int orig_niothreads = vm-def-iothreads; +unsigned int exp_niothreads = vm-def-iothreads; +int new_niothreads = 0; +qemuMonitorIOThreadInfoPtr *new_iothreads = NULL; +virCgroupPtr cgroup_iothread = NULL; +char *mem_mask = NULL; +virDomainIOThreadIDDefPtr iothrid; + +if (virDomainIOThreadIDFind(vm-def, iothread_id)) { +virReportError(VIR_ERR_INVALID_ARG, + _(an IOThread is already using iothread_id '%u'), + iothread_id); +goto cleanup; +} + +if (virAsprintf(alias, iothread%u, iothread_id) 0) +return -1; + +qemuDomainObjEnterMonitor(driver, vm); + +rc = qemuMonitorAddObject(priv-mon, iothread, alias, NULL); +exp_niothreads++; +if (rc 0) +goto exit_monitor; + +/* After hotplugging the IOThreads we need to re-detect the + * IOThreads thread_id's, adjust the cgroups, thread affinity, + * and add the thread_id to the vm-def-iothreadids list. + */ +if ((new_niothreads = qemuMonitorGetIOThreads(priv-mon, + new_iothreads)) 0) +goto exit_monitor; + +if (qemuDomainObjExitMonitor(driver, vm) 0) +goto cleanup; + +if (new_niothreads != exp_niothreads) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(got wrong number of IOThread ids from QEMU monitor. + got %d, wanted %d), + new_niothreads, exp_niothreads); +vm-def-iothreads = new_niothreads; +goto cleanup; +} +vm-def-iothreads = exp_niothreads; + +if (virDomainNumatuneGetMode(vm-def-numa, -1) == +VIR_DOMAIN_NUMATUNE_MEM_STRICT +virDomainNumatuneMaybeFormatNodeset(vm-def-numa, +priv-autoNodeset, +mem_mask, -1) 0) +goto cleanup; + + +/* + * If we've successfully added an IOThread, find out where we added it + * in the QEMU IOThread list, so we can add it to our iothreadids list + */ +for (idx = 0; idx new_niothreads; idx++) { +if (STREQ(new_iothreads[idx]-name, alias)) +break; +} + +if (idx == new_niothreads) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(cannot find new IOThread '%u' in QEMU monitor.), + iothread_id); +goto cleanup; +} + +if (!(iothrid = virDomainIOThreadIDAdd(vm-def, iothread_id))) +goto cleanup; + +iothrid-thread_id = new_iothreads[idx]-thread_id; + +/* Add IOThread to cgroup if present */ +if (priv-cgroup) { +cgroup_iothread = +qemuDomainAddCgroupForThread(priv-cgroup, + VIR_CGROUP_THREAD_IOTHREAD, + iothread_id, mem_mask, + iothrid-thread_id); +if (!cgroup_iothread) +goto cleanup; +} + +/* Inherit def-cpuset */ +if (vm-def-cpumask) { +if (!(iothrid-cpumask = virBitmapNewCopy(vm-def-cpumask))) +goto cleanup; Copying the existing default mask is not necessary here. You only have to set it now. It will be set automatically the next time the VM will start. Also you still didn't add the case where automatic numa placement is used. Perhaps because I'm
Re: [libvirt] [RFC 0/7] Live Migration with Pass-through Devices proposal
On 04/23/2015 04:34 AM, Chen Fan wrote: On 04/20/2015 06:29 AM, Laine Stump wrote: On 04/17/2015 04:53 AM, Chen Fan wrote: - on destination side, check whether need to hotplug new NIC according to specified XML. usually, we use migrate --xml command option to specify the destination host NIC mac address to hotplug a new NIC, because source side passthrough NIC mac address is different, then hotplug the deivce according to the destination XML configuration. Why does the MAC address need to be different? Are you suggesting doing this with passed-through non-SRIOV NICs? An SRIOV virtual function gets its MAC address from the libvirt config, so it's very simple to use the same MAC address across the migration. Any network card that would be able to do this on any sort of useful scale will be SRIOV-capable (or should be replaced with one that is - some of them are not that expensive). Hi Laine, I think SRIOV virtual NIC to support migration is good idea, but I also think some passthrough NIC without SRIOV-capable. for these NIC devices we only able to use hostdev to specify the passthrough function, so for these NIC I think we should support too. As I think you've already discovered, passing through non-SRIOV NICS is problematic. It is completely impossible for the host to change their MAC address before assigning them to the guest - the guest's driver sees standard netdev hardware and resets it, which resets the MAC address to the original value burned into the firmware. This makes management more complicated, especially when you get into scenarios such as what we're discussing (i.e. migration) where the actual hardware (and thus MAC address) may be different from one run to the next. Since libvirt's interface element requires a fixed MAC address in the XML, it's not possible to have an interface that gets the actual device from a network pool (without some serious hacking to that code), and there is no support for plain (non-network) hostdev device pools; there would need to be a separate (nonexistent) driver for that. Since the hostdev element relies on the PCI address of the device (in the source subelement, which also must be fixed) to determine which device to passthrough, a domain config with a hostdev that could be run on two different machines would require the device to reside at exactly the same PCI address on both machines, which is a very serious limitation to have in an environment large enough that migrating domains is a requirement. Also, non-SRIOV NICs are limited to a single device per physical port, meaning probably at most 4 devices per physical host PCIe slot, and this results in a greatly reduced density on the host (and even more so on the switch that connects to the host!) compared to even the old Intel 82576 cards, which have 14 VFs (7VFs x 2 ethernet ports). Think about it - with an 82576, you can get 14 guests into 1 PCIe slot and 2 switch ports, while the same number of guests with non-SRIOV would take 4 PCIe slots and 14(!) switch ports. The difference is even more striking when comparing to chips like the 82599 (64 VFs per port x 2), or a Mellanox (also 64?) or SolarFlare (128?) card. And don't forget that, because you don't have pools of devices to be automatically chosen from, that each guest domain that will be migrated requires a reserved NIC on *every* machine it will be migrated to (no other domain can be configured to use that NIC, in order to avoid conflicts). Of course you could complicate the software by adding a driver that manages pools of generic hostdevs, and coordinates MAC address changes with the guest (part of what you're suggesting), but all that extra complexity not only takes a lot of time and effort to develop, it also creates more code that needs to be maintained and tested for regressions at each release. The alternative is to just spend $130 per host for an 82576 or Intel I350 card (these are the cheapest SRIOV options I'm aware of). When compared to the total cost of any hardware installation large enough to support migration and have performance requirements high enough that NIC passthrough is needed, this is a trivial amount. I guess the bottom line of all this is that (in my opinion, of course :-) supporting useful migration of domains that used passed-through non-SRIOV NICs would be an interesting experiment, but I don't see much utility to it, other than scratching an intellectual itch, and I'm concerned that it would create more long term maintenance cost than it was worth. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 4/9] Move iothreadspin information into iothreadids
... + +if (!(iothrid = virDomainIOThreadIDFind(def, iothreadid))) { +if (!(iothrid = virDomainIOThreadIDAdd(def, iothreadid))) Why are you adding a iothread definition here? This code is executed after the iothread info should already be parsed so adding a thread here doesn't make sense. The section here should ressemble the check [1] that you've removed. Because you required that the iothreadpin information to be included in the iothreadid; however, if someone hasn't defined iothreadids's, then there won't be any until the PostParse is called - thus we have to add one here so that we can store the pin information for an iothread Okay, I didn't see that because I've remembered a different version of patch 1 where you allocated the full list upfront. By the way, the approach in this series now has the following loophole: 1) Specify iothreads nelemes(iothreadids) 2) Specify iothreadpin for thread . Now one of the threads is added with id due to the code above. Thus I think that the PostParse callback code should probably be removed and the missing threads should be numbered right after iohtreadids is parsed so that this patch doesn't allow that. I've moved the code in patch 1 ... -virBufferAsprintf(buf, iothreadpin iothread='%u' , - def-cputune.iothreadspin[i]-id); +/* Ignore iothreadids with no cpumask or a cpumask that + * inherits from cpuset of vcpu. */ +if (!def-iothreadids[i]-cpumask || +virBitmapEqual(def-cpumask, def-iothreadids[i]-cpumask)) +continue; ... here. Also why are you explicitly rejecting cpumask that is equal to the default one? If a user explicitly specifies it that way then the info would be lost. Additionally, def-cpumask here is used on the iothread automatically without filling it to the structure if it's used so there's no need to do that. Prior review - it's already there - see commit id '938fb12f' I'm fairly certain I was specifically asked to do that... It doesn't make much sense though. The default pinning is used from def-cpumap if the specific is not present. If the specific pinning is removed it should be freed. OK, but I contend this needs to be a separate patch after this series so that all 3 are fixed at once. ... diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index cdf0aaf..5282449 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1149,7 +1149,7 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm) virCgroupPtr cgroup_iothread = NULL; qemuDomainObjPrivatePtr priv = vm-privateData; virDomainDefPtr def = vm-def; -size_t i, j; +size_t i; unsigned long long period = vm-def-cputune.period; long long quota = vm-def-cputune.quota; char *mem_mask = NULL; @@ -1214,18 +1214,11 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm) /* default cpu masks */ if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) cpumask = priv-autoCpuset; +else if (def-iothreadids[i]-cpumask) +cpumask = def-iothreadids[i]-cpumask; This has to be the first case. Even if VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO is specified, it cannot override what the user configured pinning. OK, but see qemuSetupCgroupForVcpu and qemuSetupCgroupForEmulator... Vcpu has one way of doing it and Emulator slightly different, but both check AUTO first. VCPU checks AUTO first, then sets to default. Then it checks the vcpupin and resets to whatever is there. Emulator checks AUTO first, then it checks if emulatorpin is set and uses that, and finally falls back to default if defined. Um, I made a mistake when I refactored qemuSetupCgroupForEmulator. qemuSetupCgroupForVcpu is the right approach. I will follow ForVcpu and someone else can fix ForEmulator I followed emulatorpin If I'm wrong, then emulator and vcpu are wrong, but I think fixing that is a separate patch. yes. else cpumask = def-cpumask; -/* specific cpu mask */ -for (j = 0; j def-cputune.niothreadspin; j++) { -if (def-cputune.iothreadspin[j]-id == -def-iothreadids[i]-iothread_id) { -cpumask = def-cputune.iothreadspin[j]-cpumask; -break; -} Finally, this can be removed! -} - if (cpumask qemuSetupCgroupCpusetCpus(cgroup_iothread, cpumask) 0) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0f95cc7..ee13d08 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5894,19 +5894,14 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, goto cleanup; for (i = 0; i targetDef-niothreadids; i++) {
Re: [libvirt] [RFC 0/7] Live Migration with Pass-through Devices proposal
On 04/22/2015 12:22 AM, Chen Fan wrote: Hi Laine, Thanks for your review for my patches. and do you know that solarflare's patches have made some update version since https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html ? if not, I hope to go on to complete this work. ;) I haven't heard of any updates. Their priorities may have changed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/9] conf: Add new domain XML element 'iothreadids'
... @@ -3298,6 +3325,21 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; } +/* Fully populate the IOThread ID list */ +if (def-iothreads def-iothreads != def-niothreadids) { +unsigned int iothread_id = 1; +while (def-niothreadids != def-iothreads) { +if (!virDomainIOThreadIDFind(def, iothread_id)) { +virDomainIOThreadIDDefPtr iothrid; + +if (!(iothrid = virDomainIOThreadIDAdd(def, iothread_id))) +return -1; Unfortunately, fixing the iothread list after you parse iothread pinning in patch 4 makes a loophole where you might force arbitrary iothread ID without using iothreadids. This code will probably need to be moved after the parsing code, despite the fact that the postparse callback is better place to do such checks. +iothrid-autofill = true; +} +iothread_id++; +} +} + if (virDomainDefGetMemoryInitial(def) == 0) { virReportError(VIR_ERR_XML_ERROR, %s, _(Memory size must be specified via memory or in the The rest of this patch looks good, but I'd like to see the above part fixed before my final ACK. I'll move it and post v5 for 1/9 here as well as 4/9 I haven't yet got down to 8/9 I'd like to get this in before code freeze... Tks - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] parallels: implement domainDetachDevice and domainDetachDeviceFlags
Maxim Nestratov (2): parallels: add prlsdkDelDisk and prlsdkGetDiskIndex functions parallels: implement domainDetachDevice and domainDetachDeviceFlags -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] parallels: implement domainDetachDevice and domainDetachDeviceFlags
New functions utilize previosly added prlsdkDelDisk and prlsdkGetDiskIndex Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_driver.c | 71 ++ src/parallels/parallels_sdk.c| 30 src/parallels/parallels_sdk.h|3 ++ 3 files changed, 104 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 07f1311..b667e02 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1098,6 +1098,75 @@ static int parallelsDomainAttachDevice(virDomainPtr dom, const char *xml) VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_AFFECT_LIVE); } +static int parallelsDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, +unsigned int flags) +{ +int ret = -1; +parallelsConnPtr privconn = dom-conn-privateData; +virDomainDeviceDefPtr dev = NULL; +virDomainObjPtr privdom = NULL; +bool domactive = false; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + +privdom = parallelsDomObjFromDomain(dom); +if (privdom == NULL) +return -1; + +if (!(flags VIR_DOMAIN_AFFECT_CONFIG)) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(device detach needs VIR_DOMAIN_AFFECT_CONFIG + flag to be set)); +goto cleanup; +} + +domactive = virDomainObjIsActive(privdom); +if (!domactive (flags VIR_DOMAIN_AFFECT_LIVE)) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(cannot do live update a device on + inactive domain)); +goto cleanup; +} +if (domactive !(flags VIR_DOMAIN_AFFECT_LIVE)) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(Updates on a running domain need + VIR_DOMAIN_AFFECT_LIVE flag)); +} + +dev = virDomainDeviceDefParse(xml, privdom-def, privconn-caps, + privconn-xmlopt, VIR_DOMAIN_XML_INACTIVE); +if (dev == NULL) +goto cleanup; + +switch (dev-type) { +case VIR_DOMAIN_DEVICE_DISK: +ret = prlsdkDetachVolume(dom-conn, privdom, dev-data.disk); +if (ret) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(disk detach failed)); +goto cleanup; +} +break; +default: +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _(device type '%s' cannot be detached), + virDomainDeviceTypeToString(dev-type)); +break; +} + +ret = 0; + cleanup: +virObjectUnlock(privdom); +return ret; +} + +static int parallelsDomainDetachDevice(virDomainPtr dom, const char *xml) +{ +return parallelsDomainDetachDeviceFlags(dom, xml, +VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_AFFECT_LIVE); +} + static virHypervisorDriver parallelsDriver = { .name = Parallels, .connectOpen = parallelsConnectOpen,/* 0.10.0 */ @@ -1134,6 +1203,8 @@ static virHypervisorDriver parallelsDriver = { .domainUndefineFlags = parallelsDomainUndefineFlags, /* 1.2.10 */ .domainAttachDevice = parallelsDomainAttachDevice, /* 1.2.15 */ .domainAttachDeviceFlags = parallelsDomainAttachDeviceFlags, /* 1.2.15 */ +.domainDetachDevice = parallelsDomainDetachDevice, /* 1.2.15 */ +.domainDetachDeviceFlags = parallelsDomainDetachDeviceFlags, /* 1.2.15 */ .domainIsActive = parallelsDomainIsActive, /* 1.2.10 */ .connectDomainEventRegisterAny = parallelsConnectDomainEventRegisterAny, /* 1.2.10 */ .connectDomainEventDeregisterAny = parallelsConnectDomainEventDeregisterAny, /* 1.2.10 */ diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 5e6e21c..94274a2 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -3106,6 +3106,36 @@ prlsdkGetDiskIndex(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk) return idx; } +int prlsdkDetachVolume(virConnectPtr conn, + virDomainObjPtr dom, + virDomainDiskDefPtr disk) +{ +int ret = -1, idx; +parallelsConnPtr privconn = conn-privateData; +parallelsDomObjPtr privdom = dom-privateData; +PRL_HANDLE job = PRL_INVALID_HANDLE; + +idx = prlsdkGetDiskIndex(privdom-sdkdom, disk); +if (idx 0) +goto cleanup; + +job = PrlVm_BeginEdit(privdom-sdkdom); +if (PRL_FAILED(waitJob(job, privconn-jobTimeout))) +goto cleanup; + +ret = prlsdkDelDisk(privdom-sdkdom, idx); +if (ret == 0) { +job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE); +if (PRL_FAILED(waitJob(job, privconn-jobTimeout))) { +ret = -1; +goto cleanup; +} +
[libvirt] [PATCH 1/2] parallels: add prlsdkDelDisk and prlsdkGetDiskIndex functions
Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c | 65 + 1 files changed, 65 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index d54f894..5e6e21c 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2844,6 +2844,25 @@ static void prlsdkDelNet(parallelsConnPtr privconn, virDomainNetDefPtr net) PrlHandle_Free(vnet); } +static int prlsdkDelDisk(PRL_HANDLE sdkdom, int idx) +{ +int ret = -1; +PRL_RESULT pret; +PRL_HANDLE sdkdisk = PRL_INVALID_HANDLE; + +pret = PrlVmCfg_GetHardDisk(sdkdom, idx, sdkdisk); +prlsdkCheckRetGoto(pret, cleanup); + +pret = PrlVmDev_Remove(sdkdisk); +prlsdkCheckRetGoto(pret, cleanup); + +ret = 0; + + cleanup: +PrlHandle_Free(sdkdisk); +return ret; +} + static int prlsdkAddDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk, bool bootDisk) { PRL_RESULT pret; @@ -3042,6 +3061,52 @@ prlsdkAttachVolume(virConnectPtr conn, } static int +prlsdkGetDiskIndex(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk) +{ +int idx = -1; +char *buf = NULL; +PRL_UINT32 buflen = 0; +PRL_RESULT pret; +PRL_UINT32 hddCount; +PRL_UINT32 i; +PRL_HANDLE hdd = PRL_INVALID_HANDLE; + +pret = PrlVmCfg_GetHardDisksCount(sdkdom, hddCount); +prlsdkCheckRetGoto(pret, cleanup); + +for (i = 0; i hddCount; ++i) { + +pret = PrlVmCfg_GetHardDisk(sdkdom, i, hdd); +prlsdkCheckRetGoto(pret, cleanup); + +pret = PrlVmDev_GetFriendlyName(hdd, 0, buflen); +prlsdkCheckRetGoto(pret, cleanup); + +if (VIR_ALLOC_N(buf, buflen) 0) +goto cleanup; + +pret = PrlVmDev_GetFriendlyName(hdd, buf, buflen); +prlsdkCheckRetGoto(pret, cleanup); + +if (STRNEQ(disk-src-path, buf)) { + +PrlHandle_Free(hdd); +hdd = PRL_INVALID_HANDLE; +VIR_FREE(buf); +continue; +} + +VIR_FREE(buf); +idx = i; +break; +} + + cleanup: +PrlHandle_Free(hdd); +return idx; +} + +static int prlsdkAddFS(PRL_HANDLE sdkdom, virDomainFSDefPtr fs) { PRL_RESULT pret; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] tests: Add virtTestCompareToFile
Replaces a common pattern used in many test files --- tests/bhyvexml2argvtest.c| 41 ++-- tests/bhyvexml2xmltest.c | 9 + tests/cputest.c | 11 +-- tests/domaincapstest.c | 9 + tests/lxcxml2xmltest.c | 9 + tests/networkxml2conftest.c | 9 + tests/networkxml2firewalltest.c | 9 + tests/networkxml2xmltest.c | 9 + tests/networkxml2xmlupdatetest.c | 9 + tests/nodeinfotest.c | 18 ++ tests/nwfilterxml2firewalltest.c | 11 +-- tests/nwfilterxml2xmltest.c | 9 + tests/qemucaps2xmltest.c | 27 -- tests/qemumonitorjsontest.c | 9 ++--- tests/qemuxml2argvtest.c | 13 + tests/qemuxmlnstest.c| 13 + tests/secretxml2xmltest.c| 9 + tests/sexpr2xmltest.c| 9 + tests/storagepoolxml2xmltest.c | 9 + tests/storagevolxml2argvtest.c | 14 +- tests/storagevolxml2xmltest.c| 9 + tests/sysinfotest.c | 10 ++ tests/testutils.c| 33 tests/testutils.h| 2 ++ tests/vircaps2xmltest.c | 10 +- tests/vircgrouptest.c| 9 + tests/vmx2xmltest.c | 9 + tests/xencapstest.c | 9 + tests/xlconfigtest.c | 18 ++ tests/xmconfigtest.c | 18 ++ tests/xml2sexprtest.c| 9 + tests/xml2vmxtest.c | 10 +- 32 files changed, 78 insertions(+), 324 deletions(-) diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 26d0075..1cce2aa 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -19,7 +19,6 @@ static int testCompareXMLToArgvFiles(const char *xml, const char *ldcmdline, const char *dmcmdline) { -char *expectargv = NULL, *expectld = NULL, *expectdm = NULL; int len; char *actualargv = NULL, *actualld = NULL, *actualdm = NULL; virDomainDefPtr vmdef = NULL; @@ -54,51 +53,23 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(actualld = virCommandToString(ldcmd))) goto out; -len = virtTestLoadFile(cmdline, expectargv); -if (len 0) +if (virtTestCompareToFile(actualargv, cmdline) 0) goto out; -if (len expectargv[len - 1] == '\n') -expectargv[len - 1] = '\0'; - -len = virtTestLoadFile(ldcmdline, expectld); -if (len 0) +if (virtTestCompareToFile(actualld, ldcmdline) 0) goto out; -if (len expectld[len - 1] == '\n') -expectld[len - 1] = '\0'; - -len = virFileReadAllQuiet(dmcmdline, 1000, expectdm); -if (len 0) { -if (actualdm != NULL) { -virtTestDifference(stderr, , actualdm); -goto out; -} -} else if (len expectdm[len - 1] == '\n') { -expectdm[len - 1] = '\0'; -} - -if (STRNEQ(expectargv, actualargv)) { -virtTestDifference(stderr, expectargv, actualargv); +if (virtTestCompareToFile(formatted, xml) 0) goto out; -} -if (STRNEQ(expectld, actualld)) { -virtTestDifference(stderr, expectld, actualld); -goto out; -} - -if (expectdm STRNEQ(expectdm, actualdm)) { -virtTestDifference(stderr, expectdm, actualdm); -goto out; +if (virFileExists(dmcmdline) || actualdm) { +if (virtTestCompareToFile(actualdm, dmcmdline) 0) +goto out; } ret = 0; out: -VIR_FREE(expectargv); -VIR_FREE(expectld); -VIR_FREE(expectdm); VIR_FREE(actualargv); VIR_FREE(actualld); VIR_FREE(actualdm); diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index 740c957..826baea 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -14,14 +14,10 @@ static bhyveConn driver; static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { -char *outXmlData = NULL; char *actual = NULL; virDomainDefPtr def = NULL; int ret = -1; -if (virtTestLoadFile(outxml, outXmlData) 0) -goto fail; - if (!(def = virDomainDefParseFile(inxml, driver.caps, driver.xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto fail; @@ -29,15 +25,12 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml) if (!(actual = virDomainDefFormat(def, VIR_DOMAIN_DEF_FORMAT_INACTIVE))) goto fail; -if (STRNEQ(outXmlData, actual)) { -virtTestDifference(stderr, outXmlData, actual); +if (virtTestCompareToFile(actual, outxml) 0) goto fail; -} ret = 0; fail: -
[libvirt] [PATCH 1/4] tests: Add VIR_TEST_DEBUG and VIR_TEST_VERBOSE
To remove a bunch of TestGetDebug()/TestGetVerbose() checks --- tests/cputest.c | 34 - tests/jsontest.c | 48 + tests/nodeinfotest.c | 2 +- tests/qemuargv2xmltest.c | 15 ++-- tests/qemuhelptest.c | 6 +- tests/qemuhotplugtest.c | 19 +++-- tests/qemumonitortest.c | 24 +++ tests/qemuxml2argvtest.c | 14 ++-- tests/qemuxmlnstest.c| 6 +- tests/securityselinuxlabeltest.c | 6 +- tests/statstest.c| 3 +- tests/testutils.c| 2 +- tests/testutils.h| 12 tests/testutilslxc.c | 2 +- tests/testutilsqemu.c| 2 +- tests/utiltest.c | 30 +++- tests/virbuftest.c | 60 +++- tests/virhashtest.c | 152 +-- tests/virpcitest.c | 4 +- tests/virportallocatortest.c | 35 - 20 files changed, 188 insertions(+), 288 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index bf7a48f..56bcc90 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -235,13 +235,11 @@ cpuTestCompare(const void *arg) virResetLastError(); if (data-result != result) { -if (virTestGetVerbose()) { -fprintf(stderr, \nExpected result %s, got %s\n, +VIR_TEST_VERBOSE(\nExpected result %s, got %s\n, cpuTestCompResStr(data-result), cpuTestCompResStr(result)); -/* Pad to line up with test name ... in virTestRun */ -fprintf(stderr, %74s, ... ); -} +/* Pad to line up with test name ... in virTestRun */ +VIR_TEST_VERBOSE(%74s, ... ); goto cleanup; } @@ -337,8 +335,8 @@ cpuTestBaseline(const void *arg) virResetLastError(); if (!baseline) { ret = 0; -} else if (virTestGetVerbose()) { -fprintf(stderr, \n%-70s... , +} else { +VIR_TEST_VERBOSE(\n%-70s... , cpuBaseline was expected to fail but it succeeded); } goto cleanup; @@ -364,11 +362,9 @@ cpuTestBaseline(const void *arg) cmp = cpuCompare(cpus[i], baseline, false); if (cmp != VIR_CPU_COMPARE_SUPERSET cmp != VIR_CPU_COMPARE_IDENTICAL) { -if (virTestGetVerbose()) { -fprintf(stderr, -\nbaseline CPU is incompatible with CPU %zu\n, i); -fprintf(stderr, %74s, ... ); -} +VIR_TEST_VERBOSE(\nbaseline CPU is incompatible with CPU %zu\n, + i); +VIR_TEST_VERBOSE(%74s, ... ); ret = -1; goto cleanup; } @@ -438,13 +434,11 @@ cpuTestHasFeature(const void *arg) virResetLastError(); if (data-result != result) { -if (virTestGetVerbose()) { -fprintf(stderr, \nExpected result %s, got %s\n, -cpuTestBoolWithErrorStr(data-result), -cpuTestBoolWithErrorStr(result)); -/* Pad to line up with test name ... in virTestRun */ -fprintf(stderr, %74s, ... ); -} +VIR_TEST_VERBOSE(\nExpected result %s, got %s\n, +cpuTestBoolWithErrorStr(data-result), +cpuTestBoolWithErrorStr(result)); +/* Pad to line up with test name ... in virTestRun */ +VIR_TEST_VERBOSE(%74s, ... ); goto cleanup; } @@ -483,7 +477,7 @@ cpuTestRun(const char *name, const struct data *data) char *log; if ((log = virtTestLogContentAndReset()) strlen(log) 0) -fprintf(stderr, \n%s\n, log); +VIR_TEST_DEBUG(\n%s\n, log); VIR_FREE(log); } diff --git a/tests/jsontest.c b/tests/jsontest.c index a2a42e3..f27943e 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -27,23 +27,19 @@ testJSONFromString(const void *data) if (info-pass) { if (!json) { -if (virTestGetVerbose()) -fprintf(stderr, Fail to parse %s\n, info-doc); +VIR_TEST_VERBOSE(Fail to parse %s\n, info-doc); ret = -1; goto cleanup; } else { -if (virTestGetDebug()) -fprintf(stderr, Parsed %s\n, info-doc); +VIR_TEST_DEBUG(Parsed %s\n, info-doc); } } else { if (json) { -if (virTestGetVerbose()) -fprintf(stderr, Should not have parsed %s\n, info-doc); +VIR_TEST_VERBOSE(Should not have parsed %s\n, info-doc); ret = -1; goto cleanup; } else { -if (virTestGetDebug()) -fprintf(stderr, Fail to parse %s\n, info-doc); +VIR_TEST_DEBUG(Fail to parse %s\n, info-doc); } } @@ -66,8 +62,7
[libvirt] [PATCH 2/4] tests: Use *DefParseFile more
--- tests/bhyvexml2xmltest.c | 9 ++--- tests/domainconftest.c | 7 +-- tests/lxcxml2xmltest.c | 8 ++-- tests/networkxml2conftest.c| 7 +-- tests/networkxml2xmltest.c | 6 +- tests/networkxml2xmlupdatetest.c | 6 +- tests/nwfilterxml2xmltest.c| 6 +- tests/qemuagenttest.c | 9 ++--- tests/secretxml2xmltest.c | 6 +- tests/securityselinuxlabeltest.c | 7 +-- tests/storagebackendsheepdogtest.c | 20 +++- tests/storagepoolxml2xmltest.c | 6 +- tests/storagevolxml2argvtest.c | 26 -- tests/storagevolxml2xmltest.c | 12 ++-- tests/xlconfigtest.c | 9 ++--- tests/xmconfigtest.c | 9 ++--- tests/xml2sexprtest.c | 9 ++--- tests/xml2vmxtest.c| 9 ++--- 18 files changed, 31 insertions(+), 140 deletions(-) diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index 2e742cf..740c957 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -14,20 +14,16 @@ static bhyveConn driver; static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { -char *inXmlData = NULL; char *outXmlData = NULL; char *actual = NULL; virDomainDefPtr def = NULL; int ret = -1; -if (virtTestLoadFile(inxml, inXmlData) 0) -goto fail; - if (virtTestLoadFile(outxml, outXmlData) 0) goto fail; -if (!(def = virDomainDefParseString(inXmlData, driver.caps, driver.xmlopt, -VIR_DOMAIN_DEF_PARSE_INACTIVE))) +if (!(def = virDomainDefParseFile(inxml, driver.caps, driver.xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto fail; if (!(actual = virDomainDefFormat(def, VIR_DOMAIN_DEF_FORMAT_INACTIVE))) @@ -41,7 +37,6 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml) ret = 0; fail: -VIR_FREE(inXmlData); VIR_FREE(outXmlData); VIR_FREE(actual); virDomainDefFree(def); diff --git a/tests/domainconftest.c b/tests/domainconftest.c index b27dd14..53c40c1 100644 --- a/tests/domainconftest.c +++ b/tests/domainconftest.c @@ -43,7 +43,6 @@ struct testGetFilesystemData { static int testGetFilesystem(const void *opaque) { int ret = -1; -char *xmlData = NULL; virDomainDefPtr def = NULL; char *filename = NULL; const struct testGetFilesystemData *data = opaque; @@ -53,10 +52,7 @@ static int testGetFilesystem(const void *opaque) abs_srcdir, data-filename) 0) goto cleanup; -if (virtTestLoadFile(filename, xmlData) 0) -goto cleanup; - -if (!(def = virDomainDefParseString(xmlData, caps, xmlopt, 0))) +if (!(def = virDomainDefParseFile(filename, caps, xmlopt, 0))) goto cleanup; fsdef = virDomainGetFilesystemForTarget(def, @@ -79,7 +75,6 @@ static int testGetFilesystem(const void *opaque) cleanup: virDomainDefFree(def); -VIR_FREE(xmlData); VIR_FREE(filename); return ret; } diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c index 37997f5..a9b5419 100644 --- a/tests/lxcxml2xmltest.c +++ b/tests/lxcxml2xmltest.c @@ -25,19 +25,16 @@ static virDomainXMLOptionPtr xmlopt; static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live) { -char *inXmlData = NULL; char *outXmlData = NULL; char *actual = NULL; int ret = -1; virDomainDefPtr def = NULL; -if (virtTestLoadFile(inxml, inXmlData) 0) -goto fail; if (virtTestLoadFile(outxml, outXmlData) 0) goto fail; -if (!(def = virDomainDefParseString(inXmlData, caps, xmlopt, -live ? 0 : VIR_DOMAIN_DEF_PARSE_INACTIVE))) +if (!(def = virDomainDefParseFile(inxml, caps, xmlopt, + live ? 0 : VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto fail; if (!virDomainDefCheckABIStability(def, def)) { @@ -55,7 +52,6 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live) ret = 0; fail: -VIR_FREE(inXmlData); VIR_FREE(outXmlData); VIR_FREE(actual); virDomainDefFree(def); diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 6df161f..1d9a772 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -21,7 +21,6 @@ static int testCompareXMLToConfFiles(const char *inxml, const char *outconf, dnsmasqCapsPtr caps) { -char *inXmlData = NULL; char *outConfData = NULL; char *actual = NULL; int ret = -1; @@ -31,13 +30,10 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, dnsmasqCapsPtr char *pidfile = NULL; dnsmasqContext *dctx = NULL; -if (virtTestLoadFile(inxml, inXmlData) 0) -goto fail; - if
[libvirt] [PATCH 0/4] tests: Cleanups and improvements
First 3 patches are uninteresting cleanups, removing a bunch of boiler plate code. The last patch adds a way to easily regenerate test output, like when adding a new test case (plop in the qemu XML, use the env var and the test suite will create foo.args for you), or when a code changes causes test suite output to expectedly change. Cole Robinson (4): tests: Add VIR_TEST_DEBUG and VIR_TEST_VERBOSE tests: Use *DefParseFile more tests: Add virtTestCompareToFile tests: Add VIR_TEST_REGENERATE_OUTPUT HACKING| 7 ++ docs/hacking.html.in | 12 +++ tests/bhyvexml2argvtest.c | 41 ++ tests/bhyvexml2xmltest.c | 18 + tests/cputest.c| 45 --- tests/domaincapstest.c | 9 +-- tests/domainconftest.c | 7 +- tests/jsontest.c | 48 +--- tests/lxcxml2xmltest.c | 17 + tests/networkxml2conftest.c| 16 +--- tests/networkxml2firewalltest.c| 9 +-- tests/networkxml2xmltest.c | 15 +--- tests/networkxml2xmlupdatetest.c | 15 +--- tests/nodeinfotest.c | 20 + tests/nwfilterxml2firewalltest.c | 11 +-- tests/nwfilterxml2xmltest.c| 15 +--- tests/qemuagenttest.c | 9 +-- tests/qemuargv2xmltest.c | 15 ++-- tests/qemucaps2xmltest.c | 27 +-- tests/qemuhelptest.c | 6 +- tests/qemuhotplugtest.c| 19 ++--- tests/qemumonitorjsontest.c| 9 +-- tests/qemumonitortest.c| 24 +++--- tests/qemuxml2argvtest.c | 27 ++- tests/qemuxmlnstest.c | 19 + tests/secretxml2xmltest.c | 15 +--- tests/securityselinuxlabeltest.c | 13 +--- tests/sexpr2xmltest.c | 9 +-- tests/statstest.c | 3 +- tests/storagebackendsheepdogtest.c | 20 + tests/storagepoolxml2xmltest.c | 15 +--- tests/storagevolxml2argvtest.c | 40 ++ tests/storagevolxml2xmltest.c | 21 + tests/sysinfotest.c| 10 +-- tests/testutils.c | 74 ++ tests/testutils.h | 14 tests/testutilslxc.c | 2 +- tests/testutilsqemu.c | 2 +- tests/utiltest.c | 30 +++- tests/virbuftest.c | 60 +++ tests/vircaps2xmltest.c| 10 +-- tests/vircgrouptest.c | 9 +-- tests/virhashtest.c| 152 - tests/virpcitest.c | 4 +- tests/virportallocatortest.c | 35 +++-- tests/vmx2xmltest.c| 9 +-- tests/xencapstest.c| 9 +-- tests/xlconfigtest.c | 27 +-- tests/xmconfigtest.c | 27 +-- tests/xml2sexprtest.c | 18 + tests/xml2vmxtest.c| 19 + 51 files changed, 340 insertions(+), 767 deletions(-) -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] virDomainObjList: Introduce yet another hash table
This hash table will contain the same data as already existing one. The only difference is that while the first table uses domain uuid as key, the new table uses domain name. This will allow much faster (and lockless) lookups by domain name. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 51 +++--- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d03..41963cc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -65,6 +65,10 @@ struct _virDomainObjList { /* uuid string - virDomainObj mapping * for O(1), lockless lookup-by-uuid */ virHashTable *objs; + +/* name - virDomainObj mapping for O(1), + * lockless lookup-by-name */ +virHashTable *objsName; }; @@ -1042,7 +1046,8 @@ virDomainObjListPtr virDomainObjListNew(void) if (!(doms = virObjectLockableNew(virDomainObjListClass))) return NULL; -if (!(doms-objs = virHashCreate(50, virObjectFreeHashData))) { +if (!(doms-objs = virHashCreate(50, virObjectFreeHashData)) || +!(doms-objsName = virHashCreate(50, virObjectFreeHashData))) { virObjectUnref(doms); return NULL; } @@ -1056,6 +1061,7 @@ static void virDomainObjListDispose(void *obj) virDomainObjListPtr doms = obj; virHashFree(doms-objs); +virHashFree(doms-objsName); } @@ -1137,27 +1143,15 @@ virDomainObjListFindByUUIDRef(virDomainObjListPtr doms, return virDomainObjListFindByUUIDInternal(doms, uuid, true); } -static int virDomainObjListSearchName(const void *payload, - const void *name ATTRIBUTE_UNUSED, - const void *data) -{ -virDomainObjPtr obj = (virDomainObjPtr)payload; -int want = 0; - -virObjectLock(obj); -if (STREQ(obj-def-name, (const char *)data)) -want = 1; -virObjectUnlock(obj); -return want; -} - virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms, const char *name) { virDomainObjPtr obj; + virObjectLock(doms); -obj = virHashSearch(doms-objs, virDomainObjListSearchName, name); +obj = virHashLookup(doms-objsName, name); virObjectRef(obj); +virObjectUnlock(doms); if (obj) { virObjectLock(obj); if (obj-removing) { @@ -1165,7 +1159,6 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms, obj = NULL; } } -virObjectUnlock(doms); return obj; } @@ -2544,7 +2537,7 @@ virDomainObjListAddLocked(virDomainObjListPtr doms, oldDef); } else { /* UUID does not match, but if a name matches, refuse it */ -if ((vm = virHashSearch(doms-objs, virDomainObjListSearchName, def-name))) { +if ((vm = virHashLookup(doms-objsName, def-name))) { virObjectLock(vm); virUUIDFormat(vm-def-uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, @@ -2562,6 +2555,15 @@ virDomainObjListAddLocked(virDomainObjListPtr doms, virObjectUnref(vm); return NULL; } + +if (virHashAddEntry(doms-objsName, def-name, vm) 0) { +virHashRemoveEntry(doms-objs, uuidstr); +return NULL; +} + +/* Since domain is in two hash tables, increment the + * reference counter */ +virObjectRef(vm); } cleanup: return vm; @@ -2719,6 +2721,7 @@ void virDomainObjListRemove(virDomainObjListPtr doms, virObjectLock(doms); virObjectLock(dom); virHashRemoveEntry(doms-objs, uuidstr); +virHashRemoveEntry(doms-objsName, dom-def-name); virObjectUnlock(dom); virObjectUnref(dom); virObjectUnlock(doms); @@ -2736,9 +2739,10 @@ void virDomainObjListRemoveLocked(virDomainObjListPtr doms, char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom-def-uuid, uuidstr); -virObjectUnlock(dom); virHashRemoveEntry(doms-objs, uuidstr); +virHashRemoveEntry(doms-objsName, dom-def-name); +virObjectUnlock(dom); } static int @@ -21587,6 +21591,15 @@ virDomainObjListLoadStatus(virDomainObjListPtr doms, if (virHashAddEntry(doms-objs, uuidstr, obj) 0) goto error; +if (virHashAddEntry(doms-objsName, obj-def-name, obj) 0) { +virHashRemoveEntry(doms-objs, uuidstr); +goto error; +} + +/* Since domain is in two hash tables, increment the + * reference counter */ +virObjectRef(obj); + if (notify) (*notify)(obj, 1, opaque); -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] Introduce virDomainObjEndAPI
This is basically turning qemuDomObjEndAPI into a more general function. Other drivers which gets a reference to domain objects may benefit from this function too. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 22 ++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 7 +-- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aad4ec0..686c614 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2460,6 +2460,28 @@ void virDomainObjAssignDef(virDomainObjPtr domain, } +/** + * virDomainObjEndAPI: + * @vm: domain object + * + * Finish working with a domain object in an API. This function + * clears whatever was left of a domain that was gathered using + * virDomainObjListFindByUUIDRef(). Currently that means only unlocking and + * decrementing the reference counter of that domain. And in order to + * make sure the caller does not access the domain, the pointer is + * cleared. + */ +void +virDomainObjEndAPI(virDomainObjPtr *vm) +{ +if (!*vm) +return; + +virObjectUnlock(*vm); +virObjectUnref(*vm); +*vm = NULL; +} + /* * diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 25d3ee6..9955052 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2397,6 +2397,8 @@ virDomainObjPtr virDomainObjListFindByUUIDRef(virDomainObjListPtr doms, virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms, const char *name); +void virDomainObjEndAPI(virDomainObjPtr *vm); + bool virDomainObjTaint(virDomainObjPtr obj, virDomainTaintFlags taint); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8c50ea2..e6555f1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -374,6 +374,7 @@ virDomainNostateReasonTypeFromString; virDomainNostateReasonTypeToString; virDomainObjAssignDef; virDomainObjCopyPersistentDef; +virDomainObjEndAPI; virDomainObjFormat; virDomainObjGetMetadata; virDomainObjGetPersistentDef; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1368386..b588c11 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2988,12 +2988,7 @@ qemuDomainAgentAvailable(virDomainObjPtr vm, void qemuDomObjEndAPI(virDomainObjPtr *vm) { -if (!*vm) -return; - -virObjectUnlock(*vm); -virObjectUnref(*vm); -*vm = NULL; +virDomainObjEndAPI(vm); } -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [RFC 0/7] Live Migration with Pass-through Devices proposal
On 04/22/2015 01:20 PM, Dr. David Alan Gilbert wrote: * Daniel P. Berrange (berra...@redhat.com) wrote: On Wed, Apr 22, 2015 at 06:12:25PM +0100, Dr. David Alan Gilbert wrote: * Daniel P. Berrange (berra...@redhat.com) wrote: On Wed, Apr 22, 2015 at 06:01:56PM +0100, Dr. David Alan Gilbert wrote: * Daniel P. Berrange (berra...@redhat.com) wrote: On Fri, Apr 17, 2015 at 04:53:02PM +0800, Chen Fan wrote: backgrond: Live migration is one of the most important features of virtualization technology. With regard to recent virtualization techniques, performance of network I/O is critical. Current network I/O virtualization (e.g. Para-virtualized I/O, VMDq) has a significant performance gap with native network I/O. Pass-through network devices have near native performance, however, they have thus far prevented live migration. No existing methods solve the problem of live migration with pass-through devices perfectly. There was an idea to solve the problem in website: https://www.kernel.org/doc/ols/2008/ols2008v2-pages-261-267.pdf Please refer to above document for detailed information. So I think this problem maybe could be solved by using the combination of existing technology. and the following steps are we considering to implement: - before boot VM, we anticipate to specify two NICs for creating bonding device (one plugged and one virtual NIC) in XML. here we can specify the NIC's mac addresses in XML, which could facilitate qemu-guest-agent to find the network interfaces in guest. - when qemu-guest-agent startup in guest it would send a notification to libvirt, then libvirt will call the previous registered initialize callbacks. so through the callback functions, we can create the bonding device according to the XML configuration. and here we use netcf tool which can facilitate to create bonding device easily. I'm not really clear on why libvirt/guest agent needs to be involved in this. I think configuration of networking is really something that must be left to the guest OS admin to control. I don't think the guest agent should be trying to reconfigure guest networking itself, as that is inevitably going to conflict with configuration attempted by things in the guest like NetworkManager or systemd-networkd. IOW, if you want to do this setup where the guest is given multiple NICs connected to the same host LAN, then I think we should just let the gues admin configure bonding in whatever manner they decide is best for their OS install. I disagree; there should be a way for the admin not to have to do this manually; however it should interact well with existing management stuff. At the simplest, something that marks the two NICs in a discoverable way so that they can be seen that they're part of a set; with just that ID system then an installer or setup tool can notice them and offer to put them into a bond automatically; I'd assume it would be possible to add a rule somewhere that said anything with the same ID would automatically be added to the bond. I didn't mean the admin would literally configure stuff manually. I really just meant that the guest OS itself should decide how it is done, whether NetworkManager magically does the right thing, or the person building the cloud disk image provides a magic udev rule, or $something else. I just don't think that the QEMU guest agent should be involved, as that will definitely trample all over other things that manage networking in the guest. OK, good, that's about the same level I was at. I could see this being solved in the cloud disk images by using cloud-init metadata to mark the NICs as being in a set, or perhaps there is some magic you could define in SMBIOS tables, or something else again. A cloud-init based solution wouldn't need any QEMU work, but an SMBIOS solution might. Would either of these work with hotplug though? I guess as the VM starts off with the pair of NICs, then when you remove one and add it back after migration then you don't need any more information added; so yes cloud-init or SMBIOS would do it. (I was thinking SMBIOS stuff in the way that you get device/slot numbering that NIC naming is sometimes based off). What about if we hot-add a new NIC later on (not during migration); a normal hot-add of a NIC now turns into a hot-add of two new NICs; how do we pass the information at hot-add time to provide that? Hmm, yes, actually hotplug would be a problem with that. A even simpler idea would be to just keep things real dumb and simply use the same MAC address for both NICs. Once you put them in a bond device, the kernel will be copying the MAC address of the first NIC into the second NIC anyway, so unless I'm missing something, we might as well just use the same MAC address for both right away. That makes it easy for guest to discover NICs in the same set and works with hotplug trivially.
[libvirt] [PATCH v2] parallels: fix crash in prlsdkAddNet in case of CT definition
Since net-model is not defined for containers we shouldn't touch it. In case network adapter model is defined, a warning about ignoring it is shown. Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c | 29 ++--- 1 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index d54f894..c67044c 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2725,7 +2725,8 @@ static const char * prlsdkFormatMac(virMacAddrPtr mac, char *macstr) static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, -virDomainNetDefPtr net) +virDomainNetDefPtr net, +bool isCt) { PRL_RESULT pret; PRL_HANDLE sdknet = PRL_INVALID_HANDLE; @@ -2757,19 +2758,25 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, pret = PrlVmDevNet_SetMacAddress(sdknet, macstr); prlsdkCheckRetGoto(pret, cleanup); -if (STREQ(net-model, rtl8139)) { -pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); -} else if (STREQ(net-model, e1000)) { -pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); -} else if (STREQ(net-model, virtio)) { -pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_VIRTIO); +if (isCt) { +if (net-model) + VIR_WARN(Setting network adapter for containers is not + supported by Parallels Cloud Server.); } else { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, +if (STREQ(net-model, rtl8139)) { +pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); +} else if (STREQ(net-model, e1000)) { +pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); +} else if (STREQ(net-model, virtio)) { +pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_VIRTIO); +} else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Specified network adapter model is not supported by Parallels Cloud Server.)); -goto cleanup; +goto cleanup; +} +prlsdkCheckRetGoto(pret, cleanup); } -prlsdkCheckRetGoto(pret, cleanup); if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) { if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { @@ -3153,7 +3160,7 @@ prlsdkDoApplyConfig(virConnectPtr conn, } for (i = 0; i def-nnets; i++) { -if (prlsdkAddNet(sdkdom, conn-privateData, def-nets[i]) 0) +if (prlsdkAddNet(sdkdom, conn-privateData, def-nets[i], IS_CT(def)) 0) goto error; } -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/9] Implement Add/Del IOThreads
So obviously I messed something up with the threading on the responses - thought I had it right, but not quite I guess. Oh well - I think it's obvious what I was trying to do ;-) v5 updates: Patch 1/10 (forgot to edit the 10) * Fixes the syntax check * Moves the PostParse autofill of iothreadids right after the possible reading from XML Patch 4/9 * Add virDomainIOThreadIDArrayHasPin which will looking through the niothreadids[] array for any iothreadid[] with a cpumask defined * Fail in virDomainIOThreadPinDefParseXML if the iothreadid cannot be found * Remove [n]iothreadspin references * Left the cpumask checking to default - I think all should be handled in a later/separate patch * Fixed qemuSetupCgroupForIOThreads cpumask ordering Patch 5/9 * Adjusted the test to use 'fifo' *and* priority='1' for second entry (got a failure in a different test after my first pass) Patch 7.5/9 * NEW - Code that will remove the iothread_id bit from the ids mask. I did test this and it worked like I expected... Patch 8/9 * Not quite sure what was meant by auto numa placement so I didn't add that into the patch I sent, but if the following is what you meant, then I can squash it in (and define cpumask appropriately) +if (vm-def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) +cpumask = priv-autoCpuset; +else +cpumask = vm-def-cpumask; + +if (cpumask) { +if (qemuDomainHotplugPinThread(cpumask, iothread_id, * Remove qemuProcessSetSchedParams * Remove niothreadspin * Remove the virNumaIsAvailable() hunk from qemuDomainChgIOThread Tks, John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 6/9] sheepdog: Check for validity of pool source hostname
If there is a pool source hostname provided, let's make sure the name can be resolved before trying to connect to it via a virCommand sequence. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_sheepdog.c | 38 +- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index af15c3b..b92c7a3 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -1,7 +1,7 @@ /* * storage_backend_sheepdog.c: storage backend for Sheepdog handling * - * Copyright (C) 2013-2014 Red Hat, Inc. + * Copyright (C) 2013-2015 Red Hat, Inc. * Copyright (C) 2012 Wido den Hollander * Copyright (C) 2012 Frank Spijkerman * Copyright (C) 2012 Sebastian Wiedenroth @@ -40,9 +40,6 @@ static int virStorageBackendSheepdogRefreshVol(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol); -void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, - virStoragePoolObjPtr pool); - int virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool, char *output) @@ -90,7 +87,7 @@ virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool, return -1; } -void +static int virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virStoragePoolObjPtr pool) { @@ -99,6 +96,8 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, if (pool-def-source.nhost 0) { if (pool-def-source.hosts[0].name != NULL) address = pool-def-source.hosts[0].name; +if (virSocketAddrParseName(NULL, address, AF_UNSPEC, IPPROTO_TCP) 0) +return -1; if (pool-def-source.hosts[0].port) port = pool-def-source.hosts[0].port; } @@ -106,6 +105,7 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virCommandAddArgFormat(cmd, %s, address); virCommandAddArg(cmd, -p); virCommandAddArgFormat(cmd, %d, port); +return 0; } static int @@ -151,7 +151,8 @@ virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, size_t i; virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, list, -r, NULL); -virStorageBackendSheepdogAddHostArg(cmd, pool); +if (virStorageBackendSheepdogAddHostArg(cmd, pool) 0) +goto cleanup; virCommandSetOutputBuffer(cmd, output); if (virCommandRun(cmd, NULL) 0) goto cleanup; @@ -196,7 +197,8 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virCommandPtr cmd; cmd = virCommandNewArgList(COLLIE, node, info, -r, NULL); -virStorageBackendSheepdogAddHostArg(cmd, pool); +if (virStorageBackendSheepdogAddHostArg(cmd, pool) 0) +goto cleanup; virCommandSetOutputBuffer(cmd, output); if (virCommandRun(cmd, NULL) 0) goto cleanup; @@ -218,13 +220,16 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, unsigned int flags) { +int ret = -1; virCheckFlags(0, -1); virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, delete, vol-name, NULL); -virStorageBackendSheepdogAddHostArg(cmd, pool); -int ret = virCommandRun(cmd, NULL); +if (virStorageBackendSheepdogAddHostArg(cmd, pool) 0) +goto cleanup; +ret = virCommandRun(cmd, NULL); + cleanup: virCommandFree(cmd); return ret; } @@ -275,7 +280,8 @@ virStorageBackendSheepdogBuildVol(virConnectPtr conn, cmd = virCommandNewArgList(COLLIE, vdi, create, vol-name, NULL); virCommandAddArgFormat(cmd, %llu, vol-target.capacity); -virStorageBackendSheepdogAddHostArg(cmd, pool); +if (virStorageBackendSheepdogAddHostArg(cmd, pool) 0) +goto cleanup; if (virCommandRun(cmd, NULL) 0) goto cleanup; @@ -355,11 +361,12 @@ virStorageBackendSheepdogRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { -int ret; +int ret = -1; char *output = NULL; virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, list, vol-name, -r, NULL); -virStorageBackendSheepdogAddHostArg(cmd, pool); +if (virStorageBackendSheepdogAddHostArg(cmd, pool) 0) +goto cleanup; virCommandSetOutputBuffer(cmd, output); ret = virCommandRun(cmd, NULL); @@ -391,14 +398,17 @@ virStorageBackendSheepdogResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long capacity, unsigned int flags) { +int ret = -1; virCheckFlags(0, -1);
[libvirt] [PATCH v2 2/9] socketaddr: Introduce virSocketAddrParseName
Add a new function virSocketAddrParseName which unlike virSocketAddrParse will be capable of processing a network address that needs to be looked up and resolved. Signed-off-by: John Ferlan jfer...@redhat.com --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 52 ++-- src/util/virsocketaddr.h | 7 ++- 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8c50ea2..848b44a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2065,6 +2065,7 @@ virSocketAddrNumericFamily; virSocketAddrParse; virSocketAddrParseIPv4; virSocketAddrParseIPv6; +virSocketAddrParseName; virSocketAddrPrefixToNetmask; virSocketAddrSetIPv4Addr; virSocketAddrSetPort; diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 0e9a39c..83518a0 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -80,6 +80,7 @@ static int virSocketAddrParseInternal(struct addrinfo **res, const char *val, int family, + int protocol, bool isNumeric, bool reportError) { @@ -93,6 +94,7 @@ virSocketAddrParseInternal(struct addrinfo **res, memset(hints, 0, sizeof(hints)); hints.ai_family = family; +hints.ai_protocol = protocol; if (isNumeric) hints.ai_flags = AI_NUMERICHOST; if ((err = getaddrinfo(val, NULL, hints, res)) != 0) { @@ -123,7 +125,7 @@ int virSocketAddrParse(virSocketAddrPtr addr, const char *val, int family) int len; struct addrinfo *res; -if (virSocketAddrParseInternal(res, val, family, true, true) 0) +if (virSocketAddrParseInternal(res, val, family, 0, true, true) 0) return -1; if (res == NULL) { @@ -143,6 +145,51 @@ int virSocketAddrParse(virSocketAddrPtr addr, const char *val, int family) return len; } +/** + * virSocketAddrParseName: + * @addr: where to store the return value, optional. + * @val: either a numeric network address IPv4 or IPv6 or a network + * hostname to be looked up and resolved. + * @family: address family to pass down to getaddrinfo + * @protocol: specific protocol to use + * + * Mostly a wrapper for getaddrinfo() extracting the address storage + * from the numeric string like 1.2.3.4 or 2001:db8:85a3:0:0:8a2e:370:7334 + * or a resolvable network hostname such as redhat.com + * + * Returns the length of the network address or -1 in case of error. + */ +int +virSocketAddrParseName(virSocketAddrPtr addr, + const char *val, + int family, + int protocol) +{ +int len; +struct addrinfo *res; + +if (virSocketAddrParseInternal(res, val, family, + protocol, false, true) 0) +return -1; + +if (res == NULL) { +virReportError(VIR_ERR_SYSTEM_ERROR, + _(No addresses found for '%s' using family='%d' + and protocol='%d'), + val, family, protocol); +return -1; +} + +len = res-ai_addrlen; +if (addr != NULL) { +memcpy(addr-data.stor, res-ai_addr, len); +addr-len = res-ai_addrlen; +} + +freeaddrinfo(res); +return len; +} + /* * virSocketAddrParseIPv4: * @val: an IPv4 numeric address @@ -880,7 +927,8 @@ virSocketAddrNumericFamily(const char *address) struct addrinfo *res; unsigned short family; -if (virSocketAddrParseInternal(res, address, AF_UNSPEC, true, false) 0) +if (virSocketAddrParseInternal(res, address, AF_UNSPEC, + 0, true, false) 0) return -1; family = res-ai_addr-sa_family; diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 99ab46f..3bbf119 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2013 Red Hat, Inc. + * Copyright (C) 2009-2015 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 @@ -78,6 +78,11 @@ int virSocketAddrParse(virSocketAddrPtr addr, const char *val, int family); +int virSocketAddrParseName(virSocketAddrPtr addr, + const char *val, + int family, + int protocol); + int virSocketAddrParseIPv4(virSocketAddrPtr addr, const char *val); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] network: check for bridge name conflict with existing devices
Since some people use the same naming convention as libvirt for bridge devices they create outside the context of libvirt, it is much nicer if we check for those devices when looking for a bridge device name to auto-assign to a new network. --- src/network/bridge_driver.c | 25 + 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index abacae3..3b879cd 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2037,11 +2037,13 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, /* Create and configure the bridge device */ if (!network-def-bridge) { -/* this can only happen if the config files were edited - * directly. Otherwise networkValidate() (called after parsing - * the XML from networkCreateXML() and networkDefine()) - * guarantees we will have a valid bridge name before this - * point. +/* bridge name can only be empty if the config files were + * edited directly. Otherwise networkValidate() (called after + * parsing the XML from networkCreateXML() and + * networkDefine()) guarantees we will have a valid bridge + * name before this point. Since hand editing of the config + * files is explicitly prohibited we can, with clear + * conscience, log an error and fail at this point. */ virReportError(VIR_ERR_INTERNAL_ERROR, _(network '%s' has no bridge name defined), @@ -2762,8 +2764,9 @@ static int networkIsPersistent(virNetworkPtr net) /* * networkFindUnusedBridgeName() - try to find a bridge name that is - * unused by the currently configured libvirt networks, and set this - * network's name to that new name. + * unused by the currently configured libvirt networks, as well as by + * the host system itself (possibly created by someone/something other + * than libvirt). Set this network's name to that new name. */ static int networkFindUnusedBridgeName(virNetworkObjListPtr nets, @@ -2777,7 +2780,13 @@ networkFindUnusedBridgeName(virNetworkObjListPtr nets, do { if (virAsprintf(newname, templ, id) 0) goto cleanup; -if (!virNetworkBridgeInUse(nets, newname, def-name)) { +/* check if this name is used in another libvirt network or + * there is an existing device with that name. ignore errors + * from virNetDevExists(), just in case it isn't implemented + * on this platform (probably impossible). + */ +if (!(virNetworkBridgeInUse(nets, newname, def-name) || + virNetDevExists(newname) == 1)) { VIR_FREE(def-bridge); /*could contain template */ def-bridge = newname; ret = 0; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] skip names of devices already existing on host when auto-assigning bridge name
Shivaprasad Bhat earlier sent patches to fix this problem which added a call to virNetDevExists() in the conf directory: https://www.redhat.com/archives/libvir-list/2015-March/msg00397.html (V1) https://www.redhat.com/archives/libvir-list/2015-March/msg00569.html (V2) https://www.redhat.com/archives/libvir-list/2015-March/msg01174.html (V3) https://www.redhat.com/archives/libvir-list/2015-March/msg01297.html (V4) After discussing it with him a few times (both email and irc), I decided that the only way to really understand the problems he described (related to locking) with my counterproposal (do the checking in the bridge driver itself), was to try fixing it myself. I do now understand the problems he encountered with locking, but also found that the instance of the call to the Find a bridge name and set it function that caused the problem was unnecessary (see the commit message of patch 1). The first patch here essentially reimplements current functionality but moving the majority of the code from conf/network_conf.c to network/bridge_driver.c. The 2nd patch adds the all-important virNetDevExists() call to the function that is looking for an unused bridge name. Laine Stump (2): network: move auto-assign of bridge name from XML parser to net driver network: check for bridge name conflict with existing devices src/conf/network_conf.c | 60 --- src/conf/network_conf.h | 9 + src/libvirt_private.syms| 2 +- src/network/bridge_driver.c | 98 - 4 files changed, 99 insertions(+), 70 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] network: move auto-assign of bridge name from XML parser to net driver
We already check that any auto-assigned bridge device name for a virtual network (e.g. virbr1) doesn't conflict with the bridge name for any existing libvirt network (via virNetworkSetBridgeName() in conf/network_conf.c). We also want to check that the name doesn't conflict with any bridge device created on the host system outside the control of libvirt (history: possibly due to the ploriferation of references to libvirt's bridge devices in HOWTO documents all around the web, it is not uncommon for an admin to manually create a bridge in their host's system network config and name it virbrX). To add such a check to virNetworkBridgeInUse() (which is called by virNetworkSetBridgeName()) we would have to call virNetDevExists() (from util/virnetdev.c); this function calls ioctl(SIOCGIFFLAGS), which everyone on the mailing list agreed should not be done from an XML parsing function in the conf directory. To remedy that problem, this patch removes virNetworkSetBridgeName() from conf/network_conf.c and puts an identically functioning networkBridgeNameValidate() in network/bridge_driver.c (because it's reasonable for the bridge driver to call virNetDevExists(), although we don't do that yet because I wanted this patch to have as close to 0 effect on function as possible). There are a couple of inevitable changes though: 1) We no longer check the bridge name during virNetworkLoadConfig(). Close examination of the code shows that this wasn't necessary anyway - the only *correct* way to get XML into the config files is via networkDefine(), and networkDefine() will always call networkValidate(), which previously called virNetworkSetBridgeName() (and now calls networkBridgeNameValidate()). This means that the only way the bridge name can be unset during virNetworkLoadConfig() is if someone edited the config file on disk by hand (which we explicitly prohibit). 2) Just on the off chance that somebody *has* edited the file by hand, rather than crashing when they try to start their malformed network, a check for non-NULL bridge name has been added to networkStartNetworkVirtual(). (For those wondering why I don't instead call networkValidateBridgeName() there to set a bridge name if one wasn't present - the problem is that during networkStartNetworkVirtual(), the lock for the network being started has already been acquired, but the lock for the network list itself *has not* (because we aren't adding/removing a network). But virNetworkBridgeInuse() iterates through *all* networks (including this one) and locks each network as it is checked for a duplicate entry; it is necessary to lock each network even before checking if it is the designated skip network because otherwise some other thread might acquire the list lock and delete the very entry we're examining. In the end, permitting a setting of the bridge name during network start would require that we lock the entire network list during any networkStartNetwork(), which eliminates a *lot* of parallelism that we've worked so hard to achieve (it can make a huge difference during libvirtd startup). So rather than try to adjust for someone playing against the rules, I choose to instead give them the error they deserve.) 3) virNetworkAllocateBridge() (now removed) would leak any template string set as the bridge name. Its replacement networkFindUnusedBridgeName() doesn't leak the template string - it is properly freed. --- src/conf/network_conf.c | 60 -- src/conf/network_conf.h | 9 + src/libvirt_private.syms| 2 +- src/network/bridge_driver.c | 89 - 4 files changed, 90 insertions(+), 70 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index f4a9df0..aa8d6c6 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -45,7 +45,6 @@ #include virfile.h #include virstring.h -#define MAX_BRIDGE_ID 256 #define VIR_FROM_THIS VIR_FROM_NETWORK /* currently, /sbin/tc implementation allows up to 16 bits for minor class size */ #define CLASS_ID_BITMAP_SIZE (116) @@ -3123,11 +3122,6 @@ virNetworkObjPtr virNetworkLoadConfig(virNetworkObjListPtr nets, virNetworkSetBridgeMacAddr(def); virNetworkSaveConfig(configDir, def); } -/* Generate a bridge if none is specified, but don't check for collisions - * if a bridge is hardcoded, so the network is at least defined. - */ -if (virNetworkSetBridgeName(nets, def, 0)) -goto error; } else { /* Throw away MAC address for other forward types, * which could have been generated by older libvirt RPMs */ @@ -3303,60 +3297,6 @@ int virNetworkBridgeInUse(virNetworkObjListPtr nets, return obj != NULL; } -char *virNetworkAllocateBridge(virNetworkObjListPtr nets, - const char
[libvirt] [PATCH v2 8/9] virsocketaddr: Introduce virSocketAddrIsSameTCPHost
Add new API to be able to compare two TCP host name strings or numeric IP Addresses in order to determine if the resolved and translated names match. Signed-off-by: John Ferlan jfer...@redhat.com --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 62 src/util/virsocketaddr.h | 2 ++ 3 files changed, 65 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 848b44a..34cb871 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2058,6 +2058,7 @@ virSocketAddrGetRange; virSocketAddrIsNetmask; virSocketAddrIsNumericLocalhost; virSocketAddrIsPrivate; +virSocketAddrIsSameTCPHost; virSocketAddrIsWildcard; virSocketAddrMask; virSocketAddrMaskByPrefix; diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 993d460..c602604 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -23,6 +23,7 @@ #include config.h +#include viralloc.h #include virsocketaddr.h #include virerror.h #include virstring.h @@ -980,3 +981,64 @@ virSocketAddrIsNumericLocalhost(const char *addr) return false; } + + +/** + * virSocketAddrIsSameTCPHost: + * @host1: either a numeric network address IPv4 or IPv6 or a network hostname + * @host2: either a numeric network address IPv4 or IPv6 or a network hostname + * + * For each hostname, get the array of resolved addresses and in a loop make + * compare the translated host IP Address looking for duplicates. + * + * Returns 1 if the hosts match, 0 if there is no match, and -1 on error + */ +int +virSocketAddrIsSameTCPHost(const char *host1, + const char *host2) +{ +int ret = -1; +struct addrinfo *reshost1 = NULL, *reshost2 = NULL; +struct addrinfo *curhost1, *curhost2; +char *host1rslv = NULL, *host2rslv = NULL; + +if (virSocketAddrParseInternal(reshost1, host1, AF_UNSPEC, IPPROTO_TCP, + false, true) 0) +goto cleanup; + +if (virSocketAddrParseInternal(reshost2, host2, AF_UNSPEC, IPPROTO_TCP, + false, true) 0) +goto cleanup; + +for (curhost1 = reshost1; curhost1; curhost1 = curhost1-ai_next) { +if (!(host1rslv = virSocketAddrGetNumericHost(curhost1-ai_addr, + curhost1-ai_addrlen, + false, NULL))) +goto cleanup; +for (curhost2 = reshost2; curhost2; curhost2 = curhost2-ai_next) { +if (curhost1-ai_family != curhost2-ai_family) +continue; + +if (!(host2rslv = virSocketAddrGetNumericHost(curhost2-ai_addr, + curhost2-ai_addrlen, + false, NULL))) +goto cleanup; + +if (STREQ(host1rslv, host2rslv)) { +ret = 1; +goto cleanup; +} +} +} + +ret = 0; + + cleanup: +if (reshost1) +freeaddrinfo(reshost1); +if (reshost2) +freeaddrinfo(reshost2); +VIR_FREE(host1rslv); +VIR_FREE(host2rslv); +return ret; +} diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 3bbf119..2f9f358 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -137,4 +137,6 @@ bool virSocketAddrIsWildcard(const virSocketAddr *addr); int virSocketAddrNumericFamily(const char *address); bool virSocketAddrIsNumericLocalhost(const char *addr); + +int virSocketAddrIsSameTCPHost(const char *host1, const char *host2); #endif /* __VIR_SOCKETADDR_H__ */ -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 9/9] storage: Check for duplicate resolved host name at pool create
https://bugzilla.redhat.com/show_bug.cgi?id=1171984 During storage pool Create or CreateXML add a call prior to the backend startPool that will compare the resolved source host name of the network storage pool to be started against the active, network storage pools resolved host name to ensure the to be started definition won't duplicate an existing, active pool for the same pool source path's. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/storage_conf.c | 80 +++- src/conf/storage_conf.h | 6 +++- src/libvirt_private.syms | 1 + src/storage/storage_driver.c | 9 - 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 4852dfb..4793817 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1,7 +1,7 @@ /* * storage_conf.c: config handling for storage driver * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -43,6 +43,7 @@ #include virbuffer.h #include viralloc.h #include virfile.h +#include virsocketaddr.h #include virstring.h #include virlog.h @@ -2571,6 +2572,83 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, return ret; } +/* + * Prior to starting a storage pool, this API will check to see whether + * any currently active pool is already using the same source dir/device + * and same resolved host name as the proposed definition. To do this, + * query the host in order to compare the resolved names for the pool + * and the definition. + * + * Returns 0 if the definition doesn't match any host, returns -1 on error + */ +int +virStoragePoolSourceHostCompare(virStoragePoolObjListPtr pools, +virStoragePoolDefPtr def) +{ +int ret = -1; +size_t i; + +if (def-type != VIR_STORAGE_POOL_NETFS +def-type != VIR_STORAGE_POOL_GLUSTER +def-type != VIR_STORAGE_POOL_ISCSI +def-type != VIR_STORAGE_POOL_SHEEPDOG) +return 0; + +for (i = 0; i pools-count; i++) { +virStoragePoolObjPtr pool = pools-objs[i]; +virStoragePoolSourcePtr poolsrc, defsrc; + +virStoragePoolObjLock(pool); + +poolsrc = pool-def-source; +defsrc = def-source; +if (pool-active pool-def-type == def-type +poolsrc-nhost == 1 defsrc-nhost == 1) { + +bool matchsrc = false; +const char *poolhost = poolsrc-hosts[0].name; +const char *defhost = defsrc-hosts[0].name; + +/* Only check the port if the to be started pool supplied one */ +if (defsrc-hosts[0].port != 0) { +if (poolsrc-hosts[0].port != defsrc-hosts[0].port) { +virStoragePoolObjUnlock(pool); +continue; +} +} + +if (pool-def-type == VIR_STORAGE_POOL_NETFS || +pool-def-type == VIR_STORAGE_POOL_GLUSTER) { +matchsrc = STREQ(poolsrc-dir, defsrc-dir); +} else if (pool-def-type == VIR_STORAGE_POOL_ISCSI) { +if (virStoragePoolSourceFindDuplicateDevices(pool, def)) +matchsrc = true; +} else if (pool-def-type == VIR_STORAGE_POOL_SHEEPDOG) { +matchsrc = true; +} + +if (matchsrc) { +ret = virSocketAddrIsSameTCPHost(poolhost, defhost); +virStoragePoolObjUnlock(pool); +if (ret == 0) +continue; +if (ret == 1) { +virReportError(VIR_ERR_OPERATION_FAILED, + _(Storage source conflict with pool: '%s'), + pool-def-name); +ret = -1; +} +goto cleanup; +} +} +virStoragePoolObjUnlock(pool); +} +ret = 0; + + cleanup: +return ret; +} + void virStoragePoolObjLock(virStoragePoolObjPtr obj) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 7471006..0d46376 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -1,7 +1,7 @@ /* * storage_conf.h: config handling for storage driver * - * Copyright (C) 2006-2008, 2010-2014 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010-2015 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -417,6 +417,10 @@ int virStoragePoolSourceFindDuplicate(virConnectPtr conn, virStoragePoolObjListPtr pools, virStoragePoolDefPtr def); +int virStoragePoolSourceHostCompare(virStoragePoolObjListPtr pools, +virStoragePoolDefPtr def) +ATTRIBUTE_NONNULL(1)
[libvirt] [PATCH v2 7/9] virsockaddr: Split up functionality of virSocketAddrFormatFull
Create a local API virSocketAddrGetNumericHost which can be used by a future patch in order to obtain the numeric host nameinfo data Signed-off-by: John Ferlan jfer...@redhat.com --- src/util/virsocketaddr.c | 60 ++-- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 83518a0..993d460 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -333,6 +333,42 @@ virSocketAddrFormat(const virSocketAddr *addr) } +static char * +virSocketAddrGetNumericHost(const struct sockaddr *sa, +socklen_t salen, +bool withService, +const char *separator) +{ +char host[NI_MAXHOST], port[NI_MAXSERV]; +char *addrstr; +int err; +int flags = NI_NUMERICHOST; + +if (withService) +flags |= NI_NUMERICSERV; + +if ((err = getnameinfo(sa, salen, + host, sizeof(host), + port, sizeof(port), + flags)) != 0) { +virReportError(VIR_ERR_SYSTEM_ERROR, + _(Cannot convert socket address to string: %s), + gai_strerror(err)); +return NULL; +} + +if (withService) { +if (virAsprintf(addrstr, %s%s%s, host, separator, port) == -1) +return NULL; +} else { +if (VIR_STRDUP(addrstr, host) 0) +return NULL; +} + +return addrstr; +} + + /* * virSocketAddrFormatFull: * @addr: an initialized virSocketAddrPtr @@ -348,9 +384,7 @@ virSocketAddrFormatFull(const virSocketAddr *addr, bool withService, const char *separator) { -char host[NI_MAXHOST], port[NI_MAXSERV]; char *addrstr; -int err; if (addr == NULL) { virReportError(VIR_ERR_INVALID_ARG, %s, _(Missing address)); @@ -371,26 +405,8 @@ virSocketAddrFormatFull(const virSocketAddr *addr, return addrstr; } -if ((err = getnameinfo(addr-data.sa, - addr-len, - host, sizeof(host), - port, sizeof(port), - NI_NUMERICHOST | NI_NUMERICSERV)) != 0) { -virReportError(VIR_ERR_SYSTEM_ERROR, - _(Cannot convert socket address to string: %s), - gai_strerror(err)); -return NULL; -} - -if (withService) { -if (virAsprintf(addrstr, %s%s%s, host, separator, port) == -1) -goto error; -} else { -if (VIR_STRDUP(addrstr, host) 0) -goto error; -} - -return addrstr; +return virSocketAddrGetNumericHost(addr-data.sa, addr-len, + withService, separator); error: return NULL; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/9] socketaddr: Add isNumeric flag to virSocketAddrParseInternal
Adjust virSocketAddrParseInternal to take a boolean 'isNumeric' in order to determine whether to set ai_flags = AI_NUMERICHOST; - IOW - expect a numeric IP Address of sorts in the 'val' to be resolved. Signed-off-by: John Ferlan jfer...@redhat.com --- src/util/virsocketaddr.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 67ed330..0e9a39c 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2014 Red Hat, Inc. + * Copyright (C) 2009-2015 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 @@ -80,6 +80,7 @@ static int virSocketAddrParseInternal(struct addrinfo **res, const char *val, int family, + bool isNumeric, bool reportError) { struct addrinfo hints; @@ -92,7 +93,8 @@ virSocketAddrParseInternal(struct addrinfo **res, memset(hints, 0, sizeof(hints)); hints.ai_family = family; -hints.ai_flags = AI_NUMERICHOST; +if (isNumeric) +hints.ai_flags = AI_NUMERICHOST; if ((err = getaddrinfo(val, NULL, hints, res)) != 0) { if (reportError) virReportError(VIR_ERR_SYSTEM_ERROR, @@ -121,7 +123,7 @@ int virSocketAddrParse(virSocketAddrPtr addr, const char *val, int family) int len; struct addrinfo *res; -if (virSocketAddrParseInternal(res, val, family, true) 0) +if (virSocketAddrParseInternal(res, val, family, true, true) 0) return -1; if (res == NULL) { @@ -878,7 +880,7 @@ virSocketAddrNumericFamily(const char *address) struct addrinfo *res; unsigned short family; -if (virSocketAddrParseInternal(res, address, AF_UNSPEC, false) 0) +if (virSocketAddrParseInternal(res, address, AF_UNSPEC, true, false) 0) return -1; family = res-ai_addr-sa_family; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/9] gluster: Check for validity of pool source hostname
Prior to attempting to open the gluster connection, let's make sure we can resolve the source pool hostname. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_gluster.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index d2e79bc..a10f784 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -1,7 +1,7 @@ /* * storage_backend_gluster.c: storage backend for Gluster handling * - * Copyright (C) 2013-2014 Red Hat, Inc. + * Copyright (C) 2013-2015 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 @@ -96,6 +96,10 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool) trailing_slash = false; } +if (virSocketAddrParseName(NULL, pool-def-source.hosts[0].name, + AF_UNSPEC, IPPROTO_TCP) 0) +return NULL; + if (VIR_ALLOC(ret) 0) return NULL; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/9] Additional host name check for network storage pools
v1 here: http://www.redhat.com/archives/libvir-list/2015-April/msg00873.html Essentially a rewrite of v1 changing to use virsocketaddr.* rather than virutil.* Also making check for hostname duplication prior to startPool rather than during XML processing John Ferlan (9): socketaddr: Add isNumeric flag to virSocketAddrParseInternal socketaddr: Introduce virSocketAddrParseName iscsi: Check for validity of pool source hostname netfs: Check for validity of pool source hostname gluster: Check for validity of pool source hostname sheepdog: Check for validity of pool source hostname virsockaddr: Split up functionality of virSocketAddrFormatFull virsocketaddr: Introduce virSocketAddrIsSameTCPHost storage: Check for duplicate resolved host name at pool create src/conf/storage_conf.c| 80 ++- src/conf/storage_conf.h| 6 +- src/libvirt_private.syms | 3 + src/storage/storage_backend_fs.c | 5 +- src/storage/storage_backend_gluster.c | 6 +- src/storage/storage_backend_iscsi.c| 6 +- src/storage/storage_backend_sheepdog.c | 38 --- src/storage/storage_driver.c | 9 +- src/util/virsocketaddr.c | 180 - src/util/virsocketaddr.h | 9 +- 10 files changed, 295 insertions(+), 47 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/9] iscsi: Check for validity of pool source hostname
Ensure that the pool that's being started has a source pool hostname that can be resolved before trying to start an iSCSI session. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_iscsi.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 197d333..0482dfb 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -1,7 +1,7 @@ /* * storage_backend_iscsi.c: storage backend for iSCSI handling * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2015 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -385,6 +385,10 @@ virStorageBackendISCSIStartPool(virConnectPtr conn, return -1; } +if (virSocketAddrParseName(NULL, pool-def-source.hosts[0].name, + AF_UNSPEC, IPPROTO_TCP) 0) +return -1; + if (pool-def-source.ndevice != 1 || pool-def-source.devices[0].path == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/9] netfs: Check for validity of pool source hostname
Before attempting to mount the netfs pool, ensure the source pool hostname can be resolved to something this host knows. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_fs.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 521dc70..0263913 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1,7 +1,7 @@ /* * storage_backend_fs.c: storage backend for FS and directory handling * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2015 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -408,6 +408,9 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) %s, _(missing source path)); return -1; } +if (virSocketAddrParseName(NULL, pool-def-source.hosts[0].name, + AF_UNSPEC, IPPROTO_TCP) 0) +return -1; } else { if (pool-def-source.ndevice != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 04/9] Move iothreadspin information into iothreadids
Remove the iothreadspin array from cputune and replace with a cpumask to be stored in the iothreadids list. Adjust the test output because our printing goes in order of the iothreadids list now. Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatdomain.html.in | 10 +- src/conf/domain_conf.c | 112 ++--- src/conf/domain_conf.h | 3 +- src/qemu/qemu_cgroup.c | 13 +-- src/qemu/qemu_driver.c | 75 -- src/qemu/qemu_process.c| 10 +- .../qemuxml2xmlout-cputune-iothreads.xml | 2 +- 7 files changed, 86 insertions(+), 139 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 518f7c5..7af6bd7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -624,11 +624,11 @@ and attribute codecpuset/code of element codevcpu/code is not specified, the IOThreads are pinned to all the physical CPUs by default. There are two required attributes, the attribute - codeiothread/code specifies the IOThread id and the attribute - codecpuset/code specifying which physical CPUs to pin to. The - codeiothread/code value begins at 1 through the number of - a href=#elementsIOThreadsAllocationcodeiothreads/code/a - allocated to the domain. A value of 0 is not permitted. + codeiothread/code specifies the IOThread ID and the attribute + codecpuset/code specifying which physical CPUs to pin to. See + the codeiothreadids/code + a href=#elementsIOThreadsAllocationcodedescription/code/a + for valid codeiothread/code values. span class=sinceSince 1.2.9/span /dd dtcodeshares/code/dt diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 15514bb..083ded7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2115,11 +2115,25 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin) } +static bool +virDomainIOThreadIDArrayHasPin(virDomainDefPtr def) +{ +size_t i; + +for (i = 0; i def-niothreadids; i++) { +if (def-iothreadids[i]-cpumask) +return true; +} +return false; +} + + void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def) { if (!def) return; +virBitmapFree(def-cpumask); VIR_FREE(def); } @@ -2342,9 +2356,6 @@ void virDomainDefFree(virDomainDefPtr def) virDomainPinDefFree(def-cputune.emulatorpin); -virDomainPinDefArrayFree(def-cputune.iothreadspin, - def-cputune.niothreadspin); - for (i = 0; i def-cputune.nvcpusched; i++) virBitmapFree(def-cputune.vcpusched[i].ids); VIR_FREE(def-cputune.vcpusched); @@ -13302,74 +13313,77 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node, * and an iothreadspin has the form * iothreadpin iothread='1' cpuset='2'/ */ -static virDomainPinDefPtr +static int virDomainIOThreadPinDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, -int iothreads) +virDomainDefPtr def) { -virDomainPinDefPtr def; +int ret = -1; +virDomainIOThreadIDDefPtr iothrid; +virBitmapPtr cpumask; xmlNodePtr oldnode = ctxt-node; unsigned int iothreadid; char *tmp = NULL; -if (VIR_ALLOC(def) 0) -return NULL; - ctxt-node = node; if (!(tmp = virXPathString(string(./@iothread), ctxt))) { virReportError(VIR_ERR_XML_ERROR, %s, _(missing iothread id in iothreadpin)); -goto error; +goto cleanup; } if (virStrToLong_uip(tmp, NULL, 10, iothreadid) 0) { virReportError(VIR_ERR_XML_ERROR, _(invalid setting for iothread '%s'), tmp); -goto error; +goto cleanup; } VIR_FREE(tmp); if (iothreadid == 0) { virReportError(VIR_ERR_XML_ERROR, %s, _(zero is an invalid iothread id value)); -goto error; +goto cleanup; } -/* IOThreads are numbered iothread1...iothreadn, where - * n is the iothreads value */ -if (iothreadid iothreads) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(iothread id must not exceed iothreads)); -goto error; +if (!(iothrid = virDomainIOThreadIDFind(def, iothreadid))) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Cannot find 'iothread' : %u), + iothreadid); } -def-id = iothreadid; - if (!(tmp = virXMLPropString(node, cpuset))) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(missing cpuset for iothreadpin)); -goto error; +goto cleanup; } -if (virBitmapParse(tmp,
Re: [libvirt] [PATCH 2/4] tests: Use *DefParseFile more
On 04/23/2015 02:19 PM, Cole Robinson wrote: --- tests/bhyvexml2xmltest.c | 9 ++--- tests/domainconftest.c | 7 +-- tests/lxcxml2xmltest.c | 8 ++-- tests/networkxml2conftest.c| 7 +-- tests/networkxml2xmltest.c | 6 +- tests/networkxml2xmlupdatetest.c | 6 +- tests/nwfilterxml2xmltest.c| 6 +- tests/qemuagenttest.c | 9 ++--- tests/secretxml2xmltest.c | 6 +- tests/securityselinuxlabeltest.c | 7 +-- tests/storagebackendsheepdogtest.c | 20 +++- tests/storagepoolxml2xmltest.c | 6 +- tests/storagevolxml2argvtest.c | 26 -- tests/storagevolxml2xmltest.c | 12 ++-- tests/xlconfigtest.c | 9 ++--- tests/xmconfigtest.c | 9 ++--- tests/xml2sexprtest.c | 9 ++--- tests/xml2vmxtest.c| 9 ++--- 18 files changed, 31 insertions(+), 140 deletions(-) diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index 2e742cf..740c957 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -14,20 +14,16 @@ static bhyveConn driver; static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { -char *inXmlData = NULL; char *outXmlData = NULL; char *actual = NULL; virDomainDefPtr def = NULL; int ret = -1; -if (virtTestLoadFile(inxml, inXmlData) 0) -goto fail; - if (virtTestLoadFile(outxml, outXmlData) 0) goto fail; -if (!(def = virDomainDefParseString(inXmlData, driver.caps, driver.xmlopt, -VIR_DOMAIN_DEF_PARSE_INACTIVE))) +if (!(def = virDomainDefParseFile(inxml, driver.caps, driver.xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) You'll see a minor conflict here due to a recent change pushed upstream. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: cgroup: Fix priorities when setting emulatorpin
On 04/23/2015 10:19 AM, Peter Krempa wrote: Use the custom emulator pin setting with the highest priority same as with vcpupin. --- src/qemu/qemu_cgroup.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) ACK - John (FWIW: I'll adjust my code to follow) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 01/10] conf: Add new domain XML element 'iothreadids'
Adding a new XML element 'iothreadids' in order to allow defining specific IOThread ID's rather than relying on the algorithm to assign IOThread ID's starting at 1 and incrementing to iothreads count. This will allow future patches to be able to add new IOThreads by a specific iothread_id and of course delete any exisiting IOThread. Each iothreadids element will have 'n' iothread children elements which will have attribute id. The id will allow for definition of any valid (eg 0) iothread_id value. On input, if any iothreadids iothread's are provided, they will be marked so that we only print out what we read in. On input, if no iothreadids are provided, the PostParse code will self generate a list of ID's starting at 1 and going to the number of iothreads defined for the domain (just like the current algorithm numbering scheme). A future patch will rework the existing algorithm to make use of the iothreadids list. On output, only print out the iothreadids if they were read in. Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatdomain.html.in | 30 +++ docs/schemas/domaincommon.rng | 12 +++ src/conf/domain_conf.c| 203 +- src/conf/domain_conf.h| 18 src/libvirt_private.syms | 4 + 5 files changed, 265 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e921749..518f7c5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -521,6 +521,18 @@ ... lt;/domaingt; /pre +pre +lt;domaingt; + ... + lt;iothreadidsgt; +lt;iothread id=2/gt; +lt;iothread id=4/gt; +lt;iothread id=6/gt; +lt;iothread id=8/gt; + lt;/iothreadidsgt; + ... +lt;/domaingt; +/pre dl dtcodeiothreads/code/dt @@ -530,7 +542,25 @@ virtio-blk-pci and virtio-blk-ccw target storage devices. There should be only 1 or 2 IOThreads per host CPU. There may be more than one supported device assigned to each IOThread. +span class=sinceSince 1.2.8/span /dd + dtcodeiothreadids/code/dt + dd +The optional codeiothreadids/code element provides the capability +to specifically define the IOThread ID's for the domain. By default, +IOThread ID's are sequentially numbered starting from 1 through the +number of codeiothreads/code defined for the domain. The +codeid/code attribute is used to define the IOThread ID. The +codeid/code attribute must be a positive integer greater than 0. +If there are less codeiothreadids/code defined than +codeiothreads/code defined for the domain, then libvirt will +sequentially fill codeiothreadids/code starting at 1 avoiding +any predefined codeid/code. If there are more +codeiothreadids/code defined than codeiothreads/code +defined for the domain, then the codeiothreads/code value +will be adjusted accordingly. +span class=sinceSince 1.2.15/span + /dd /dl h3a name=elementsCPUTuningCPU Tuning/a/h3 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 19461f5..7072954 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -539,6 +539,18 @@ /optional optional +element name=iothreadids + zeroOrMore +element name=iothread + attribute name=id +ref name=unsignedInt/ + /attribute +/element + /zeroOrMore +/element + /optional + + optional ref name=blkiotune/ /optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9aad782..5c89388 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2114,6 +2114,32 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin) return NULL; } + +void +virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def) +{ +if (!def) +return; +VIR_FREE(def); +} + + +static void +virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def, +int nids) +{ +size_t i; + +if (!def) +return; + +for (i = 0; i nids; i++) +virDomainIOThreadIDDefFree(def[i]); + +VIR_FREE(def); +} + + void virDomainPinDefFree(virDomainPinDefPtr def) { @@ -2310,6 +2336,8 @@ void virDomainDefFree(virDomainDefPtr def) virCPUDefFree(def-cpu); +virDomainIOThreadIDDefArrayFree(def-iothreadids, def-niothreadids); + virDomainPinDefArrayFree(def-cputune.vcpupin, def-cputune.nvcpupin); virDomainPinDefFree(def-cputune.emulatorpin); @@ -13148,6 +13176,54 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, return idmap; } +/* Parse the XML definition for an IOThread ID + * + * Format is : + * + * iothreads4/iothreads + * iothreadids + * iothread id='1'/ + * iothread id='3'/ + * iothread id='5'/ + *
[libvirt] [PATCH v5 8/9] qemu: Add support to Add/Delete IOThreads
Add qemuDomainAddIOThread and qemuDomainDelIOThread in order to add or remove an IOThread to/from the host either for live or config optoins The implementation for the 'live' option will use the iothreadpids list in order to make decision, while the 'config' option will use the iothreadids list. Additionally, for deletion each may have to adjust the iothreadpin list. IOThreads are implemented by qmp objects, the code makes use of the existing qemuMonitorAddObject or qemuMonitorDelObject APIs. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_audit.c | 9 ++ src/conf/domain_audit.h | 6 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 375 +++ 4 files changed, 391 insertions(+) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 3e93d97..4ea10d2 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -790,6 +790,15 @@ virDomainAuditVcpu(virDomainObjPtr vm, return virDomainAuditResource(vm, vcpu, oldvcpu, newvcpu, reason, success); } +void +virDomainAuditIOThread(virDomainObjPtr vm, + unsigned int oldiothread, unsigned int newiothread, + const char *reason, bool success) +{ +return virDomainAuditResource(vm, iothread, oldiothread, newiothread, + reason, success); +} + static void virDomainAuditLifecycle(virDomainObjPtr vm, const char *op, const char *reason, bool success) diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 4c1ef90..97dadca 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -102,6 +102,12 @@ void virDomainAuditVcpu(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void virDomainAuditIOThread(virDomainObjPtr vm, +unsigned int oldiothread, +unsigned int newiothread, +const char *reason, +bool success) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); void virDomainAuditSecurityLabel(virDomainObjPtr vm, bool success) ATTRIBUTE_NONNULL(1); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0877907..2f72636 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -127,6 +127,7 @@ virDomainAuditDisk; virDomainAuditFS; virDomainAuditHostdev; virDomainAuditInit; +virDomainAuditIOThread; virDomainAuditMemory; virDomainAuditNet; virDomainAuditNetDevice; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f5216bd..56d953f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6155,6 +6155,379 @@ qemuDomainPinIOThread(virDomainPtr dom, return ret; } +static int +qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int iothread_id) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +char *alias = NULL; +size_t idx; +int rc = -1; +int ret = -1; +unsigned int orig_niothreads = vm-def-iothreads; +unsigned int exp_niothreads = vm-def-iothreads; +int new_niothreads = 0; +qemuMonitorIOThreadInfoPtr *new_iothreads = NULL; +virCgroupPtr cgroup_iothread = NULL; +char *mem_mask = NULL; +virDomainIOThreadIDDefPtr iothrid; + +if (virDomainIOThreadIDFind(vm-def, iothread_id)) { +virReportError(VIR_ERR_INVALID_ARG, + _(an IOThread is already using iothread_id '%u'), + iothread_id); +goto cleanup; +} + +if (virAsprintf(alias, iothread%u, iothread_id) 0) +return -1; + +qemuDomainObjEnterMonitor(driver, vm); + +rc = qemuMonitorAddObject(priv-mon, iothread, alias, NULL); +exp_niothreads++; +if (rc 0) +goto exit_monitor; + +/* After hotplugging the IOThreads we need to re-detect the + * IOThreads thread_id's, adjust the cgroups, thread affinity, + * and add the thread_id to the vm-def-iothreadids list. + */ +if ((new_niothreads = qemuMonitorGetIOThreads(priv-mon, + new_iothreads)) 0) +goto exit_monitor; + +if (qemuDomainObjExitMonitor(driver, vm) 0) +goto cleanup; + +if (new_niothreads != exp_niothreads) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(got wrong number of IOThread ids from QEMU monitor. + got %d, wanted %d), + new_niothreads, exp_niothreads); +vm-def-iothreads = new_niothreads; +goto cleanup; +} +vm-def-iothreads = exp_niothreads; + +if (virDomainNumatuneGetMode(vm-def-numa, -1) == +VIR_DOMAIN_NUMATUNE_MEM_STRICT +virDomainNumatuneMaybeFormatNodeset(vm-def-numa, +
[libvirt] [PATCH v5 7.5/9] domain: Introduce virDomainIOThreadSchedDelId
We're about to allow IOThreads to be deleted, but an iothreadid may be included in some domain thread sched, so add a new API to allow removing an iothread from some entry. Then during the writing of the threadsched data and an additional check to determine whether the bitmap is all clear before writing it out. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 20 src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 3 files changed, 22 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 846a744..bc975f3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17434,6 +17434,24 @@ virDomainIOThreadIDDel(virDomainDefPtr def, } } +void +virDomainIOThreadSchedDelId(virDomainDefPtr def, +unsigned int iothreadid) +{ +size_t i; + +if (!def-cputune.iothreadsched || !def-cputune.niothreadsched) +return; + +for (i = 0; i def-cputune.niothreadsched; i++) { +if (virBitmapIsBitSet(def-cputune.iothreadsched[i].ids, iothreadid)) { +ignore_value(virBitmapClearBit(def-cputune.iothreadsched[i].ids, + iothreadid)); +return; +} +} +} + virDomainPinDefPtr virDomainPinFind(virDomainPinDefPtr *def, int npin, @@ -20876,6 +20894,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainThreadSchedParamPtr sp = def-cputune.iothreadsched[i]; char *ids = NULL; +if (virBitmapIsAllClear(sp-ids)) +continue; if (!(ids = virBitmapFormat(sp-ids))) goto error; virBufferAsprintf(buf, iothreadsched iothreads='%s' scheduler='%s', diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7daaeed..4ae89fd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2615,6 +2615,7 @@ virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(virDomainDefPtr def, virDomainIOThreadIDDefPtr virDomainIOThreadIDAdd(virDomainDefPtr def, unsigned int iothread_id); void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id); +void virDomainIOThreadSchedDelId(virDomainDefPtr def, unsigned int iothread_id); unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a0f4152..0877907 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -328,6 +328,7 @@ virDomainIOThreadIDAdd; virDomainIOThreadIDDefFree; virDomainIOThreadIDDel; virDomainIOThreadIDFind; +virDomainIOThreadSchedDelId; virDomainLeaseDefFree; virDomainLeaseIndex; virDomainLeaseInsert; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] tests: Add virtTestCompareToFile
On 04/23/2015 02:19 PM, Cole Robinson wrote: Replaces a common pattern used in many test files --- tests/bhyvexml2argvtest.c| 41 ++-- tests/bhyvexml2xmltest.c | 9 + tests/cputest.c | 11 +-- tests/domaincapstest.c | 9 + tests/lxcxml2xmltest.c | 9 + tests/networkxml2conftest.c | 9 + tests/networkxml2firewalltest.c | 9 + tests/networkxml2xmltest.c | 9 + tests/networkxml2xmlupdatetest.c | 9 + tests/nodeinfotest.c | 18 ++ tests/nwfilterxml2firewalltest.c | 11 +-- tests/nwfilterxml2xmltest.c | 9 + tests/qemucaps2xmltest.c | 27 -- tests/qemumonitorjsontest.c | 9 ++--- tests/qemuxml2argvtest.c | 13 + tests/qemuxmlnstest.c| 13 + tests/secretxml2xmltest.c| 9 + tests/sexpr2xmltest.c| 9 + tests/storagepoolxml2xmltest.c | 9 + tests/storagevolxml2argvtest.c | 14 +- tests/storagevolxml2xmltest.c| 9 + tests/sysinfotest.c | 10 ++ tests/testutils.c| 33 tests/testutils.h| 2 ++ tests/vircaps2xmltest.c | 10 +- tests/vircgrouptest.c| 9 + tests/vmx2xmltest.c | 9 + tests/xencapstest.c | 9 + tests/xlconfigtest.c | 18 ++ tests/xmconfigtest.c | 18 ++ tests/xml2sexprtest.c| 9 + tests/xml2vmxtest.c | 10 +- 32 files changed, 78 insertions(+), 324 deletions(-) Nice! ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 05/9] conf: Adjust the iothreadsched expectations
With iothreadid's allowing any 'id' value for an iothread_id, the iothreadsched code needs a slight adjustment to allow for any unsigned int value in order to create the bitmap of ids that will have scheduler adjustments. Adjusted the doc description as well. Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatdomain.html.in| 16 src/conf/domain_conf.c | 2 +- .../qemuxml2argv-cputune-iothreadsched-toomuch.xml | 3 ++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7af6bd7..0767a2a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -691,10 +691,18 @@ type (values codebatch/code, codeidle/code, codefifo/code, coderr/code) for particular vCPU/IOThread threads (based on codevcpus/code and codeiothreads/code, leaving out -codevcpus/code/codeiothreads/code sets the default). For -real-time schedulers (codefifo/code, coderr/code), priority must -be specified as well (and is ignored for non-real-time ones). The value -range for the priority depends on the host kernel (usually 1-99). +codevcpus/code/codeiothreads/code sets the default). Valid +codevcpus/code values start at 0 through one less than the +number of vCPU's defined for the domain. Valid codeiothreads/code +values are described in the codeiothreadids/code +a href=#elementsIOThreadsAllocationcodedescription/code/a. +If no codeiothreadids/code are defined, then libvirt numbers +IOThreads from 1 to the number of codeiothreads/code available +for the domain. For real-time schedulers (codefifo/code, +coderr/code), priority must real-time schedulers +(codefifo/code, coderr/code), priority must be specified as +well (and is ignored for non-real-time ones). The value range +for the priority depends on the host kernel (usually 1-99). span class=sinceSince 1.2.13/span /dd diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 083ded7..846a744 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14327,7 +14327,7 @@ virDomainDefParseXML(xmlDocPtr xml, for (i = 0; i def-cputune.niothreadsched; i++) { if (virDomainThreadSchedParse(nodes[i], - 1, def-iothreads, + 1, UINT_MAX, iothreads, def-cputune.iothreadsched[i]) 0) goto error; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml index 1540969..7cae303 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml @@ -13,7 +13,8 @@ vcpupin vcpu='1' cpuset='1'/ emulatorpin cpuset='1'/ vcpusched vcpus='0-1' scheduler='fifo' priority='1'/ -iothreadsched iothreads='2' scheduler='batch'/ +iothreadsched iothreads='1' scheduler='batch'/ +iothreadsched iothreads='2' scheduler='fifo' priority='1'/ /cputune os type arch='i686' machine='pc'hvm/type -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] tests: Add VIR_TEST_DEBUG and VIR_TEST_VERBOSE
On 04/23/2015 02:19 PM, Cole Robinson wrote: To remove a bunch of TestGetDebug()/TestGetVerbose() checks --- tests/cputest.c | 34 - tests/jsontest.c | 48 + tests/nodeinfotest.c | 2 +- tests/qemuargv2xmltest.c | 15 ++-- tests/qemuhelptest.c | 6 +- tests/qemuhotplugtest.c | 19 +++-- tests/qemumonitortest.c | 24 +++ tests/qemuxml2argvtest.c | 14 ++-- tests/qemuxmlnstest.c| 6 +- tests/securityselinuxlabeltest.c | 6 +- tests/statstest.c| 3 +- tests/testutils.c| 2 +- tests/testutils.h| 12 tests/testutilslxc.c | 2 +- tests/testutilsqemu.c| 2 +- tests/utiltest.c | 30 +++- tests/virbuftest.c | 60 +++- tests/virhashtest.c | 152 +-- tests/virpcitest.c | 4 +- tests/virportallocatortest.c | 35 - 20 files changed, 188 insertions(+), 288 deletions(-) rather than make you crawl through all of this to find the comment - testutils.h fails make syntax-check when cppi is installed, due to improper indentation (you need to put a space between # and define). It's also missing an update to the copyright date. Other than that looks fine. Nice cleanup! ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] tests: Add VIR_TEST_REGENERATE_OUTPUT
On 04/23/2015 02:20 PM, Cole Robinson wrote: If this enviroment variable is set, the virTestCompareToFile helper will overwrite the file content we are comparing against, if the file doesn't exist or it doesn't match the expected input. This is useful when adding new test cases, or making changes that generate a lot of output churn. --- HACKING | 7 +++ docs/hacking.html.in | 12 tests/testutils.c| 45 +++-- 3 files changed, 46 insertions(+), 18 deletions(-) diff --git a/HACKING b/HACKING index 94d9d2c..cfc507a 100644 --- a/HACKING +++ b/HACKING @@ -136,6 +136,13 @@ Also, individual tests can be run from inside the tests/ directory, like: ./qemuxml2xmltest +If you are adding new test cases, or making changes that alter existing test +output, you can use the environment variable VIR_TEST_REGENERATE_OUTPUT to +quickly update the saved test data. Of course you still need to review the +changes to ensure they are correct. Maybe say review the changes *VERY CAREFULLY* or something like that. This is incredibly convenient, but could make it easier for someone to gloss over a regression that happens to be introduced with the same patch where they are adding some new functionality. [...] -if (STRNEQ(fixedcontent ? fixedcontent : strcontent, filecontent)) { +if (STRNEQ_NULLABLE(fixedcontent ? fixedcontent : strcontent, +filecontent)) { +if (regenerate) { +if (virFileWriteStr(filename, strcontent, 0666) 0) +goto failure; +goto out; +} virtTestDifference(stderr, strcontent, filecontent); goto failure; Okay, so you still report a failure for this test, but in the process you update the test file. Makes sense - that way you can see which tests are getting updated. This is *really* cool! ACK++ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: qemu: Couple aarch64 CPU tests
- Make sure aarch64 host-passthrough works correctly - Make sure libvirt doesn't choke on cpu model=host, which is what virt-install/virt-manager were incorrectly specifying up until recently. --- .../qemuxml2argv-aarch64-cpu-model-host.args | 1 + .../qemuxml2argv-aarch64-cpu-model-host.xml| 28 ++ .../qemuxml2argv-aarch64-cpu-passthrough.args | 1 + .../qemuxml2argv-aarch64-cpu-passthrough.xml | 27 + tests/qemuxml2argvtest.c | 6 + .../qemuxml2argv-aarch64-cpu-passthrough.xml | 27 + tests/qemuxml2xmltest.c| 2 ++ 7 files changed, 92 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-model-host.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-model-host.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-passthrough.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-passthrough.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2argv-aarch64-cpu-passthrough.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-model-host.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-model-host.args new file mode 100644 index 000..802ea11 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-model-host.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none /usr/bin/qemu-system-aarch64 -S -M virt -cpu host -m 1024 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -boot c -usb -drive file=/aarch64.raw,if=none,id=drive-virtio-disk0 -device virtio-blk-device,drive=drive-virtio-disk0,id=virtio-disk0 \ No newline at end of file diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-model-host.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-model-host.xml new file mode 100644 index 000..53d528f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-model-host.xml @@ -0,0 +1,28 @@ +domain type='kvm' + nameaarch64test/name + uuid496d7ea8-9739-544b-4ebd-ef08be936e8b/uuid + memory unit='KiB'1048576/memory + currentMemory unit='KiB'1048576/currentMemory + vcpu placement='static'1/vcpu + os +type arch='aarch64' machine='virt'hvm/type +boot dev='hd'/ + /os + features +acpi/ +apic/ +pae/ + /features + cpu mode='host-passthrough'/ + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashrestart/on_crash + devices +emulator/usr/bin/qemu-system-aarch64/emulator +disk type='file' device='disk' + source file='/aarch64.raw'/ + target dev='vda' bus='virtio'/ +/disk + /devices +/domain diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-passthrough.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-passthrough.args new file mode 100644 index 000..802ea11 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-passthrough.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none /usr/bin/qemu-system-aarch64 -S -M virt -cpu host -m 1024 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -boot c -usb -drive file=/aarch64.raw,if=none,id=drive-virtio-disk0 -device virtio-blk-device,drive=drive-virtio-disk0,id=virtio-disk0 \ No newline at end of file diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-passthrough.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-passthrough.xml new file mode 100644 index 000..4cdf387 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-cpu-passthrough.xml @@ -0,0 +1,27 @@ +domain type=kvm + nameaarch64test/name + uuid496d7ea8-9739-544b-4ebd-ef08be936e8b/uuid + memory1048576/memory + currentMemory1048576/currentMemory + vcpu placement='static'1/vcpu + os +type arch=aarch64 machine=virthvm/type + /os + features +acpi/ +apic/ +pae/ + /features + cpu mode='host-passthrough'/ + clock offset=utc/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashrestart/on_crash + devices +emulator/usr/bin/qemu-system-aarch64/emulator +disk type='file' device='disk' + source file='/aarch64.raw'/ + target dev='vda' bus='virtio'/ +/disk + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1fe445a..055ceee 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1525,6 +1525,12 @@ mymain(void) DO_TEST(aarch64-virt-default-nic, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_VIRTIO_MMIO); +DO_TEST(aarch64-cpu-passthrough, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, +QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_VIRTIO_MMIO, +QEMU_CAPS_CPU_HOST, QEMU_CAPS_KVM); +DO_TEST(aarch64-cpu-model-host, QEMU_CAPS_DEVICE,
Re: [libvirt] [PATCH 4/4] tests: Add VIR_TEST_REGENERATE_OUTPUT
On 04/23/2015 03:46 PM, Laine Stump wrote: On 04/23/2015 02:20 PM, Cole Robinson wrote: If this enviroment variable is set, the virTestCompareToFile helper will overwrite the file content we are comparing against, if the file doesn't exist or it doesn't match the expected input. This is useful when adding new test cases, or making changes that generate a lot of output churn. --- HACKING | 7 +++ docs/hacking.html.in | 12 tests/testutils.c| 45 +++-- 3 files changed, 46 insertions(+), 18 deletions(-) diff --git a/HACKING b/HACKING index 94d9d2c..cfc507a 100644 --- a/HACKING +++ b/HACKING @@ -136,6 +136,13 @@ Also, individual tests can be run from inside the tests/ directory, like: ./qemuxml2xmltest +If you are adding new test cases, or making changes that alter existing test +output, you can use the environment variable VIR_TEST_REGENERATE_OUTPUT to +quickly update the saved test data. Of course you still need to review the +changes to ensure they are correct. Maybe say review the changes *VERY CAREFULLY* or something like that. This is incredibly convenient, but could make it easier for someone to gloss over a regression that happens to be introduced with the same patch where they are adding some new functionality. [...] -if (STRNEQ(fixedcontent ? fixedcontent : strcontent, filecontent)) { +if (STRNEQ_NULLABLE(fixedcontent ? fixedcontent : strcontent, +filecontent)) { +if (regenerate) { +if (virFileWriteStr(filename, strcontent, 0666) 0) +goto failure; +goto out; +} virtTestDifference(stderr, strcontent, filecontent); goto failure; Okay, so you still report a failure for this test, but in the process you update the test file. Makes sense - that way you can see which tests are getting updated. Actually no failure is reported, see goto out; vs goto failure; However git diff/git status still makes it very obvious what files have been added or changed, so I think that's okay. This is *really* cool! ACK++ Thanks for the review. I pushed the the docs suggestion, and the cppi fix for patch #1 - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] virDomainObjListNew: Use virObjectFreeHashData
There's no point in duplicating virObjectFreeHashData() in a separate function. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ccd3bdf..aad4ec0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1032,13 +1032,6 @@ virDomainDeviceDefCheckUnsupportedMemoryDevice(virDomainDeviceDefPtr dev) } -static void -virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) -{ -virDomainObjPtr obj = payload; -virObjectUnref(obj); -} - virDomainObjListPtr virDomainObjListNew(void) { virDomainObjListPtr doms; @@ -1049,7 +1042,7 @@ virDomainObjListPtr virDomainObjListNew(void) if (!(doms = virObjectLockableNew(virDomainObjListClass))) return NULL; -if (!(doms-objs = virHashCreate(50, virDomainObjListDataFree))) { +if (!(doms-objs = virHashCreate(50, virObjectFreeHashData))) { virObjectUnref(doms); return NULL; } -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] virDomainObjListAddLocked: s/false/NULL/ for @oldDef
It's a pointer after all. We should initialize it to NULL instead of false. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9aad782..ccd3bdf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2490,7 +2490,7 @@ virDomainObjListAddLocked(virDomainObjListPtr doms, char uuidstr[VIR_UUID_STRING_BUFLEN]; if (oldDef) -*oldDef = false; +*oldDef = NULL; virUUIDFormat(def-uuid, uuidstr); -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] virDomainObjListFindByName: Return referenced object
Every domain that grabs a domain object to work over should reference it to make sure it won't disappear meanwhile. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/bhyve/bhyve_driver.c | 3 +- src/conf/domain_conf.c | 1 + src/libxl/libxl_driver.c | 10 ++--- src/lxc/lxc_driver.c | 3 +- src/openvz/openvz_driver.c | 11 +++-- src/parallels/parallels_driver.c | 3 +- src/qemu/qemu_driver.c | 14 ++ src/test/test_driver.c | 93 +--- src/uml/uml_driver.c | 15 +++ 9 files changed, 51 insertions(+), 102 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index edbf1e4..dc76cf7 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -797,8 +797,7 @@ static virDomainPtr bhyveDomainLookupByName(virConnectPtr conn, dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virDomainObjEndAPI(vm); return dom; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 686c614..d03 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1157,6 +1157,7 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms, virDomainObjPtr obj; virObjectLock(doms); obj = virHashSearch(doms-objs, virDomainObjListSearchName, name); +virObjectRef(obj); if (obj) { virObjectLock(obj); if (obj-removing) { diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 6a54c73..393f8bd 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1001,8 +1001,7 @@ libxlDomainLookupByName(virConnectPtr conn, const char *name) dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virDomainObjEndAPI(vm); return dom; } @@ -4955,12 +4954,12 @@ libxlDomainMigrateFinish3Params(virConnectPtr dconn, } if (virDomainMigrateFinish3ParamsEnsureACL(dconn, vm-def) 0) { -virObjectUnlock(vm); +virDomainObjEndAPI(vm); return NULL; } if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0) { -virObjectUnlock(vm); +virDomainObjEndAPI(vm); return NULL; } @@ -4969,8 +4968,7 @@ libxlDomainMigrateFinish3Params(virConnectPtr dconn, if (!libxlDomainObjEndJob(driver, vm)) vm = NULL; -if (vm) -virObjectUnlock(vm); +virDomainObjEndAPI(vm); return ret; } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 38d9bed..188ff80 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -321,8 +321,7 @@ static virDomainPtr lxcDomainLookupByName(virConnectPtr conn, dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virDomainObjEndAPI(vm); return dom; } diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 1bb8973..10d94ff 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -427,8 +427,7 @@ static virDomainPtr openvzDomainLookupByName(virConnectPtr conn, dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virDomainObjEndAPI(vm); return dom; } @@ -1007,6 +1006,7 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla virReportError(VIR_ERR_OPERATION_FAILED, _(Already an OPENVZ VM active with the id '%s'), vmdef-name); +virDomainObjEndAPI(vm); goto cleanup; } if (!(vm = virDomainObjListAdd(driver-domains, vmdef, @@ -1103,6 +1103,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, virReportError(VIR_ERR_OPERATION_FAILED, _(Already an OPENVZ VM defined with the id '%s'), vmdef-name); +virDomainObjEndAPI(vm); goto cleanup; } if (!(vm = virDomainObjListAdd(driver-domains, @@ -1208,8 +1209,7 @@ openvzDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virDomainObjEndAPI(vm); return ret; } @@ -2508,8 +2508,7 @@ openvzDomainMigrateFinish3Params(virConnectPtr dconn, dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virDomainObjEndAPI(vm); return dom; } diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 07f1311..1ddc14e 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -526,8 +526,7 @@ parallelsDomainLookupByName(virConnectPtr conn, const char *name) ret-id = dom-def-id; cleanup: -if (dom) -virObjectUnlock(dom); +virDomainObjEndAPI(dom); return ret; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82f34ec..47ada4b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
[libvirt] [PATCH 0/5] Make virDomainObjListFindByName run in O(1)
As discussed here: https://www.redhat.com/archives/libvir-list/2015-April/msg01135.html Michal Privoznik (5): virDomainObjListAddLocked: s/false/NULL/ for @oldDef virDomainObjListNew: Use virObjectFreeHashData Introduce virDomainObjEndAPI virDomainObjListFindByName: Return referenced object virDomainObjList: Introduce yet another hash table src/bhyve/bhyve_driver.c | 3 +- src/conf/domain_conf.c | 83 +++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 10 ++--- src/lxc/lxc_driver.c | 3 +- src/openvz/openvz_driver.c | 11 +++-- src/parallels/parallels_driver.c | 3 +- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 14 ++ src/test/test_driver.c | 93 +--- src/uml/uml_driver.c | 15 +++ 12 files changed, 110 insertions(+), 135 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] tests: Add VIR_TEST_REGENERATE_OUTPUT
If this enviroment variable is set, the virTestCompareToFile helper will overwrite the file content we are comparing against, if the file doesn't exist or it doesn't match the expected input. This is useful when adding new test cases, or making changes that generate a lot of output churn. --- HACKING | 7 +++ docs/hacking.html.in | 12 tests/testutils.c| 45 +++-- 3 files changed, 46 insertions(+), 18 deletions(-) diff --git a/HACKING b/HACKING index 94d9d2c..cfc507a 100644 --- a/HACKING +++ b/HACKING @@ -136,6 +136,13 @@ Also, individual tests can be run from inside the tests/ directory, like: ./qemuxml2xmltest +If you are adding new test cases, or making changes that alter existing test +output, you can use the environment variable VIR_TEST_REGENERATE_OUTPUT to +quickly update the saved test data. Of course you still need to review the +changes to ensure they are correct. + + VIR_TEST_REGENERATE_OUTPUT=1 ./qemuxml2argvtest + There is also a ./run script at the top level, to make it easier to run programs that have not yet been installed, as well as to wrap invocations of various tests under gdb or Valgrind. diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 53786b7..8f25bd4 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -164,6 +164,18 @@ pre ./qemuxml2xmltest /pre + +p + If you are adding new test cases, or making changes that alter + existing test output, you can use the environment variable + VIR_TEST_REGENERATE_OUTPUT to quickly update the saved test data. + Of course you still need to review the changes to ensure they are + correct. +/p +pre + VIR_TEST_REGENERATE_OUTPUT=1 ./qemuxml2argvtest +/pre + pThere is also a code./run/code script at the top level, to make it easier to run programs that have not yet been installed, as well as to wrap invocations of various tests diff --git a/tests/testutils.c b/tests/testutils.c index 8367b4b..acb2ef1 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -91,6 +91,21 @@ bool virtTestOOMActive(void) return testOOMActive; } +static unsigned int +virTestGetFlag(const char *name) +{ +char *flagStr; +unsigned int flag; + +if ((flagStr = getenv(name)) == NULL) +return 0; + +if (virStrToLong_ui(flagStr, NULL, 10, flag) 0) +return 0; + +return flag; +} + #ifdef TEST_OOM_TRACE static void virTestAllocHook(int nalloc ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) @@ -607,21 +622,30 @@ virtTestCompareToFile(const char *strcontent, int ret = -1; char *filecontent = NULL; char *fixedcontent = NULL; +bool regenerate = !!virTestGetFlag(VIR_TEST_REGENERATE_OUTPUT); -if (virtTestLoadFile(filename, filecontent) 0) +if (virtTestLoadFile(filename, filecontent) 0 !regenerate) goto failure; -if (filecontent[strlen(filecontent) - 1] == '\n' +if (filecontent +filecontent[strlen(filecontent) - 1] == '\n' strcontent[strlen(strcontent) - 1] != '\n') { if (virAsprintf(fixedcontent, %s\n, strcontent) 0) goto failure; } -if (STRNEQ(fixedcontent ? fixedcontent : strcontent, filecontent)) { +if (STRNEQ_NULLABLE(fixedcontent ? fixedcontent : strcontent, +filecontent)) { +if (regenerate) { +if (virFileWriteStr(filename, strcontent, 0666) 0) +goto failure; +goto out; +} virtTestDifference(stderr, strcontent, filecontent); goto failure; } + out: ret = 0; failure: VIR_FREE(fixedcontent); @@ -692,21 +716,6 @@ virtTestLogContentAndReset(void) } -static unsigned int -virTestGetFlag(const char *name) -{ -char *flagStr; -unsigned int flag; - -if ((flagStr = getenv(name)) == NULL) -return 0; - -if (virStrToLong_ui(flagStr, NULL, 10, flag) 0) -return 0; - -return flag; -} - unsigned int virTestGetDebug(void) { -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] qemu: Fix issues with socket/nvram directories
Couple cleanups and bug fixes dealing with qemu directory creation and permissions. Cole Robinson (4): qemu: conf: Clarify paths that are relative to libDir qemu: chown autoDumpPath on driver startup qemu: Build channel autosocket directory at driver startup qemu: Build nvram directory at driver startup libvirt.spec.in | 6 -- src/Makefile.am | 2 -- src/qemu/qemu_conf.c| 28 +++- src/qemu/qemu_conf.h| 2 ++ src/qemu/qemu_domain.c | 9 - src/qemu/qemu_driver.c | 49 + src/qemu/qemu_process.c | 4 ++-- 7 files changed, 76 insertions(+), 24 deletions(-) -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] parallels: implement .domainGetMaxMemory
20.04.2015 14:43, Maxim Nestratov пишет: 19.03.2015 18:39, Dmitry Guryanov пишет: Since we haven't implemented balloon parameters tuning we can just return amount of memory in this function. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 391e927..d13a4e2 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1089,6 +1089,24 @@ parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) return ret; } +static unsigned long long +parallelsDomainGetMaxMemory(virDomainPtr domain) +{ +parallelsConnPtr privconn = domain-conn-privateData; +virDomainObjPtr dom = NULL; +int ret = -1; + +dom = virDomainObjListFindByUUID(privconn-domains, domain-uuid); +if (dom == NULL) { +parallelsDomNotFoundError(domain); +return -1; +} + +ret = dom-def-mem.max_balloon; +virObjectUnlock(dom); +return ret; +} + static virHypervisorDriver parallelsDriver = { .name = Parallels, .connectOpen = parallelsConnectOpen,/* 0.10.0 */ @@ -1133,6 +1151,7 @@ static virHypervisorDriver parallelsDriver = { .domainHasManagedSaveImage = parallelsDomainHasManagedSaveImage, /* 1.2.13 */ .domainManagedSave = parallelsDomainManagedSave, /* 1.2.14 */ .domainManagedSaveRemove = parallelsDomainManagedSaveRemove, /* 1.2.14 */ +.domainGetMaxMemory = parallelsDomainGetMaxMemory, /* 1.2.14 */ }; static virConnectDriver parallelsConnectDriver = { ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list I think it should be reworked with newly introduced parallelsDomObjFromDomain -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 4/9] Move iothreadspin information into iothreadids
On 04/23/2015 08:02 AM, Peter Krempa wrote: On Tue, Apr 21, 2015 at 19:31:25 -0400, John Ferlan wrote: Remove the iothreadspin array from cputune and replace with a cpumask to be stored in the iothreadids list Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatdomain.html.in | 10 ++-- src/conf/domain_conf.c| 118 +- src/conf/domain_conf.h| 2 +- src/qemu/qemu_cgroup.c| 13 ++--- src/qemu/qemu_driver.c| 79 +-- src/qemu/qemu_process.c | 7 +-- 6 files changed, 86 insertions(+), 143 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 518f7c5..7af6bd7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -624,11 +624,11 @@ and attribute codecpuset/code of element codevcpu/code is not specified, the IOThreads are pinned to all the physical CPUs by default. There are two required attributes, the attribute - codeiothread/code specifies the IOThread id and the attribute - codecpuset/code specifying which physical CPUs to pin to. The - codeiothread/code value begins at 1 through the number of - a href=#elementsIOThreadsAllocationcodeiothreads/code/a - allocated to the domain. A value of 0 is not permitted. + codeiothread/code specifies the IOThread ID and the attribute + codecpuset/code specifying which physical CPUs to pin to. See + the codeiothreadids/code + a href=#elementsIOThreadsAllocationcodedescription/code/a + for valid codeiothread/code values. This description is fair enough. It hints that the implicit ones are numbered too. span class=sinceSince 1.2.9/span /dd dtcodeshares/code/dt diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bd25d52..969e56f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2118,8 +2118,10 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin) void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def) { -if (def) +if (def) { +virBitmapFree(def-cpumask); VIR_FREE(def); This fixes the syntax check failure from 1/9. Byproduct of rewrites... I've change 1/9 to be (so that each patch passes check/syntax-check) +if (!def) +return; +VIR_FREE(def); Which will change this to be: if (!def) return; +virBitmapFree(def-cpumask); VIR_FREE(def); +} } @@ -2341,9 +2343,6 @@ void virDomainDefFree(virDomainDefPtr def) virDomainPinDefFree(def-cputune.emulatorpin); -virDomainPinDefArrayFree(def-cputune.iothreadspin, - def-cputune.niothreadspin); - for (i = 0; i def-cputune.nvcpusched; i++) virBitmapFree(def-cputune.vcpusched[i].ids); VIR_FREE(def-cputune.vcpusched); @@ -13316,74 +13315,77 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node, * and an iothreadspin has the form * iothreadpin iothread='1' cpuset='2'/ */ -static virDomainPinDefPtr +static int virDomainIOThreadPinDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, -int iothreads) +virDomainDefPtr def) { -virDomainPinDefPtr def; +int ret = -1; +virDomainIOThreadIDDefPtr iothrid; +virBitmapPtr cpumask; xmlNodePtr oldnode = ctxt-node; unsigned int iothreadid; char *tmp = NULL; -if (VIR_ALLOC(def) 0) -return NULL; - ctxt-node = node; if (!(tmp = virXPathString(string(./@iothread), ctxt))) { virReportError(VIR_ERR_XML_ERROR, %s, _(missing iothread id in iothreadpin)); -goto error; +goto cleanup; } if (virStrToLong_uip(tmp, NULL, 10, iothreadid) 0) { virReportError(VIR_ERR_XML_ERROR, _(invalid setting for iothread '%s'), tmp); -goto error; +goto cleanup; } VIR_FREE(tmp); if (iothreadid == 0) { virReportError(VIR_ERR_XML_ERROR, %s, _(zero is an invalid iothread id value)); -goto error; -} - -/* IOThreads are numbered iothread1...iothreadn, where - * n is the iothreads value */ -if (iothreadid iothreads) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(iothread id must not exceed iothreads)); [1] -goto error; +goto cleanup; } -def-id = iothreadid; - if (!(tmp = virXMLPropString(node, cpuset))) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(missing cpuset for iothreadpin)); -goto error; +goto cleanup; } -if (virBitmapParse(tmp, 0, def-cpumask, VIR_DOMAIN_CPUMASK_LEN)
Re: [libvirt] [PATCH] parallels: implement .domainGetMaxMemory
On 04/23/2015 03:55 PM, Maxim Nestratov wrote: 20.04.2015 14:43, Maxim Nestratov пишет: 19.03.2015 18:39, Dmitry Guryanov пишет: Since we haven't implemented balloon parameters tuning we can just return amount of memory in this function. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 391e927..d13a4e2 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1089,6 +1089,24 @@ parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) return ret; } +static unsigned long long +parallelsDomainGetMaxMemory(virDomainPtr domain) +{ +parallelsConnPtr privconn = domain-conn-privateData; +virDomainObjPtr dom = NULL; +int ret = -1; + +dom = virDomainObjListFindByUUID(privconn-domains, domain-uuid); +if (dom == NULL) { +parallelsDomNotFoundError(domain); +return -1; +} + +ret = dom-def-mem.max_balloon; +virObjectUnlock(dom); +return ret; +} + static virHypervisorDriver parallelsDriver = { .name = Parallels, .connectOpen = parallelsConnectOpen,/* 0.10.0 */ @@ -1133,6 +1151,7 @@ static virHypervisorDriver parallelsDriver = { .domainHasManagedSaveImage = parallelsDomainHasManagedSaveImage, /* 1.2.13 */ .domainManagedSave = parallelsDomainManagedSave, /* 1.2.14 */ .domainManagedSaveRemove = parallelsDomainManagedSaveRemove, /* 1.2.14 */ +.domainGetMaxMemory = parallelsDomainGetMaxMemory, /* 1.2.14 */ }; static virConnectDriver parallelsConnectDriver = { ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list I think it should be reworked with newly introduced parallelsDomObjFromDomain OK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API][PATCH 2/3] Modify the case to cover two new APIs
Test below 2 APIs in this case: networkLookupByUUIDString/networkLookupByUUID --- repos/network/network_uuid.py | 68 --- 1 file changed, 58 insertions(+), 10 deletions(-) diff --git a/repos/network/network_uuid.py b/repos/network/network_uuid.py index 02a104c..8f2ca83 100644 --- a/repos/network/network_uuid.py +++ b/repos/network/network_uuid.py @@ -1,13 +1,18 @@ #!/usr/bin/env python -# To test virsh net-uuid command +#To test virsh net-uuid command and related APIs +#To test 2 APIs in this case: +# networkLookupByUUIDString +# networkLookupByUUID import os import sys import re import commands - +import binascii import libvirt + from libvirt import libvirtError +from xml.dom import minidom from src import sharedmod @@ -15,6 +20,7 @@ required_params = ('networkname',) optional_params = {} VIRSH_NETUUID = virsh net-uuid +NWPATH = /etc/libvirt/qemu/networks/ def check_network_exists(conn, networkname, logger): check if the network exists, may or may not be active @@ -29,25 +35,43 @@ def check_network_exists(conn, networkname, logger): def check_network_uuid(networkname, UUIDString, logger): check UUID String of a network -status, ret = commands.getstatusoutput(VIRSH_NETUUID + ' %s' % networkname) +status, ret = commands.getstatusoutput(VIRSH_NETUUID + ' %s' \ +% networkname) if status: -logger.error(executing + \ + VIRSH_NETUUID + ' %s' % networkname + \ + failed) +logger.error(executing + \ + VIRSH_NETUUID + ' %s' % networkname\ + + \ + failed) logger.error(ret) return False else: UUIDString_virsh = ret[:-1] logger.debug(UUIDString from API is %s % UUIDString) -logger.debug(UUIDString from + \ + VIRSH_NETUUID + \ is %s % UUIDString_virsh) +logger.debug(UUIDString from + \ + VIRSH_NETUUID + \ is %s\ + % UUIDString_virsh) if UUIDString_virsh == UUIDString: return True else: return False +def checking_uuid(logger,nwname,nwuuid): + compare two UUIDs, one is from API, another is from network XML +global NWPATH +NWPATH = NWPATH + nwname + .xml +xml = minidom.parse(NWPATH) +network = xml.getElementsByTagName('network')[0] +uuid = network.getElementsByTagName('uuid')[0].childNodes[0].data +if uuid == nwuuid: +return True +else: + return False -def netuuid(params): - call appropriate API to generate the UUIDStirng -of a network , then compared to the output of command -virsh net-uuid +def network_uuid(params): + 1.call appropriate API to generate the UUIDStirng + of a network , then compared to the output of command + virsh net-uuid +2.check below 2 new APIs: + networkLookupByUUIDString + networkLookupByUUID +global NWPATH logger = params['logger'] networkname = params['networkname'] @@ -61,7 +85,31 @@ def netuuid(params): try: UUIDString = netobj.UUIDString() -logger.info(the UUID string of network %s is %s % (networkname, UUIDString)) + +#For a transient network, set another path +if not netobj.isPersistent() == 1: + NWPATH = /var/run/libvirt/network/ + +logger.info(the UUID string of network \%s\ is \%s\\ + % (networkname, UUIDString)) +#allowing '-' and ' ' anywhere between character pairs, just +#check one of them. +UUIDString1 = UUIDString.replace(-, ) +network1 = conn.networkLookupByUUIDString(UUIDString1) +nw_name1 = network1.name() +logger.debug(The given UUID is \%s\, the network is \%s\ using\ + networkLookupByUUIDString %(UUIDString1,nw_name1)) + +UUIDString2 = UUIDString.replace(-,) +UUID_ascii = binascii.a2b_hex(UUIDString2) +network2 = conn.networkLookupByUUID(UUID_ascii) +nw_name2 = network2.name() +logger.debug(The given UUID is \%s\, the network is \%s\ using \ +networkLookupByUUID %(UUIDString2,nw_name2)) + +if nw_name1 == nw_name2 and checking_uuid(logger,nw_name1,UUIDString): +logger.info(Successed to get network name \%s\ using \%s\\ + %(nw_name1,UUIDString)) if check_network_uuid(networkname, UUIDString, logger): logger.info(VIRSH_NETUUID + test succeeded.) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API][PATCH 1/3] Add two testing cases to test_connection.conf
Below 4 new APIs will be tested networkLookupByUUIDString/networkLookupByUUID storagePoolLookupByUUIDString/storagePoolLookupByUUID --- cases/test_connection.conf | 8 1 file changed, 8 insertions(+) diff --git a/cases/test_connection.conf b/cases/test_connection.conf index 5719937..5317d72 100644 --- a/cases/test_connection.conf +++ b/cases/test_connection.conf @@ -57,3 +57,11 @@ virconn:connection_getDomainCapabilities pc-i440fx-rhel7.0.0 virttype kvm + +network:network_uuid +networkname +default + +storage:pool_uuid +poolname +default -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: lock: unlock vm during qemuBuildCommandLine
On Thu, Apr 23, 2015 at 11:44:49 +0800, zhang bo wrote: The function qemuBuildCommandLine() may take a long time, for example if we configure tens of vifs for the guest, each may cost hundrands of milliseconds to create tap dev, senconds in total. Thus, unlock vm before calling it. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com Signed-off-by: Zhou Yimin zhouyi...@huawei.com --- src/qemu/qemu_process.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 753afe8..d1aaaec 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4628,14 +4628,18 @@ int qemuProcessStart(virConnectPtr conn, } VIR_DEBUG(Building emulator command line); +virObjectUnlock(vm); if (!(cmd = qemuBuildCommandLine(conn, driver, vm-def, priv-monConfig, priv-monJSON, priv-qemuCaps, migrateFrom, stdin_fd, snapshot, vmop, buildCommandLineCallbacks, false, qemuCheckFips(), priv-autoNodeset, - nnicindexes, nicindexes))) + nnicindexes, nicindexes))) { +virObjectLock(vm); goto cleanup; +} +virObjectLock(vm); Why do you need to unlock the object? The VM is starting at this point so you won't be able to save any time since APIs will either be blocked by a job or by the fact that the VM was not started. Would you care to explain what are the benefits of doing this? NACK unless you provide a convincing example where this would help. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: string: Avoid use of deprecated function index()
--- src/util/virstring.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index 1cd4987..e48c29a 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -976,6 +976,21 @@ static const char control_chars[] = \x10\x11\x12\x13\x14\x15\x16\x17 \x18\x19\x1A\x1B\x1C\x1D\x1E\x1F; + +static bool +virStringIsControlChar(char c) +{ +const char *cch; + +for (cch = control_chars; *cch; cch++) { +if (*cch == c) +return true; +} + +return false; +} + + bool virStringHasControlChars(const char *str) { @@ -1003,7 +1018,7 @@ virStringStripControlChars(char *str) len = strlen(str); for (i = 0, j = 0; i len; i++) { -if (index(control_chars, str[i])) +if (virStringIsControlChar(str[i])) continue; str[j++] = str[i]; -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: string: Avoid use of deprecated function index()
On Thu, Apr 23, 2015 at 10:48:06AM +0200, Peter Krempa wrote: --- src/util/virstring.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index 1cd4987..e48c29a 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -976,6 +976,21 @@ static const char control_chars[] = \x10\x11\x12\x13\x14\x15\x16\x17 \x18\x19\x1A\x1B\x1C\x1D\x1E\x1F; + +static bool +virStringIsControlChar(char c) +{ +const char *cch; + +for (cch = control_chars; *cch; cch++) { +if (*cch == c) +return true; +} + +return false; +} + + bool virStringHasControlChars(const char *str) { @@ -1003,7 +1018,7 @@ virStringStripControlChars(char *str) len = strlen(str); for (i = 0, j = 0; i len; i++) { -if (index(control_chars, str[i])) +if (virStringIsControlChar(str[i])) Why not simply s/index/strchr/ which is the recommended POSIX replacement ? 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 0/4] Introduce parallelsDomObjFromDomain()
22.04.2015 19:35, Michal Privoznik пишет: We have such wrapper in other drivers too: qemu, lxc, network, ... Michal Privoznik (4): struct _parallelsConn: Mark @domains as immutable pointer parallels: Introduce parallelsDomObjFromDomain() parallels_driver: Utilize parallelsDomObjFromDomain() parallels_sdk: Utilize parallelsDomObjFromDomain() src/parallels/parallels_driver.c | 93 ++-- src/parallels/parallels_sdk.c| 5 +-- src/parallels/parallels_utils.c | 32 ++ src/parallels/parallels_utils.h | 5 +++ 4 files changed, 51 insertions(+), 84 deletions(-) The whole series looks OK. Also I checked these patches on my working environment - it works OK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API][PATCH 3/3] Modify the case to cover two new APIs
Check two new APIs in this case: storagePoolLookupByUUIDString/storagePoolLookupByUUID --- repos/storage/pool_uuid.py | 49 +- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/repos/storage/pool_uuid.py b/repos/storage/pool_uuid.py index bb6bf63..2f371eb 100644 --- a/repos/storage/pool_uuid.py +++ b/repos/storage/pool_uuid.py @@ -1,4 +1,8 @@ #!/usr/bin/env python +#To test virsh pool-uuid command and related APIs +#To test 2 APIs in this case: +#storagePoolLookupByUUID +#storagePoolLookupByUUIDString import os import sys @@ -6,7 +10,10 @@ import re import time import commands +import binascii import libvirt + +from xml.dom import minidom from libvirt import libvirtError from src import sharedmod @@ -15,6 +22,7 @@ required_params = ('poolname',) optional_params = {} VIRSH_POOLUUID = virsh pool-uuid +POOLPATH = /etc/libvirt/storage/ def check_pool_uuid(poolname, UUIDString, logger): check UUID String of a pool @@ -32,10 +40,25 @@ def check_pool_uuid(poolname, UUIDString, logger): else: return False +def checking_uuid(logger,poolname,pooluuid): +check two uuid of pool which are from API and pool's XML +global POOLPATH +POOLPATH = POOLPATH + poolname + .xml +xml = minidom.parse(POOLPATH) +pool = xml.getElementsByTagName('pool')[0] +uuid = pool.getElementsByTagName('uuid')[0].childNodes[0].data +if uuid == pooluuid: +return True +else: + return False + def pool_uuid(params): - call appropriate API to generate the UUIDStirng + 1. call appropriate API to generate the UUIDStirng of a pool , then compared to the output of command virsh pool-uuid +2. check 2 APIs in the case: + storagePoolLookupByUUID + storagePoolLookupByUUIDString logger = params['logger'] poolname = params['poolname'] @@ -53,6 +76,30 @@ def pool_uuid(params): try: UUIDString = poolobj.UUIDString() logger.info(the UUID string of pool %s is %s % (poolname, UUIDString)) + +#For a transient pool, set another path +if not poolobj.isPersistent() == 1: +logger.info(Can not check a transient pool by now.) +return 0 +#allowing '-' and ' ' anywhere between character pairs,just check +#one of them +UUIDString1 = UUIDString.replace(-, ) +pool1 = conn.storagePoolLookupByUUIDString(UUIDString1) +pool_name1 = pool1.name() +logger.debug(The given UUID is \%s\, the pool is \%s\ using\ + storagePoolLookupByUUIDString %(UUIDString1,pool_name1)) + +UUIDString2 = UUIDString.replace(-,) +UUID_ascii = binascii.a2b_hex(UUIDString2) +pool2 = conn.storagePoolLookupByUUID(UUID_ascii) +pool_name2 = pool2.name() +logger.debug(The given UUID is \%s\, the pool is \%s\ using \ +storagePoolLookupByUUID %(UUIDString2,pool_name2)) + +if pool_name1 == pool_name2 and checking_uuid(logger,pool_name1,UUIDString): +logger.info(Successed to get pool name \%s\ using \%s\\ + %(pool_name1,UUIDString)) + if check_pool_uuid(poolname, UUIDString, logger): logger.info(VIRSH_POOLUUID + test succeeded.) return 0 -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Apparmor: allow reading block-rbd.so
--- examples/apparmor/libvirt-qemu | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 4f0bb1b..c80ece7 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -114,6 +114,7 @@ /usr/bin/qemu-sparc64 rmix, /usr/bin/qemu-x86_64 rmix, /usr/{lib,lib64}/qemu/block-curl.so mr, + /usr/{lib,lib64}/qemu/block-rbd.so mr, # for save and resume /bin/dash rmix, -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Apparmor PCI passthrough fixes
Hi all, These two simple patches intend to fix apparmor profiles for PCI passthrough with the qemu driver. Cédric Bosdonnat (2): Allow access to vendor and device file for PCI device passthrough Apparmor: allow reading block-rbd.so examples/apparmor/libvirt-qemu | 1 + src/util/virpci.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Allow access to vendor and device file for PCI device passthrough
For some devices, the $PCIDIR/vendor and $PCIDIR/device need to be read. Iterate over them to get them as well in the the generated apparmor profile. --- src/util/virpci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index 512e839..cf2a253 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1955,11 +1955,13 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, while ((direrr = virDirRead(dir, ent, pcidir)) 0) { /* Device assignment requires: * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, - * $PCIDIR/rom, $PCIDIR/reset + * $PCIDIR/rom, $PCIDIR/reset, $PCIDIR/vendor, $PCIDIR/device */ if (STREQ(ent-d_name, config) || STRPREFIX(ent-d_name, resource) || STREQ(ent-d_name, rom) || +STREQ(ent-d_name, vendor) || +STREQ(ent-d_name, device) || STREQ(ent-d_name, reset)) { if (virAsprintf(file, %s/%s, pcidir, ent-d_name) 0) goto cleanup; -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] bhyve: fix build in tests
Commit 835cf84 dropped expectedVirtTypes argument for virDomainDefParse*() functions, however bhyve tests still try to pass that to virDomainDefParseFile(), therefore build fails. Fix build by fixing virDomainDefParseFile() usage. --- tests/bhyvexml2argvtest.c | 1 - tests/bhyvexml2xmltest.c | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index c5b0681..26d0075 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -32,7 +32,6 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, -1 VIR_DOMAIN_VIRT_BHYVE, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto out; diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index a1a3701..2e742cf 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -27,7 +27,6 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml) goto fail; if (!(def = virDomainDefParseString(inXmlData, driver.caps, driver.xmlopt, -1 VIR_DOMAIN_VIRT_BHYVE, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto fail; -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't fail to reboot domains with unresponsive agent
On 23.04.2015 05:40, zhang bo wrote: just as what b8e25c35d7f80a2fadc0e51e95318e39db3d1687 did, we fall back to the ACPI method when the guest agent is unresponsive in qemuDomainReboot. Signed-off-by: YueWenyuan yueweny...@huawei.com Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- src/qemu/qemu_driver.c | 67 +- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6fc9696..964a9c5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2002,21 +2002,14 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) virDomainObjPtr vm; int ret = -1; qemuDomainObjPrivatePtr priv; -bool useAgent = false; +bool useAgent = false, agentRequested, acpiRequested; bool isReboot = true; +bool agentForced; int agentFlag = QEMU_AGENT_SHUTDOWN_REBOOT; virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN | VIR_DOMAIN_REBOOT_GUEST_AGENT, -1); -/* At most one of these two flags should be set. */ -if ((flags VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) -(flags VIR_DOMAIN_REBOOT_GUEST_AGENT)) { -virReportInvalidArg(flags, %s, -_(flags for acpi power button and guest agent are mutually exclusive)); -return -1; -} - if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -2028,38 +2021,25 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) } priv = vm-privateData; +agentRequested = flags VIR_DOMAIN_REBOOT_GUEST_AGENT; +acpiRequested = flags VIR_DOMAIN_REBOOT_ACPI_POWER_BTN; if (virDomainRebootEnsureACL(dom-conn, vm-def, flags) 0) goto cleanup; -if ((flags VIR_DOMAIN_REBOOT_GUEST_AGENT) || -(!(flags VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) - priv-agent)) +/* Prefer agent unless we were requested to not to. */ +if (agentRequested || (!flags priv-agent)) useAgent = true; Not that it would harm or anything, but this check can be made just before checkACL() like in shutdown. -if (!useAgent) { -#if WITH_YAJL -if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON)) { -if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(Reboot is not supported with this QEMU binary)); -goto cleanup; -} -} else { -#endif -virReportError(VIR_ERR_OPERATION_INVALID, %s, - _(Reboot is not supported without the JSON monitor)); -goto cleanup; -#if WITH_YAJL -} -#endif -} - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0) goto cleanup; -if (useAgent !qemuDomainAgentAvailable(vm, true)) -goto endjob; +agentForced = agentRequested !acpiRequested; +if (useAgent !qemuDomainAgentAvailable(vm, true)) { Or if (!qemuDomainAgentAvailable(vm, agentForced)) {} +if (agentForced) +goto endjob; +useAgent = false; +} if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, Not to be seen in the context, but this is the correct place to set fakeReboot. @@ -2071,7 +2051,28 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) qemuDomainObjEnterAgent(vm); ret = qemuAgentShutdown(priv-agent, agentFlag); qemuDomainObjExitAgent(vm); -} else { +} + +/* If we are not enforced to use just an agent, try ACPI + * shutdown as well in case agent did not succeed. + */ +if ((!useAgent) || +(ret 0 (acpiRequested || !flags))) { +#if WITH_YAJL +if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON)) { +if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(ACPI reboot is not supported with this QEMU binary)); +goto endjob; +} +} else { +#endif +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(ACPI reboot is not supported without the JSON monitor)); +goto endjob; +#if WITH_YAJL +} +#endif qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSystemPowerdown(priv-mon); if (qemuDomainObjExitMonitor(driver, vm) 0) So, I'm squashing this in: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 509ba94..82f34ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2024,18 +2024,18 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) agentRequested = flags VIR_DOMAIN_REBOOT_GUEST_AGENT; acpiRequested = flags
Re: [libvirt] [PATCH v4 3/9] conf: Move virDomainPinIsDuplicate and make static
On Tue, Apr 21, 2015 at 19:31:24 -0400, John Ferlan wrote: Since it's only ever referenced in domain_conf.c, make the function static, but also will need to move it to somewhere before it's referenced rather than forward referencing it. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 38 +++--- src/conf/domain_conf.h | 4 src/libvirt_private.syms | 1 - 3 files changed, 19 insertions(+), 24 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Apparmor PCI passthrough fixes
On 23.04.2015 09:38, Cédric Bosdonnat wrote: Hi all, These two simple patches intend to fix apparmor profiles for PCI passthrough with the qemu driver. Cédric Bosdonnat (2): Allow access to vendor and device file for PCI device passthrough Apparmor: allow reading block-rbd.so examples/apparmor/libvirt-qemu | 1 + src/util/virpci.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) ACK to both. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC 0/7] Live Migration with Pass-through Devices proposal
On 04/20/2015 06:29 AM, Laine Stump wrote: On 04/17/2015 04:53 AM, Chen Fan wrote: backgrond: Live migration is one of the most important features of virtualization technology. With regard to recent virtualization techniques, performance of network I/O is critical. Current network I/O virtualization (e.g. Para-virtualized I/O, VMDq) has a significant performance gap with native network I/O. Pass-through network devices have near native performance, however, they have thus far prevented live migration. No existing methods solve the problem of live migration with pass-through devices perfectly. There was an idea to solve the problem in website: https://www.kernel.org/doc/ols/2008/ols2008v2-pages-261-267.pdf Please refer to above document for detailed information. This functionality has been on my mind/bug list for a long time, but I haven't been able to pursue it much. See this BZ, along with the original patches submitted by Shradha Shah from SolarFlare: https://bugzilla.redhat.com/show_bug.cgi?id=896716 (I was a bit optimistic in my initial review of the patches - there are actually a lot of issues that weren't handled by those patches.) So I think this problem maybe could be solved by using the combination of existing technology. and the following steps are we considering to implement: - before boot VM, we anticipate to specify two NICs for creating bonding device (one plugged and one virtual NIC) in XML. here we can specify the NIC's mac addresses in XML, which could facilitate qemu-guest-agent to find the network interfaces in guest. An interesting idea, but I think that is a 2nd level enhancement, not necessary initially (and maybe not ever, due to the high possibility of it being extremely difficult to get right in 100% of the cases). - when qemu-guest-agent startup in guest it would send a notification to libvirt, then libvirt will call the previous registered initialize callbacks. so through the callback functions, we can create the bonding device according to the XML configuration. and here we use netcf tool which can facilitate to create bonding device easily. This isn't quite making sense - the bond will be on the guest, which may not have netcf installed. Anyway, I think it should be up to the guest's own system network config to have the bond already setup. If you try to impose it from outside that infrastructure, you run too much risk of running afoul of something on the guest (e.g. NetworkManager) - during migration, unplug the passthroughed NIC. then do native migration. Correct. This is the most important part. But not just unplugging it, you also need to wait until the unplug operation completes (it is asynchronous). (After this point, the emulated NIC that is part of the bond would get all of the traffic). - on destination side, check whether need to hotplug new NIC according to specified XML. usually, we use migrate --xml command option to specify the destination host NIC mac address to hotplug a new NIC, because source side passthrough NIC mac address is different, then hotplug the deivce according to the destination XML configuration. Why does the MAC address need to be different? Are you suggesting doing this with passed-through non-SRIOV NICs? An SRIOV virtual function gets its MAC address from the libvirt config, so it's very simple to use the same MAC address across the migration. Any network card that would be able to do this on any sort of useful scale will be SRIOV-capable (or should be replaced with one that is - some of them are not that expensive). Hi Laine, I think SRIOV virtual NIC to support migration is good idea, but I also think some passthrough NIC without SRIOV-capable. for these NIC devices we only able to use hostdev to specify the passthrough function, so for these NIC I think we should support too. Thanks, Chen TODO: 1. when hot add a new NIC in destination side after migration finished, the NIC device need to re-enslave on bonding device in guest. otherwise, it is offline. maybe we should consider bonding driver to support add interfaces dynamically. I never looked at the details of how SolarFlare's code handled the guest side (they have/had their own patchset they maintained for some older version of libvirt which integrated with some sort of enhanced bonding driver on the guests). I assumed the bond driver could handle this already, but have to say I never investigated. This is an example on how this might work, so I want to hear some voices about this scenario. Thanks, Chen Chen Fan (7): qemu-agent: add agent init callback when detecting guest setup qemu: add guest init event callback to do the initialize work for guest hostdev: add a 'bond' type element in hostdev element Putting this into hostdev is the wrong approach, for two reasons: 1) it doesn't account for the device to be used being in a different address on the source and
[libvirt] [PATCH] migration: Usable time statistics without requiring NTP
virDomainGetJobStats is able to report statistics of a completed migration, however to get usable downtime and total time statistics both hosts have to keep synchronized time. To provide at least some estimation of the times even when NTP daemons are not running on both hosts we can just ignore the time needed to transfer a migration cookie to the destination host. The result will be also inaccurate but a bit more predictable. The total/down time will just be at least what we report. https://bugzilla.redhat.com/show_bug.cgi?id=1213434 Signed-off-by: Jiri Denemark jdene...@redhat.com --- include/libvirt/libvirt-domain.h | 23 ++- src/qemu/qemu_domain.c | 15 +++ src/qemu/qemu_domain.h | 9 + src/qemu/qemu_migration.c| 26 +- tools/virsh-domain.c | 16 5 files changed, 75 insertions(+), 14 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 8a4fe53..5c0a382 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2548,6 +2548,16 @@ int virDomainAbortJob(virDomainPtr dom); # define VIR_DOMAIN_JOB_TIME_ELAPSED time_elapsed /** + * VIR_DOMAIN_JOB_TIME_ELAPSED_NET: + * + * virDomainGetJobStats field: time (ms) since the beginning of the + * migration job NOT including the time required to transfer control + * flow from the source host to the destination host, + * as VIR_TYPED_PARAM_ULLONG. + */ +# define VIR_DOMAIN_JOB_TIME_ELAPSED_NET time_elapsed_net + +/** * VIR_DOMAIN_JOB_TIME_REMAINING: * * virDomainGetJobStats field: remaining time (ms) for VIR_DOMAIN_JOB_BOUNDED @@ -2561,11 +2571,22 @@ int virDomainAbortJob(virDomainPtr dom); * VIR_DOMAIN_JOB_DOWNTIME: * * virDomainGetJobStats field: downtime (ms) that is expected to happen - * during migration, as VIR_TYPED_PARAM_ULLONG. + * during migration, as VIR_TYPED_PARAM_ULLONG. The real computed downtime + * between the time guest CPUs were paused and the time they were resumed + * is reported for completed migration. */ # define VIR_DOMAIN_JOB_DOWNTIME downtime /** + * VIR_DOMAIN_JOB_DOWNTIME_NET: + * + * virDomainGetJobStats field: real measured downtime (ms) NOT including + * the time required to transfer control flow from the source host to the + * destination host, as VIR_TYPED_PARAM_ULLONG. + */ +# define VIR_DOMAIN_JOB_DOWNTIME_NET downtime_net + +/** * VIR_DOMAIN_JOB_SETUP_TIME: * * virDomainGetJobStats field: total time in milliseconds spent preparing diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1368386..ff223a0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -288,6 +288,13 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, jobInfo-timeElapsed) 0) goto error; +if (jobInfo-timeDelta +jobInfo-timeElapsed jobInfo-timeDelta +virTypedParamsAddULLong(par, npar, maxpar, +VIR_DOMAIN_JOB_TIME_ELAPSED_NET, +jobInfo-timeElapsed - jobInfo-timeDelta) 0) +goto error; + if (jobInfo-type == VIR_DOMAIN_JOB_BOUNDED virTypedParamsAddULLong(par, npar, maxpar, VIR_DOMAIN_JOB_TIME_REMAINING, @@ -300,6 +307,14 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, status-downtime) 0) goto error; +if (status-downtime_set +jobInfo-timeDelta +status-downtime jobInfo-timeDelta +virTypedParamsAddULLong(par, npar, maxpar, +VIR_DOMAIN_JOB_DOWNTIME_NET, +status-downtime - jobInfo-timeDelta) 0) +goto error; + if (status-setup_time_set virTypedParamsAddULLong(par, npar, maxpar, VIR_DOMAIN_JOB_SETUP_TIME, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 6bea7c7..1b93f7d 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -100,9 +100,18 @@ struct _qemuDomainJobInfo { virDomainJobType type; unsigned long long started; /* When the async job started */ unsigned long long stopped; /* When the domain's CPUs were stopped */ +unsigned long long sent; /* When the source sent status info to the +destination (only for migrations). */ +unsigned long long received; /* When the destination host received status +info from the source (migrations only). */ /* Computed values */ unsigned long long timeElapsed; unsigned long long timeRemaining; +long long timeDelta; /* delta = sent - received, i.e., the difference +between the source and the destination time plus +the time between the end of Perform
[libvirt] [libvirt-test-API][PATCH 0/3] Modify the existing cases to cover 4 new APIs
Modify the existing cases to cover below 4 new APIs: networkLookupByUUIDString/networkLookupByUUID storagePoolLookupByUUIDString/storagePoolLookupByUUID jiahu (3): Add two testing cases to test_connection.conf Modify the case to cover two new APIs Modify the case to cover two new APIs cases/test_connection.conf| 8 + repos/network/network_uuid.py | 68 --- repos/storage/pool_uuid.py| 49 ++- 3 files changed, 114 insertions(+), 11 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: fix build in tests
On Thu, Apr 23, 2015 at 10:55:12 +0400, Roman Bogorodskiy wrote: Commit 835cf84 dropped expectedVirtTypes argument for virDomainDefParse*() functions, however bhyve tests still try to pass that to virDomainDefParseFile(), therefore build fails. Fix build by fixing virDomainDefParseFile() usage. --- tests/bhyvexml2argvtest.c | 1 - tests/bhyvexml2xmltest.c | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index c5b0681..26d0075 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -32,7 +32,6 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, -1 VIR_DOMAIN_VIRT_BHYVE, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto out; diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index a1a3701..2e742cf 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -27,7 +27,6 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml) goto fail; if (!(def = virDomainDefParseString(inXmlData, driver.caps, driver.xmlopt, -1 VIR_DOMAIN_VIRT_BHYVE, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto fail; ACK, this qualifies to be pushed without review under the build breaker rule. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: fix build in tests
Peter Krempa wrote: On Thu, Apr 23, 2015 at 10:55:12 +0400, Roman Bogorodskiy wrote: Commit 835cf84 dropped expectedVirtTypes argument for virDomainDefParse*() functions, however bhyve tests still try to pass that to virDomainDefParseFile(), therefore build fails. Fix build by fixing virDomainDefParseFile() usage. --- tests/bhyvexml2argvtest.c | 1 - tests/bhyvexml2xmltest.c | 1 - 2 files changed, 2 deletions(-) ACK, this qualifies to be pushed without review under the build breaker rule. Pushed, thanks! Roman Bogorodskiy -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: lock: unlock vm during qemuBuildCommandLine
On 23.04.2015 10:30, Peter Krempa wrote: On Thu, Apr 23, 2015 at 11:44:49 +0800, zhang bo wrote: The function qemuBuildCommandLine() may take a long time, for example if we configure tens of vifs for the guest, each may cost hundrands of milliseconds to create tap dev, senconds in total. Thus, unlock vm before calling it. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com Signed-off-by: Zhou Yimin zhouyi...@huawei.com --- src/qemu/qemu_process.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 753afe8..d1aaaec 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4628,14 +4628,18 @@ int qemuProcessStart(virConnectPtr conn, } VIR_DEBUG(Building emulator command line); +virObjectUnlock(vm); if (!(cmd = qemuBuildCommandLine(conn, driver, vm-def, priv-monConfig, priv-monJSON, priv-qemuCaps, migrateFrom, stdin_fd, snapshot, vmop, buildCommandLineCallbacks, false, qemuCheckFips(), priv-autoNodeset, - nnicindexes, nicindexes))) + nnicindexes, nicindexes))) { +virObjectLock(vm); goto cleanup; +} +virObjectLock(vm); Why do you need to unlock the object? The VM is starting at this point so you won't be able to save any time since APIs will either be blocked by a job or by the fact that the VM was not started. Not true. We still allow QUERY jobs, or APIs that lock a domain object but don't necessarily take a job. For instance, if in one thread I'm doing virDomainGetOSType(), virDomainGetInfo(), ... (essentially 'virsh dominfo') while the other just starts the domain. I don't see a reason why virDomainGetOSType() should wait for the domain to be started up. Domain state should have no affect on the guest OS type, should it? On the other hand, I don't think we can just lock and unlock the domain object as we please. qemuBuildCommandLine is a very complex function and as such it calls many others. Some of them may rely on the fact, that the object is locked by caller. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: lock: unlock vm during qemuBuildCommandLine
On Thu, Apr 23, 2015 at 11:19:34AM +0200, Michal Privoznik wrote: On 23.04.2015 10:30, Peter Krempa wrote: On Thu, Apr 23, 2015 at 11:44:49 +0800, zhang bo wrote: The function qemuBuildCommandLine() may take a long time, for example if we configure tens of vifs for the guest, each may cost hundrands of milliseconds to create tap dev, senconds in total. Thus, unlock vm before calling it. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com Signed-off-by: Zhou Yimin zhouyi...@huawei.com --- src/qemu/qemu_process.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 753afe8..d1aaaec 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4628,14 +4628,18 @@ int qemuProcessStart(virConnectPtr conn, } VIR_DEBUG(Building emulator command line); +virObjectUnlock(vm); if (!(cmd = qemuBuildCommandLine(conn, driver, vm-def, priv-monConfig, priv-monJSON, priv-qemuCaps, migrateFrom, stdin_fd, snapshot, vmop, buildCommandLineCallbacks, false, qemuCheckFips(), priv-autoNodeset, - nnicindexes, nicindexes))) + nnicindexes, nicindexes))) { +virObjectLock(vm); goto cleanup; +} +virObjectLock(vm); Why do you need to unlock the object? The VM is starting at this point so you won't be able to save any time since APIs will either be blocked by a job or by the fact that the VM was not started. Not true. We still allow QUERY jobs, or APIs that lock a domain object but don't necessarily take a job. For instance, if in one thread I'm doing virDomainGetOSType(), virDomainGetInfo(), ... (essentially 'virsh dominfo') while the other just starts the domain. I don't see a reason why virDomainGetOSType() should wait for the domain to be started up. Domain state should have no affect on the guest OS type, should it? On the other hand, I don't think we can just lock and unlock the domain object as we please. qemuBuildCommandLine is a very complex function and as such it calls many others. Some of them may rely on the fact, that the object is locked by caller. Yeah, I'm pretty wary of allowing unlocking of the config while in the build command line function. I think I'd rather than it were refactored so that it was always fast and has no side effects on external system state. ie, we should allocate all the TAP devices upfront in a separate function, and then just pass in the list of TAP device file descriptors to qemuBuildCommandLine so it can generate the CLI args fast. Then we can reliably determine that the code which allocates TAP devices is safe to run unlocked. 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] qemu: lock: unlock vm during qemuBuildCommandLine
On Thu, Apr 23, 2015 at 11:19:34 +0200, Michal Privoznik wrote: On 23.04.2015 10:30, Peter Krempa wrote: On Thu, Apr 23, 2015 at 11:44:49 +0800, zhang bo wrote: The function qemuBuildCommandLine() may take a long time, for example if we configure tens of vifs for the guest, each may cost hundrands of milliseconds to create tap dev, senconds in total. Thus, unlock vm before calling it. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com Signed-off-by: Zhou Yimin zhouyi...@huawei.com --- src/qemu/qemu_process.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 753afe8..d1aaaec 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4628,14 +4628,18 @@ int qemuProcessStart(virConnectPtr conn, } VIR_DEBUG(Building emulator command line); +virObjectUnlock(vm); if (!(cmd = qemuBuildCommandLine(conn, driver, vm-def, priv-monConfig, priv-monJSON, priv-qemuCaps, migrateFrom, stdin_fd, snapshot, vmop, buildCommandLineCallbacks, false, qemuCheckFips(), priv-autoNodeset, - nnicindexes, nicindexes))) + nnicindexes, nicindexes))) { +virObjectLock(vm); goto cleanup; +} +virObjectLock(vm); Why do you need to unlock the object? The VM is starting at this point so you won't be able to save any time since APIs will either be blocked by a job or by the fact that the VM was not started. Not true. We still allow QUERY jobs, or APIs that lock a domain object but don't necessarily take a job. For instance, if in one thread I'm doing virDomainGetOSType(), virDomainGetInfo(), ... (essentially 'virsh dominfo') while the other just starts the domain. I don't see a reason why virDomainGetOSType() should wait for the domain to be started up. Domain state should have no affect on the guest OS type, should it? OK that is technically right. I wanted to point out that most of the stats are available only for online machines or it shouldn't be much of a problem to dealy the delivery. Your example certainly doesn't illustrate why the delay to format the command line should be a problem when using libvirt. Or are you polling virDomainGetOSType every millisecond? I am curious to see why the delay would be a problem. On the other hand, I don't think we can just lock and unlock the domain object as we please. qemuBuildCommandLine is a very complex function and as such it calls many others. Some of them may rely on the fact, that the object is locked by caller. Well, you definitely can't since almost everything in there is accessing vm-def. Locking semantics would be broken right in the line after @vm was unlocked by dereferencing it. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] migration: Usable time statistics without requiring NTP
On 23.04.2015 11:18, Jiri Denemark wrote: virDomainGetJobStats is able to report statistics of a completed migration, however to get usable downtime and total time statistics both hosts have to keep synchronized time. To provide at least some estimation of the times even when NTP daemons are not running on both hosts we can just ignore the time needed to transfer a migration cookie to the destination host. The result will be also inaccurate but a bit more predictable. The total/down time will just be at least what we report. https://bugzilla.redhat.com/show_bug.cgi?id=1213434 Signed-off-by: Jiri Denemark jdene...@redhat.com --- include/libvirt/libvirt-domain.h | 23 ++- src/qemu/qemu_domain.c | 15 +++ src/qemu/qemu_domain.h | 9 + src/qemu/qemu_migration.c| 26 +- tools/virsh-domain.c | 16 5 files changed, 75 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1da687c..4b3143f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3438,18 +3443,9 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, /* Update total times with the values sent by the destination daemon */ if (mig-jobInfo) { qemuDomainObjPrivatePtr priv = vm-privateData; -if (priv-job.completed) { -qemuDomainJobInfoPtr jobInfo = priv-job.completed; -if (mig-jobInfo-status.downtime_set) { -jobInfo-status.downtime = mig-jobInfo-status.downtime; -jobInfo-status.downtime_set = true; -} -if (mig-jobInfo-timeElapsed) -jobInfo-timeElapsed = mig-jobInfo-timeElapsed; -} else { -priv-job.completed = mig-jobInfo; -mig-jobInfo = NULL; -} +VIR_FREE(priv-job.completed); +priv-job.completed = mig-jobInfo; +mig-jobInfo = NULL; } if (flags VIR_MIGRATE_OFFLINE) @@ -4041,6 +4037,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, if (priv-job.completed) { qemuDomainJobInfoUpdateTime(priv-job.completed); qemuDomainJobInfoUpdateDowntime(priv-job.completed); +ignore_value(virTimeMillisNow(priv-job.completed-sent)); So here you mark the time of start of the migration (on the source)... } if (priv-job.current-type == VIR_DOMAIN_JOB_UNBOUNDED) @@ -5164,8 +5161,11 @@ qemuMigrationFinish(virQEMUDriverPtr driver, } if (mig-jobInfo) { -priv-job.completed = mig-jobInfo; +qemuDomainJobInfoPtr jobInfo = mig-jobInfo; +priv-job.completed = jobInfo; mig-jobInfo = NULL; +if (jobInfo-sent virTimeMillisNow(jobInfo-received) == 0) +jobInfo-timeDelta = jobInfo-received - jobInfo-sent; ... and here, once the migration is finished, you compute the time difference. What I am worried about is, what if time on both machines is so off that this value makes no sense (e.g. timeDelta would be a negative number)? Moreover, don't we have it as a migration prerequisite that time on the both machines needs to be synchronized? Or is this meant for much finer granularity and I'm just babbling? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: lock: unlock vm during qemuBuildCommandLine
On 23.04.2015 11:32, Peter Krempa wrote: On Thu, Apr 23, 2015 at 11:19:34 +0200, Michal Privoznik wrote: On 23.04.2015 10:30, Peter Krempa wrote: On Thu, Apr 23, 2015 at 11:44:49 +0800, zhang bo wrote: The function qemuBuildCommandLine() may take a long time, for example if we configure tens of vifs for the guest, each may cost hundrands of milliseconds to create tap dev, senconds in total. Thus, unlock vm before calling it. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com Signed-off-by: Zhou Yimin zhouyi...@huawei.com --- src/qemu/qemu_process.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 753afe8..d1aaaec 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4628,14 +4628,18 @@ int qemuProcessStart(virConnectPtr conn, } VIR_DEBUG(Building emulator command line); +virObjectUnlock(vm); if (!(cmd = qemuBuildCommandLine(conn, driver, vm-def, priv-monConfig, priv-monJSON, priv-qemuCaps, migrateFrom, stdin_fd, snapshot, vmop, buildCommandLineCallbacks, false, qemuCheckFips(), priv-autoNodeset, - nnicindexes, nicindexes))) + nnicindexes, nicindexes))) { +virObjectLock(vm); goto cleanup; +} +virObjectLock(vm); Why do you need to unlock the object? The VM is starting at this point so you won't be able to save any time since APIs will either be blocked by a job or by the fact that the VM was not started. Not true. We still allow QUERY jobs, or APIs that lock a domain object but don't necessarily take a job. For instance, if in one thread I'm doing virDomainGetOSType(), virDomainGetInfo(), ... (essentially 'virsh dominfo') while the other just starts the domain. I don't see a reason why virDomainGetOSType() should wait for the domain to be started up. Domain state should have no affect on the guest OS type, should it? OK that is technically right. I wanted to point out that most of the stats are available only for online machines or it shouldn't be much of a problem to dealy the delivery. Your example certainly doesn't illustrate why the delay to format the command line should be a problem when using libvirt. Or are you polling virDomainGetOSType every millisecond? I am curious to see why the delay would be a problem. Yep, I'm too. So far we don't really care much about libvirt response time (otherwise our critical sections would be much shorter), but maybe it's an issue for somebody. On the other hand, I don't think we can just lock and unlock the domain object as we please. qemuBuildCommandLine is a very complex function and as such it calls many others. Some of them may rely on the fact, that the object is locked by caller. Well, you definitely can't since almost everything in there is accessing vm-def. Locking semantics would be broken right in the line after @vm was unlocked by dereferencing it. Well, anything that would change a domain definition should grap a MODIFY job. But such jobs are serialized, so even if we unlock the domain we should be okay to still access vm-def. What I am more worried about are all the small functions that interact with system or something. Currently they are serialized by @vm lock, but one we remove it ... Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: lock: unlock vm during qemuBuildCommandLine
On 2015/4/23 17:46, Michal Privoznik wrote: On 23.04.2015 11:32, Peter Krempa wrote: On Thu, Apr 23, 2015 at 11:19:34 +0200, Michal Privoznik wrote: On 23.04.2015 10:30, Peter Krempa wrote: On Thu, Apr 23, 2015 at 11:44:49 +0800, zhang bo wrote: The function qemuBuildCommandLine() may take a long time, for example if we configure tens of vifs for the guest, each may cost hundrands of milliseconds to create tap dev, senconds in total. Thus, unlock vm before calling it. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com Signed-off-by: Zhou Yimin zhouyi...@huawei.com --- src/qemu/qemu_process.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 753afe8..d1aaaec 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4628,14 +4628,18 @@ int qemuProcessStart(virConnectPtr conn, } VIR_DEBUG(Building emulator command line); +virObjectUnlock(vm); if (!(cmd = qemuBuildCommandLine(conn, driver, vm-def, priv-monConfig, priv-monJSON, priv-qemuCaps, migrateFrom, stdin_fd, snapshot, vmop, buildCommandLineCallbacks, false, qemuCheckFips(), priv-autoNodeset, - nnicindexes, nicindexes))) + nnicindexes, nicindexes))) { +virObjectLock(vm); goto cleanup; +} +virObjectLock(vm); Why do you need to unlock the object? The VM is starting at this point so you won't be able to save any time since APIs will either be blocked by a job or by the fact that the VM was not started. Not true. We still allow QUERY jobs, or APIs that lock a domain object but don't necessarily take a job. For instance, if in one thread I'm doing virDomainGetOSType(), virDomainGetInfo(), ... (essentially 'virsh dominfo') while the other just starts the domain. I don't see a reason why virDomainGetOSType() should wait for the domain to be started up. Domain state should have no affect on the guest OS type, should it? OK that is technically right. I wanted to point out that most of the stats are available only for online machines or it shouldn't be much of a problem to dealy the delivery. Your example certainly doesn't illustrate why the delay to format the command line should be a problem when using libvirt. Or are you polling virDomainGetOSType every millisecond? I am curious to see why the delay would be a problem. Yep, I'm too. So far we don't really care much about libvirt response time (otherwise our critical sections would be much shorter), but maybe it's an issue for somebody. The specific semantic is: migrating multiple guests simultaneously, downtime of each guest will add up, even to an unacceptable value. 1 suppose the downtime is 2 seconds when we migrate only 1 guest at one time. 2 suppose it costs 0.5sec to create each tapDev, and each guest has 20 vifs, that's 10 seconds for qemuBuildCommandLine. 3 now, we migrate 10 guest simultaneously, the downtime of the guests will vary from seconds to even 100 seconds. The reason for the problem is that: 1 guestA locks vm while creating each tapDev(virNetDevTapCreate) in qemuBuildCommandLine(), for 10seconds 2 guestB calls qemuMigrationPrepareAny-*virDomainObjListAdd* to get its vm object, which locks 'doms' and waits for the vm lock. 3 doms will be locked until guestA unlock its vm, we say that's 10 seconds. 4 guestC calls qemuDomainMigrateFinish3-virDomainObjListFindByName, which tries to lock doms. because it's now locked by guestB, guestC blocks here, and it can't be unpaused for at least 10 seconds. 5 then comes to guestD, guestE, guestF, etc, the downtime will be added up, to even 50 seconds or more. 6 the command 'virsh list' is blocked as well. Thus, we think the problem must be solved. On the other hand, I don't think we can just lock and unlock the domain object as we please. qemuBuildCommandLine is a very complex function and as such it calls many others. Some of them may rely on the fact, that the object is locked by caller. Well, you definitely can't since almost everything in there is accessing vm-def. Locking semantics would be broken right in the line after @vm was unlocked by dereferencing it. Well, anything that would change a domain definition should grap a MODIFY job. But such jobs are serialized, so even if we unlock the domain we should be okay to still access vm-def. What I am more worried about are all the small functions that interact with system or something. Currently they are serialized by @vm lock, but one we remove it ... Michal . After the discussion above, maybe it's better to move virNetDevTapCreate() prior to qemuBuildCommandLine(), what do you think about that?
Re: [libvirt] [PATCH] qemu: lock: unlock vm during qemuBuildCommandLine
On Thu, Apr 23, 2015 at 07:00:21PM +0800, zhang bo wrote: The reason for the problem is that: 1 guestA locks vm while creating each tapDev(virNetDevTapCreate) in qemuBuildCommandLine(), for 10seconds 2 guestB calls qemuMigrationPrepareAny-*virDomainObjListAdd* to get its vm object, which locks 'doms' and waits for the vm lock. 3 doms will be locked until guestA unlock its vm, we say that's 10 seconds. 4 guestC calls qemuDomainMigrateFinish3-virDomainObjListFindByName, which tries to lock doms. because it's Ok, this is the real core problem - FindByName has a bad impl that requires iterating over every single guest. Unfortunately due to the design of the migration API we can't avoid this call, but we could add a second hash table of name - virDomainObj so we make it O(1) and lock-less. now locked by guestB, guestC blocks here, and it can't be unpaused for at least 10 seconds. 5 then comes to guestD, guestE, guestF, etc, the downtime will be added up, to even 50 seconds or more. 6 the command 'virsh list' is blocked as well. Thus, we think the problem must be solved. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 6/9] virstoragefile: Add quorum in virstoragefile
Add VIR_STORAGE_TYPE_QUORUM in virStorageType. Add VIR_STORAGE_FILE_QUORUM in virStorageFileFormat. Add threshold value in _virStorageSource Signed-off-by: Matthias Gatto matthias.ga...@outscale.com --- docs/formatdomain.html.in | 23 --- docs/schemas/domaincommon.rng | 21 - docs/schemas/storagecommon.rng | 1 + docs/schemas/storagevol.rng| 1 + src/conf/domain_conf.c | 2 ++ src/qemu/qemu_command.c| 1 + src/qemu/qemu_driver.c | 4 src/qemu/qemu_migration.c | 1 + src/util/virstoragefile.c | 25 + src/util/virstoragefile.h | 3 +++ 10 files changed, 70 insertions(+), 12 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e921749..7d058ec 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1878,8 +1878,9 @@ dd Valid values are file, block, dir (span class=sincesince 0.7.5/span), -network (span class=sincesince 0.8.7/span), or -volume (span class=sincesince 1.0.5/span) +network (span class=sincesince 0.8.7/span), +volume (span class=sincesince 1.0.5/span), or +quorum (span class=sincesince 1.2.13/span) and refer to the underlying source for the disk. /dd dtcodedevice/code attribute @@ -1940,6 +1941,14 @@ codesnapshot='yes'/code with a transient disk generally does not make sense. /dd + dtcodethreshold/code attribute + span class=sincesince 1.2.13/span/dt +dd +Only use with a quorum disk. +Indicate the minimum of positive vote a quorum must have to validate +a data to be write. The minimum value is 2. The number of backingStores +contain by the quorum must be superior to the threshold. +/dd /dl /dd dtcodesource/code/dt @@ -2010,6 +2019,11 @@ 'file=/dev/disk/by-path/ip-example.com:3260-iscsi-iqn.2013-07.com.example:iscsi-pool-lun-1'). /p /dd +dtcodetype='quorum'/code +span class=sincesince 1.2.13/span/dt + dd + A quorum contain no source element, but a serie of backingStores instead. + /dd /dl With file, block, and volume, one or more optional sub-elements codeseclabel/code, a href=#seclabeldescribed @@ -2149,7 +2163,10 @@ property, but using existing external files for snapshot or block copy operations requires the end user to pre-create the file correctly). The following attributes and sub-elements are -supported in codebackingStore/code: +supported in. +span class=sinceSince 1.2.15/span. This elements is used to +describe a quorum child. +codebackingStore/code: dl dtcodetype/code attribute/dt dd diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 19461f5..6cd834e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1217,9 +1217,15 @@ /interleave /define + define name=diskBackingStoreArray +zeroOrMore + ref name=diskBackingStore/ +/zeroOrMore + /define + define name=diskBackingChain choice - ref name=diskBackingStore/ + ref name=diskBackingStoreArray/ ref name=diskBackingStoreLast/ /choice /define @@ -1261,9 +1267,22 @@ ref name=diskSourceDir/ ref name=diskSourceNetwork/ ref name=diskSourceVolume/ + ref name=diskSourceQuorum/ /choice /define + define name=diskSourceQuorum +optional + attribute name=type +valuequorum/value + /attribute + attribute name=threshold +ref name=unsignedInt/ + /attribute +/optional + /define + + define name=diskSourceFile optional attribute name=type diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index 5f71b10..ba9f485 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -76,6 +76,7 @@ valuefat/value valuevhd/value valueploop/value + valuequorum/value ref name='storageFormatBacking'/ /choice /define diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 7450547..a718576 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -20,6 +20,7 @@ valuedir/value valuenetwork/value valuenetdir/value +valuequorum/value /choice /attribute /optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 09f0bca..a3a6c13 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5928,6 +5928,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
[libvirt] [PATCH v5 8/9] qemu: Add quorum support in qemuBuildDriveDevStr
Allow to libvirt to build the quorum string used by quemu. Add 2 static functions: qemuBuildQuorumStr and qemuBuildAndAppendDriveStrToVirBuffer. qemuBuildQuorumStr is made because a quorum can have another quorum as a child, so we may need to call qemuBuildQuorumStr recursively. qemuBuildQuorumFileSourceStr was basically made to share the code use to build the source between qemuBuildQuorumStr and qemuBuildDriveStr, but there is some difference betwin the syntax use by libvirt to declare a disk and the one qemu need to build a quorum: a quorum need a syntaxe like: domaine_name.children.X.file.filename=filename where libvirt don't use file.filename= but directly file=. Therfore I use this function only for quorum. But as explained in the cover letter and here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html We miss some informations in _virStorageSource to have a complet quorum support in libvirt. Ideally I'd like to refactore virDomainDiskDefFormat to allow qemuBuildQuorumStr to call this function in a loop. Signed-off-by: Matthias Gatto matthias.ga...@outscale.com --- src/qemu/qemu_command.c | 110 1 file changed, 110 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6c40d3e..80cbb7d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3479,6 +3479,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) return -1; } +static bool +qemuBuildQuorumFileSourceStr(virConnectPtr conn, + virStorageSourcePtr src, + virBuffer *opt, + const char *toAppend) +{ +char *source = NULL; +int actualType = virStorageSourceGetActualType(src); + +if (qemuGetDriveSourceString(src, conn, source) 0) +goto error; + +if (source) { + +virBufferAsprintf(opt, ,%sfilename=, toAppend); + +switch (actualType) { +case VIR_STORAGE_TYPE_DIR: +/* QEMU only supports magic FAT format for now */ +if (src-format 0 +src-format != VIR_STORAGE_FILE_FAT) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(unsupported disk driver type for '%s'), + virStorageFileFormatTypeToString(src-format)); +goto error; +} + +if (!src-readonly) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(cannot create virtual FAT disks in read-write mode)); +goto error; +} + +virBufferAddLit(opt, fat:); + +break; + +default: +break; +} +virBufferAsprintf(opt, %s, source); +} + +return true; + error: +return false; +} + + +static bool +qemuBuildQuorumStr(virConnectPtr conn, + virDomainDiskDefPtr disk, + virStorageSourcePtr src, + virBuffer *opt, + const char *toAppend) +{ +char *tmp = NULL; +int ret; +virStorageSourcePtr backingStore; +size_t i; + +if (!src-threshold) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(threshold missing in the quorum configuration)); +return false; +} +if (src-nBackingStores 2) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(a quorum must have at last 2 children)); +return false; +} +if (src-threshold src-nBackingStores) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(threshold must not exceed the number of childrens)); +return false; +} +virBufferAsprintf(opt, ,%svote-threshold=%lu, + toAppend, src-threshold); +for (i = 0; i src-nBackingStores; ++i) { +backingStore = virStorageSourceGetBackingStore(src, i); +ret = virAsprintf(tmp, %schildren.%lu.file., toAppend, i); +if (ret 0) +return false; + +virBufferAsprintf(opt, ,%schildren.%lu.driver=%s, + toAppend, i, + virStorageFileFormatTypeToString(backingStore-format)); + +if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) == false) +goto error; + +/* This operation avoid me to made another copy */ +tmp[ret - sizeof(file)] = '\0'; +if (backingStore-type == VIR_STORAGE_TYPE_QUORUM) { +if (!qemuBuildQuorumStr(conn, disk, backingStore, opt, tmp)) +goto error; +} +VIR_FREE(tmp); +} +return true; + error: +VIR_FREE(tmp); +return false; +} + /* Qemu 1.2 and later have a binary flag -enable-fips that must be * used for VNC auth to obey FIPS settings; but the flag only @@ -3952,6 +4057,11 @@ qemuBuildDriveStr(virConnectPtr conn,
[libvirt] [PATCH v5 2/9] virstoragefile: Always use virStorageSourceGetBackingStore to get backing store
Uniformize backing store usage by calling virStorageSourceGetBackingStore instead of setting backing store manually. Signed-off-by: Matthias Gatto matthias.ga...@outscale.com Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c| 7 --- src/conf/storage_conf.c | 6 +++--- src/qemu/qemu_cgroup.c| 4 ++-- src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_driver.c| 18 ++ src/qemu/qemu_monitor_json.c | 4 +++- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 ++-- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 20 src/storage/storage_backend_fs.c | 31 +-- src/storage/storage_backend_gluster.c | 8 +--- src/storage/storage_backend_logical.c | 10 ++ src/util/virstoragefile.c | 20 ++-- tests/virstoragetest.c| 14 +++--- 15 files changed, 84 insertions(+), 68 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9aad782..2a05291 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17751,7 +17751,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, /* We currently don't output seclabels for backing chain element */ if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) 0 || virDomainDiskBackingStoreFormat(buf, -backingStore-backingStore, + virStorageSourceGetBackingStore(backingStore, 0), backingStore-backingStoreRaw, idx + 1) 0) return -1; @@ -17873,7 +17873,8 @@ virDomainDiskDefFormat(virBufferPtr buf, /* Don't format backingStore to inactive XMLs until the code for * persistent storage of backing chains is ready. */ if (!(flags VIR_DOMAIN_DEF_FORMAT_INACTIVE) -virDomainDiskBackingStoreFormat(buf, def-src-backingStore, +virDomainDiskBackingStoreFormat(buf, + virStorageSourceGetBackingStore(def-src, 0), def-src-backingStoreRaw, 1) 0) return -1; @@ -22044,7 +22045,7 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, } } -for (tmp = disk-src; tmp; tmp = tmp-backingStore) { +for (tmp = disk-src; tmp; tmp = virStorageSourceGetBackingStore(tmp, 0)) { int actualType = virStorageSourceGetActualType(tmp); /* execute the callback only for local storage */ if (actualType != VIR_STORAGE_TYPE_NETWORK diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 4852dfb..5781374 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1326,7 +1326,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (virStorageSize(unit, capacity, ret-target.capacity) 0) goto error; } else if (!(flags VIR_VOL_XML_PARSE_NO_CAPACITY) - !((flags VIR_VOL_XML_PARSE_OPT_CAPACITY) ret-target.backingStore)) { + !((flags VIR_VOL_XML_PARSE_OPT_CAPACITY) virStorageSourceGetBackingStore(ret-target, 0))) { virReportError(VIR_ERR_XML_ERROR, %s, _(missing capacity element)); goto error; } @@ -1635,9 +1635,9 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, def-target, target) 0) goto cleanup; -if (def-target.backingStore +if (virStorageSourceGetBackingStore(def-target, 0) virStorageVolTargetDefFormat(options, buf, - def-target.backingStore, + virStorageSourceGetBackingStore(def-target, 0), backingStore) 0) goto cleanup; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index e83342d..44363c9 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -121,7 +121,7 @@ qemuSetupDiskCgroup(virDomainObjPtr vm, virStorageSourcePtr next; bool forceReadonly = false; -for (next = disk-src; next; next = next-backingStore) { +for (next = disk-src; next; next = virStorageSourceGetBackingStore(next, 0)) { if (qemuSetImageCgroupInternal(vm, next, false, forceReadonly) 0) return -1; @@ -139,7 +139,7 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm, { virStorageSourcePtr next; -for (next = disk-src; next; next = next-backingStore) { +for (next = disk-src; next; next = virStorageSourceGetBackingStore(next, 0)) { if (qemuSetImageCgroup(vm, next, true) 0) return -1; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1368386..5415f04 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c
[libvirt] [PATCH v5 7/9] domain_conf: Read and Write quorum config
Add the capabiltty to libvirt to parse and format the quorum syntax as described here: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html Signed-off-by: Matthias Gatto matthias.ga...@outscale.com --- src/conf/domain_conf.c | 164 +++-- 1 file changed, 119 insertions(+), 45 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a3a6c13..ec93b6f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5952,20 +5952,56 @@ virDomainDiskSourceParse(xmlNodePtr node, } +static bool +virDomainDiskThresholdParse(virStorageSourcePtr src, +xmlNodePtr node) +{ +char *threshold = virXMLPropString(node, threshold); +int ret; + +if (!threshold) { +virReportError(VIR_ERR_XML_ERROR, + %s, _(missing threshold in quorum)); +return false; +} +ret = virStrToLong_ul(threshold, NULL, 10, src-threshold); +if (ret 0 || src-threshold 2) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(unexpected threshold %s), + threshold must be a decimal number superior to 2 + and inferior to the number of children); +VIR_FREE(threshold); +return false; +} +VIR_FREE(threshold); +return true; +} + + static int virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, - virStorageSourcePtr src) + virStorageSourcePtr src, + xmlNodePtr node, + size_t pos) { virStorageSourcePtr backingStore = NULL; xmlNodePtr save_ctxt = ctxt-node; -xmlNodePtr source; +xmlNodePtr source = NULL; char *type = NULL; char *format = NULL; int ret = -1; -if (!(ctxt-node = virXPathNode(./backingStore[*], ctxt))) { -ret = 0; -goto cleanup; +if (src-type == VIR_STORAGE_TYPE_QUORUM) { +if (!node) { +ret = 0; +goto cleanup; +} +ctxt-node = node; +} else { +if (!(ctxt-node = virXPathNode(./backingStore[*], ctxt))) { +ret = 0; +goto cleanup; +} } if (VIR_ALLOC(backingStore) 0) @@ -5997,6 +6033,25 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } +if (backingStore-type == VIR_STORAGE_TYPE_QUORUM) { +xmlNodePtr cur = NULL; + +if (!virDomainDiskThresholdParse(backingStore, node)) +goto cleanup; + +for (cur = node-children; cur != NULL; cur = cur-next) { +if (xmlStrEqual(cur-name, BAD_CAST backingStore)) { +if ((virDomainDiskBackingStoreParse(ctxt, +backingStore, +cur, + backingStore-nBackingStores) 0)) { +goto cleanup; +} +} +} +goto exit; +} + if (!(source = virXPathNode(./source, ctxt))) { virReportError(VIR_ERR_XML_ERROR, %s, _(missing disk backing store source)); @@ -6004,10 +6059,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, } if (virDomainDiskSourceParse(source, ctxt, backingStore) 0 || -virDomainDiskBackingStoreParse(ctxt, backingStore) 0) +virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) 0) goto cleanup; -if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + exit: +if (!virStorageSourceSetBackingStore(src, backingStore, pos)) goto cleanup; ret = 0; @@ -6020,7 +6076,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, return ret; } - #define VENDOR_LEN 8 #define PRODUCT_LEN 16 @@ -6518,6 +6573,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } else if (xmlStrEqual(cur-name, BAD_CAST boot)) { /* boot is parsed as part of virDomainDeviceInfoParseXML */ +} else if (xmlStrEqual(cur-name, BAD_CAST backingStore)) { +if (virDomainDiskBackingStoreParse(ctxt, def-src, cur, + def-src-nBackingStores) 0) +goto error; } } cur = cur-next; @@ -6541,12 +6600,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, def-device = VIR_DOMAIN_DISK_DEVICE_DISK; } +if (def-src-type == VIR_STORAGE_TYPE_QUORUM +!virDomainDiskThresholdParse(def-src, node)) +goto error; + +snapshot = virXMLPropString(node, snapshot); + /* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present. LUN is for raw access CD-ROMs * that are not attached to a physical device
[libvirt] [PATCH v5 3/9] virstoragefile: Add virStorageSourceSetBackingStore
As explained for virStorageSourceGetBackingStore, quorum allows multiple backing store, this make the operation to set bs complex because we have to check if the backingStore is used as an array or a pointer, and set it differently in both case. In order to help the manipulation of backing store, I've added a function virStorageSourceSetBackingStore. For now virStorageSourceSetBackingStore don't handle the case where we have more than one backing store in virStorageSource. Signed-off-by: Matthias Gatto matthias.ga...@outscale.com Signed-off-by: John Ferlan jfer...@redhat.com --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 10 ++ src/util/virstoragefile.h | 3 +++ 3 files changed, 14 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6893054..ee0db4f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2124,6 +2124,7 @@ virStorageSourceParseRBDColonString; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; virStorageSourcePoolModeTypeToString; +virStorageSourceSetBackingStore; virStorageTypeFromString; virStorageTypeToString; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 620f490..d69f49d 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1817,6 +1817,16 @@ virStorageSourceGetBackingStore(const virStorageSource *src, } +bool +virStorageSourceSetBackingStore(virStorageSourcePtr src, +virStorageSourcePtr backingStore, +size_t pos ATTRIBUTE_UNUSED) +{ +src-backingStore = backingStore; +return !!src-backingStore; +} + + /** * virStorageSourcePtr: * diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 4262b36..5f76324 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -289,6 +289,9 @@ struct _virStorageSource { # define DEV_BSIZE 512 # endif +bool virStorageSourceSetBackingStore(virStorageSourcePtr src, +virStorageSourcePtr backingStore, +size_t pos); virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos); -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 0/9] qemu: Add quorum support to libvirt
The purpose of these patches is to introduce quorum for libvirt I've try to follow this proposal: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html This feature ask for 6 task: 1) Allow a _virStorageSource to contain more than one backing store. Because all the actual libvirt code use the backingStore field as a pointer and we needs want to change that, I've decide to encapsulate the backingStore field to simplifie the array manipulation. 2) Add the missing field a quorum need in _virStorageSource and the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in their respectives enums. 3) Parse and format the xml Because a quorum allows to have more than one backing store at the same level we need to change virDomainDiskDefFormat and virDomainDiskDefParseXML to call virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse in a loop. virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse can call themself recursively in a loop because a quorum can contain another quorum 4) Add nodename We need to add nodename support in _virStorageSource because qemu use them for their child. 5) Build qemu string As for the xml, we have to call the function which create quorum recursively. But this task have the problem explained here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html The _virStorageSource missing some informations that can be passed to a child, and therefore this version of quorum is incomplet. 6) Allow to hotplug/change a disk in a quorum This part is not present in these patches because for this task we have to use blockdev-add, and currently libvirt use device_add for hotpluging that doesn't allow to hotplug quorum childs. There is 3 way to handle this problem: 1) create a virDomainBlockDevAdd function in libvirt witch call blockdev-add. 2) use blockdev-add instead of device_add in qemuMonitorJSONAddDevice 3) write a hack which uses blockdev-add when only attaching quorum (but i'm pretty sure this solution is not the good one) V2: -Rebase on master -Add Documentation V3: -Transforme the backingStore field in virStorageSource into an array of pointer instead of a pointer -Modify virStorageSourceSetBackingStore to allow it to expand the backingStore size. V4: -Rebase on master V5: -Rebase on master -patch 1-4/9: use patchs from John Ferlan -patch 4/9: check return of virStorageSourceSetBackingStore -patch 5/9: report type of error on virStorageSourceSetBackingStore Matthias Gatto (9): virstoragefile: Add virStorageSourceGetBackingStore virstoragefile: Always use virStorageSourceGetBackingStore to get backing store virstoragefile: Add virStorageSourceSetBackingStore virstoragefile: Always use virStorageSourceSetBackingStore to set backing store virstoragefile: change backingStore to backingStores. virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name docs/formatdomain.html.in | 30 +- docs/schemas/domaincommon.rng | 26 - docs/schemas/storagecommon.rng| 1 + docs/schemas/storagevol.rng | 1 + src/conf/domain_conf.c| 195 ++ src/conf/storage_conf.c | 23 ++-- src/libvirt_private.syms | 2 + src/qemu/qemu_cgroup.c| 4 +- src/qemu/qemu_command.c | 114 src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_driver.c| 39 --- src/qemu/qemu_migration.c | 1 + src/qemu/qemu_monitor_json.c | 4 +- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 22 ++-- src/storage/storage_backend_fs.c | 38 --- src/storage/storage_backend_gluster.c | 11 +- src/storage/storage_backend_logical.c | 15 ++- src/storage/storage_driver.c | 3 +- src/util/virstoragefile.c | 122 ++--- src/util/virstoragefile.h | 13 ++- tests/virstoragetest.c| 18 ++-- 24 files changed, 551 insertions(+), 141 deletions(-) -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 1/9] virstoragefile: Add virStorageSourceGetBackingStore
Create virStorageSourceGetBackingStore function in preparation for quorum: Actually, if we want to get a backing store inside a virStorageSource we have to do it manually(src-backingStore = backingStore). The problem is that with a quorum, a virStorageSource can contain multiple backing stores, and src-backingStore can be treated as a virStorageSourcePtr or a virStorageSourcePtrPtr. Due to these reason, we need to simplify the manipulation of virStorageSource, and create a function to get the asked backingStore in a virStorageSource For now, this function only return the backingStore field Signed-off-by: Matthias Gatto matthias.ga...@outscale.com --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 8 src/util/virstoragefile.h | 3 +++ 3 files changed, 12 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8c50ea2..6893054 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2114,6 +2114,7 @@ virStorageSourceClear; virStorageSourceCopy; virStorageSourceFree; virStorageSourceGetActualType; +virStorageSourceGetBackingStore; virStorageSourceGetSecurityLabelDef; virStorageSourceInitChainElement; virStorageSourceIsEmpty; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 9507ca1..e6ed573 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1809,6 +1809,14 @@ virStorageSourcePoolDefCopy(const virStorageSourcePoolDef *src) } +virStorageSourcePtr +virStorageSourceGetBackingStore(const virStorageSource *src, +size_t pos ATTRIBUTE_UNUSED) +{ +return src-backingStore; +} + + /** * virStorageSourcePtr: * diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index aa17a00..4262b36 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -289,6 +289,9 @@ struct _virStorageSource { # define DEV_BSIZE 512 # endif +virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource *src, +size_t pos); + int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid); int virStorageFileProbeFormatFromBuf(const char *path, char *buf, -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 4/9] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store
Replace the parts of the code where a backing store is set manually with virStorageSourceSetBackingStore Signed-off-by: Matthias Gatto matthias.ga...@outscale.com Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c| 3 ++- src/conf/storage_conf.c | 17 ++--- src/qemu/qemu_driver.c| 17 +++-- src/storage/storage_backend_fs.c | 7 +-- src/storage/storage_backend_gluster.c | 5 +++-- src/storage/storage_backend_logical.c | 9 ++--- src/storage/storage_driver.c | 3 ++- src/util/virstoragefile.c | 8 +--- tests/virstoragetest.c| 4 ++-- 9 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a05291..09f0bca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6006,7 +6006,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virDomainDiskBackingStoreParse(ctxt, backingStore) 0) goto cleanup; -src-backingStore = backingStore; +if (!virStorageSourceSetBackingStore(src, backingStore, 0)) +goto cleanup; ret = 0; cleanup: diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 5781374..ca3a6d5 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1254,6 +1254,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, char *capacity = NULL; char *unit = NULL; char *backingStore = NULL; +virStorageSourcePtr backingStorePtr; xmlNodePtr node; xmlNodePtr *nodes = NULL; size_t i; @@ -1290,20 +1291,22 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, } if ((backingStore = virXPathString(string(./backingStore/path), ctxt))) { -if (VIR_ALLOC(ret-target.backingStore) 0) +if (VIR_ALLOC(backingStorePtr) 0) goto error; -ret-target.backingStore-path = backingStore; +if (!virStorageSourceSetBackingStore(ret-target, backingStorePtr, 0)) +goto error; +backingStorePtr-path = backingStore; backingStore = NULL; if (options-formatFromString) { char *format = virXPathString(string(./backingStore/format/@type), ctxt); if (format == NULL) -ret-target.backingStore-format = options-defaultFormat; +backingStorePtr-format = options-defaultFormat; else -ret-target.backingStore-format = (options-formatFromString)(format); +backingStorePtr-format = (options-formatFromString)(format); -if (ret-target.backingStore-format 0) { +if (backingStorePtr-format 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(unknown volume format type %s), format); VIR_FREE(format); @@ -1312,9 +1315,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, VIR_FREE(format); } -if (VIR_ALLOC(ret-target.backingStore-perms) 0) +if (VIR_ALLOC(backingStorePtr-perms) 0) goto error; -if (virStorageDefParsePerms(ctxt, ret-target.backingStore-perms, +if (virStorageDefParsePerms(ctxt, backingStorePtr-perms, ./backingStore/permissions, DEFAULT_VOL_PERM_MODE) 0) goto error; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 26be9d6..1a98601 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14274,13 +14274,18 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, /* Update vm in place to match changes. */ need_unlink = false; -newDiskSrc-backingStore = disk-src; -disk-src = newDiskSrc; +if (!virStorageSourceSetBackingStore(newDiskSrc, disk-src, 0)) { +ret = -1; +goto cleanup; +} newDiskSrc = NULL; if (persistDisk) { -persistDiskSrc-backingStore = persistDisk-src; -persistDisk-src = persistDiskSrc; +if (!virStorageSourceSetBackingStore(persistDiskSrc, + persistDisk-src, 0)) { +ret = -1; +goto cleanup; +} persistDiskSrc = NULL; } @@ -14323,13 +14328,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, /* Update vm in place to match changes. */ tmp = disk-src; disk-src = virStorageSourceGetBackingStore(tmp, 0); -tmp-backingStore = NULL; +ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0)); virStorageSourceFree(tmp); if (persistDisk) { tmp = persistDisk-src; persistDisk-src = virStorageSourceGetBackingStore(tmp, 0); -tmp-backingStore = NULL; +ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0)); virStorageSourceFree(tmp); } } diff --git
[libvirt] [PATCH v5 9/9] virstoragefile: Add node-name
Add nodename inside virstoragefile During xml backingStore parsing, look for a nodename attribute in the disk declaration if this one is a quorum, if a nodename is found, add it to the virStorageSource otherwise create a new one with a random name. Take inspiration from this patch to create the nodename: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03209.html Durring xml backingStore formating, look for a nodename attribute inside the virStorageSource struct, and add it to the disk element. Use the nodename to create the quorum in qemuBuildQuorumStr. Signed-off-by: Matthias Gatto matthias.ga...@outscale.com --- docs/formatdomain.html.in | 7 +++ docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c| 27 +++ src/qemu/qemu_command.c | 3 +++ src/util/virstoragefile.c | 4 src/util/virstoragefile.h | 1 + 6 files changed, 47 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7d058ec..d9afe36 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2183,6 +2183,13 @@ codevda[2]/code refers to the backing store with codeindex='2'/code of the disk with codevda/code target. /dd + dtcodenodename/code attribute + span class=sincesince 1.2.13/span/dt + dd +When the backing store is a quorum child, we can use this attribute +to define the node-name of a child. If this atribute is undefine, +a random nodename is generate. + /dd dtcodeformat/code sub-element/dt dd The codeformat/code element contains codetype/code diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6cd834e..30694d9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1236,6 +1236,11 @@ ref name=positiveInteger/ /attribute interleave +optional + attribute name=nodename +text/ + /attribute +/optional ref name=diskSource/ ref name=diskBackingChain/ ref name=diskFormat/ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ec93b6f..46eeae2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -54,6 +54,7 @@ #include network_conf.h #include virtpm.h #include virstring.h +#include virrandom.h #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -5978,6 +5979,8 @@ virDomainDiskThresholdParse(virStorageSourcePtr src, } +#define GEN_NODE_NAME_PREFIXlibvirt +#define GEN_NODE_NAME_MAX_LEN (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8) static int virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virStorageSourcePtr src, @@ -6020,6 +6023,26 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } +if (src-type == VIR_STORAGE_TYPE_QUORUM) { +char *nodeName = NULL; + +if ((nodeName = virXMLPropString(ctxt-node, nodename))) { +backingStore-nodeName = nodeName; +} else { +size_t len; + +if (VIR_ALLOC_N(nodeName, GEN_NODE_NAME_MAX_LEN) 0) +goto cleanup; +if (snprintf(nodeName, GEN_NODE_NAME_MAX_LEN, + %s%08x, GEN_NODE_NAME_PREFIX, (int)pos) 0) +goto cleanup; +for (len = strlen(nodeName); len GEN_NODE_NAME_MAX_LEN - 1; ++len) +nodeName[len] = virRandomInt('Z' - 'A') + 'A'; +nodeName[GEN_NODE_NAME_MAX_LEN - 1] = '\0'; +backingStore-nodeName = nodeName; +} +} + if (!(format = virXPathString(string(./format/@type), ctxt))) { virReportError(VIR_ERR_XML_ERROR, %s, _(missing disk backing store format)); @@ -6075,6 +6098,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, ctxt-node = save_ctxt; return ret; } +#undef GEN_NODE_NAME_PREFIX +#undef GEN_NODE_NAME_MAX_LEN #define VENDOR_LEN 8 #define PRODUCT_LEN 16 @@ -17817,6 +17842,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, type, idx); if (backingStore-threshold) virBufferAsprintf(buf, threshold='%lu', backingStore-threshold); +if (backingStore-nodeName) +virBufferAsprintf(buf, nodename='%s', backingStore-nodeName); virBufferAddLit(buf, \n); virBufferAdjustIndent(buf, 2); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 80cbb7d..ac09750 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3567,6 +3567,9 @@ qemuBuildQuorumStr(virConnectPtr conn, toAppend, i, virStorageFileFormatTypeToString(backingStore-format)); +virBufferAsprintf(opt, ,%schildren.%lu.node-name=%s, + toAppend, i, backingStore-nodeName); +
[libvirt] [PATCH v5 5/9] virstoragefile: change backingStore to backingStores.
The backingStore field was a virStorageSourcePtr. because a quorum can contain more that one backingStore at the same level it's now a 'virStorageSourcePtr *'. This patch rename src-backingStore to src-backingStores, add a static function virStorageSourceExpandBackingStore (virStorageSourcePushBackingStore in the V2) and made the necessary modification to virStorageSourceSetBackingStore and virStorageSourceGetBackingStore. virStorageSourceSetBackingStore can now expand size of src-backingStores by calling virStorageSourceExpandBackingStore if necessary. Signed-off-by: Matthias Gatto matthias.ga...@outscale.com --- src/storage/storage_backend.c| 2 +- src/storage/storage_backend_fs.c | 2 +- src/util/virstoragefile.c| 75 +++- src/util/virstoragefile.h| 3 +- 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 269a93b..b0eb976 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -487,7 +487,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } -if (vol-target.backingStore) { +if (vol-target.backingStores) { virReportError(VIR_ERR_NO_SUPPORT, %s, _(backing storage not supported for raw volumes)); goto cleanup; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bab2569..dd9ccb5 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1048,7 +1048,7 @@ static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } -if (vol-target.backingStore) { +if (vol-target.backingStores) { virReportError(VIR_ERR_NO_SUPPORT, %s, _(backing storage not supported for directories volumes)); return -1; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 234a72b..f0450b5 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1809,21 +1809,72 @@ virStorageSourcePoolDefCopy(const virStorageSourcePoolDef *src) } +/** + * virStorageSourceGetBackingStore: + * @src: virStorageSourcePtr containing the backing stores + * @pos: position of the backing store to get + * + * return the backingStore at the position of @pos + */ virStorageSourcePtr -virStorageSourceGetBackingStore(const virStorageSource *src, -size_t pos ATTRIBUTE_UNUSED) +virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos) { -return src-backingStore; +if (!src || !src-backingStores || pos = src-nBackingStores) +return NULL; +return src-backingStores[pos]; } +/** + * virStorageSourcePushBackingStore: + * + * Expand src-backingStores and update src-nBackingStores + */ +static bool +virStorageSourceExpandBackingStore(virStorageSourcePtr src, size_t nbr) +{ +if (!src) { +virReportError(VIR_ERR_INTERNAL_ERROR, + %s, _(src is NULL)); +return false; +} +if (src-nBackingStores 0) { +if (VIR_EXPAND_N(src-backingStores, src-nBackingStores, nbr) 0) +goto allocation_failed; +} else { +if (VIR_ALLOC_N(src-backingStores, nbr) 0) +goto allocation_failed; +src-nBackingStores += nbr; +} +return true; + allocation_failed: +virReportOOMError(); +return false; +} + + +/** + * virStorageSourceSetBackingStore: + * @src: virStorageSourcePtr to hold @backingStore + * @backingStore: backingStore to store + * @pos: position of the backing store to store + * + * Set @backingStore at @pos in src-backingStores. + * If src-backingStores don't have the space to contain + * @backingStore, we expand src-backingStores + */ bool virStorageSourceSetBackingStore(virStorageSourcePtr src, virStorageSourcePtr backingStore, -size_t pos ATTRIBUTE_UNUSED) +size_t pos) { -src-backingStore = backingStore; -return !!src-backingStore; +if (!src) +return false; +if (pos = src-nBackingStores +!virStorageSourceExpandBackingStore(src, pos - src-nBackingStores + 1)) +return false; +src-backingStores[pos] = backingStore; +return true; } @@ -2038,6 +2089,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src) void virStorageSourceBackingStoreClear(virStorageSourcePtr def) { +size_t i; + if (!def) return; @@ -2045,8 +2098,14 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def-backingStoreRaw); /* recursively free backing chain */ -virStorageSourceFree(virStorageSourceGetBackingStore(def, 0)); -virStorageSourceSetBackingStore(def, NULL, 0); +for (i = 0; i def-nBackingStores; ++i) +virStorageSourceFree(virStorageSourceGetBackingStore(def, i)); +
Re: [libvirt] [PATCH 1/2] parallels: fix crash in prlsdkAddNet in case of CT definition
On 04/22/2015 10:49 PM, Maxim Nestratov wrote: Since net-model is not defined for containers we shouldn't touch it Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c | 27 +++ 1 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 59ca62e..988bcc6 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2728,7 +2728,8 @@ static const char * prlsdkFormatMac(virMacAddrPtr mac, char *macstr) static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, -virDomainNetDefPtr net) +virDomainNetDefPtr net, +bool isCt) { PRL_RESULT pret; PRL_HANDLE sdknet = PRL_INVALID_HANDLE; @@ -2760,19 +2761,21 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, pret = PrlVmDevNet_SetMacAddress(sdknet, macstr); prlsdkCheckRetGoto(pret, cleanup); -if (STREQ(net-model, rtl8139)) { -pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); -} else if (STREQ(net-model, e1000)) { -pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); -} else if (STREQ(net-model, virtio)) { -pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_VIRTIO); -} else { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, +if (!isCt) { +if (STREQ(net-model, rtl8139)) { +pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); +} else if (STREQ(net-model, e1000)) { +pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); +} else if (STREQ(net-model, virtio)) { +pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_VIRTIO); +} else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Specified network adapter model is not supported by Parallels Cloud Server.)); -goto cleanup; +goto cleanup; +} +prlsdkCheckRetGoto(pret, cleanup); } Could you, please, show some warning in case adapter model is specified for a container? -prlsdkCheckRetGoto(pret, cleanup); if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) { if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { @@ -3156,7 +3159,7 @@ prlsdkDoApplyConfig(virConnectPtr conn, } for (i = 0; i def-nnets; i++) { -if (prlsdkAddNet(sdkdom, conn-privateData, def-nets[i]) 0) +if (prlsdkAddNet(sdkdom, conn-privateData, def-nets[i], IS_CT(def)) 0) goto error; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Introduce parallelsDomObjFromDomain()
On 04/22/2015 07:35 PM, Michal Privoznik wrote: We have such wrapper in other drivers too: qemu, lxc, network, ... Michal Privoznik (4): struct _parallelsConn: Mark @domains as immutable pointer parallels: Introduce parallelsDomObjFromDomain() parallels_driver: Utilize parallelsDomObjFromDomain() parallels_sdk: Utilize parallelsDomObjFromDomain() src/parallels/parallels_driver.c | 93 ++-- src/parallels/parallels_sdk.c| 5 +-- src/parallels/parallels_utils.c | 32 ++ src/parallels/parallels_utils.h | 5 +++ 4 files changed, 51 insertions(+), 84 deletions(-) ACK, but somebody has already pushed this series :) -- Dmitry Guryanov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Introduce parallelsDomObjFromDomain()
On 23.04.2015 11:33, Maxim Nestratov wrote: 22.04.2015 19:35, Michal Privoznik пишет: We have such wrapper in other drivers too: qemu, lxc, network, ... Michal Privoznik (4): struct _parallelsConn: Mark @domains as immutable pointer parallels: Introduce parallelsDomObjFromDomain() parallels_driver: Utilize parallelsDomObjFromDomain() parallels_sdk: Utilize parallelsDomObjFromDomain() src/parallels/parallels_driver.c | 93 ++-- src/parallels/parallels_sdk.c| 5 +-- src/parallels/parallels_utils.c | 32 ++ src/parallels/parallels_utils.h | 5 +++ 4 files changed, 51 insertions(+), 84 deletions(-) The whole series looks OK. Also I checked these patches on my working environment - it works OK. Okay, I take it as ACK :) I've pushed these. Thanks! Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] parallels: fix IS_CT macro
On 04/22/2015 10:49 PM, Maxim Nestratov wrote: CT stands for containers, i.e. def-os.type should be compared with VIR_DOMAIN_OSTYPE_EXE rather than VIR_DOMAIN_OSTYPE_HVM Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_utils.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h index 5731381..c9cba0f 100644 --- a/src/parallels/parallels_utils.h +++ b/src/parallels/parallels_utils.h @@ -37,7 +37,7 @@ virReportErrorHelper(VIR_FROM_TEST, VIR_ERR_OPERATION_FAILED, __FILE__, \ __FUNCTION__, __LINE__, _(Can't parse prlctl output)) -# define IS_CT(def) (def-os.type == VIR_DOMAIN_OSTYPE_HVM) +# define IS_CT(def) (def-os.type == VIR_DOMAIN_OSTYPE_EXE) # define parallelsDomNotFoundError(domain) \ do { \ ACKed and pushed -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] parallels: fix crash in prlsdkAddNet in case of CT definition
23.04.2015 15:44, Dmitry Guryanov пишет: On 04/22/2015 10:49 PM, Maxim Nestratov wrote: Since net-model is not defined for containers we shouldn't touch it Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c | 27 +++ 1 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 59ca62e..988bcc6 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2728,7 +2728,8 @@ static const char * prlsdkFormatMac(virMacAddrPtr mac, char *macstr) static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, -virDomainNetDefPtr net) +virDomainNetDefPtr net, +bool isCt) { PRL_RESULT pret; PRL_HANDLE sdknet = PRL_INVALID_HANDLE; @@ -2760,19 +2761,21 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, pret = PrlVmDevNet_SetMacAddress(sdknet, macstr); prlsdkCheckRetGoto(pret, cleanup); -if (STREQ(net-model, rtl8139)) { -pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); -} else if (STREQ(net-model, e1000)) { -pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); -} else if (STREQ(net-model, virtio)) { -pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_VIRTIO); -} else { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, +if (!isCt) { +if (STREQ(net-model, rtl8139)) { +pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); +} else if (STREQ(net-model, e1000)) { +pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); +} else if (STREQ(net-model, virtio)) { +pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_VIRTIO); +} else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Specified network adapter model is not supported by Parallels Cloud Server.)); -goto cleanup; +goto cleanup; +} +prlsdkCheckRetGoto(pret, cleanup); } Could you, please, show some warning in case adapter model is specified for a container? Ok. -prlsdkCheckRetGoto(pret, cleanup); if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) { if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { @@ -3156,7 +3159,7 @@ prlsdkDoApplyConfig(virConnectPtr conn, } for (i = 0; i def-nnets; i++) { -if (prlsdkAddNet(sdkdom, conn-privateData, def-nets[i]) 0) +if (prlsdkAddNet(sdkdom, conn-privateData, def-nets[i], IS_CT(def)) 0) goto error; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] parallels: implement .domainGetMaxMemory
On Thu, Apr 23, 2015 at 04:21:27PM +0300, Dmitry Guryanov wrote: Since we haven't implemented balloon parameters tuning we can just return amount of memory in this function. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c | 15 +++ 1 file changed, 15 insertions(+) ACK 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] migration: Usable time statistics without requiring NTP
On Thu, Apr 23, 2015 at 11:40:11 +0200, Michal Privoznik wrote: On 23.04.2015 11:18, Jiri Denemark wrote: virDomainGetJobStats is able to report statistics of a completed migration, however to get usable downtime and total time statistics both hosts have to keep synchronized time. To provide at least some estimation of the times even when NTP daemons are not running on both hosts we can just ignore the time needed to transfer a migration cookie to the destination host. The result will be also inaccurate but a bit more predictable. The total/down time will just be at least what we report. https://bugzilla.redhat.com/show_bug.cgi?id=1213434 Signed-off-by: Jiri Denemark jdene...@redhat.com --- include/libvirt/libvirt-domain.h | 23 ++- src/qemu/qemu_domain.c | 15 +++ src/qemu/qemu_domain.h | 9 + src/qemu/qemu_migration.c| 26 +- tools/virsh-domain.c | 16 5 files changed, 75 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1da687c..4b3143f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3438,18 +3443,9 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, /* Update total times with the values sent by the destination daemon */ if (mig-jobInfo) { qemuDomainObjPrivatePtr priv = vm-privateData; -if (priv-job.completed) { -qemuDomainJobInfoPtr jobInfo = priv-job.completed; -if (mig-jobInfo-status.downtime_set) { -jobInfo-status.downtime = mig-jobInfo-status.downtime; -jobInfo-status.downtime_set = true; -} -if (mig-jobInfo-timeElapsed) -jobInfo-timeElapsed = mig-jobInfo-timeElapsed; -} else { -priv-job.completed = mig-jobInfo; -mig-jobInfo = NULL; -} +VIR_FREE(priv-job.completed); +priv-job.completed = mig-jobInfo; +mig-jobInfo = NULL; } if (flags VIR_MIGRATE_OFFLINE) @@ -4041,6 +4037,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, if (priv-job.completed) { qemuDomainJobInfoUpdateTime(priv-job.completed); qemuDomainJobInfoUpdateDowntime(priv-job.completed); +ignore_value(virTimeMillisNow(priv-job.completed-sent)); So here you mark the time of start of the migration (on the source)... This is actually the end of migration, i.e., just be for we sent the cookie to the destination. } if (priv-job.current-type == VIR_DOMAIN_JOB_UNBOUNDED) @@ -5164,8 +5161,11 @@ qemuMigrationFinish(virQEMUDriverPtr driver, } if (mig-jobInfo) { -priv-job.completed = mig-jobInfo; +qemuDomainJobInfoPtr jobInfo = mig-jobInfo; +priv-job.completed = jobInfo; mig-jobInfo = NULL; +if (jobInfo-sent virTimeMillisNow(jobInfo-received) == 0) +jobInfo-timeDelta = jobInfo-received - jobInfo-sent; ... and here, once the migration is finished, you compute the time difference. And here, when we get the cookie, we compute a difference between now and the time stored in sent. Which means timeDelta will contain the difference between now on the source and now on the destination plus any time required to transfer and process the cookie. What I am worried about is, what if time on both machines is so off that this value makes no sense (e.g. timeDelta would be a negative number)? timeDelta is signed and can of course be negative (if the destination is behind the source) and there's no problem with this. The only problem would be when the difference is so large it would overflow, but this means the difference would have to be 2^63, which is something like 292 million years, which cannot even be represented by time_t. I don't think we need to worry about this. Moreover, don't we have it as a migration prerequisite that time on the both machines needs to be synchronized? No, there's no such requirement. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] parallels: implement .domainGetMaxMemory
Since we haven't implemented balloon parameters tuning we can just return amount of memory in this function. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c | 15 +++ 1 file changed, 15 insertions(+) Changes in v2: use parallelsDomObjFromDomain function diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 07f1311..c791b5b 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1098,6 +1098,20 @@ static int parallelsDomainAttachDevice(virDomainPtr dom, const char *xml) VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_AFFECT_LIVE); } +static unsigned long long +parallelsDomainGetMaxMemory(virDomainPtr domain) +{ +virDomainObjPtr dom = NULL; +int ret = -1; + +if (!(dom = parallelsDomObjFromDomain(domain))) +return -1; + +ret = dom-def-mem.max_balloon; +virObjectUnlock(dom); +return ret; +} + static virHypervisorDriver parallelsDriver = { .name = Parallels, .connectOpen = parallelsConnectOpen,/* 0.10.0 */ @@ -1144,6 +1158,7 @@ static virHypervisorDriver parallelsDriver = { .domainHasManagedSaveImage = parallelsDomainHasManagedSaveImage, /* 1.2.13 */ .domainManagedSave = parallelsDomainManagedSave, /* 1.2.14 */ .domainManagedSaveRemove = parallelsDomainManagedSaveRemove, /* 1.2.14 */ +.domainGetMaxMemory = parallelsDomainGetMaxMemory, /* 1.2.15 */ }; static virConnectDriver parallelsConnectDriver = { -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 8/9] qemu: Add support to Add/Delete IOThreads
On Tue, Apr 21, 2015 at 19:31:29 -0400, John Ferlan wrote: Add qemuDomainAddIOThread and qemuDomainDelIOThread in order to add or remove an IOThread to/from the host either for live or config optoins The implementation for the 'live' option will use the iothreadpids list in order to make decision, while the 'config' option will use the iothreadids list. Additionally, for deletion each may have to adjust the iothreadpin list. IOThreads are implemented by qmp objects, the code makes use of the existing qemuMonitorAddObject or qemuMonitorDelObject APIs. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_audit.c | 9 ++ src/conf/domain_audit.h | 6 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 401 +++ 4 files changed, 417 insertions(+) ... diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ee13d08..c34abc9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6137,6 +6137,405 @@ qemuDomainPinIOThread(virDomainPtr dom, return ret; } +static int +qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int iothread_id) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +char *alias = NULL; +size_t idx; +int rc = -1; +int ret = -1; +unsigned int orig_niothreads = vm-def-iothreads; +unsigned int exp_niothreads = vm-def-iothreads; +int new_niothreads = 0; +qemuMonitorIOThreadInfoPtr *new_iothreads = NULL; +virCgroupPtr cgroup_iothread = NULL; +char *mem_mask = NULL; +virDomainIOThreadIDDefPtr iothrid; + +if (virDomainIOThreadIDFind(vm-def, iothread_id)) { +virReportError(VIR_ERR_INVALID_ARG, + _(an IOThread is already using iothread_id '%u'), + iothread_id); +goto cleanup; +} + +if (virAsprintf(alias, iothread%u, iothread_id) 0) +return -1; + +qemuDomainObjEnterMonitor(driver, vm); + +rc = qemuMonitorAddObject(priv-mon, iothread, alias, NULL); +exp_niothreads++; +if (rc 0) +goto exit_monitor; + +/* After hotplugging the IOThreads we need to re-detect the + * IOThreads thread_id's, adjust the cgroups, thread affinity, + * and add the thread_id to the vm-def-iothreadids list. + */ +if ((new_niothreads = qemuMonitorGetIOThreads(priv-mon, + new_iothreads)) 0) +goto exit_monitor; + +if (qemuDomainObjExitMonitor(driver, vm) 0) +goto cleanup; + +if (new_niothreads != exp_niothreads) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(got wrong number of IOThread ids from QEMU monitor. + got %d, wanted %d), + new_niothreads, exp_niothreads); +vm-def-iothreads = new_niothreads; +goto cleanup; +} +vm-def-iothreads = exp_niothreads; + +if (virDomainNumatuneGetMode(vm-def-numa, -1) == +VIR_DOMAIN_NUMATUNE_MEM_STRICT +virDomainNumatuneMaybeFormatNodeset(vm-def-numa, +priv-autoNodeset, +mem_mask, -1) 0) +goto cleanup; + + +/* + * If we've successfully added an IOThread, find out where we added it + * in the QEMU IOThread list, so we can add it to our iothreadids list + */ +for (idx = 0; idx new_niothreads; idx++) { +if (STREQ(new_iothreads[idx]-name, alias)) +break; +} + +if (idx == new_niothreads) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(cannot find new IOThread '%u' in QEMU monitor.), + iothread_id); +goto cleanup; +} + +if (!(iothrid = virDomainIOThreadIDAdd(vm-def, iothread_id))) +goto cleanup; + +iothrid-thread_id = new_iothreads[idx]-thread_id; + +/* Add IOThread to cgroup if present */ +if (priv-cgroup) { +cgroup_iothread = +qemuDomainAddCgroupForThread(priv-cgroup, + VIR_CGROUP_THREAD_IOTHREAD, + iothread_id, mem_mask, + iothrid-thread_id); +if (!cgroup_iothread) +goto cleanup; +} + +/* Inherit def-cpuset */ +if (vm-def-cpumask) { +if (!(iothrid-cpumask = virBitmapNewCopy(vm-def-cpumask))) +goto cleanup; Copying the existing default mask is not necessary here. You only have to set it now. It will be set automatically the next time the VM will start. Also you still didn't add the case where automatic numa placement is used. +vm-def-cputune.niothreadspin++; As said, this shouldn't be
Re: [libvirt] [PATCH v4 4/9] Move iothreadspin information into iothreadids
On Thu, Apr 23, 2015 at 08:58:26 -0400, John Ferlan wrote: On 04/23/2015 08:02 AM, Peter Krempa wrote: On Tue, Apr 21, 2015 at 19:31:25 -0400, John Ferlan wrote: Remove the iothreadspin array from cputune and replace with a cpumask to be stored in the iothreadids list Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatdomain.html.in | 10 ++-- src/conf/domain_conf.c| 118 +- src/conf/domain_conf.h| 2 +- src/qemu/qemu_cgroup.c| 13 ++--- src/qemu/qemu_driver.c| 79 +-- src/qemu/qemu_process.c | 7 +-- 6 files changed, 86 insertions(+), 143 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 518f7c5..7af6bd7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -624,11 +624,11 @@ and attribute codecpuset/code of element codevcpu/code is not specified, the IOThreads are pinned to all the physical CPUs by default. There are two required attributes, the attribute - codeiothread/code specifies the IOThread id and the attribute - codecpuset/code specifying which physical CPUs to pin to. The - codeiothread/code value begins at 1 through the number of - a href=#elementsIOThreadsAllocationcodeiothreads/code/a - allocated to the domain. A value of 0 is not permitted. + codeiothread/code specifies the IOThread ID and the attribute + codecpuset/code specifying which physical CPUs to pin to. See + the codeiothreadids/code + a href=#elementsIOThreadsAllocationcodedescription/code/a + for valid codeiothread/code values. This description is fair enough. It hints that the implicit ones are numbered too. span class=sinceSince 1.2.9/span /dd dtcodeshares/code/dt diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bd25d52..969e56f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2118,8 +2118,10 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin) void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def) { -if (def) +if (def) { +virBitmapFree(def-cpumask); VIR_FREE(def); This fixes the syntax check failure from 1/9. Byproduct of rewrites... I've change 1/9 to be (so that each patch passes check/syntax-check) +if (!def) +return; +VIR_FREE(def); Which will change this to be: if (!def) return; +virBitmapFree(def-cpumask); VIR_FREE(def); +} } @@ -2341,9 +2343,6 @@ void virDomainDefFree(virDomainDefPtr def) virDomainPinDefFree(def-cputune.emulatorpin); -virDomainPinDefArrayFree(def-cputune.iothreadspin, - def-cputune.niothreadspin); - for (i = 0; i def-cputune.nvcpusched; i++) virBitmapFree(def-cputune.vcpusched[i].ids); VIR_FREE(def-cputune.vcpusched); @@ -13316,74 +13315,77 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node, * and an iothreadspin has the form * iothreadpin iothread='1' cpuset='2'/ */ -static virDomainPinDefPtr +static int virDomainIOThreadPinDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, -int iothreads) +virDomainDefPtr def) { -virDomainPinDefPtr def; +int ret = -1; +virDomainIOThreadIDDefPtr iothrid; +virBitmapPtr cpumask; xmlNodePtr oldnode = ctxt-node; unsigned int iothreadid; char *tmp = NULL; -if (VIR_ALLOC(def) 0) -return NULL; - ctxt-node = node; if (!(tmp = virXPathString(string(./@iothread), ctxt))) { virReportError(VIR_ERR_XML_ERROR, %s, _(missing iothread id in iothreadpin)); -goto error; +goto cleanup; } if (virStrToLong_uip(tmp, NULL, 10, iothreadid) 0) { virReportError(VIR_ERR_XML_ERROR, _(invalid setting for iothread '%s'), tmp); -goto error; +goto cleanup; } VIR_FREE(tmp); if (iothreadid == 0) { virReportError(VIR_ERR_XML_ERROR, %s, _(zero is an invalid iothread id value)); -goto error; -} - -/* IOThreads are numbered iothread1...iothreadn, where - * n is the iothreads value */ -if (iothreadid iothreads) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(iothread id must not exceed iothreads)); [1] -goto error; +goto cleanup; } -def-id = iothreadid; - if (!(tmp = virXMLPropString(node, cpuset))) {
Re: [libvirt] [PATCH v4 1/9] conf: Add new domain XML element 'iothreadids'
On Tue, Apr 21, 2015 at 19:31:22 -0400, John Ferlan wrote: Adding a new XML element 'iothreadids' in order to allow defining specific IOThread ID's rather than relying on the algorithm to assign IOThread ID's starting at 1 and incrementing to iothreads count. This will allow future patches to be able to add new IOThreads by a specific iothread_id and of course delete any exisiting IOThread. Each iothreadids element will have 'n' iothread children elements which will have attribute id. The id will allow for definition of any valid (eg 0) iothread_id value. On input, if any iothreadids iothread's are provided, they will be marked so that we only print out what we read in. On input, if no iothreadids are provided, the PostParse code will self generate a list of ID's starting at 1 and going to the number of iothreads defined for the domain (just like the current algorithm numbering scheme). A future patch will rework the existing algorithm to make use of the iothreadids list. On output, only print out the iothreadids if they were read in. Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatdomain.html.in | 30 +++ docs/schemas/domaincommon.rng | 12 +++ src/conf/domain_conf.c| 200 +- src/conf/domain_conf.h| 18 src/libvirt_private.syms | 4 + 5 files changed, 262 insertions(+), 2 deletions(-) ... diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 479b4c2..da1bb7e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2114,6 +2114,31 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin) return NULL; } + +void +virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def) +{ +if (def) +VIR_FREE(def); This breaks syntax check. +} + + +static void +virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def, +int nids) +{ +size_t i; + +if (!def) +return; + +for (i = 0; i nids; i++) +virDomainIOThreadIDDefFree(def[i]); + +VIR_FREE(def); +} + + void virDomainPinDefFree(virDomainPinDefPtr def) { @@ -3298,6 +3325,21 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; } +/* Fully populate the IOThread ID list */ +if (def-iothreads def-iothreads != def-niothreadids) { +unsigned int iothread_id = 1; +while (def-niothreadids != def-iothreads) { +if (!virDomainIOThreadIDFind(def, iothread_id)) { +virDomainIOThreadIDDefPtr iothrid; + +if (!(iothrid = virDomainIOThreadIDAdd(def, iothread_id))) +return -1; Unfortunately, fixing the iothread list after you parse iothread pinning in patch 4 makes a loophole where you might force arbitrary iothread ID without using iothreadids. This code will probably need to be moved after the parsing code, despite the fact that the postparse callback is better place to do such checks. +iothrid-autofill = true; +} +iothread_id++; +} +} + if (virDomainDefGetMemoryInitial(def) == 0) { virReportError(VIR_ERR_XML_ERROR, %s, _(Memory size must be specified via memory or in the The rest of this patch looks good, but I'd like to see the above part fixed before my final ACK. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list