Re: [libvirt] [PATCH v3 2/6] qemuProcessHandleBlockJob: Take status into account
On Fri, Feb 13, 2015 at 16:24:29 +0100, Michal Privoznik wrote: Upon BLOCK_JOB_COMPLETED event delivery, we check if the job has completed (in qemuMonitorJSONHandleBlockJobImpl()). For better image, the event looks something like this: timestamp: {seconds: 1423582694, microseconds: 372666}, event: BLOCK_JOB_COMPLETED, data: {device: drive-virtio-disk0, len: 8412790784, offset: 409993216, speed: 8796093022207, type: mirror, error: No space left on device}} If len does not equal offset it's considered an error, and we can clearly see error field filled in. However, later in the event processing this case was handled no differently to case of job being aborted via separate API. It's time that we start differentiate these two because of the future work. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_process.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f30acbb..9cfc0f3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1103,7 +1103,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, case VIR_DOMAIN_BLOCK_JOB_CANCELED: virStorageSourceFree(disk-mirror); disk-mirror = NULL; -disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; +disk-mirrorState = status == VIR_DOMAIN_BLOCK_JOB_FAILED ? +VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : VIR_DOMAIN_DISK_MIRROR_STATE_NONE; disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; save = true; break; ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Getting Introduced to the Community
Hi, My name is Gaurav Bansal and I would like to introduce myself to the developers of this community. I'm (technically) a junior in the Computer Science degree program at Birla Institute of Technology and Science Pilani. I am well versed with Python, C/C++, Java, PHP and bash. I have been using the qemu tool and the virt-manager for almost 6 months now and I'm comfortable with it. I was very much fascinated with the concept of Virtualization and wanted to explore it more and what could be a better way to start off than KVM. I was having a look at the GSoC 2015 ideas http://qemu-project.org/Google_Summer_of_Code_2015 page and the particular project of Running docker containers using virt-sandbox http://qemu-project.org/Google_Summer_of_Code_2015#Running_docker_containers_using_virt-sandbox. I have some basic information about the docker images and have worked using the libvirt API. I have also tried the proof of concept uploaded by Daniel for downloading the docker images. Could you please guide me through where should I look forward that would be helpful for the project? - Gaurav Bansal Github : https://github.com/gauravb7090 IRC : gauravb7090 LinkedIn http://in.linkedin.com/in/gauravb7090/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemuBuildMemoryBackendStr: Report backend requirement more appropriately
On Thu, Feb 12, 2015 at 18:03:53 +0100, Michal Privoznik wrote: So, when building the '-numa' command line, the qemuBuildMemoryBackendStr() function does quite a lot of checks to chose the best backend, or to check if one is in fact needed. However, it returned that backend is needed even for this little fella: numatune memory mode=strict nodeset=0,2 / /numatune This can be guaranteed via CGroups entirely, there's no need to use memory-backend-ram to let qemu know where to get memory from. Well, as long as there's no memnode/ element, which explicitly requires the backend. Long story short, we wouldn't have to care, as qemu works either way. However, the problem is migration (as always). Previously, libvirt would have started qemu with: -numa node,memory=X in this case and restricted memory placement in CGroups. Today, libvirt creates more complicated command line: -object memory-backend-ram,id=ram-node0,size=X -numa node,memdev=ram-node0 Again, one wouldn't find anything wrong with these two approaches. Both work just fine. Unless you try to migrated from the older libvirt s/migrated/migrate/ into the newer one. These two approaches are, unfortunately, not compatible. My suggestion is, in order to allow users to migrate, lets use the older approach for as long as the newer one is not needed. Signed-off-by: Michal Privoznik mpriv...@redhat.com ... diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml index 01bbb3d..fe7c465 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml @@ -1,4 +1,4 @@ -domain type='kvm' +domain type='qemu' namedummy2/name uuid4d92ec27-9ebf-400b-ae91-20c71c647c19/uuid memory unit='KiB'131072/memory Looks like a useless change. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/6] qemuDomainObjPrivate: Introduce blockJob condition
On Fri, Feb 13, 2015 at 16:24:31 +0100, Michal Privoznik wrote: So far the condition is not used, but will be. There are some operations, where we actively wait for a block job to complete. Instead of locking and unlocking the domain object, entering and leaving monitor, lets just use a condition and let our monitor event handling code wake up when needed. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_domain.c | 4 +++- src/qemu/qemu_domain.h | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 99c46d4..28961d2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -406,7 +406,8 @@ qemuDomainObjPrivateAlloc(void) goto error; } -if (virCondInit(priv-unplugFinished) 0) +if (virCondInit(priv-unplugFinished) 0 || +virCondInit(priv-blockJob) 0) goto error; if (!(priv-devs = virChrdevAlloc())) @@ -439,6 +440,7 @@ qemuDomainObjPrivateFree(void *data) VIR_FREE(priv-origname); virCondDestroy(priv-unplugFinished); +virCondDestroy(priv-blockJob); virChrdevFree(priv-devs); /* This should never be non-NULL if we get here, but just in case... */ diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b2c3881..db9ffac 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -186,6 +186,8 @@ struct _qemuDomainObjPrivate { const char *unpluggingDevice; /* alias of the device that is being unplugged */ char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */ +virCond blockJob; /* signals that one of disks translated state of a block job */ Wouldn't signals whenever a block job changes its state or something similar be more readable? + bool hookRun; /* true if there was a hook run over this domain */ virBitmapPtr autoNodeset; }; Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] docs: add page about virtlockd setup
On 02/16/2015 04:59 AM, Daniel P. Berrange wrote: Introduce some basic docs describing the virtlockd setup. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- docs/locking-lockd.html.in | 160 + docs/locking.html.in | 2 +- docs/sitemap.html.in | 4 ++ 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 docs/locking-lockd.html.in +p + The virtlockd daemon is a single purpose binary which + focus exclusively on the task of acquiring and holding s/focus/focuses/ + locks on behalf of running virtual machines. It is + designed to offer a low overhead, portable locking + scheme can be used out of the box on virtualization + hosts with minimal configuration overheads. It makes + use of the POSIX fcntl advisory locking capability + to hold locks, which is supported by the majority of + commonly used filesystems s/$/./ +/p + +h2a name=sanlockvirtlockd daemon setup/a/h2 + +p + In most OS, the virtlockd daemon itself will not require + any upfront configuration work. It is installed by default + when libvirtd is present, and a systemd socket unit is + registered such that the daemon will be automatically + started when first required. With OS that predate systemd + though, it will be neccessary to start it at boot time, s/neccessary/necessary/ +p + The default behaviour of the lockd plugin is to acquire locks + directly on the virtual disk images associated with the guest + lt;diskgt; elements. This ensures it can run out of the box + with not configuration, providing locking for disk images on s/not/no/ -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] numatune: Prefer old approach
On Thu, Feb 12, 2015 at 18:03:50 +0100, Michal Privoznik wrote: Consider the following part of domain XML: numatune memory mode='static' nodeset=0,2/ /numatune cpu numa cell id='0' cpus='0' memory='65536' unit='KiB'/ /numa /cpu Yes, this have a great potential of breaking things. Especially, this will break migration between previous two or three upstream releases and current release we are working on, because libvirt started domains in more complicated way (even if not needed). After these patches, domains will be started in simpler way which is incompatible. On the other hand, we get backward compatibility with much more releases than we are about to break. Michal Privoznik (3): numatune_conf: Expose virDomainNumatuneNodeSpecified qemuxml2argvtest: Fake response from numad qemuBuildMemoryBackendStr: Report backend requirement more appropriately ACK, but see the two nits I found in 3/3. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/6] qemuProcessHandleBlockJob: Set disk-mirrorState more often
On Fri, Feb 13, 2015 at 16:24:28 +0100, Michal Privoznik wrote: Currently, upon BLOCK_JOB_* event, disk-mirrorState is not updated each time. The callback code handling the events checks if a blockjob was started via our public APIs prior to setting the mirrorState. However, some block jobs may be started internally (e.g. during storage migration), in which case we don't bother with setting disk-mirror (there's nothing we can set it to anyway), or other fields. But it will come handy if we update the mirrorState in these cases too. The event wasn't delivered just for fun - we've started the job after all. So, in this commit, the mirrorState is set to whatever job status we've obtained. Of course, there are some actions on some statuses that we want to perform. But instead of if {} else if {} else {} ... enumeration, let's move to switch(). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_process.c | 35 --- 1 file changed, 20 insertions(+), 15 deletions(-) ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 22/24] conf: numa: Add accessor to NUMA node's memory access mode
--- src/conf/numa_conf.c | 10 +- src/conf/numa_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 5 +++-- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index acda415..4906687 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -769,7 +769,7 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, virBufferAddLit(buf, numa\n); virBufferAdjustIndent(buf, 2); for (i = 0; i ncells; i++) { -memAccess = def-cells[i].memAccess; +memAccess = virDomainNumaGetNodeMemoryAccessMode(def, i); if (!(cpustr = virBitmapFormat(virDomainNumaGetNodeCpumask(def, i return -1; @@ -832,3 +832,11 @@ virDomainNumaGetNodeCpumask(virCPUDefPtr numa, { return numa-cells[node].cpumask; } + + +virNumaMemAccess +virDomainNumaGetNodeMemoryAccessMode(virCPUDefPtr numa, + size_t node) +{ +return numa-cells[node].memAccess; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index ab88ab7..0ea2c93 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -89,6 +89,8 @@ int virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr numatune, size_t virDomainNumaGetNodeCount(virCPUDefPtr numa); virBitmapPtr virDomainNumaGetNodeCpumask(virCPUDefPtr numa, size_t node); +virNumaMemAccess virDomainNumaGetNodeMemoryAccessMode(virCPUDefPtr numa, + size_t node); /* * Formatters diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 770803b..1f1ce14 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -631,6 +631,7 @@ virDomainNumaEquals; virDomainNumaFree; virDomainNumaGetNodeCount; virDomainNumaGetNodeCpumask; +virDomainNumaGetNodeMemoryAccessMode; virDomainNumaNew; virDomainNumatuneFormatNodeset; virDomainNumatuneFormatXML; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d276da3..05545ee 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4558,7 +4558,8 @@ qemuBuildMemoryBackendStr(unsigned long long size, virDomainHugePagePtr hugepage = NULL; virDomainNumatuneMemMode mode; const long system_page_size = virGetSystemPageSizeKB(); -virNumaMemAccess memAccess = def-cpu-cells[guestNode].memAccess; +virNumaMemAccess memAccess = virDomainNumaGetNodeMemoryAccessMode(def-cpu, guestNode); + size_t i; char *mem_path = NULL; virBitmapPtr nodemask = NULL; @@ -7185,7 +7186,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, if (rc == 0) needBackend = true; } else { -if (def-cpu-cells[i].memAccess) { +if (virDomainNumaGetNodeMemoryAccessMode(def-cpu, i)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Shared memory mapping is not supported with this QEMU)); -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 21/24] conf: numa: Add accesor for the NUMA node cpu mask
Add virDomainNumaGetNodeCpumask() and refactor a few places that would get the cpu mask without the helper. --- src/conf/numa_conf.c | 12 ++-- src/conf/numa_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 2 +- 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 61dfea0..acda415 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -771,7 +771,7 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, for (i = 0; i ncells; i++) { memAccess = def-cells[i].memAccess; -if (!(cpustr = virBitmapFormat(def-cells[i].cpumask))) +if (!(cpustr = virBitmapFormat(virDomainNumaGetNodeCpumask(def, i return -1; virBufferAddLit(buf, cell); @@ -799,7 +799,7 @@ virDomainNumaGetCPUCountTotal(virCPUDefPtr numa) unsigned int ret = 0; for (i = 0; i numa-ncells; i++) -ret += virBitmapCountBits(numa-cells[i].cpumask); +ret += virBitmapCountBits(virDomainNumaGetNodeCpumask(numa, i)); return ret; } @@ -824,3 +824,11 @@ virDomainNumaGetNodeCount(virCPUDefPtr numa) return numa-ncells; } + + +virBitmapPtr +virDomainNumaGetNodeCpumask(virCPUDefPtr numa, +size_t node) +{ +return numa-cells[node].cpumask; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 55a9fbe..ab88ab7 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -87,6 +87,8 @@ int virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr numatune, int cellid); size_t virDomainNumaGetNodeCount(virCPUDefPtr numa); +virBitmapPtr virDomainNumaGetNodeCpumask(virCPUDefPtr numa, + size_t node); /* * Formatters diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4ba5fd0..770803b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -630,6 +630,7 @@ virNodeDeviceObjUnlock; virDomainNumaEquals; virDomainNumaFree; virDomainNumaGetNodeCount; +virDomainNumaGetNodeCpumask; virDomainNumaNew; virDomainNumatuneFormatNodeset; virDomainNumatuneFormatXML; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f009570..d276da3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7196,7 +7196,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, for (i = 0; i ncells; i++) { VIR_FREE(cpumask); -if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask))) +if (!(cpumask = virBitmapFormat(virDomainNumaGetNodeCpumask(def-cpu, i goto cleanup; if (strchr(cpumask, ',') -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/24] conf: Refactor virDomainNumaDefCPUParseXML
Rewrite the function to save a few local variables and reorder the code to make more sense. Additionally the ncells_max member of the virCPUDef structure is used only for tracking alocation when parsing the numa definition, which can be avoided by switching to VIR_ALLOC_N as the array is not resized after initial allocation. --- src/conf/cpu_conf.c | 1 - src/conf/cpu_conf.h | 1 - src/conf/numa_conf.c | 129 +++--- tests/testutilsqemu.c | 1 - 4 files changed, 58 insertions(+), 74 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index f14b37a..98b309b 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -154,7 +154,6 @@ virCPUDefCopy(const virCPUDef *cpu) if (cpu-ncells) { if (VIR_ALLOC_N(copy-cells, cpu-ncells) 0) goto error; -copy-ncells_max = copy-ncells = cpu-ncells; for (i = 0; i cpu-ncells; i++) { copy-cells[i].mem = cpu-cells[i].mem; diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 46fce25..e201656 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -127,7 +127,6 @@ struct _virCPUDef { size_t nfeatures_max; virCPUFeatureDefPtr features; size_t ncells; -size_t ncells_max; virCellDefPtr cells; unsigned int cells_cpus; }; diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index e36a4be..c50369d 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -688,45 +688,36 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def, { xmlNodePtr *nodes = NULL; xmlNodePtr oldNode = ctxt-node; +char *tmp = NULL; int n; size_t i; int ret = -1; -if (virXPathNode(/domain/cpu/numa[1], ctxt)) { -VIR_FREE(nodes); +/* check if NUMA definition is present */ +if (!virXPathNode(/domain/cpu/numa[1], ctxt)) +return 0; -n = virXPathNodeSet(/domain/cpu/numa[1]/cell, ctxt, nodes); -if (n = 0) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(NUMA topology defined without NUMA cells)); -goto error; -} +if ((n = virXPathNodeSet(/domain/cpu/numa[1]/cell, ctxt, nodes)) = 0) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(NUMA topology defined without NUMA cells)); +goto cleanup; +} + +if (VIR_ALLOC_N(def-cells, n) 0) +goto cleanup; +def-ncells = n; -if (VIR_RESIZE_N(def-cells, def-ncells_max, - def-ncells, n) 0) -goto error; - -def-ncells = n; - -for (i = 0; i n; i++) { -char *cpus, *memAccessStr; -int rc, ncpus = 0; -unsigned int cur_cell; -char *tmp = NULL; - -tmp = virXMLPropString(nodes[i], id); -if (!tmp) { -cur_cell = i; -} else { -rc = virStrToLong_ui(tmp, NULL, 10, cur_cell); -if (rc == -1) { -virReportError(VIR_ERR_XML_ERROR, - _(Invalid 'id' attribute in NUMA cell: %s), - tmp); -VIR_FREE(tmp); -goto error; -} -VIR_FREE(tmp); +for (i = 0; i n; i++) { +int rc, ncpus = 0; +unsigned int cur_cell = i; + +/* cells are in order of parsing or explicitly numbered */ +if ((tmp = virXMLPropString(nodes[i], id))) { +if (virStrToLong_uip(tmp, NULL, 10, cur_cell) 0) { +virReportError(VIR_ERR_XML_ERROR, + _(Invalid 'id' attribute in NUMA cell: '%s'), + tmp); +goto cleanup; } if (cur_cell = n) { @@ -734,60 +725,56 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def, _(Exactly one 'cell' element per guest NUMA cell allowed, non-contiguous ranges or ranges not starting from 0 are not allowed)); -goto error; -} - -if (def-cells[cur_cell].cpustr) { -virReportError(VIR_ERR_XML_ERROR, - _(Duplicate NUMA cell info for cell id '%u'), - cur_cell); -goto error; +goto cleanup; } +} +VIR_FREE(tmp); -cpus = virXMLPropString(nodes[i], cpus); -if (!cpus) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(Missing 'cpus' attribute in NUMA cell)); -goto error; -} -def-cells[cur_cell].cpustr = cpus; +if (def-cells[cur_cell].cpumask) { +virReportError(VIR_ERR_XML_ERROR, + _(Duplicate NUMA cell info for cell id '%u'), +
[libvirt] [PATCH 24/24] conf: Move all NUMA configuration to virDomainNuma
For historical reasons data regarding NUMA configuration were split between the CPU definition and numatune. We cannot do anything about the XML still being split, but we certainly can at least store the relevant data in one place. This patch moves the NUMA stuff to the right place. --- src/conf/cpu_conf.c | 26 +++ src/conf/cpu_conf.h | 12 +++-- src/conf/domain_conf.c | 8 +++--- src/conf/numa_conf.c| 67 +++-- src/conf/numa_conf.h| 20 +++ src/cpu/cpu.c | 2 +- src/qemu/qemu_command.c | 20 +++ tests/cputest.c | 2 +- tests/testutilsqemu.c | 2 -- 9 files changed, 63 insertions(+), 96 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 0e3ce26..4e33934 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -73,16 +73,11 @@ virCPUDefFreeModel(virCPUDefPtr def) void virCPUDefFree(virCPUDefPtr def) { -size_t i; - if (!def) return; virCPUDefFreeModel(def); -for (i = 0; i def-ncells; i++) -virBitmapFree(def-cells[i].cpumask); -VIR_FREE(def-cells); VIR_FREE(def-vendor_id); VIR_FREE(def); @@ -126,7 +121,6 @@ virCPUDefPtr virCPUDefCopy(const virCPUDef *cpu) { virCPUDefPtr copy; -size_t i; if (!cpu || VIR_ALLOC(copy) 0) return NULL; @@ -143,20 +137,6 @@ virCPUDefCopy(const virCPUDef *cpu) if (virCPUDefCopyModel(copy, cpu, false) 0) goto error; -if (cpu-ncells) { -if (VIR_ALLOC_N(copy-cells, cpu-ncells) 0) -goto error; - -for (i = 0; i cpu-ncells; i++) { -copy-cells[i].mem = cpu-cells[i].mem; - -copy-cells[i].cpumask = virBitmapNewCopy(cpu-cells[i].cpumask); - -if (!copy-cells[i].cpumask) -goto error; -} -} - return copy; error: @@ -429,11 +409,12 @@ virCPUDefParseXML(xmlNodePtr node, char * virCPUDefFormat(virCPUDefPtr def, +virDomainNumaPtr numa, bool updateCPU) { virBuffer buf = VIR_BUFFER_INITIALIZER; -if (virCPUDefFormatBufFull(buf, def, updateCPU) 0) +if (virCPUDefFormatBufFull(buf, def, numa, updateCPU) 0) goto cleanup; if (virBufferCheckError(buf) 0) @@ -450,6 +431,7 @@ virCPUDefFormat(virCPUDefPtr def, int virCPUDefFormatBufFull(virBufferPtr buf, virCPUDefPtr def, + virDomainNumaPtr numa, bool updateCPU) { if (!def) @@ -489,7 +471,7 @@ virCPUDefFormatBufFull(virBufferPtr buf, if (virCPUDefFormatBuf(buf, def, updateCPU) 0) return -1; -if (virDomainNumaDefCPUFormat(buf, def) 0) +if (virDomainNumaDefCPUFormat(buf, numa) 0) return -1; virBufferAdjustIndent(buf, -2); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index d2563e2..705ba6d 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -29,6 +29,7 @@ # include virxml.h # include virbitmap.h # include virarch.h +# include numa_conf.h # define VIR_CPU_VENDOR_ID_LENGTH 12 @@ -90,13 +91,6 @@ struct _virCPUFeatureDef { int policy; /* enum virCPUFeaturePolicy */ }; -typedef struct _virCellDef virCellDef; -typedef virCellDef *virCellDefPtr; -struct _virCellDef { -virBitmapPtr cpumask; /* CPUs that are part of this node */ -unsigned long long mem; /* Node memory in kB */ -int memAccess; /* virNumaMemAccess */ -}; typedef struct _virCPUDef virCPUDef; typedef virCPUDef *virCPUDefPtr; @@ -115,8 +109,6 @@ struct _virCPUDef { size_t nfeatures; size_t nfeatures_max; virCPUFeatureDefPtr features; -size_t ncells; -virCellDefPtr cells; }; @@ -145,6 +137,7 @@ virCPUDefIsEqual(virCPUDefPtr src, char * virCPUDefFormat(virCPUDefPtr def, +virDomainNumaPtr numa, bool updateCPU); int @@ -154,6 +147,7 @@ virCPUDefFormatBuf(virBufferPtr buf, int virCPUDefFormatBufFull(virBufferPtr buf, virCPUDefPtr def, + virDomainNumaPtr numa, bool updateCPU); int diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d34b9c4..fe73a5f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13517,11 +13517,10 @@ virDomainDefParseXML(xmlDocPtr xml, } -if (virDomainNumaDefCPUParseXML(def-cpu, ctxt) 0) +if (virDomainNumaDefCPUParseXML(def-numa, ctxt) 0) goto error; -if (def-cpu -virDomainNumaGetCPUCountTotal(def-cpu) def-maxvcpus) { +if (virDomainNumaGetCPUCountTotal(def-numa) def-maxvcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Number of CPUs in numa exceeds the vcpu count)); @@ -13531,7 +13530,6 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainNumatuneParseXML(def-numa,
[libvirt] [PATCH 07/24] conf: Move enum virMemAccess to the NUMA code and rename it
Name it virNumaMemAccess and add it to conf/numa_conf.[ch] Note that to avoid a circular dependency the type of the NUMA cell memAccess variable was changed to int. It will be turned back later after the circular dependency will not exist. --- src/conf/cpu_conf.c | 6 -- src/conf/cpu_conf.h | 12 +--- src/conf/numa_conf.c| 11 --- src/conf/numa_conf.h| 9 + src/qemu/qemu_command.c | 10 +- 5 files changed, 23 insertions(+), 25 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 28fbead..4923618 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -56,12 +56,6 @@ VIR_ENUM_IMPL(virCPUFeaturePolicy, VIR_CPU_FEATURE_LAST, disable, forbid) -VIR_ENUM_IMPL(virMemAccess, VIR_MEM_ACCESS_LAST, - default, - shared, - private) - - void ATTRIBUTE_NONNULL(1) virCPUDefFreeModel(virCPUDefPtr def) { diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index d6efba7..0163384 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -83,16 +83,6 @@ typedef enum { VIR_ENUM_DECL(virCPUFeaturePolicy) -typedef enum { -VIR_MEM_ACCESS_DEFAULT, -VIR_MEM_ACCESS_SHARED, -VIR_MEM_ACCESS_PRIVATE, - -VIR_MEM_ACCESS_LAST, -} virMemAccess; - -VIR_ENUM_DECL(virMemAccess) - typedef struct _virCPUFeatureDef virCPUFeatureDef; typedef virCPUFeatureDef *virCPUFeatureDefPtr; struct _virCPUFeatureDef { @@ -105,7 +95,7 @@ typedef virCellDef *virCellDefPtr; struct _virCellDef { virBitmapPtr cpumask; /* CPUs that are part of this node */ unsigned long long mem; /* Node memory in kB */ -virMemAccess memAccess; +int memAccess; /* virNumaMemAccess */ }; typedef struct _virCPUDef virCPUDef; diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index bcb8023..eea4172 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -43,6 +43,11 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement, static, auto); +VIR_ENUM_IMPL(virNumaMemAccess, VIR_NUMA_MEM_ACCESS_LAST, + default, + shared, + private); + typedef struct _virDomainNumaNode virDomainNumaNode; typedef virDomainNumaNode *virDomainNumaNodePtr; @@ -757,7 +762,7 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def, goto cleanup; if ((tmp = virXMLPropString(nodes[i], memAccess))) { -if ((rc = virMemAccessTypeFromString(tmp)) = 0) { +if ((rc = virNumaMemAccessTypeFromString(tmp)) = 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(Invalid 'memAccess' attribute value '%s'), tmp); @@ -783,7 +788,7 @@ int virDomainNumaDefCPUFormat(virBufferPtr buf, virCPUDefPtr def) { -virMemAccess memAccess; +virNumaMemAccess memAccess; char *cpustr; size_t i; @@ -805,7 +810,7 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, virBufferAddLit(buf, unit='KiB'); if (memAccess) virBufferAsprintf(buf, memAccess='%s', - virMemAccessTypeToString(memAccess)); + virNumaMemAccessTypeToString(memAccess)); virBufferAddLit(buf, /\n); VIR_FREE(cpustr); } diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 0adeaa4..ca89e05 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -46,6 +46,15 @@ typedef enum { VIR_ENUM_DECL(virDomainNumatunePlacement) VIR_ENUM_DECL(virDomainNumatuneMemMode) +typedef enum { +VIR_NUMA_MEM_ACCESS_DEFAULT, +VIR_NUMA_MEM_ACCESS_SHARED, +VIR_NUMA_MEM_ACCESS_PRIVATE, + +VIR_NUMA_MEM_ACCESS_LAST +} virNumaMemAccess; + +VIR_ENUM_DECL(virNumaMemAccess) void virDomainNumaFree(virDomainNumaPtr numa); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c4ae596..b6fca9c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4558,7 +4558,7 @@ qemuBuildMemoryBackendStr(unsigned long long size, virDomainHugePagePtr hugepage = NULL; virDomainNumatuneMemMode mode; const long system_page_size = virGetSystemPageSizeKB(); -virMemAccess memAccess = def-cpu-cells[guestNode].memAccess; +virNumaMemAccess memAccess = def-cpu-cells[guestNode].memAccess; size_t i; char *mem_path = NULL; virBitmapPtr nodemask = NULL; @@ -4651,18 +4651,18 @@ qemuBuildMemoryBackendStr(unsigned long long size, goto cleanup; switch (memAccess) { -case VIR_MEM_ACCESS_SHARED: +case VIR_NUMA_MEM_ACCESS_SHARED: if (virJSONValueObjectAdd(props, b:share, true, NULL) 0) goto cleanup; break; -case VIR_MEM_ACCESS_PRIVATE: +case VIR_NUMA_MEM_ACCESS_PRIVATE: if (virJSONValueObjectAdd(props, b:share, false, NULL) 0) goto cleanup; break; -case
[libvirt] [PATCH 23/24] conf: numa: Add setter/getter for NUMA node memory size
Add the helpers and refactor places where the value is accessed without them. --- src/conf/numa_conf.c | 20 +++- src/conf/numa_conf.h | 7 +++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 10 ++ 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 4906687..ee7e65d 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -777,7 +777,8 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, virBufferAddLit(buf, cell); virBufferAsprintf(buf, id='%zu', i); virBufferAsprintf(buf, cpus='%s', cpustr); -virBufferAsprintf(buf, memory='%llu', def-cells[i].mem); +virBufferAsprintf(buf, memory='%llu', + virDomainNumaGetNodeMemorySize(def, i)); virBufferAddLit(buf, unit='KiB'); if (memAccess) virBufferAsprintf(buf, memAccess='%s', @@ -840,3 +841,20 @@ virDomainNumaGetNodeMemoryAccessMode(virCPUDefPtr numa, { return numa-cells[node].memAccess; } + + +unsigned long long +virDomainNumaGetNodeMemorySize(virCPUDefPtr numa, + size_t node) +{ +return numa-cells[node].mem; +} + + +void +virDomainNumaSetNodeMemorySize(virCPUDefPtr numa, + size_t node, + unsigned long long size) +{ +numa-cells[node].mem = size; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 0ea2c93..eadab43 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -91,6 +91,9 @@ virBitmapPtr virDomainNumaGetNodeCpumask(virCPUDefPtr numa, size_t node); virNumaMemAccess virDomainNumaGetNodeMemoryAccessMode(virCPUDefPtr numa, size_t node); +unsigned long long virDomainNumaGetNodeMemorySize(virCPUDefPtr numa, + size_t node); + /* * Formatters @@ -114,6 +117,10 @@ int virDomainNumatuneSet(virDomainNumaPtr numa, virBitmapPtr nodeset) ATTRIBUTE_NONNULL(1); +void virDomainNumaSetNodeMemorySize(virCPUDefPtr numa, +size_t node, +unsigned long long size); + /* * Other accessors */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1f1ce14..8988b61 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -632,7 +632,9 @@ virDomainNumaFree; virDomainNumaGetNodeCount; virDomainNumaGetNodeCpumask; virDomainNumaGetNodeMemoryAccessMode; +virDomainNumaGetNodeMemorySize; virDomainNumaNew; +virDomainNumaSetNodeMemorySize; virDomainNumatuneFormatNodeset; virDomainNumatuneFormatXML; virDomainNumatuneGetMode; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 05545ee..a69d004 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4745,7 +4745,8 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def, if (virAsprintf(alias, ram-node%zu, cell) 0) goto cleanup; -if ((rc = qemuBuildMemoryBackendStr(def-cpu-cells[cell].mem, 0, cell, +if ((rc = qemuBuildMemoryBackendStr(virDomainNumaGetNodeMemorySize(def-cpu, cell), +0, cell, NULL, auto_nodeset, def, qemuCaps, cfg, backendType, props, false)) 0) @@ -7173,8 +7174,8 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, /* using of -numa memdev= cannot be combined with -numa mem=, thus we * need to check which approach to use */ for (i = 0; i ncells; i++) { -unsigned long long cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024); -def-cpu-cells[i].mem = cellmem * 1024; +unsigned long long cellmem = virDomainNumaGetNodeMemorySize(def-cpu, i); +virDomainNumaSetNodeMemorySize(def-cpu, i, VIR_ROUND_UP(cellmem, 1024)); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { @@ -7224,7 +7225,8 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, if (needBackend) virBufferAsprintf(buf, ,memdev=ram-node%zu, i); else -virBufferAsprintf(buf, ,mem=%llu, def-cpu-cells[i].mem / 1024); +virBufferAsprintf(buf, ,mem=%llu, + virDomainNumaGetNodeMemorySize(def-cpu, i) / 1024); virCommandAddArgBuffer(cmd, buf); } -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/24] conf: numa: Rename virDomainNumatune to virDomainNuma
The structure will gradually become the only place for NUMA related config, thus rename it appropriately. --- src/conf/domain_conf.c| 10 +++--- src/conf/domain_conf.h| 2 +- src/conf/numa_conf.c | 78 +-- src/conf/numa_conf.h | 34 +-- src/libvirt_private.syms | 4 +-- src/lxc/lxc_cgroup.c | 4 +-- src/lxc/lxc_controller.c | 6 ++-- src/lxc/lxc_native.c | 2 +- src/parallels/parallels_sdk.c | 2 +- src/qemu/qemu_cgroup.c| 12 +++ src/qemu/qemu_command.c | 8 ++--- src/qemu/qemu_driver.c| 20 +-- src/qemu/qemu_process.c | 4 +-- 13 files changed, 93 insertions(+), 93 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 06ed0fd..c9a65e1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2249,7 +2249,7 @@ void virDomainDefFree(virDomainDefPtr def) virBitmapFree(def-cputune.iothreadsched[i].ids); VIR_FREE(def-cputune.iothreadsched); -virDomainNumatuneFree(def-numatune); +virDomainNumaFree(def-numa); virSysinfoDefFree(def-sysinfo); @@ -13508,14 +13508,14 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } -if (virDomainNumatuneParseXML(def-numatune, +if (virDomainNumatuneParseXML(def-numa, def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC, def-cpu ? def-cpu-ncells : 0, ctxt) 0) goto error; -if (virDomainNumatuneHasPlacementAuto(def-numatune) +if (virDomainNumatuneHasPlacementAuto(def-numa) !def-cpumask !def-cputune.vcpupin !def-cputune.emulatorpin !def-cputune.iothreadspin) def-placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO; @@ -19894,7 +19894,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (cputune) virBufferAddLit(buf, /cputune\n); -if (virDomainNumatuneFormatXML(buf, def-numatune) 0) +if (virDomainNumatuneFormatXML(buf, def-numa) 0) goto error; if (def-resource) @@ -22322,7 +22322,7 @@ virDomainDefNeedsPlacementAdvice(virDomainDefPtr def) if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) return true; -if (virDomainNumatuneHasPlacementAuto(def-numatune)) +if (virDomainNumatuneHasPlacementAuto(def-numa)) return true; return false; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c45e303..1e04886 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2091,7 +2091,7 @@ struct _virDomainDef { virDomainCputune cputune; -virDomainNumatunePtr numatune; +virDomainNumaPtr numa; virDomainResourceDefPtr resource; virDomainIdMapDef idmap; diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index d8f1739..bcb8023 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -43,10 +43,10 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement, static, auto); -typedef struct _virDomainNumatuneNode virDomainNumatuneNode; -typedef virDomainNumatuneNode *virDomainNumatuneNodePtr; +typedef struct _virDomainNumaNode virDomainNumaNode; +typedef virDomainNumaNode *virDomainNumaNodePtr; -struct _virDomainNumatune { +struct _virDomainNuma { struct { bool specified; virBitmapPtr nodeset; @@ -54,7 +54,7 @@ struct _virDomainNumatune { virDomainNumatunePlacement placement; } memory; /* pinning for all the memory */ -struct _virDomainNumatuneNode { +struct _virDomainNumaNode { virBitmapPtr nodeset; virDomainNumatuneMemMode mode; } *mem_nodes; /* fine tuning per guest node */ @@ -65,7 +65,7 @@ struct _virDomainNumatune { static inline bool -virDomainNumatuneNodeSpecified(virDomainNumatunePtr numatune, +virDomainNumatuneNodeSpecified(virDomainNumaPtr numatune, int cellid) { if (numatune @@ -77,7 +77,7 @@ virDomainNumatuneNodeSpecified(virDomainNumatunePtr numatune, } static int -virDomainNumatuneNodeParseXML(virDomainNumatunePtr *numatunePtr, +virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr, size_t ncells, xmlXPathContextPtr ctxt) { @@ -85,7 +85,7 @@ virDomainNumatuneNodeParseXML(virDomainNumatunePtr *numatunePtr, int n = 0; int ret = -1; size_t i = 0; -virDomainNumatunePtr numatune = *numatunePtr; +virDomainNumaPtr numatune = *numatunePtr; xmlNodePtr *nodes = NULL; if ((n = virXPathNodeSet(./numatune/memnode, ctxt, nodes)) 0) { @@ -126,7 +126,7 @@ virDomainNumatuneNodeParseXML(virDomainNumatunePtr *numatunePtr, for (i = 0; i n; i++) { int mode = 0; unsigned int cellid = 0; -virDomainNumatuneNodePtr mem_node = NULL; +
Re: [libvirt] [PATCH] qemu: check the validity of a pointer
On Mon, Feb 16, 2015 at 21:17:17 +0100, Matthias Gatto wrote: In the current code if mem_mask is NULL there is a goto error, but we freeing it without knowing if mem_mask is NULL or not, therefor I've had a check. Signed-off-by: Matthias Gatto matthias.ga...@outscale.com --- src/qemu/qemu_cgroup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index ca255c8..926726c 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -810,7 +810,8 @@ qemuRestoreCgroupState(virDomainObjPtr vm) goto error; cleanup: -VIR_FREE(mem_mask); +if (mem_mask) +VIR_FREE(mem_mask); NACK, VIR_FREE actually checks if the pointer is NULL before freeing it. The real problem is that 'mem_mask' is not initialized to NULL. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Fix memory ABI stability check issues
Note that this series applies only on top of the NUMA config unification series: http://www.redhat.com/archives/libvir-list/2015-February/msg00532.html Please see individual patches for explanation Peter Krempa (3): conf: ABI: Hugepage backing definition is not guest ABI conf: ABI: Memballoon setting is not guest ABI conf: numa: Check ABI stability of NUMA configuration src/conf/domain_conf.c | 31 ++- src/conf/numa_conf.c | 37 + src/conf/numa_conf.h | 3 +++ src/libvirt_private.syms | 1 + 4 files changed, 43 insertions(+), 29 deletions(-) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/24] conf: numa: Format numatune XML only if necessary
Do a content-aware check if formatting of the numatune element is necessary. Later on the def-numa structure will be always present so we cannot decide only on the basis whether it's allocated. --- src/conf/numa_conf.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index d5ba27f..359bdff 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -278,11 +278,22 @@ virDomainNumatuneFormatXML(virBufferPtr buf, { const char *tmp = NULL; char *nodeset = NULL; +bool nodesetSpecified = false; size_t i = 0; if (!numatune) return 0; +for (i = 0; i numatune-nmem_nodes; i++) { +if (numatune-mem_nodes[i].nodeset) { +nodesetSpecified = true; +break; +} +} + +if (!nodesetSpecified !numatune-memory.specified) +return 0; + virBufferAddLit(buf, numatune\n); virBufferAdjustIndent(buf, 2); -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/24] conf: Separate helper for creating domain objects
Move the existing virDomainDefNew to virDomainDefNewFull as it's setting a few things in the conf and re-introduce virDomainDefNew as a function without parameters for common use. --- src/conf/domain_conf.c | 20 src/conf/domain_conf.h | 7 --- src/libvirt_private.syms | 1 + src/xen/xen_hypervisor.c | 8 src/xen/xend_internal.c | 4 ++-- src/xen/xm_internal.c| 4 ++-- 6 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 870efc9..420b713 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2314,13 +2314,25 @@ virDomainObjNew(virDomainXMLOptionPtr xmlopt) } -virDomainDefPtr virDomainDefNew(const char *name, -const unsigned char *uuid, -int id) +virDomainDefPtr +virDomainDefNew(void) +{ +virDomainDefPtr ret; + +ignore_value(VIR_ALLOC(ret)); + +return ret; +} + + +virDomainDefPtr +virDomainDefNewFull(const char *name, +const unsigned char *uuid, +int id) { virDomainDefPtr def; -if (VIR_ALLOC(def) 0) +if (!(def = virDomainDefNew())) return NULL; if (VIR_STRDUP(def-name, name) 0) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1e04886..86db2ab 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2414,9 +2414,10 @@ void virDomainDefFree(virDomainDefPtr vm); virDomainChrDefPtr virDomainChrDefNew(void); -virDomainDefPtr virDomainDefNew(const char *name, -const unsigned char *uuid, -int id); +virDomainDefPtr virDomainDefNew(void); +virDomainDefPtr virDomainDefNewFull(const char *name, +const unsigned char *uuid, +int id); enum { VIR_DOMAIN_OBJ_LIST_ADD_LIVE = (1 0), diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 278124c..4ba2142 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -204,6 +204,7 @@ virDomainDefMaybeAddController; virDomainDefMaybeAddInput; virDomainDefNeedsPlacementAdvice; virDomainDefNew; +virDomainDefNewFull; virDomainDefParseFile; virDomainDefParseNode; virDomainDefParseString; diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 31a2a1b..bc498ff 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -2634,9 +2634,9 @@ xenHypervisorLookupDomainByID(virConnectPtr conn, int id) if (!name) return NULL; -ret = virDomainDefNew(name, - XEN_GETDOMAININFO_UUID(dominfo), - id); +ret = virDomainDefNewFull(name, + XEN_GETDOMAININFO_UUID(dominfo), + id); VIR_FREE(name); return ret; } @@ -2699,7 +2699,7 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) if (!name) return NULL; -ret = virDomainDefNew(name, uuid, id); +ret = virDomainDefNewFull(name, uuid, id); if (ret) ret-id = id; VIR_FREE(name); diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index c2b9098..ab03c1c 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1152,7 +1152,7 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) if (tmp) id = sexpr_int(root, domain/domid); -return virDomainDefNew(name, uuid, id); +return virDomainDefNewFull(name, uuid, id); error: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2117,7 +2117,7 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (name == NULL) return NULL; -ret = virDomainDefNew(name, uuid, id); +ret = virDomainDefNewFull(name, uuid, id); VIR_FREE(name); return ret; diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index dc6f88a..354ab3f 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -844,7 +844,7 @@ xenXMDomainLookupByName(virConnectPtr conn, const char *domname) if (!(entry = virHashLookup(priv-configCache, filename))) goto cleanup; -ret = virDomainDefNew(domname, entry-def-uuid, -1); +ret = virDomainDefNewFull(domname, entry-def-uuid, -1); cleanup: xenUnifiedUnlock(priv); @@ -887,7 +887,7 @@ xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (!(entry = virHashSearch(priv-configCache, xenXMDomainSearchForUUID, (const void *)uuid))) goto cleanup; -ret = virDomainDefNew(entry-def-name, uuid, -1); +ret = virDomainDefNewFull(entry-def-name, uuid, -1); cleanup: xenUnifiedUnlock(priv); -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/24] conf: numa: Refactor logic in virDomainNumatuneParseXML
Shuffling around the logic will allow to simplify the code quite a bit. As an additional bonus the change in the logic now reports an error if automatic placement is selected and individual placement is configured. --- src/conf/numa_conf.c | 53 +--- 1 file changed, 21 insertions(+), 32 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 18c21d5..d5ba27f 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -229,42 +229,31 @@ virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr, *numatunePtr = NULL; } -if (!node placement_static) { -if (virDomainNumatuneNodeParseXML(numatunePtr, ncells, ctxt) 0) -goto cleanup; -return 0; -} +if (!placement_static !node) +placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO; -if (!node) { -/* We know that placement_mode is auto if we're here */ -ret = virDomainNumatuneSet(numatunePtr, - placement_static, - VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO, - -1, - NULL); -goto cleanup; -} +if (node) { +if ((tmp = virXMLPropString(node, mode)) +(mode = virDomainNumatuneMemModeTypeFromString(tmp)) 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Unsupported NUMA memory tuning mode '%s'), tmp); +goto cleanup; +} +VIR_FREE(tmp); -if ((tmp = virXMLPropString(node, mode)) -(mode = virDomainNumatuneMemModeTypeFromString(tmp)) 0) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(Unsupported NUMA memory tuning mode '%s'), tmp); -goto cleanup; -} -VIR_FREE(tmp); +if ((tmp = virXMLPropString(node, placement)) +(placement = virDomainNumatunePlacementTypeFromString(tmp)) 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Unsupported NUMA memory placement mode '%s'), tmp); +goto cleanup; +} +VIR_FREE(tmp); -if ((tmp = virXMLPropString(node, placement)) -(placement = virDomainNumatunePlacementTypeFromString(tmp)) 0) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(Unsupported NUMA memory placement mode '%s'), tmp); -goto cleanup; +if ((tmp = virXMLPropString(node, nodeset)) +virBitmapParse(tmp, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN) 0) +goto cleanup; +VIR_FREE(tmp); } -VIR_FREE(tmp); - -if ((tmp = virXMLPropString(node, nodeset)) -virBitmapParse(tmp, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN) 0) -goto cleanup; -VIR_FREE(tmp); if (virDomainNumatuneSet(numatunePtr, placement_static, -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 14/24] conf: Allocate domain definition with the new helper
Use the virDomainDefNew() helper to allocate the definition instead of doing it via VIR_ALLOC. --- src/conf/domain_conf.c| 2 +- src/lxc/lxc_native.c | 2 +- src/openvz/openvz_conf.c | 2 +- src/parallels/parallels_sdk.c | 2 +- src/phyp/phyp_driver.c| 2 +- src/qemu/qemu_command.c | 2 +- src/vbox/vbox_common.c| 8 src/vmx/vmx.c | 2 +- src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c| 2 +- src/xenconfig/xen_xm.c| 2 +- tests/openvzutilstest.c | 2 +- tests/securityselinuxtest.c | 2 +- 13 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 420b713..6dea109 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12910,7 +12910,7 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(schema); } -if (VIR_ALLOC(def) 0) +if (!(def = virDomainDefNew())) return NULL; if (!(flags VIR_DOMAIN_DEF_PARSE_INACTIVE)) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index cd3b86b..99613af 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -1002,7 +1002,7 @@ lxcParseConfigString(const char *config) if (!(properties = virConfReadMem(config, 0, VIR_CONF_FLAG_LXC_FORMAT))) return NULL; -if (VIR_ALLOC(vmdef) 0) +if (!(vmdef = virDomainDefNew())) goto error; if (virUUIDGenerate(vmdef-uuid) 0) { diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index f955dda..848e230 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -543,7 +543,7 @@ int openvzLoadDomains(struct openvz_driver *driver) } *line++ = '\0'; -if (VIR_ALLOC(def) 0) +if (!(def = virDomainDefNew())) goto cleanup; def-virtType = VIR_DOMAIN_VIRT_OPENVZ; diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 8c05b32..d5ea00a 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1186,7 +1186,7 @@ prlsdkLoadDomain(parallelsConnPtr privconn, virCheckNonNullArgGoto(privconn, error); virCheckNonNullArgGoto(sdkdom, error); -if (VIR_ALLOC(def) 0) +if (!(def = virDomainDefNew())) goto error; if (!olddom) { diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index d05f897..d69e29c 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1708,7 +1708,7 @@ phypDomainAttachDevice(virDomainPtr domain, const char *xml) virBuffer buf = VIR_BUFFER_INITIALIZER; char *domain_name = NULL; -if (VIR_ALLOC(def) 0) +if (!(def = virDomainDefNew())) goto cleanup; domain_name = escape_specialcharacters(domain-name); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b6fca9c..8b660f0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11867,7 +11867,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, return NULL; } -if (VIR_ALLOC(def) 0) +if (!(def = virDomainDefNew())) goto error; /* allocate the cmdlinedef up-front; if it's unused, we'll free it later */ diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index deca490..55d3624 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3860,7 +3860,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) if (openSessionForMachine(data, dom-uuid, iid, machine, false) 0) goto cleanup; -if (VIR_ALLOC(def) 0) +if (!(def = virDomainDefNew())) goto cleanup; gVBoxAPI.UIMachine.GetAccessible(machine, accessible); @@ -4114,7 +4114,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom, return ret; VBOX_IID_INITIALIZE(iid); -if (VIR_ALLOC(def) 0) +if (!(def = virDomainDefNew())) return ret; if (VIR_STRDUP(def-os.type, hvm) 0) @@ -4246,7 +4246,7 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml) return ret; VBOX_IID_INITIALIZE(iid); -if (VIR_ALLOC(def) 0) +if (!(def = virDomainDefNew())) return ret; if (VIR_STRDUP(def-os.type, hvm) 0) @@ -6032,7 +6032,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot-name))) goto cleanup; -if (VIR_ALLOC(def) 0 || VIR_ALLOC(def-dom) 0) +if (VIR_ALLOC(def) 0 || !(def-dom = virDomainDefNew())) goto cleanup; if (VIR_STRDUP(def-name, snapshot-name) 0) goto cleanup; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 2a794c7..ac2542a 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1298,7 +1298,7 @@ virVMXParseConfig(virVMXContext *ctx, } /* Allocate domain def */ -if (VIR_ALLOC(def) 0) +if (!(def = virDomainDefNew())) goto cleanup; def-virtType = VIR_DOMAIN_VIRT_VMWARE; diff --git
[libvirt] [PATCH 00/24] Move all NUMA related configuration into one structure
Due to historical madne^Wreasons the NUMA configuration is split between /domain/cpu and /domain/numatune. Internally the data was also split into two places. We can't do anything about the external representation but we certainly can store all the definitions in one place internally. This series does that. Peter Krempa (24): conf: Move numatune_conf to numa_conf conf: Move NUMA cell parsing code from cpu conf to numa conf conf: Refactor virDomainNumaDefCPUParseXML conf: numa: Don't duplicate NUMA cell cpumask conf: Move NUMA cell formatter to numa_conf conf: numa: Rename virDomainNumatune to virDomainNuma conf: Move enum virMemAccess to the NUMA code and rename it conf: numa: Recalculate rather than remember total NUMA cpu count conf: numa: Improve error message in case a numa node doesn't have cpus conf: numa: Reformat virDomainNumatuneParseXML conf: numa: Refactor logic in virDomainNumatuneParseXML conf: numa: Format numatune XML only if necessary conf: Separate helper for creating domain objects conf: Allocate domain definition with the new helper conf: numa: Always allocate the NUMA config conf: numa: Avoid re-allocation of the NUMA conf numa: conf: Tweak parameters of virDomainNumatuneSet conf: numa: Don't pass double pointer to virDomainNumatuneParseXML qemu: command: Unify retrieval of NUMA cell count in qemuBuildNumaArgStr conf: numa: Add helper to get guest NUMA node count and refactor users conf: numa: Add accesor for the NUMA node cpu mask conf: numa: Add accessor to NUMA node's memory access mode conf: numa: Add setter/getter for NUMA node memory size conf: Move all NUMA configuration to virDomainNuma po/POTFILES.in | 2 +- src/Makefile.am| 2 +- src/conf/cpu_conf.c| 151 +--- src/conf/cpu_conf.h| 25 +- src/conf/domain_conf.c | 59 ++- src/conf/domain_conf.h | 11 +- src/conf/{numatune_conf.c = numa_conf.c} | 429 +++-- src/conf/{numatune_conf.h = numa_conf.h} | 77 ++-- src/cpu/cpu.c | 2 +- src/libvirt_private.syms | 13 +- src/lxc/lxc_cgroup.c | 4 +- src/lxc/lxc_controller.c | 6 +- src/lxc/lxc_native.c | 4 +- src/openvz/openvz_conf.c | 2 +- src/parallels/parallels_sdk.c | 4 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_cgroup.c | 12 +- src/qemu/qemu_command.c| 52 +-- src/qemu/qemu_driver.c | 20 +- src/qemu/qemu_process.c| 4 +- src/vbox/vbox_common.c | 8 +- src/vmx/vmx.c | 2 +- src/xen/xen_hypervisor.c | 8 +- src/xen/xend_internal.c| 4 +- src/xen/xm_internal.c | 4 +- src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c | 2 +- src/xenconfig/xen_xm.c | 2 +- tests/cputest.c| 2 +- tests/openvzutilstest.c| 2 +- .../qemuxml2argv-numatune-memnode.xml | 2 +- .../qemuxml2xmlout-numatune-memnode.xml| 2 +- tests/securityselinuxtest.c| 2 +- tests/testutilsqemu.c | 4 - 34 files changed, 503 insertions(+), 424 deletions(-) rename src/conf/{numatune_conf.c = numa_conf.c} (60%) rename src/conf/{numatune_conf.h = numa_conf.h} (50%) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: check the validity of a pointer
In the current code if mem_mask is NULL there is a goto error, but we freeing it without knowing if mem_mask is NULL or not, therefor I've had a check. Signed-off-by: Matthias Gatto matthias.ga...@outscale.com --- src/qemu/qemu_cgroup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index ca255c8..926726c 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -810,7 +810,8 @@ qemuRestoreCgroupState(virDomainObjPtr vm) goto error; cleanup: -VIR_FREE(mem_mask); +if (mem_mask) +VIR_FREE(mem_mask); virBitmapFree(all_nodes); return; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] conf: ABI: Hugepage backing definition is not guest ABI
The backing of the vm's memory isn't influencing the guest ABI thus shouldn't be checked. --- src/conf/domain_conf.c | 24 1 file changed, 24 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe73a5f..9a7060d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15969,30 +15969,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src, dst-mem.cur_balloon, src-mem.cur_balloon); goto error; } -if (src-mem.nhugepages != dst-mem.nhugepages) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(Target domain huge pages count %zu does not match source %zu), - dst-mem.nhugepages, src-mem.nhugepages); -goto error; -} -for (i = 0; i src-mem.nhugepages; i++) { -virDomainHugePagePtr src_huge = src-mem.hugepages[i]; -virDomainHugePagePtr dst_huge = dst-mem.hugepages[i]; - -if (src_huge-size != dst_huge-size) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(Target domain huge page size %llu - does not match source %llu), - dst_huge-size, src_huge-size); -goto error; -} - -if (!virBitmapEqual(src_huge-nodemask, dst_huge-nodemask)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(Target huge page nodemask does not match source)); -goto error; -} -} if (src-vcpus != dst-vcpus) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 2/3] conf: ABI: Memballoon setting is not guest ABI
The current balloon setting does not influence the ABI of the guest and the driver should take the change just fine. --- I personally don't think that this check is useful, but I'll happily drop the patch if anybody thinks otherwise. src/conf/domain_conf.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a7060d..eee01b6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15963,12 +15963,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src, dst-mem.max_balloon, src-mem.max_balloon); goto error; } -if (src-mem.cur_balloon != dst-mem.cur_balloon) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(Target domain current memory %lld does not match source %lld), - dst-mem.cur_balloon, src-mem.cur_balloon); -goto error; -} if (src-vcpus != dst-vcpus) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 20/24] conf: numa: Add helper to get guest NUMA node count and refactor users
Add an accessor so that a later refactor is simpler. --- src/conf/domain_conf.c | 2 +- src/conf/numa_conf.c | 15 +-- src/conf/numa_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 6 +++--- 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ceaf092..d34b9c4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13531,7 +13531,7 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainNumatuneParseXML(def-numa, def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC, - def-cpu ? def-cpu-ncells : 0, + virDomainNumaGetNodeCount(def-cpu), ctxt) 0) goto error; diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 8adec6f..61dfea0 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -760,14 +760,15 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, { virNumaMemAccess memAccess; char *cpustr; +size_t ncells = virDomainNumaGetNodeCount(def); size_t i; -if (def-ncells == 0) +if (ncells == 0) return 0; virBufferAddLit(buf, numa\n); virBufferAdjustIndent(buf, 2); -for (i = 0; i def-ncells; i++) { +for (i = 0; i ncells; i++) { memAccess = def-cells[i].memAccess; if (!(cpustr = virBitmapFormat(def-cells[i].cpumask))) @@ -813,3 +814,13 @@ virDomainNumaNew(void) return ret; } + + +size_t +virDomainNumaGetNodeCount(virCPUDefPtr numa) +{ +if (!numa) +return 0; + +return numa-ncells; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 69eccf2..55a9fbe 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -86,6 +86,8 @@ int virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr numatune, virBitmapPtr *retNodeset, int cellid); +size_t virDomainNumaGetNodeCount(virCPUDefPtr numa); + /* * Formatters */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6a746cf..4ba5fd0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -629,6 +629,7 @@ virNodeDeviceObjUnlock; # conf/numa_conf.h virDomainNumaEquals; virDomainNumaFree; +virDomainNumaGetNodeCount; virDomainNumaNew; virDomainNumatuneFormatNodeset; virDomainNumatuneFormatXML; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 65d8874..f009570 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7118,7 +7118,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, bool needBackend = false; int rc; int ret = -1; -size_t ncells = def-cpu-ncells; +size_t ncells = virDomainNumaGetNodeCount(def-cpu); const long system_page_size = virGetSystemPageSizeKB(); if (virDomainNumatuneHasPerNodeBinding(def-numa) @@ -8315,7 +8315,7 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, -m); def-mem.max_balloon = VIR_DIV_UP(def-mem.max_balloon, 1024) * 1024; virCommandAddArgFormat(cmd, %llu, def-mem.max_balloon / 1024); -if (def-mem.nhugepages (!def-cpu || !def-cpu-ncells)) { +if (def-mem.nhugepages !virDomainNumaGetNodeCount(def-cpu)) { const long system_page_size = virGetSystemPageSizeKB(); char *mem_path = NULL; @@ -8395,7 +8395,7 @@ qemuBuildCommandLine(virConnectPtr conn, } } -if (def-cpu def-cpu-ncells) +if (virDomainNumaGetNodeCount(def-cpu)) if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) 0) goto error; -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 18/24] conf: numa: Don't pass double pointer to virDomainNumatuneParseXML
virDomainNumatuneParseXML now doesn't allocate the def-numa object any longer so we don't need to pass a double pointer. --- src/conf/domain_conf.c | 2 +- src/conf/numa_conf.c | 23 +++ src/conf/numa_conf.h | 2 +- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 83c5fd9..ceaf092 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13528,7 +13528,7 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } -if (virDomainNumatuneParseXML(def-numa, +if (virDomainNumatuneParseXML(def-numa, def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC, def-cpu ? def-cpu-ncells : 0, diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 1b8232e..8adec6f 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -82,7 +82,7 @@ virDomainNumatuneNodeSpecified(virDomainNumaPtr numatune, } static int -virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr, +virDomainNumatuneNodeParseXML(virDomainNumaPtr numa, size_t ncells, xmlXPathContextPtr ctxt) { @@ -90,7 +90,6 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr, int n = 0; int ret = -1; size_t i = 0; -virDomainNumaPtr numatune = *numatunePtr; xmlNodePtr *nodes = NULL; if ((n = virXPathNodeSet(./numatune/memnode, ctxt, nodes)) 0) { @@ -102,8 +101,8 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr, if (!n) return 0; -if (numatune numatune-memory.specified -numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) { +if (numa-memory.specified +numa-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Per-node binding is not compatible with automatic NUMA placement.)); @@ -117,11 +116,11 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr, goto cleanup; } -VIR_FREE(numatune-mem_nodes); -if (VIR_ALLOC_N(numatune-mem_nodes, ncells) 0) +VIR_FREE(numa-mem_nodes); +if (VIR_ALLOC_N(numa-mem_nodes, ncells) 0) goto cleanup; -numatune-nmem_nodes = ncells; +numa-nmem_nodes = ncells; for (i = 0; i n; i++) { int mode = 0; @@ -144,14 +143,14 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr, } VIR_FREE(tmp); -if (cellid = numatune-nmem_nodes) { +if (cellid = numa-nmem_nodes) { virReportError(VIR_ERR_XML_ERROR, %s, _(Argument 'cellid' in memnode element must correspond to existing guest's NUMA cell)); goto cleanup; } -mem_node = numatune-mem_nodes[cellid]; +mem_node = numa-mem_nodes[cellid]; if (mem_node-nodeset) { virReportError(VIR_ERR_XML_ERROR, @@ -194,7 +193,7 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr, } int -virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr, +virDomainNumatuneParseXML(virDomainNumaPtr numa, bool placement_static, size_t ncells, xmlXPathContextPtr ctxt) @@ -245,14 +244,14 @@ virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr, VIR_FREE(tmp); } -if (virDomainNumatuneSet(*numatunePtr, +if (virDomainNumatuneSet(numa, placement_static, placement, mode, nodeset) 0) goto cleanup; -if (virDomainNumatuneNodeParseXML(numatunePtr, ncells, ctxt) 0) +if (virDomainNumatuneNodeParseXML(numa, ncells, ctxt) 0) goto cleanup; ret = 0; diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 790f3bf..69eccf2 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -62,7 +62,7 @@ void virDomainNumaFree(virDomainNumaPtr numa); /* * XML Parse/Format functions */ -int virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr, +int virDomainNumatuneParseXML(virDomainNumaPtr numa, bool placement_static, size_t ncells, xmlXPathContextPtr ctxt) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 19/24] qemu: command: Unify retrieval of NUMA cell count in qemuBuildNumaArgStr
The function uses the cell count in 6 places. Add a temp variable to hold the count as it will greatly simplify the refactor. --- src/qemu/qemu_command.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8b660f0..65d8874 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7118,6 +7118,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, bool needBackend = false; int rc; int ret = -1; +size_t ncells = def-cpu-ncells; const long system_page_size = virGetSystemPageSizeKB(); if (virDomainNumatuneHasPerNodeBinding(def-numa) @@ -7150,10 +7151,10 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, continue; } -if (def-cpu-ncells) { +if (ncells) { /* Fortunately, we allow only guest NUMA nodes to be continuous * starting from zero. */ -pos = def-cpu-ncells - 1; +pos = ncells - 1; } next_bit = virBitmapNextSetBit(def-mem.hugepages[i].nodemask, pos); @@ -7165,12 +7166,12 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, } } -if (VIR_ALLOC_N(nodeBackends, def-cpu-ncells) 0) +if (VIR_ALLOC_N(nodeBackends, ncells) 0) goto cleanup; /* using of -numa memdev= cannot be combined with -numa mem=, thus we * need to check which approach to use */ -for (i = 0; i def-cpu-ncells; i++) { +for (i = 0; i ncells; i++) { unsigned long long cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024); def-cpu-cells[i].mem = cellmem * 1024; @@ -7193,7 +7194,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, } } -for (i = 0; i def-cpu-ncells; i++) { +for (i = 0; i ncells; i++) { VIR_FREE(cpumask); if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask))) goto cleanup; @@ -7232,7 +7233,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, VIR_FREE(cpumask); if (nodeBackends) { -for (i = 0; i def-cpu-ncells; i++) +for (i = 0; i ncells; i++) VIR_FREE(nodeBackends[i]); VIR_FREE(nodeBackends); -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 16/24] conf: numa: Avoid re-allocation of the NUMA conf
As the numa object is now always present we can avoid the ad-hoc allocation code. --- src/conf/numa_conf.c | 26 ++ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 2a5fb3c..70a38d6 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -117,11 +117,6 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr *numatunePtr, goto cleanup; } -if (!numatune VIR_ALLOC(numatune) 0) -goto cleanup; - -*numatunePtr = numatune; - VIR_FREE(numatune-mem_nodes); if (VIR_ALLOC_N(numatune-mem_nodes, ncells) 0) goto cleanup; @@ -224,11 +219,6 @@ virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr, node = virXPathNode(./numatune/memory[1], ctxt); -if (*numatunePtr) { -virDomainNumaFree(*numatunePtr); -*numatunePtr = NULL; -} - if (!placement_static !node) placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO; @@ -454,27 +444,20 @@ virDomainNumatuneSet(virDomainNumaPtr *numatunePtr, int mode, virBitmapPtr nodeset) { -bool created = false; int ret = -1; -virDomainNumaPtr numatune; +virDomainNumaPtr numatune = *numatunePtr; /* No need to do anything in this case */ if (mode == -1 placement == -1 !nodeset) return 0; -if (!(*numatunePtr)) { -if (VIR_ALLOC(*numatunePtr) 0) -goto cleanup; - -created = true; +if (!numatune-memory.specified) { if (mode == -1) mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT; if (placement == -1) placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_DEFAULT; } -numatune = *numatunePtr; - /* Range checks */ if (mode != -1 (mode 0 || mode = VIR_DOMAIN_NUMATUNE_MEM_LAST)) { @@ -534,11 +517,6 @@ virDomainNumatuneSet(virDomainNumaPtr *numatunePtr, ret = 0; cleanup: -if (ret 0 created) { -virDomainNumaFree(*numatunePtr); -*numatunePtr = NULL; -} - return ret; } -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 17/24] numa: conf: Tweak parameters of virDomainNumatuneSet
As virDomainNumatuneSet now doesn't allocate the virDomainNuma object any longer it's not necessary to pass the pointer to a pointer to store the object as it will not change any longer. While touching the parameter definitions I've also changed the name of the parameter to numa. --- src/conf/numa_conf.c | 28 +--- src/conf/numa_conf.h | 2 +- src/lxc/lxc_native.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 70a38d6..1b8232e 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -245,7 +245,7 @@ virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr, VIR_FREE(tmp); } -if (virDomainNumatuneSet(numatunePtr, +if (virDomainNumatuneSet(*numatunePtr, placement_static, placement, mode, @@ -438,20 +438,19 @@ virDomainNumatuneMaybeFormatNodeset(virDomainNumaPtr numatune, } int -virDomainNumatuneSet(virDomainNumaPtr *numatunePtr, +virDomainNumatuneSet(virDomainNumaPtr numa, bool placement_static, int placement, int mode, virBitmapPtr nodeset) { int ret = -1; -virDomainNumaPtr numatune = *numatunePtr; /* No need to do anything in this case */ if (mode == -1 placement == -1 !nodeset) return 0; -if (!numatune-memory.specified) { +if (!numa-memory.specified) { if (mode == -1) mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT; if (placement == -1) @@ -476,26 +475,25 @@ virDomainNumatuneSet(virDomainNumaPtr *numatunePtr, } if (mode != -1) -numatune-memory.mode = mode; +numa-memory.mode = mode; if (nodeset) { -virBitmapFree(numatune-memory.nodeset); -numatune-memory.nodeset = virBitmapNewCopy(nodeset); -if (!numatune-memory.nodeset) +virBitmapFree(numa-memory.nodeset); +if (!(numa-memory.nodeset = virBitmapNewCopy(nodeset))) goto cleanup; if (placement == -1) placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC; } if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_DEFAULT) { -if (numatune-memory.nodeset || placement_static) +if (numa-memory.nodeset || placement_static) placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC; else placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO; } if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC -!numatune-memory.nodeset) { +!numa-memory.nodeset) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(nodeset for NUMA memory tuning must be set if 'placement' is 'static')); @@ -504,15 +502,15 @@ virDomainNumatuneSet(virDomainNumaPtr *numatunePtr, /* setting nodeset when placement auto is invalid */ if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO -numatune-memory.nodeset) { -virBitmapFree(numatune-memory.nodeset); -numatune-memory.nodeset = NULL; +numa-memory.nodeset) { +virBitmapFree(numa-memory.nodeset); +numa-memory.nodeset = NULL; } if (placement != -1) -numatune-memory.placement = placement; +numa-memory.placement = placement; -numatune-memory.specified = true; +numa-memory.specified = true; ret = 0; diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index e69489d..790f3bf 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -101,7 +101,7 @@ int virDomainNumatuneMaybeFormatNodeset(virDomainNumaPtr numatune, /* * Setters */ -int virDomainNumatuneSet(virDomainNumaPtr *numatunePtr, +int virDomainNumatuneSet(virDomainNumaPtr numa, bool placement_static, int placement, int mode, diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 99613af..abf07ce 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -847,7 +847,7 @@ lxcSetCpusetTune(virDomainDefPtr def, virConfPtr properties) value-str) { if (virBitmapParse(value-str, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN) 0) return -1; -if (virDomainNumatuneSet(def-numa, +if (virDomainNumatuneSet(def-numa, def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC, VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1c2ace9..0706303 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9543,7 +9543,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, qemuDomainSetNumaParamsLive(vm, nodeset) 0) goto endjob; -if
[libvirt] [PATCH 15/24] conf: numa: Always allocate the NUMA config
Since our formatter now handles well if the config is allocated and not filled and the parser always frees the definition before parsing we can safely always-allocate the NUMA config. This will help in later patches as the parser will be refactored to just fill the data. --- src/conf/domain_conf.c | 10 +- src/conf/numa_conf.c | 11 +++ src/conf/numa_conf.h | 1 + src/libvirt_private.syms | 1 + 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6dea109..83c5fd9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2319,9 +2319,17 @@ virDomainDefNew(void) { virDomainDefPtr ret; -ignore_value(VIR_ALLOC(ret)); +if (VIR_ALLOC(ret) 0) +return NULL; + +if (!(ret-numa = virDomainNumaNew())) +goto error; return ret; + + error: +virDomainDefFree(ret); +return NULL; } diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 359bdff..2a5fb3c 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -827,3 +827,14 @@ virDomainNumaGetCPUCountTotal(virCPUDefPtr numa) return ret; } + + +virDomainNumaPtr +virDomainNumaNew(void) +{ +virDomainNumaPtr ret = NULL; + +ignore_value(VIR_ALLOC(ret)); + +return ret; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index fa6b89b..e69489d 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -56,6 +56,7 @@ typedef enum { VIR_ENUM_DECL(virNumaMemAccess) +virDomainNumaPtr virDomainNumaNew(void); void virDomainNumaFree(virDomainNumaPtr numa); /* diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4ba2142..6a746cf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -629,6 +629,7 @@ virNodeDeviceObjUnlock; # conf/numa_conf.h virDomainNumaEquals; virDomainNumaFree; +virDomainNumaNew; virDomainNumatuneFormatNodeset; virDomainNumatuneFormatXML; virDomainNumatuneGetMode; -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/24] conf: Move NUMA cell parsing code from cpu conf to numa conf
For weird historical reasons NUMA cells are added as a subelement of cpu while the actual configuration is done in numatune. This patch splits out the cell parser code from cpu config to NUMA config. Note that the changes to the code are minimal just to make it work and the function will be refactored in the next patch. --- src/conf/cpu_conf.c| 90 --- src/conf/domain_conf.c | 17 +--- src/conf/numa_conf.c | 111 + src/conf/numa_conf.h | 4 ++ 4 files changed, 126 insertions(+), 96 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 4a367a1..f14b37a 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -426,96 +426,6 @@ virCPUDefParseXML(xmlNodePtr node, def-features[i].policy = policy; } -if (virXPathNode(./numa[1], ctxt)) { -VIR_FREE(nodes); -n = virXPathNodeSet(./numa[1]/cell, ctxt, nodes); -if (n = 0) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(NUMA topology defined without NUMA cells)); -goto error; -} - -if (VIR_RESIZE_N(def-cells, def-ncells_max, - def-ncells, n) 0) -goto error; - -def-ncells = n; - -for (i = 0; i n; i++) { -char *cpus, *memAccessStr; -int ret, ncpus = 0; -unsigned int cur_cell; -char *tmp = NULL; - -tmp = virXMLPropString(nodes[i], id); -if (!tmp) { -cur_cell = i; -} else { -ret = virStrToLong_ui(tmp, NULL, 10, cur_cell); -if (ret == -1) { -virReportError(VIR_ERR_XML_ERROR, - _(Invalid 'id' attribute in NUMA cell: %s), - tmp); -VIR_FREE(tmp); -goto error; -} -VIR_FREE(tmp); -} - -if (cur_cell = n) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(Exactly one 'cell' element per guest - NUMA cell allowed, non-contiguous ranges or - ranges not starting from 0 are not allowed)); -goto error; -} - -if (def-cells[cur_cell].cpustr) { -virReportError(VIR_ERR_XML_ERROR, - _(Duplicate NUMA cell info for cell id '%u'), - cur_cell); -goto error; -} - -cpus = virXMLPropString(nodes[i], cpus); -if (!cpus) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(Missing 'cpus' attribute in NUMA cell)); -goto error; -} -def-cells[cur_cell].cpustr = cpus; - -ncpus = virBitmapParse(cpus, 0, def-cells[cur_cell].cpumask, - VIR_DOMAIN_CPUMASK_LEN); -if (ncpus = 0) -goto error; -def-cells_cpus += ncpus; - -ctxt-node = nodes[i]; -if (virDomainParseMemory(./@memory, ./@unit, ctxt, - def-cells[cur_cell].mem, true, false) 0) -goto cleanup; - -memAccessStr = virXMLPropString(nodes[i], memAccess); -if (memAccessStr) { -int rc = virMemAccessTypeFromString(memAccessStr); - -if (rc = 0) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(Invalid 'memAccess' attribute - value '%s'), - memAccessStr); -VIR_FREE(memAccessStr); -goto error; -} - -def-cells[cur_cell].memAccess = rc; - -VIR_FREE(memAccessStr); -} -} -} - cleanup: ctxt-node = oldnode; VIR_FREE(fallback); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b13cae8..06ed0fd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13495,12 +13495,17 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } -if (def-cpu-cells_cpus def-maxvcpus) { -virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(Number of CPUs in numa exceeds the - vcpu count)); -goto error; -} +} + +if (virDomainNumaDefCPUParseXML(def-cpu, ctxt) 0) +goto error; + +if (def-cpu +def-cpu-cells_cpus def-maxvcpus) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Number of CPUs in numa exceeds the + vcpu count)); +goto error; } if
[libvirt] [PATCH 01/24] conf: Move numatune_conf to numa_conf
For a while now there are two places that gather information about NUMA related guest configuration. While the XML can't be changed we can at least store the data in one place in the definition. Rename the numatune_conf.[ch] files to numa_conf as later patches will move the rest of the definitions from the cpu definition to this one. --- po/POTFILES.in| 2 +- src/Makefile.am | 2 +- src/conf/domain_conf.h| 2 +- src/conf/{numatune_conf.c = numa_conf.c} | 6 +++--- src/conf/{numatune_conf.h = numa_conf.h} | 10 +- src/libvirt_private.syms | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) rename src/conf/{numatune_conf.c = numa_conf.c} (99%) rename src/conf/{numatune_conf.h = numa_conf.h} (96%) diff --git a/po/POTFILES.in b/po/POTFILES.in index 3064037..c25bfce 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -27,7 +27,7 @@ src/conf/netdev_vport_profile_conf.c src/conf/network_conf.c src/conf/networkcommon_conf.c src/conf/node_device_conf.c -src/conf/numatune_conf.c +src/conf/numa_conf.c src/conf/nwfilter_conf.c src/conf/nwfilter_params.c src/conf/object_event.c diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..85624be 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -273,7 +273,7 @@ DOMAIN_CONF_SOURCES = \ conf/domain_audit.c conf/domain_audit.h \ conf/domain_nwfilter.c conf/domain_nwfilter.h \ conf/snapshot_conf.c conf/snapshot_conf.h \ - conf/numatune_conf.c conf/numatune_conf.h + conf/numa_conf.c conf/numa_conf.h OBJECT_EVENT_SOURCES = \ conf/object_event.c conf/object_event.h \ diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 325afa8..c45e303 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -38,7 +38,7 @@ # include virsocketaddr.h # include networkcommon_conf.h # include nwfilter_params.h -# include numatune_conf.h +# include numa_conf.h # include virnetdevmacvlan.h # include virsysinfo.h # include virnetdevvportprofile.h diff --git a/src/conf/numatune_conf.c b/src/conf/numa_conf.c similarity index 99% rename from src/conf/numatune_conf.c rename to src/conf/numa_conf.c index 7f322ea..f6a8248 100644 --- a/src/conf/numatune_conf.c +++ b/src/conf/numa_conf.c @@ -1,7 +1,7 @@ /* - * numatune_conf.c + * numa_conf.c * - * Copyright (C) 2014 Red Hat, Inc. + * Copyright (C) 2014-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 @@ -22,7 +22,7 @@ #include config.h -#include numatune_conf.h +#include numa_conf.h #include domain_conf.h #include viralloc.h diff --git a/src/conf/numatune_conf.h b/src/conf/numa_conf.h similarity index 96% rename from src/conf/numatune_conf.h rename to src/conf/numa_conf.h index 28c4ce2..bcc8c8a 100644 --- a/src/conf/numatune_conf.h +++ b/src/conf/numa_conf.h @@ -1,7 +1,7 @@ /* - * numatune_conf.h + * numa_conf.h * - * Copyright (C) 2014 Red Hat, Inc. + * Copyright (C) 2014-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 @@ -20,8 +20,8 @@ * Author: Martin Kletzander mklet...@redhat.com */ -#ifndef __NUMATUNE_CONF_H__ -# define __NUMATUNE_CONF_H__ +#ifndef __NUMA_CONF_H__ +# define __NUMA_CONF_H__ # include libxml/xpath.h @@ -111,4 +111,4 @@ int virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr numatune); bool virDomainNumatuneNodesetIsAvailable(virDomainNumatunePtr numatune, virBitmapPtr auto_nodeset); -#endif /* __NUMATUNE_CONF_H__ */ +#endif /* __NUMA_CONF_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c07a561..edd54b8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -625,7 +625,7 @@ virNodeDeviceObjRemove; virNodeDeviceObjUnlock; -# conf/numatune_conf.h +# conf/numa_conf.h virDomainNumatuneEquals; virDomainNumatuneFormatNodeset; virDomainNumatuneFormatXML; -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/24] conf: numa: Improve error message in case a numa node doesn't have cpus
Currently the code would exit without reporting an error as virBitmapParse reports one only if it fails to parse the bitmap, whereas the code was jumping to the error label even in case 0 cpus were correctly parsed in the map. --- src/conf/numa_conf.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 41a7e01..391ef15 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -749,9 +749,14 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def, } if (virBitmapParse(tmp, 0, def-cells[cur_cell].cpumask, - VIR_DOMAIN_CPUMASK_LEN) = 0) + VIR_DOMAIN_CPUMASK_LEN) 0) goto cleanup; +if (virBitmapIsAllClear(def-cells[cur_cell].cpumask)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(NUMA cell %d has no vCPUs assigned), cur_cell); +goto cleanup; +} VIR_FREE(tmp); ctxt-node = nodes[i]; -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 2/3] conf: ABI: Memballoon setting is not guest ABI
On Mon, Feb 16, 2015 at 08:50:43PM +0100, Peter Krempa wrote: The current balloon setting does not influence the ABI of the guest and the driver should take the change just fine. --- I personally don't think that this check is useful, but I'll happily drop the patch if anybody thinks otherwise. src/conf/domain_conf.c | 6 -- 1 file changed, 6 deletions(-) Yes, you can change the balloon target on the fly, so this isn't something we need to keep as stable across migration. The question though is whether libvirt/QEMU will correctly handle the changed balloon setting during migration. ie does libvirt correctly issue a 'balloon' monitor command on the target host if it has changed in the target XML ? If it does, then I agree this check can go.. diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a7060d..eee01b6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15963,12 +15963,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src, dst-mem.max_balloon, src-mem.max_balloon); goto error; } -if (src-mem.cur_balloon != dst-mem.cur_balloon) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(Target domain current memory %lld does not match source %lld), - dst-mem.cur_balloon, src-mem.cur_balloon); -goto error; -} 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 3/3] conf: numa: Check ABI stability of NUMA configuration
Add helper to compare initial sizes of indivitual NUMA nodes and the map of belonging vCPUs. Other configuration is not ABI. --- src/conf/domain_conf.c | 3 +++ src/conf/numa_conf.c | 37 + src/conf/numa_conf.h | 3 +++ src/libvirt_private.syms | 1 + 4 files changed, 44 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eee01b6..15d4fe0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15964,6 +15964,9 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; } +if (!virDomainNumaCheckABIStability(src-numa, dst-numa)) +goto error; + if (src-vcpus != dst-vcpus) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(Target domain vCPU count %d does not match source %d), diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 0a6f902..70044d5 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -814,6 +814,43 @@ virDomainNumaNew(void) } +bool +virDomainNumaCheckABIStability(virDomainNumaPtr src, + virDomainNumaPtr tgt) +{ +size_t i; + +if (virDomainNumaGetNodeCount(src) != virDomainNumaGetNodeCount(tgt)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Target NUMA node count '%zu' doesn't match + source '%zu'), + virDomainNumaGetNodeCount(tgt), + virDomainNumaGetNodeCount(src)); +return false; +} + +for (i = 0; i virDomainNumaGetNodeCount(src); i++) { +if (src-mem_nodes[i].mem != tgt-mem_nodes[i].mem) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Size of target NUMA node %zu (%llu) doesn't + match source (%llu)), i, + tgt-mem_nodes[i].mem, src-mem_nodes[i].mem); +return false; +} + +if (!virBitmapEqual(src-mem_nodes[i].cpumask, +tgt-mem_nodes[i].cpumask)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Processor mask of target NUMA node %zu doesn't + match source), i); +return false; +} +} + +return true; +} + + size_t virDomainNumaGetNodeCount(virDomainNumaPtr numa) { diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 8b3c437..23265af 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -125,6 +125,9 @@ void virDomainNumaSetNodeMemorySize(virDomainNumaPtr numa, bool virDomainNumaEquals(virDomainNumaPtr n1, virDomainNumaPtr n2); +bool virDomainNumaCheckABIStability(virDomainNumaPtr src, +virDomainNumaPtr tgt); + bool virDomainNumatuneHasPlacementAuto(virDomainNumaPtr numatune); bool virDomainNumatuneHasPerNodeBinding(virDomainNumaPtr numatune); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8988b61..c02e089 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -627,6 +627,7 @@ virNodeDeviceObjUnlock; # conf/numa_conf.h +virDomainNumaCheckABIStability; virDomainNumaEquals; virDomainNumaFree; virDomainNumaGetNodeCount; -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/24] conf: Move NUMA cell formatter to numa_conf
Move the code that formats the /domain/cpu/numa element to numa_conf as it belongs there. --- src/conf/cpu_conf.c | 29 - src/conf/numa_conf.c | 37 + src/conf/numa_conf.h | 1 + 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 11ad5f4..28fbead 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -495,8 +495,11 @@ virCPUDefFormatBufFull(virBufferPtr buf, virArchToString(def-arch)); if (virCPUDefFormatBuf(buf, def, updateCPU) 0) return -1; -virBufferAdjustIndent(buf, -2); +if (virDomainNumaDefCPUFormat(buf, def) 0) +return -1; + +virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, /cpu\n); return 0; @@ -591,30 +594,6 @@ virCPUDefFormatBuf(virBufferPtr buf, } } -if (def-ncells) { -virBufferAddLit(buf, numa\n); -virBufferAdjustIndent(buf, 2); -for (i = 0; i def-ncells; i++) { -virMemAccess memAccess = def-cells[i].memAccess; -char *cpustr = NULL; - -if (!(cpustr = virBitmapFormat(def-cells[i].cpumask))) -return -1; - -virBufferAddLit(buf, cell); -virBufferAsprintf(buf, id='%zu', i); -virBufferAsprintf(buf, cpus='%s', cpustr); -virBufferAsprintf(buf, memory='%llu', def-cells[i].mem); -virBufferAddLit(buf, unit='KiB'); -if (memAccess) -virBufferAsprintf(buf, memAccess='%s', - virMemAccessTypeToString(memAccess)); -virBufferAddLit(buf, /\n); -VIR_FREE(cpustr); -} -virBufferAdjustIndent(buf, -2); -virBufferAddLit(buf, /numa\n); -} return 0; } diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 965d67b..d8f1739 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -777,3 +777,40 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def, VIR_FREE(tmp); return ret; } + + +int +virDomainNumaDefCPUFormat(virBufferPtr buf, + virCPUDefPtr def) +{ +virMemAccess memAccess; +char *cpustr; +size_t i; + +if (def-ncells == 0) +return 0; + +virBufferAddLit(buf, numa\n); +virBufferAdjustIndent(buf, 2); +for (i = 0; i def-ncells; i++) { +memAccess = def-cells[i].memAccess; + +if (!(cpustr = virBitmapFormat(def-cells[i].cpumask))) +return -1; + +virBufferAddLit(buf, cell); +virBufferAsprintf(buf, id='%zu', i); +virBufferAsprintf(buf, cpus='%s', cpustr); +virBufferAsprintf(buf, memory='%llu', def-cells[i].mem); +virBufferAddLit(buf, unit='KiB'); +if (memAccess) +virBufferAsprintf(buf, memAccess='%s', + virMemAccessTypeToString(memAccess)); +virBufferAddLit(buf, /\n); +VIR_FREE(cpustr); +} +virBufferAdjustIndent(buf, -2); +virBufferAddLit(buf, /numa\n); + +return 0; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 276d25a..9202355 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -114,5 +114,6 @@ bool virDomainNumatuneNodesetIsAvailable(virDomainNumatunePtr numatune, virBitmapPtr auto_nodeset); int virDomainNumaDefCPUParseXML(virCPUDefPtr def, xmlXPathContextPtr ctxt); +int virDomainNumaDefCPUFormat(virBufferPtr buf, virCPUDefPtr def); #endif /* __NUMA_CONF_H__ */ -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/24] conf: numa: Recalculate rather than remember total NUMA cpu count
It's easier to recalculate the number in the one place it's used as having a separate varialbe to track it. It will also help with moving the NUMA code to the separate module. --- src/conf/cpu_conf.c| 1 - src/conf/cpu_conf.h| 1 - src/conf/domain_conf.c | 2 +- src/conf/numa_conf.c | 23 +-- src/conf/numa_conf.h | 3 +++ tests/testutilsqemu.c | 1 - 6 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 4923618..0e3ce26 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -155,7 +155,6 @@ virCPUDefCopy(const virCPUDef *cpu) if (!copy-cells[i].cpumask) goto error; } -copy-cells_cpus = cpu-cells_cpus; } return copy; diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 0163384..d2563e2 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -117,7 +117,6 @@ struct _virCPUDef { virCPUFeatureDefPtr features; size_t ncells; virCellDefPtr cells; -unsigned int cells_cpus; }; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c9a65e1..870efc9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13501,7 +13501,7 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; if (def-cpu -def-cpu-cells_cpus def-maxvcpus) { +virDomainNumaGetCPUCountTotal(def-cpu) def-maxvcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Number of CPUs in numa exceeds the vcpu count)); diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index eea4172..41a7e01 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -713,7 +713,7 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def, def-ncells = n; for (i = 0; i n; i++) { -int rc, ncpus = 0; +int rc; unsigned int cur_cell = i; /* cells are in order of parsing or explicitly numbered */ @@ -748,12 +748,10 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def, goto cleanup; } -ncpus = virBitmapParse(tmp, 0, def-cells[cur_cell].cpumask, - VIR_DOMAIN_CPUMASK_LEN); - -if (ncpus = 0) +if (virBitmapParse(tmp, 0, def-cells[cur_cell].cpumask, + VIR_DOMAIN_CPUMASK_LEN) = 0) goto cleanup; -def-cells_cpus += ncpus; + VIR_FREE(tmp); ctxt-node = nodes[i]; @@ -819,3 +817,16 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, return 0; } + + +unsigned int +virDomainNumaGetCPUCountTotal(virCPUDefPtr numa) +{ +size_t i; +unsigned int ret = 0; + +for (i = 0; i numa-ncells; i++) +ret += virBitmapCountBits(numa-cells[i].cpumask); + +return ret; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index ca89e05..fa6b89b 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -125,4 +125,7 @@ bool virDomainNumatuneNodesetIsAvailable(virDomainNumaPtr numatune, int virDomainNumaDefCPUParseXML(virCPUDefPtr def, xmlXPathContextPtr ctxt); int virDomainNumaDefCPUFormat(virBufferPtr buf, virCPUDefPtr def); +unsigned int virDomainNumaGetCPUCountTotal(virCPUDefPtr numa); + + #endif /* __NUMA_CONF_H__ */ diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 64f709a..4a3df8a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -244,7 +244,6 @@ virCapsPtr testQemuCapsInit(void) host_cpu_features, /* features */ 0, /* ncells */ NULL, /* cells */ -0, /* cells_cpus */ }; if ((caps = virCapabilitiesNew(host_cpu.arch, -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/24] conf: numa: Reformat virDomainNumatuneParseXML
Make collapse few of the conditions so that the program flow is more clear. --- src/conf/numa_conf.c | 32 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 391ef15..18c21d5 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -245,32 +245,24 @@ virDomainNumatuneParseXML(virDomainNumaPtr *numatunePtr, goto cleanup; } -tmp = virXMLPropString(node, mode); -if (tmp) { -mode = virDomainNumatuneMemModeTypeFromString(tmp); -if (mode 0) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(Unsupported NUMA memory tuning mode '%s'), - tmp); -goto cleanup; -} +if ((tmp = virXMLPropString(node, mode)) +(mode = virDomainNumatuneMemModeTypeFromString(tmp)) 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Unsupported NUMA memory tuning mode '%s'), tmp); +goto cleanup; } VIR_FREE(tmp); -tmp = virXMLPropString(node, placement); -if (tmp) { -placement = virDomainNumatunePlacementTypeFromString(tmp); -if (placement 0) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(Unsupported NUMA memory placement mode '%s'), - tmp); -goto cleanup; -} +if ((tmp = virXMLPropString(node, placement)) +(placement = virDomainNumatunePlacementTypeFromString(tmp)) 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Unsupported NUMA memory placement mode '%s'), tmp); +goto cleanup; } VIR_FREE(tmp); -tmp = virXMLPropString(node, nodeset); -if (tmp virBitmapParse(tmp, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN) 0) +if ((tmp = virXMLPropString(node, nodeset)) +virBitmapParse(tmp, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN) 0) goto cleanup; VIR_FREE(tmp); -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/24] conf: numa: Don't duplicate NUMA cell cpumask
The mask was stored both as a bitmap and as a string. The string is used for XML output only. Remove the string, as it can be reconstructed from the bitmap. The test change is necessary as the bitmap formatter doesn't optimize using the '^' operator. --- src/conf/cpu_conf.c| 14 +++--- src/conf/cpu_conf.h| 1 - src/conf/numa_conf.c | 3 +-- tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml | 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml | 2 +- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 98b309b..11ad5f4 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -86,10 +86,8 @@ virCPUDefFree(virCPUDefPtr def) virCPUDefFreeModel(def); -for (i = 0; i def-ncells; i++) { +for (i = 0; i def-ncells; i++) virBitmapFree(def-cells[i].cpumask); -VIR_FREE(def-cells[i].cpustr); -} VIR_FREE(def-cells); VIR_FREE(def-vendor_id); @@ -162,9 +160,6 @@ virCPUDefCopy(const virCPUDef *cpu) if (!copy-cells[i].cpumask) goto error; - -if (VIR_STRDUP(copy-cells[i].cpustr, cpu-cells[i].cpustr) 0) -goto error; } copy-cells_cpus = cpu-cells_cpus; } @@ -601,16 +596,21 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAdjustIndent(buf, 2); for (i = 0; i def-ncells; i++) { virMemAccess memAccess = def-cells[i].memAccess; +char *cpustr = NULL; + +if (!(cpustr = virBitmapFormat(def-cells[i].cpumask))) +return -1; virBufferAddLit(buf, cell); virBufferAsprintf(buf, id='%zu', i); -virBufferAsprintf(buf, cpus='%s', def-cells[i].cpustr); +virBufferAsprintf(buf, cpus='%s', cpustr); virBufferAsprintf(buf, memory='%llu', def-cells[i].mem); virBufferAddLit(buf, unit='KiB'); if (memAccess) virBufferAsprintf(buf, memAccess='%s', virMemAccessTypeToString(memAccess)); virBufferAddLit(buf, /\n); +VIR_FREE(cpustr); } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, /numa\n); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index e201656..d6efba7 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -104,7 +104,6 @@ typedef struct _virCellDef virCellDef; typedef virCellDef *virCellDefPtr; struct _virCellDef { virBitmapPtr cpumask; /* CPUs that are part of this node */ -char *cpustr; /* CPUs stored in string form for dumpxml */ unsigned long long mem; /* Node memory in kB */ virMemAccess memAccess; }; diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index c50369d..965d67b 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -749,8 +749,7 @@ virDomainNumaDefCPUParseXML(virCPUDefPtr def, if (ncpus = 0) goto cleanup; def-cells_cpus += ncpus; - -def-cells[cur_cell].cpustr = tmp; +VIR_FREE(tmp); ctxt-node = nodes[i]; if (virDomainParseMemory(./@memory, ./@unit, ctxt, diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml index 8912017..73dfdf5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml @@ -17,7 +17,7 @@ numa cell id='0' cpus='0' memory='20002' unit='KiB'/ cell id='1' cpus='1-27,29' memory='660066' unit='KiB'/ - cell id='2' cpus='28-31,^29' memory='24002400' unit='KiB'/ + cell id='2' cpus='28,30-31' memory='24002400' unit='KiB'/ /numa /cpu clock offset='utc'/ diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml index ffc57cf..7542125 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml @@ -17,7 +17,7 @@ numa cell id='0' cpus='0' memory='20002' unit='KiB'/ cell id='1' cpus='1-27,29' memory='660066' unit='KiB'/ - cell id='2' cpus='28-31,^29' memory='24002400' unit='KiB'/ + cell id='2' cpus='28,30-31' memory='24002400' unit='KiB'/ /numa /cpu clock offset='utc'/ -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/4] qemu: Implement the qemu driver fetch for IOThreads
Depending on the flags passed, either attempt to return the active/live IOThread data for the domain or the config data. The active/live path will call into the Monitor in order to get the IOThread data and then correlate the thread_id's returned from the monitor to the currently running system/threads in order to ascertain the affinity for each iothread_id. The config path will map each of the configured IOThreads and return any configured iothreadspin data Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_driver.c | 217 + 1 file changed, 217 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 709f468..510ab41 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5541,6 +5541,222 @@ qemuDomainGetMaxVcpus(virDomainPtr dom) VIR_DOMAIN_VCPU_MAXIMUM)); } +static int +qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainIOThreadsInfoPtr **info) +{ +qemuDomainObjPrivatePtr priv; +qemuMonitorIOThreadsInfoPtr *iothreads = NULL; +virDomainIOThreadsInfoPtr *info_ret = NULL; +int niothreads = 0; +int maxcpu, hostcpus, maplen; +size_t i; +int ret = -1; + +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(cannot list IOThreads for an inactive domain)); +goto endjob; +} + +priv = vm-privateData; +if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(IOThreads not supported with this binary)); +goto endjob; +} + +if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) 0) +goto endjob; +niothreads = qemuMonitorGetIOThreads(priv-mon, iothreads); +if (qemuDomainObjExitMonitor(driver, vm) 0) +goto endjob; +if (niothreads 0) +goto endjob; + +/* Nothing to do */ +if (niothreads == 0) { +ret = 0; +goto endjob; +} + +if ((hostcpus = nodeGetCPUCount()) 0) +goto endjob; + +maplen = VIR_CPU_MAPLEN(hostcpus); +maxcpu = maplen * 8; +if (maxcpu hostcpus) +maxcpu = hostcpus; + +if (VIR_ALLOC_N(info_ret, niothreads) 0) +goto endjob; + +for (i = 0; i niothreads; i++) { +virBitmapPtr map = NULL; +unsigned char *tmpmap = NULL; +int tmpmaplen = 0; + +if (VIR_ALLOC(info_ret[i]) 0) +goto endjob; + +if (virStrToLong_ui(iothreads[i]-name + strlen(iothread), NULL, 10, +info_ret[i]-iothread_id) 0) +goto endjob; + +if (VIR_ALLOC_N(info_ret[i]-cpumap, maplen) 0) +goto endjob; + +if (virProcessGetAffinity(iothreads[i]-thread_id, map, maxcpu) 0) +goto endjob; + +virBitmapToData(map, tmpmap, tmpmaplen); +if (tmpmaplen maplen) +tmpmaplen = maplen; +memcpy(info_ret[i]-cpumap, tmpmap, tmpmaplen); +info_ret[i]-cpumaplen = tmpmaplen; + +VIR_FREE(tmpmap); +virBitmapFree(map); +} + +*info = info_ret; +info_ret = NULL; +ret = niothreads; + + endjob: +qemuDomainObjEndJob(driver, vm); + + cleanup: +if (info_ret) { +for (i = 0; i niothreads; i++) +virDomainIOThreadsInfoFree(info_ret[i]); +VIR_FREE(info_ret); +} + +return ret; +} + +static int +qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, + virDomainIOThreadsInfoPtr **info) +{ +virDomainIOThreadsInfoPtr *info_ret = NULL; +virDomainVcpuPinDefPtr *iothreadspin_list; +virBitmapPtr cpumask = NULL; +unsigned char *cpumap; +int maxcpu, hostcpus, maplen; +size_t i, pcpu; +bool pinned; +int ret = -1; + +if (targetDef-iothreads == 0) +return 0; + +if ((hostcpus = nodeGetCPUCount()) 0) +goto cleanup; + +maplen = VIR_CPU_MAPLEN(hostcpus); +maxcpu = maplen * 8; +if (maxcpu hostcpus) +maxcpu = hostcpus; + +if (VIR_ALLOC_N(info_ret, targetDef-iothreads) 0) +goto cleanup; + +for (i = 0; i targetDef-iothreads; i++) { +if (VIR_ALLOC(info_ret[i]) 0) +goto cleanup; + +/* IOThreads being counting at 1 */ +info_ret[i]-iothread_id = i + 1; + +if (VIR_ALLOC_N(info_ret[i]-cpumap, maplen) 0) +goto cleanup; + +/* Initialize the cpumap */ +info_ret[i]-cpumaplen = maplen; +memset(info_ret[i]-cpumap, 0xff, maplen); +if (maxcpu % 8) +info_ret[i]-cpumap[maplen - 1] = (1 maxcpu % 8) - 1; +} + +/* If iothreadspin setting exists, there are unused physical cpus */ +
[libvirt] [PATCH v2 2/4] remote: Implement the remote plumbing for virDomainGetIOThreads
Implement the remote plumbing for virDomainGetIOThreads Signed-off-by: John Ferlan jfer...@redhat.com --- daemon/remote.c | 74 +++- src/remote/remote_driver.c | 81 +++- src/remote/remote_protocol.x | 28 +-- src/remote_protocol-structs | 19 +++ src/rpc/gendispatch.pl | 1 + 5 files changed, 199 insertions(+), 4 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index d657a09..f41b1ca 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1,7 +1,7 @@ /* * remote.c: handlers for RPC method calls * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-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 @@ -2269,6 +2269,78 @@ remoteDispatchDomainGetVcpus(virNetServerPtr server ATTRIBUTE_UNUSED, } static int +remoteDispatchDomainGetIOThreadsInfo(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_iothreads_info_args *args, + remote_domain_get_iothreads_info_ret *ret) +{ +int rv = -1; +size_t i; +struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); +virDomainIOThreadsInfoPtr *info = NULL; +virDomainPtr dom = NULL; +remote_domain_iothreads_info *dst; +int ninfo = 0; + +if (!priv-conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +if ((ninfo = virDomainGetIOThreadsInfo(dom, info, args-flags)) 0) +goto cleanup; + +if (ninfo REMOTE_IOTHREADS_INFO_MAX) { +virReportError(VIR_ERR_RPC, + _(Too many IOThreads in info: %d for limit %d), + ninfo, REMOTE_IOTHREADS_INFO_MAX); +goto cleanup; +} + +if (ninfo) { +if (VIR_ALLOC_N(ret-info.info_val, ninfo) 0) +goto cleanup; + +ret-info.info_len = ninfo; + +for (i = 0; i ninfo; i++) { +dst = ret-info.info_val[i]; +dst-iothread_id = info[i]-iothread_id; + +/* No need to allocate/copy the cpumap if we make the reasonable + * assumption that unsigned char and char are the same size. + */ +dst-cpumap.cpumap_len = info[i]-cpumaplen; +dst-cpumap.cpumap_val = (char *)info[i]-cpumap; +info[i]-cpumap = NULL; +} +} else { +ret-info.info_len = 0; +ret-info.info_val = NULL; +} + +ret-ret = ninfo; + +rv = 0; + + cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +virObjectUnref(dom); +if (ninfo = 0) +for (i = 0; i ninfo; i++) +virDomainIOThreadsInfoFree(info[i]); +VIR_FREE(info); + +return rv; +} + +static int remoteDispatchDomainMigratePrepare(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d4fd658..944a590 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2,7 +2,7 @@ * remote_driver.c: driver to provide access to libvirtd running * on a remote machine * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-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 @@ -2316,6 +2316,84 @@ remoteDomainGetVcpus(virDomainPtr domain, } static int +remoteDomainGetIOThreadsInfo(virDomainPtr dom, + virDomainIOThreadsInfoPtr **info, + unsigned int flags) +{ +int rv = -1; +size_t i; +struct private_data *priv = dom-conn-privateData; +remote_domain_get_iothreads_info_args args; +remote_domain_get_iothreads_info_ret ret; +remote_domain_iothreads_info *src; +virDomainIOThreadsInfoPtr *info_ret = NULL; + +remoteDriverLock(priv); + +make_nonnull_domain(args.dom, dom); + +args.flags = flags; + +memset(ret, 0, sizeof(ret)); + +if (call(dom-conn, priv, 0, REMOTE_PROC_DOMAIN_GET_IOTHREADS_INFO, + (xdrproc_t)xdr_remote_domain_get_iothreads_info_args, + (char *)args, + (xdrproc_t)xdr_remote_domain_get_iothreads_info_ret, + (char *)ret) == -1) +goto done; + +if (ret.info.info_len REMOTE_IOTHREADS_INFO_MAX) { +
Re: [libvirt] [PATCH] Comment out variables/functions that are unused.
On 2/16/15 3:16 AM, Martin Kletzander wrote: On Fri, Feb 13, 2015 at 08:18:53PM +, Gary R Hook wrote: Avoids complaints when the compiler is configured to warn-unused. A few files contain unnecessary code that results in the compiler erroring out when -Wunused* options are used. Comment out the code until such time as it is needed. --- src/libxl/libxl_conf.c | 2 ++ tests/virnetsockettest.c | 8 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 0555b91..f8db4d2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -305,7 +305,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) regmatch_t subs[4]; char *saveptr = NULL; size_t i; +/* virArch hostarch = caps-host.arch; +*/ How come you see this unused? It is used about 100 lines later being compared to VIR_ARCH_X86_64. Ugh. You are right, and I can't believe I overlooked this. The problem is a bad Ubuntu patch that removes the reference to the variable, precipitating the compiler complaint. My apologies. I shall have to fix this in our build environment. struct guest_arch guest_archs[32]; int nr_guest_archs = 0; diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 5d91f26..988ab43 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -333,9 +333,10 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) return ret; } +/* static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED) { -virNetSocketPtr csock = NULL; /* Client socket */ +virNetSocketPtr csock = NULL; / * Client socket * / Ugly, guess what happens if someone was about uncomment the function. If you used #if 0 around the function it'd be way easier, or I might even consider removing that function, BUT... Now I'm really annoyed. Same problem here: a badly implemented patch from Ubuntu (or Debian, but I don't really care which). Let's leave this at: thank you for making me look just a bit further (as I should have to begin with) and thank you very much for your time. I am sorry to have wasted it. -- Gary R Hook Senior Kernel Engineer NIMBOXX, Inc -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] Implement public API for virDomainGetIOThreadsInfo
Add virDomainGetIOThreadsInfo in order to returned a list of virDomainIOThreadsInfoPtr structures which list the IOThread ID and the CPU Affinity map for each IOThread for domain. For an active domain, the live data will be returned, while for an inactive domain, the config data will be returned. The API supports either the --live or --config flag, but not both. Also added virDomainIOThreadsInfoFree in order to free the cpumap and the array entry structure. Signed-off-by: John Ferlan jfer...@redhat.com --- include/libvirt/libvirt-domain.h | 21 ++- src/driver-hypervisor.h | 8 - src/libvirt-domain.c | 75 +++- src/libvirt_public.syms | 6 4 files changed, 107 insertions(+), 3 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4dbd7f5..60423fd 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -4,7 +4,7 @@ * Description: Provides APIs for the management of domains * Author: Daniel Veillard veill...@redhat.com * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-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 @@ -1568,6 +1568,25 @@ int virDomainGetEmulatorPinInfo (virDomainPtr domain, unsigned int flags); /** + * virIOThreadsInfo: + * + * The data structure for information about IOThreads in a domain + */ +typedef struct _virDomainIOThreadsInfo virDomainIOThreadsInfo; +typedef virDomainIOThreadsInfo *virDomainIOThreadsInfoPtr; +struct _virDomainIOThreadsInfo { +unsigned int iothread_id; /* IOThread ID */ +unsigned char *cpumap; /* CPU map for thread */ +int cpumaplen; /* cpumap size */ +}; + +void virDomainIOThreadsInfoFree(virDomainIOThreadsInfoPtr info); + +int virDomainGetIOThreadsInfo(virDomainPtr domain, + virDomainIOThreadsInfoPtr **info, + unsigned int flags); + +/** * VIR_USE_CPU: * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN/OUT) * @cpu: the physical CPU number diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index a1198ad..2ce1a51 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1,7 +1,7 @@ /* * driver-hypervisor.h: entry points for hypervisor drivers * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-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 @@ -381,6 +381,11 @@ typedef int (*virDrvDomainGetMaxVcpus)(virDomainPtr domain); typedef int +(*virDrvDomainGetIOThreadsInfo)(virDomainPtr domain, +virDomainIOThreadsInfoPtr **info, +unsigned int flags); + +typedef int (*virDrvDomainGetSecurityLabel)(virDomainPtr domain, virSecurityLabelPtr seclabel); @@ -1254,6 +1259,7 @@ struct _virHypervisorDriver { virDrvDomainGetEmulatorPinInfo domainGetEmulatorPinInfo; virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; +virDrvDomainGetIOThreadsInfo domainGetIOThreadsInfo; virDrvDomainGetSecurityLabel domainGetSecurityLabel; virDrvDomainGetSecurityLabelList domainGetSecurityLabelList; virDrvNodeGetSecurityModel nodeGetSecurityModel; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 492e90a..4a4e392 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -1,7 +1,7 @@ /* * libvirt-domain.c: entry points for virDomainPtr APIs * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-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 @@ -7891,6 +7891,79 @@ virDomainGetMaxVcpus(virDomainPtr domain) /** + * virDomainGetIOThreadsInfo: + * @dom: a domain object + * @info: pointer to an array of virDomainIOThreadsInfo structures (OUT) + * @flags: bitwise-OR of virDomainModificationImpact + * Must not be VIR_DOMAIN_AFFECT_LIVE and + * VIR_DOMAIN_AFFECT_CONFIG concurrently. + * + * Fetch IOThreads of an active domain including the cpumap information to + * determine on which CPU the IOThread has affinity to run. + * + * Returns the number of IOThreads or -1 in case of error. + * On success, the array of information is stored into @info. The caller is + * responsible for calling virDomainIOThreadsInfoFree() on each array element, + * then calling free() on @info. On error, @info is set to NULL. + */ +int +virDomainGetIOThreadsInfo(virDomainPtr dom, +
[libvirt] [PATCH v2 4/4] virsh: Add 'iothreads' command
Add the 'iothreads' command to display IOThread Info data. Allow for [--live] or [--config] options in order to display live or config data for an active domain. An active domain may return: $ virsh iothreads $dom IOThread ID CPU Affinity - 1 2 2 3 3 0-3 $ echo $? 0 For domains which don't have IOThreads the following is returned: $ virsh iothreads $dom error: No IOThreads found for the domain $ echo $? 0 For domains which are not running the following is returned: $ virsh iothreads $dom --live error: Unable to get domain IOThreads information error: Requested operation is not valid: cannot list IOThreads for an inactive domain $ echo $? 1 Editing a running domains configuration and modifying the iothreadpin data for thread 3 from nothing provided to setting a cpuset of '0-1' and then displaying using --config could display: $ virsh iothreads f18iothr --config IOThread ID CPU Affinity - 1 2 2 3 3 0-1 $ Signed-off-by: John Ferlan jfer...@redhat.com --- tools/virsh-domain.c | 94 tools/virsh.pod | 12 +++ 2 files changed, 106 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2506b89..3b4480d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6797,6 +6797,94 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) } /* + * iothreads command + */ +static const vshCmdInfo info_iothreads[] = { +{.name = help, + .data = N_(view domain IOThreads) +}, +{.name = desc, + .data = N_(Returns basic information about the domain IOThreads.) +}, +{.name = NULL} +}; +static const vshCmdOptDef opts_iothreads[] = { +{.name = domain, + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_(domain name, id or uuid) +}, +{.name = config, + .type = VSH_OT_BOOL, + .help = N_(affect next boot) +}, +{.name = live, + .type = VSH_OT_BOOL, + .help = N_(affect running domain) +}, +{.name = current, + .type = VSH_OT_BOOL, + .help = N_(affect current domain) +}, +{.name = NULL} +}; + +static bool +cmdIOThreadsInfo(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom; +bool config = vshCommandOptBool(cmd, config); +bool live = vshCommandOptBool(cmd, live); +bool current = vshCommandOptBool(cmd, current); +int niothreads = 0; +virDomainIOThreadsInfoPtr *info; +size_t i; +int maxcpu; +unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + +VSH_EXCLUSIVE_OPTIONS_VAR(current, live); +VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + +if (config) +flags |= VIR_DOMAIN_AFFECT_CONFIG; +if (live) +flags |= VIR_DOMAIN_AFFECT_LIVE; + +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +return false; + +if ((maxcpu = vshNodeGetCPUCount(ctl-conn)) 0) +goto cleanup; + +if ((niothreads = virDomainGetIOThreadsInfo(dom, info, flags)) 0) { +vshError(ctl, _(Unable to get domain IOThreads information)); +goto cleanup; +} + +if (niothreads == 0) { +vshError(ctl, _(No IOThreads found for the domain)); +goto cleanup; +} + +vshPrintExtra(ctl, %-15s %-15s\n, + _(IOThread ID), _(CPU Affinity)); +vshPrintExtra(ctl, -\n); +for (i = 0; i niothreads; i++) { + +vshPrintExtra(ctl, %-15u , info[i]-iothread_id); +ignore_value(vshPrintPinInfo(info[i]-cpumap, info[i]-cpumaplen, + maxcpu, 0)); +vshPrint(ctl, \n); +virDomainIOThreadsInfoFree(info[i]); +} +VIR_FREE(info); + + cleanup: +virDomainFree(dom); +return niothreads = 0; +} + +/* * cpu-compare command */ static const vshCmdInfo info_cpu_compare[] = { @@ -12697,6 +12785,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_inject_nmi, .flags = 0 }, +{.name = iothreads, + .handler = cmdIOThreadsInfo, + .opts = opts_iothreads, + .info = info_iothreads, + .flags = 0 +}, {.name = send-key, .handler = cmdSendKey, .opts = opts_send_key, diff --git a/tools/virsh.pod b/tools/virsh.pod index 50de32c..6d0a6fe 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1360,6 +1360,18 @@ If I--timeout is specified, the command gives up waiting for events after Iseconds have elapsed. With I--loop, the command prints all events until a timeout or interrupt key. +=item Biothreads Idomain [[I--live] [I--config] | [I--current]] + +Display basic domain IOThreads information including the IOThread ID and +the CPU Affinity for each IOThread. + +If I--live is specified, get the IOThreads data from the running guest. If +the guest is not running, an error is returned. +If
[libvirt] [PATCH v2 0/4] Introduce display of IOThreads Information
v1: http://www.redhat.com/archives/libvir-list/2015-February/msg00402.html Changes since v1: * Remove 'thread_id' from public display * Add --live, --config, --current flags * Adjust patch 1 to accept/check the flags, modify comments * Adjust patch 3 to handle 'Live' or 'Config' options. Moved most of v1 patch code into the *Live function and add the *Config function to generate the IOThread config data * Adjust patch 4 to allow/check live/config options and set flags John Ferlan (4): Implement public API for virDomainGetIOThreadsInfo remote: Implement the remote plumbing for virDomainGetIOThreads qemu: Implement the qemu driver fetch for IOThreads virsh: Add 'iothreads' command daemon/remote.c | 74 - include/libvirt/libvirt-domain.h | 21 +++- src/driver-hypervisor.h | 8 +- src/libvirt-domain.c | 75 +- src/libvirt_public.syms | 6 ++ src/qemu/qemu_driver.c | 217 +++ src/remote/remote_driver.c | 81 ++- src/remote/remote_protocol.x | 28 - src/remote_protocol-structs | 19 src/rpc/gendispatch.pl | 1 + tools/virsh-domain.c | 94 + tools/virsh.pod | 12 +++ 12 files changed, 629 insertions(+), 7 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add support for the midonet virtualport type
On 02/16/2015 08:46 PM, Antoni Segura Puimedon wrote: From: Antoni Segura Puimedon t...@midokura.com Midonet is an opensource virtual networking that over lays the IP network between hypervisors. Currently, such networks can be made with the openvswitch virtualport type. With this patch, one will be able to setup such configurations by binding vNICs to Midonet overlay ports. Signed-off-by: Antoni Segura Puimedon t...@midokura.com --- configure.ac | 4 ++ docs/schemas/networkcommon.rng | 12 + src/Makefile.am | 1 + src/conf/domain_conf.h | 1 + src/conf/netdev_vport_profile_conf.c | 3 +- src/libvirt_private.syms | 5 ++ src/qemu/qemu_hotplug.c | 25 +++--- src/qemu/qemu_process.c | 13 +++-- src/util/virnetdevmidonet.c | 97 src/util/virnetdevmidonet.h | 37 ++ src/util/virnetdevtap.c | 11 ++-- src/util/virnetdevvportprofile.c | 1 + src/util/virnetdevvportprofile.h | 5 +- 13 files changed, 197 insertions(+), 18 deletions(-) create mode 100644 src/util/virnetdevmidonet.c create mode 100644 src/util/virnetdevmidonet.h I haven't checked the rest for completeness yet, but thought I'd point out in advance that you need to also document the new type in formatdomain.html.in. Also, I don't see any changes here to the network configuration to support a network of this type. That can be a second step though. (A very useful thing - it would allow migration of a guest from a host using a standard Linux host bridge to a host using this new network connection type, for example). You may also need to go through the hypervisor drivers other than qemu and put in explicit checks for your new virtualport type and log an UNSUPPORTED message if it's encountered. Another thing - for awhile now everyone has been putting the config/doc/schema changes in one patch, the utility functions in a second, and the glue that ties them together in qemu/lxc/etc in a third. That can make it easier to review, test, and also in the case that some other new feature might use the same utility functions it makes it easier to backport one feature without needing to backport the other. etc etc. I'll go through the code that that's here in the morning. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Enhancing block/disk migration in libvirt
On Mon, Feb 16, 2015 at 11:34:51AM +0100, Michal Privoznik wrote: On 16.02.2015 11:16, Daniel P. Berrange wrote: This doesn't really belong in the XML. The XML configs are for controlling the guest configuration, not functional behaviour of the APIs. So we need to fit any neccessary information into the migration API parameters instead, or define a new migration API for it. At minimum we just want a list of disks to be migrated. We have a migrate API that takes an array of virTypedParameter. We can use that to let users pass a list of disks to migrate. Thanks I'll look at adding that to the API. Yours Tony. pgpt_mijMKu6V.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Enhancing block/disk migration in libvirt
On Mon, Feb 16, 2015 at 11:12:14AM +0100, Kashyap Chamarthy wrote: On a related note, some weeks ago, I captured some context and analysis (including some debugging done by Dan Berrange and Eric Blake on public IRC channel mid-last year) for the above bug, since the actual bug has too many comments: https://kashyapc.fedorapeople.org/virt/openstack/nova-live-snapshot-bug-1334398-context-and-analysis.txt The cause of it is almost narrowed down here by Dan in this comment https://bugs.launchpad.net/nova/+bug/1334398/comments/27 Great thanks. Yours Tony. pgp5frJJOkybY.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Fix virFileFindResource in VPATH
Jiri Denemark (2): virFileFindResource: Add support for looking in source tree Search for schemas and cpu_map.xml in source tree src/Makefile.am | 2 ++ src/conf/domain_conf.c | 6 ++--- src/cpu/cpu_map.c | 6 ++--- src/driver.c| 1 + src/libvirt_private.syms| 1 + src/locking/lock_driver_lockd.c | 1 + src/locking/lock_manager.c | 1 + src/remote/remote_driver.c | 1 + src/util/virfile.c | 49 + src/util/virfile.h | 12 +- 10 files changed, 64 insertions(+), 16 deletions(-) -- 2.3.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] virFileFindResource: Add support for looking in source tree
Not all file we want to find using virFileFindResource{,Full} are generated when libvirt is built, some of them (such as RNG schemas) are distributed with sources. The current API was not able to find source files if libvirt was built in VPATH. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/Makefile.am | 2 ++ src/driver.c| 1 + src/locking/lock_driver_lockd.c | 1 + src/locking/lock_manager.c | 1 + src/remote/remote_driver.c | 1 + src/util/virfile.c | 39 ++- src/util/virfile.h | 8 +++- 7 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..a938d7e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,6 +20,7 @@ abs_builddir = $(shell pwd) abs_topbuilddir = $(shell cd .. pwd) abs_srcdir = $(shell cd $(srcdir) pwd) +abs_topsrcdir = $(shell cd $(srcdir)/.. pwd) # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets @@ -32,6 +33,7 @@ INCLUDES =-I../gnulib/lib \ -I$(srcdir)/util\ -DIN_LIBVIRT\ -Dabs_topbuilddir=\$(abs_topbuilddir)\ \ + -Dabs_topsrcdir=\$(abs_topsrcdir)\ \ $(GETTEXT_CPPFLAGS) AM_CFLAGS =$(LIBXML_CFLAGS)\ diff --git a/src/driver.c b/src/driver.c index 1be32ef..1271597 100644 --- a/src/driver.c +++ b/src/driver.c @@ -57,6 +57,7 @@ virDriverLoadModule(const char *name) libvirt_driver_, .so, src/.libs, +VIR_FILE_FIND_RESOURCE_VPATH_BUILD, LIBDIR /libvirt/connection-driver, LIBVIRT_DRIVER_DIR))) return NULL; diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 2af3f22..53a7256 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -254,6 +254,7 @@ static virNetClientPtr virLockManagerLockDaemonConnectionNew(bool privileged, !(daemonPath = virFileFindResourceFull(virtlockd, NULL, NULL, src, + VIR_FILE_FIND_RESOURCE_VPATH_BUILD, SBINDIR, VIRTLOCKD_PATH))) goto error; diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index ec90d04..554e9e2 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -143,6 +143,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name, NULL, .so, src/.libs, + VIR_FILE_FIND_RESOURCE_VPATH_BUILD, LIBDIR /libvirt/lock-driver, LIBVIRT_LOCK_MANAGER_PLUGIN_DIR))) goto cleanup; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d4fd658..4830a48 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -888,6 +888,7 @@ doRemoteOpen(virConnectPtr conn, !(daemonPath = virFileFindResourceFull(libvirtd, NULL, NULL, daemon, + VIR_FILE_FIND_RESOURCE_VPATH_BUILD, SBINDIR, LIBVIRTD_PATH))) goto failed; diff --git a/src/util/virfile.c b/src/util/virfile.c index 1b004d6..e60896f 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1618,7 +1618,8 @@ static bool useDirOverride; * @filename: libvirt distributed filename without any path * @prefix: optional string to prepend to filename * @suffix: optional string to append to filename - * @builddir: location of the binary in the source tree build tree + * @builddir: location of the filename in the build tree + * @vpath: switch between source and build directory in VPATH build * @installdir: location of the installed binary * @envname: environment variable used to override all dirs * @@ -1627,8 +1628,14 @@ static bool useDirOverride; * run from the source tree. Otherwise it will return the * path in the installed location. * + * If libvirt was built in
[libvirt] [PATCH 2/2] Search for schemas and cpu_map.xml in source tree
Both RNG schemas and cpu_map.xml are distributed in source tarball. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/conf/domain_conf.c | 6 +++--- src/cpu/cpu_map.c| 6 +++--- src/libvirt_private.syms | 1 + src/util/virfile.c | 10 ++ src/util/virfile.h | 4 5 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b13cae8..b3d63f8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12886,9 +12886,9 @@ virDomainDefParseXML(xmlDocPtr xml, bool primaryVideo = false; if (flags VIR_DOMAIN_DEF_PARSE_VALIDATE) { -char *schema = virFileFindResource(domain.rng, - docs/schemas, - PKGDATADIR /schemas); +char *schema = virFileFindResourceSrc(domain.rng, + docs/schemas, + PKGDATADIR /schemas); if (!schema) return NULL; if (virXMLValidateAgainstSchema(schema, xml) 0) { diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index b77f688..8c005d4 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -86,9 +86,9 @@ int cpuMapLoad(const char *arch, int element; char *mapfile; -if (!(mapfile = virFileFindResource(cpu_map.xml, -src/cpu, -PKGDATADIR))) +if (!(mapfile = virFileFindResourceSrc(cpu_map.xml, + src/cpu, + PKGDATADIR))) return -1; VIR_DEBUG(Loading CPU map from %s, mapfile); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c07a561..488cde6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1346,6 +1346,7 @@ virFileFindHugeTLBFS; virFileFindMountPoint; virFileFindResource; virFileFindResourceFull; +virFileFindResourceSrc; virFileGetHugepageSize; virFileGetMountReverseSubtree; virFileGetMountSubtree; diff --git a/src/util/virfile.c b/src/util/virfile.c index e60896f..fc3d0a2 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1695,6 +1695,16 @@ virFileFindResource(const char *filename, installdir, NULL); } +char * +virFileFindResourceSrc(const char *filename, + const char *srcdir, + const char *installdir) +{ +return virFileFindResourceFull(filename, NULL, NULL, + srcdir, VIR_FILE_FIND_RESOURCE_VPATH_SOURCE, + installdir, NULL); +} + /** * virFileActivateDirOverride: diff --git a/src/util/virfile.h b/src/util/virfile.h index 0e481c2..ddce797 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -172,6 +172,10 @@ char *virFileFindResource(const char *filename, const char *builddir, const char *installdir) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +char *virFileFindResourceSrc(const char *filename, + const char *srcdir, + const char *installdir) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); char *virFileFindResourceFull(const char *filename, const char *prefix, const char *suffix, -- 2.3.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] virFileFindResource: Add support for looking in source tree
On Mon, Feb 16, 2015 at 03:11:29PM +0100, Jiri Denemark wrote: Not all file we want to find using virFileFindResource{,Full} are generated when libvirt is built, some of them (such as RNG schemas) are distributed with sources. The current API was not able to find source files if libvirt was built in VPATH. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/Makefile.am | 2 ++ src/driver.c| 1 + src/locking/lock_driver_lockd.c | 1 + src/locking/lock_manager.c | 1 + src/remote/remote_driver.c | 1 + src/util/virfile.c | 39 ++- src/util/virfile.h | 8 +++- 7 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..a938d7e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,6 +20,7 @@ abs_builddir = $(shell pwd) abs_topbuilddir = $(shell cd .. pwd) abs_srcdir = $(shell cd $(srcdir) pwd) +abs_topsrcdir = $(shell cd $(srcdir)/.. pwd) # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets @@ -32,6 +33,7 @@ INCLUDES = -I../gnulib/lib \ -I$(srcdir)/util\ -DIN_LIBVIRT\ -Dabs_topbuilddir=\$(abs_topbuilddir)\ \ + -Dabs_topsrcdir=\$(abs_topsrcdir)\ \ $(GETTEXT_CPPFLAGS) AM_CFLAGS = $(LIBXML_CFLAGS)\ diff --git a/src/driver.c b/src/driver.c index 1be32ef..1271597 100644 --- a/src/driver.c +++ b/src/driver.c @@ -57,6 +57,7 @@ virDriverLoadModule(const char *name) libvirt_driver_, .so, src/.libs, + VIR_FILE_FIND_RESOURCE_VPATH_BUILD, Instead of adding yet another parameter, why don't we just change the calling convention of virFileFindResource so that instead of assuming abs_topbuilddir we just make the caller prepend the builddir. eg instead of pasing 'src/.libs' we pass abs_topbuilddir /src/.libs letting the compiler concatenate these. The enum you define is basically doing exactly that but with an extra level of indirection which doesn't seem to buy us anything @@ -1640,11 +1647,14 @@ virFileFindResourceFull(const char *filename, const char *prefix, const char *suffix, const char *builddir, +virFileFindResourceVPath vpath, const char *installdir, const char *envname) { char *ret = NULL; const char *envval = envname ? virGetEnvBlockSUID(envname) : NULL; +const char *base = ; +const char *path = ; if (!prefix) prefix = ; @@ -1652,16 +1662,25 @@ virFileFindResourceFull(const char *filename, suffix = ; if (envval) { -if (virAsprintf(ret, %s/%s%s%s, envval, prefix, filename, suffix) 0) -return NULL; +base = envval; } else if (useDirOverride) { -if (virAsprintf(ret, %s/%s/%s%s%s, abs_topbuilddir, builddir, prefix, filename, suffix) 0) -return NULL; eg, simply change this to be if (virAsprintf(ret, %s/%s/%s%s%s, builddir, prefix, filename, suffix) 0) return NULL; 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] virsh: fix IP address in vncdisplay for listen type='network'
On 02/16/2015 06:51 PM, Pavel Hrdina wrote: On Sun, Feb 15, 2015 at 04:49:09PM +0800, Luyao Huang wrote: Just like the fix for domdisplay in commit 1ba815. --- tools/virsh-domain.c | 12 1 file changed, 12 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index dc4a863..2506b89 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10269,6 +10269,18 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd) listen_addr = virXPathString(string(/domain/devices/graphics [@type='vnc']/@listen), ctxt); +if (!listen_addr) { +/* The subelement address - listen address='xyz'/ - + * *should* have been automatically backfilled into its + * parent graphics listen='xyz' (which we just tried to + * retrieve into listen_addr above) but in some cases it + * isn't, so we also do an explicit check for the + * subelement (which, by the way, doesn't exist on libvirt + * 0.9.4, so we really do need to check both places) + */ +listen_addr = virXPathString(string(/domain/devices/graphics + [@type='vnc']/listen/@address), ctxt); +} if (listen_addr == NULL || STREQ(listen_addr, 0.0.0.0)) vshPrint(ctl, :%d\n, port-5900); else -- 1.8.3.1 ACK and pushed. Thanks for the quick review. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix vm deadlock when try to use numatune in session mode
https://bugzilla.redhat.com/show_bug.cgi?id=1126762 commit 43b67f introduce a deadlock issue when we use numatune to change numa settings to a vm in session mode. Jump to endjob instead of jump to cleanup. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 709f468..1bbbe9b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9495,7 +9495,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, flags VIR_DOMAIN_AFFECT_LIVE) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, _(NUMA tuning is not available in session mode)); -goto cleanup; +goto endjob; } if (flags VIR_DOMAIN_AFFECT_LIVE) { -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] Use PAUSED state for domains that are starting up
On Mon, Feb 16, 2015 at 03:50:41PM +0100, Jiri Denemark wrote: When libvirt is starting a domain, it reports the state as SHUTOFF until it's RUNNING. This is not ideal because domain startup may take a long time (usually because of some configuration issues, firewalls blocking access to network disks, etc.) and domain lists provided by libvirt look awkward. One can see weird shutoff domains with IDs in a list of active domains or even shutoff transient domains. In any case, it looks more like a bug in libvirt than a normal state a domain goes through. A shutoff transient domain isn't too bad IMHO, but a shutoff domain with an ID number is definitely not expected. Could we perhaps address it by ensuring that we always return '-1' for ID if the state is SHUTOFF, even if def-id has a positive value ? 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] [RFC PATCH] Use PAUSED state for domains that are starting up
When libvirt is starting a domain, it reports the state as SHUTOFF until it's RUNNING. This is not ideal because domain startup may take a long time (usually because of some configuration issues, firewalls blocking access to network disks, etc.) and domain lists provided by libvirt look awkward. One can see weird shutoff domains with IDs in a list of active domains or even shutoff transient domains. In any case, it looks more like a bug in libvirt than a normal state a domain goes through. I'm not quite sure what the best way to fix this is. In this patch, I tried to use PAUSED state with STARTING_UP reason. Alternatively, we could keep using SHUTOFF state and just set STARTING_UP reason instead of UNKNOWN but it just feels wrong and wouldn't really solve the confusion when looking at virsh list. I made the change to qemu driver only in this RFC patch, I will update all drivers once we agree on the best approach. Signed-off-by: Jiri Denemark jdene...@redhat.com --- include/libvirt/libvirt-domain.h | 1 + src/conf/domain_conf.c | 3 ++- src/qemu/qemu_process.c | 22 ++ tools/virsh-domain-monitor.c | 3 ++- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4dbd7f5..90150f6 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -116,6 +116,7 @@ typedef enum { VIR_DOMAIN_PAUSED_SHUTTING_DOWN = 8, /* paused during shutdown process */ VIR_DOMAIN_PAUSED_SNAPSHOT = 9, /* paused while creating a snapshot */ VIR_DOMAIN_PAUSED_CRASHED = 10, /* paused due to a guest crash */ +VIR_DOMAIN_PAUSED_STARTING_UP = 11, /* the domain is being started */ # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_PAUSED_LAST diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b3d63f8..2b7c5bf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -661,7 +661,8 @@ VIR_ENUM_IMPL(virDomainPausedReason, VIR_DOMAIN_PAUSED_LAST, from snapshot, shutdown, snapshot, - panicked) + panicked, + starting up) VIR_ENUM_IMPL(virDomainShutdownReason, VIR_DOMAIN_SHUTDOWN_LAST, unknown, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1d4e957..d317b19 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3412,6 +3412,7 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm) virDomainState state; virDomainPausedReason reason; virDomainState newState = VIR_DOMAIN_NOSTATE; +int oldReason; int newReason; bool running; char *msg = NULL; @@ -3425,9 +3426,16 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm) if (ret 0) return -1; -state = virDomainObjGetState(vm, NULL); +state = virDomainObjGetState(vm, oldReason); -if (state == VIR_DOMAIN_PAUSED running) { +if (running +(state == VIR_DOMAIN_SHUTOFF || + (state == VIR_DOMAIN_PAUSED + oldReason == VIR_DOMAIN_PAUSED_STARTING_UP))) { +newState = VIR_DOMAIN_RUNNING; +newReason = VIR_DOMAIN_RUNNING_BOOTED; +ignore_value(VIR_STRDUP_QUIET(msg, finished booting)); +} else if (state == VIR_DOMAIN_PAUSED running) { newState = VIR_DOMAIN_RUNNING; newReason = VIR_DOMAIN_RUNNING_UNPAUSED; ignore_value(VIR_STRDUP_QUIET(msg, was unpaused)); @@ -3446,10 +3454,6 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm) ignore_value(virAsprintf(msg, was paused (%s), virDomainPausedReasonTypeToString(reason))); } -} else if (state == VIR_DOMAIN_SHUTOFF running) { -newState = VIR_DOMAIN_RUNNING; -newReason = VIR_DOMAIN_RUNNING_BOOTED; -ignore_value(VIR_STRDUP_QUIET(msg, finished booting)); } if (newState != VIR_DOMAIN_NOSTATE) { @@ -3817,7 +3821,9 @@ qemuProcessReconnect(void *opaque) goto error; state = virDomainObjGetState(obj, reason); -if (state == VIR_DOMAIN_SHUTOFF) { +if (state == VIR_DOMAIN_SHUTOFF || +(state == VIR_DOMAIN_PAUSED + reason == VIR_DOMAIN_PAUSED_STARTING_UP)) { VIR_DEBUG(Domain '%s' wasn't fully started yet, killing it, obj-def-name); goto error; @@ -4435,7 +4441,7 @@ int qemuProcessStart(virConnectPtr conn, vm-def-id = qemuDriverAllocateID(driver); qemuDomainSetFakeReboot(driver, vm, false); -virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_UNKNOWN); +virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); if (virAtomicIntInc(driver-nactive) == 1 driver-inhibitCallback) driver-inhibitCallback(true, driver-inhibitOpaque); diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index
Re: [libvirt] [PATCH 1/2] virFileFindResource: Add support for looking in source tree
On Mon, Feb 16, 2015 at 14:26:56 +, Daniel P. Berrange wrote: On Mon, Feb 16, 2015 at 03:11:29PM +0100, Jiri Denemark wrote: Not all file we want to find using virFileFindResource{,Full} are generated when libvirt is built, some of them (such as RNG schemas) are distributed with sources. The current API was not able to find source files if libvirt was built in VPATH. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/Makefile.am | 2 ++ src/driver.c| 1 + src/locking/lock_driver_lockd.c | 1 + src/locking/lock_manager.c | 1 + src/remote/remote_driver.c | 1 + src/util/virfile.c | 39 ++- src/util/virfile.h | 8 +++- 7 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..a938d7e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,6 +20,7 @@ abs_builddir = $(shell pwd) abs_topbuilddir = $(shell cd .. pwd) abs_srcdir = $(shell cd $(srcdir) pwd) +abs_topsrcdir = $(shell cd $(srcdir)/.. pwd) # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets @@ -32,6 +33,7 @@ INCLUDES =-I../gnulib/lib \ -I$(srcdir)/util\ -DIN_LIBVIRT\ -Dabs_topbuilddir=\$(abs_topbuilddir)\ \ + -Dabs_topsrcdir=\$(abs_topsrcdir)\ \ $(GETTEXT_CPPFLAGS) AM_CFLAGS =$(LIBXML_CFLAGS)\ diff --git a/src/driver.c b/src/driver.c index 1be32ef..1271597 100644 --- a/src/driver.c +++ b/src/driver.c @@ -57,6 +57,7 @@ virDriverLoadModule(const char *name) libvirt_driver_, .so, src/.libs, + VIR_FILE_FIND_RESOURCE_VPATH_BUILD, Instead of adding yet another parameter, why don't we just change the calling convention of virFileFindResource so that instead of assuming abs_topbuilddir we just make the caller prepend the builddir. eg instead of pasing 'src/.libs' we pass abs_topbuilddir /src/.libs letting the compiler concatenate these. The enum you define is basically doing exactly that but with an extra level of indirection which doesn't seem to buy us anything Yeah, I guess, that would work too, although it will either need one more allocation or making all calls to the non-Full variant of this API more complex. But we can live with it since virFileFind* APIs are used at about 11 places... Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] Use PAUSED state for domains that are starting up
On Mon, Feb 16, 2015 at 14:57:17 +, Daniel P. Berrange wrote: On Mon, Feb 16, 2015 at 03:50:41PM +0100, Jiri Denemark wrote: When libvirt is starting a domain, it reports the state as SHUTOFF until it's RUNNING. This is not ideal because domain startup may take a long time (usually because of some configuration issues, firewalls blocking access to network disks, etc.) and domain lists provided by libvirt look awkward. One can see weird shutoff domains with IDs in a list of active domains or even shutoff transient domains. In any case, it looks more like a bug in libvirt than a normal state a domain goes through. A shutoff transient domain isn't too bad IMHO, but a shutoff domain with an ID number is definitely not expected. Could we perhaps address it by ensuring that we always return '-1' for ID if the state is SHUTOFF, even if def-id has a positive value ? But we should somehow make it clear that the domain is actually there, somehow, only not completely usable. That is, one may need to actually call virsh destroy on such domain to get rid of the leftover process if something goes wrong. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] Use PAUSED state for domains that are starting up
On Mon, Feb 16, 2015 at 04:03:50PM +0100, Jiri Denemark wrote: On Mon, Feb 16, 2015 at 14:57:17 +, Daniel P. Berrange wrote: On Mon, Feb 16, 2015 at 03:50:41PM +0100, Jiri Denemark wrote: When libvirt is starting a domain, it reports the state as SHUTOFF until it's RUNNING. This is not ideal because domain startup may take a long time (usually because of some configuration issues, firewalls blocking access to network disks, etc.) and domain lists provided by libvirt look awkward. One can see weird shutoff domains with IDs in a list of active domains or even shutoff transient domains. In any case, it looks more like a bug in libvirt than a normal state a domain goes through. A shutoff transient domain isn't too bad IMHO, but a shutoff domain with an ID number is definitely not expected. Could we perhaps address it by ensuring that we always return '-1' for ID if the state is SHUTOFF, even if def-id has a positive value ? But we should somehow make it clear that the domain is actually there, somehow, only not completely usable. That is, one may need to actually call virsh destroy on such domain to get rid of the leftover process if something goes wrong. Hmm, if something goes wrong due virDomainStart though, we should be tearing down the QEMU process. IIRC we should even be kill -9'ing QEMU, so even if QEMU is stuck in an uninterruptable sleep and won't exit, once the (storage?) problem causing that sleep is resolved QEMU will exit without further intervention. Similarly calling 'destroy' more times won't make it any more likely to quit, once it has had a SIGKILL 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/2] Document virtlockd
On 16.02.2015 12:59, Daniel P. Berrange wrote: Just some docs additions wrt virtlockd Daniel P. Berrange (2): docs: split out sanlock setup docs docs: add page about virtlockd setup docs/locking-lockd.html.in | 160 + docs/locking-sanlock.html.in | 247 ++ docs/locking.html.in | 277 ++- docs/sitemap.html.in | 10 ++ 4 files changed, 450 insertions(+), 244 deletions(-) create mode 100644 docs/locking-lockd.html.in create mode 100644 docs/locking-sanlock.html.in ACK to both Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Enhancing block/disk migration in libvirt
On 16.02.2015 11:16, Daniel P. Berrange wrote: On Mon, Feb 16, 2015 at 01:42:13PM +1100, Tony Breeds wrote: Hello all, I'm new to both openstack and libvirt so I may get some of this slightly wrong[1]. Here is some context form the openstack world (which at least some of you are aware of). There are at least 2 open bug against openstack (nova) in the area of block/disk migration. 1) Live migration fails when the instance has a config-drive[2] Here openstack(nova) fails because a drive that nova expects to be migrated isn't migrated. 2) libvirt live_snapshot periodically explodes on libvirt 1.2.2 in the gate[3] Here openstack(nova) fails because a drive that nova expects NOT to be migrated is migrated. To me these are essentially the same bug/issue. There is no way to communicate with libvirt the users expectations around block/disk mirgration. My idea so far would be to add an options element to the 'disk' XML node. This element could start with 3 possible states block_migration=default: Let libvirt decide block_migration=yes: This device should be block migrated block_migration=no: This device should *NOT* be block migrated The absence of this element would be treated as default above. This would mean that all existing domain XML would still be valid and have the expected behaviour and users (such as opensatck) can be explicit about deviced that do/do not need to be block migrated. While I'm certainly open to discussing the finer points of the implementation, right now I'm interested in getting a feel for is this idea generally ok? This doesn't really belong in the XML. The XML configs are for controlling the guest configuration, not functional behaviour of the APIs. So we need to fit any neccessary information into the migration API parameters instead, or define a new migration API for it. At minimum we just want a list of disks to be migrated. We have a migrate API that takes an array of virTypedParameter. We can use that to let users pass a list of disks to migrate. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix IP address in vncdisplay for listen type='network'
On Sun, Feb 15, 2015 at 04:49:09PM +0800, Luyao Huang wrote: Just like the fix for domdisplay in commit 1ba815. --- tools/virsh-domain.c | 12 1 file changed, 12 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index dc4a863..2506b89 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10269,6 +10269,18 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd) listen_addr = virXPathString(string(/domain/devices/graphics [@type='vnc']/@listen), ctxt); +if (!listen_addr) { +/* The subelement address - listen address='xyz'/ - + * *should* have been automatically backfilled into its + * parent graphics listen='xyz' (which we just tried to + * retrieve into listen_addr above) but in some cases it + * isn't, so we also do an explicit check for the + * subelement (which, by the way, doesn't exist on libvirt + * 0.9.4, so we really do need to check both places) + */ +listen_addr = virXPathString(string(/domain/devices/graphics + [@type='vnc']/listen/@address), ctxt); +} if (listen_addr == NULL || STREQ(listen_addr, 0.0.0.0)) vshPrint(ctl, :%d\n, port-5900); else -- 1.8.3.1 ACK and pushed. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libxl and non-absolute paths
On 16.02.2015 10:18, Martin Kletzander wrote: On Fri, Feb 13, 2015 at 03:20:07PM +0100, Stefan Bader wrote: Just recently we moved to libvirt 1.2.12 for the next release. Which brought up a few problems when working with configs which we and Debian used to have. A mild complaint towards the xml validation: it would be really nice of that would be a bit more specific about what exactly it complains. It took me a while to realize that Extra element os in interleave was trying to tell me that the string of the loader element within the os section was not an absolute path. The issue here is that with libxl, I think the goal was to rather allow the library to select the path prefix (like for pygrub where the full path got removed recently). But now the xml validation disagrees. This would go for bootloader for xenpv and loader (within os) for xenfv. And for emulator in the device section. Though for that things are a bit more complicated. The libxl driver now calls that with the help option and decides from the output whether this is the traditional xen forked qemu or the upstream qemu binary. Then it selects the device model depending on that outcome. Not sure whether the libxl driver could query libxl for the path prefix. Right now the most straight forward way seems to move back to a full path for the emulator. At least now, by using the standard qemu binary for everything, we got a predictable path that does not change with Xen versions. So its possible to force migrate over to put /usr/bin/qemu-system-i386 there. But for loader and bootloader, do you think it reasonable to change the templates from absFilePath to filePath? Maybe stupid question here... How does the string with the prefix look like then? Is it something like bootloaderpygrub:/path/to/loader ? No, sorry I should probably have added that: in both cases there is only the binary name in the config and libxl extends things internally. So bootloaderpygrub/bootloader and loaderhvmloader/loader. -Stefan -Stefan --- libvirt-1.2.12.orig/docs/schemas/domaincommon.rng 2015-01-23 12:46:24. +++ libvirt-1.2.12/docs/schemas/domaincommon.rng2015-02-13 10:00:43.1616 @@ -258,7 +258,7 @@ /choice /attribute /optional -ref name=absFilePath/ +ref name=filePath/ /element /optional optional @@ -1060,7 +1060,7 @@ optional element name=bootloader choice -ref name=absFilePath/ +ref name=filePath/ empty/ /choice /element -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Comment out variables/functions that are unused.
On Fri, Feb 13, 2015 at 08:18:53PM +, Gary R Hook wrote: Avoids complaints when the compiler is configured to warn-unused. A few files contain unnecessary code that results in the compiler erroring out when -Wunused* options are used. Comment out the code until such time as it is needed. --- src/libxl/libxl_conf.c | 2 ++ tests/virnetsockettest.c | 8 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 0555b91..f8db4d2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -305,7 +305,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) regmatch_t subs[4]; char *saveptr = NULL; size_t i; +/* virArch hostarch = caps-host.arch; +*/ How come you see this unused? It is used about 100 lines later being compared to VIR_ARCH_X86_64. struct guest_arch guest_archs[32]; int nr_guest_archs = 0; diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 5d91f26..988ab43 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -333,9 +333,10 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) return ret; } +/* static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED) { -virNetSocketPtr csock = NULL; /* Client socket */ +virNetSocketPtr csock = NULL; / * Client socket * / Ugly, guess what happens if someone was about uncomment the function. If you used #if 0 around the function it'd be way easier, or I might even consider removing that function, BUT... char buf[100]; size_t i; int ret = -1; @@ -360,10 +361,12 @@ static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED) virObjectUnref(csock); return ret; } +*/ +/* static int testSocketCommandFail(const void *data ATTRIBUTE_UNUSED) { -virNetSocketPtr csock = NULL; /* Client socket */ +virNetSocketPtr csock = NULL; / * Client socket * / char buf[100]; int ret = -1; virCommandPtr cmd = virCommandNewArgList(/bin/cat, /dev/does-not-exist, NULL); @@ -383,6 +386,7 @@ static int testSocketCommandFail(const void *data ATTRIBUTE_UNUSED) virObjectUnref(csock); return ret; } +*/ .. both of these functions *are* used in the file. At least everywhere else than on win32 platforms. If that's the problem, then it should be in #ifndef WIN32 the same way as the tests using it are. struct testSSHData { const char *nodename; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list pgplxw1N2Pa1f.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] SRIOV NIC offload feature discovery
Adding functionality to libvirt that will allow it query the ethtool interface for the availability of certain NIC HW offload features --- src/conf/device_conf.h | 6 ++ src/conf/node_device_conf.c| 7 ++ src/conf/node_device_conf.h| 2 + src/libvirt_private.syms | 1 + src/node_device/node_device_udev.c | 4 ++ src/util/virnetdev.c | 134 + src/util/virnetdev.h | 12 +++- 7 files changed, 165 insertions(+), 1 deletion(-) diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index 7256cdc..091f2f0 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -62,6 +62,12 @@ struct _virInterfaceLink { unsigned int speed; /* link speed in Mbits per second */ }; +typedef struct _virDevFeature virDevFeature; +typedef virDevFeature *virDevFeaturePtr; +struct _virDevFeature { + char *name; /* device feature */ +}; + int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr); int virDevicePCIAddressParseXML(xmlNodePtr node, diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index a728a00..7f4dbfe 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -437,6 +437,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferEscapeString(buf, address%s/address\n, data-net.address); virInterfaceLinkFormat(buf, data-net.lnk); +if (data-net.features) { +for (i = 0; i data-net.nfeatures; i++) { +virBufferAsprintf(buf, feature name='%s'/\n, +data-net.features[i].name); +} +} if (data-net.subtype != VIR_NODE_DEV_CAP_NET_LAST) { const char *subtyp = virNodeDevNetCapTypeToString(data-net.subtype); @@ -1679,6 +1685,7 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case VIR_NODE_DEV_CAP_NET: VIR_FREE(data-net.ifname); VIR_FREE(data-net.address); +VIR_FREE(data-net.features); break; case VIR_NODE_DEV_CAP_SCSI_HOST: VIR_FREE(data-scsi_host.wwnn); diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index fd5d179..918523a 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -141,6 +141,8 @@ struct _virNodeDevCapsDef { char *ifname; virInterfaceLink lnk; virNodeDevNetCapType subtype; /* LAST - no subtype */ +size_t nfeatures; +virDevFeaturePtr features; } net; struct { unsigned int host; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c07a561..1d165a9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1659,6 +1659,7 @@ virNetDevAddRoute; virNetDevClearIPAddress; virNetDevDelMulti; virNetDevExists; +virNetDevGetFeatures; virNetDevGetIndex; virNetDevGetIPv4Address; virNetDevGetLinkInfo; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 03c7a0b..349733f 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -719,6 +719,10 @@ static int udevProcessNetworkInterface(struct udev_device *device, if (virNetDevGetLinkInfo(data-net.ifname, data-net.lnk) 0) goto out; +if (virNetDevGetFeatures(data-net.ifname, data-net.features, +data-net.nfeatures) 0) +goto out; + ret = 0; out: diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 2a0eb43..e513c85 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2728,3 +2728,137 @@ int virNetDevGetRxFilter(const char *ifname, *filter = fil; return ret; } + +#if defined(SIOCETHTOOL) defined(HAVE_STRUCT_IFREQ) + +/** + * _virNetDevFeatureAvailable + * This function checks for the availability of a network device feature + * + * @ifname: name of the interface + * @cmd: reference to an ethtool command structure + * + * Returns 0 on success, -1 on failure. + */ +int +_virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) +{ +int ret = -1; +int sock = -1; +virIfreq ifr; + +sock = socket(AF_LOCAL, SOCK_DGRAM, 0); +if (sock 0) { +virReportSystemError(errno, %s, _(Cannot open control socket)); +goto cleanup; +} + +strcpy(ifr.ifr_name, ifname); +ifr.ifr_data = (void*) cmd; + +if (ioctl(sock, SIOCETHTOOL, ifr) != 0) { +/* Privileged command, no error */ +if (errno == EPERM || errno == EINVAL) { +virReportSystemError(errno, %s, _(ioctl)); +/* Some kernels dont support named feature, no error */ +} else if (errno == EOPNOTSUPP) { +virReportSystemError(errno, %s, _(Warning)); +} else { +virReportSystemError(errno, %s, _(Error));
Re: [libvirt] libxl and non-absolute paths
On Fri, Feb 13, 2015 at 03:20:07PM +0100, Stefan Bader wrote: Just recently we moved to libvirt 1.2.12 for the next release. Which brought up a few problems when working with configs which we and Debian used to have. A mild complaint towards the xml validation: it would be really nice of that would be a bit more specific about what exactly it complains. It took me a while to realize that Extra element os in interleave was trying to tell me that the string of the loader element within the os section was not an absolute path. The issue here is that with libxl, I think the goal was to rather allow the library to select the path prefix (like for pygrub where the full path got removed recently). But now the xml validation disagrees. This would go for bootloader for xenpv and loader (within os) for xenfv. And for emulator in the device section. Though for that things are a bit more complicated. The libxl driver now calls that with the help option and decides from the output whether this is the traditional xen forked qemu or the upstream qemu binary. Then it selects the device model depending on that outcome. Not sure whether the libxl driver could query libxl for the path prefix. Right now the most straight forward way seems to move back to a full path for the emulator. At least now, by using the standard qemu binary for everything, we got a predictable path that does not change with Xen versions. So its possible to force migrate over to put /usr/bin/qemu-system-i386 there. But for loader and bootloader, do you think it reasonable to change the templates from absFilePath to filePath? Maybe stupid question here... How does the string with the prefix look like then? Is it something like bootloaderpygrub:/path/to/loader ? -Stefan --- libvirt-1.2.12.orig/docs/schemas/domaincommon.rng 2015-01-23 12:46:24. +++ libvirt-1.2.12/docs/schemas/domaincommon.rng2015-02-13 10:00:43.1616 @@ -258,7 +258,7 @@ /choice /attribute /optional -ref name=absFilePath/ +ref name=filePath/ /element /optional optional @@ -1060,7 +1060,7 @@ optional element name=bootloader choice -ref name=absFilePath/ +ref name=filePath/ empty/ /choice /element -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list pgp_uPbu5fD5M.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Enhancing block/disk migration in libvirt
On Mon, Feb 16, 2015 at 01:42:13PM +1100, Tony Breeds wrote: Hello all, I'm new to both openstack and libvirt so I may get some of this slightly wrong[1]. Here is some context form the openstack world (which at least some of you are aware of). There are at least 2 open bug against openstack (nova) in the area of block/disk migration. 1) Live migration fails when the instance has a config-drive[2] Here openstack(nova) fails because a drive that nova expects to be migrated isn't migrated. 2) libvirt live_snapshot periodically explodes on libvirt 1.2.2 in the gate[3] Here openstack(nova) fails because a drive that nova expects NOT to be migrated is migrated. To me these are essentially the same bug/issue. There is no way to communicate with libvirt the users expectations around block/disk mirgration. My idea so far would be to add an options element to the 'disk' XML node. This element could start with 3 possible states block_migration=default: Let libvirt decide block_migration=yes: This device should be block migrated block_migration=no: This device should *NOT* be block migrated The absence of this element would be treated as default above. This would mean that all existing domain XML would still be valid and have the expected behaviour and users (such as opensatck) can be explicit about deviced that do/do not need to be block migrated. While I'm certainly open to discussing the finer points of the implementation, right now I'm interested in getting a feel for is this idea generally ok? This doesn't really belong in the XML. The XML configs are for controlling the guest configuration, not functional behaviour of the APIs. So we need to fit any neccessary information into the migration API parameters instead, or define a new migration API for it. At minimum we just want a list of disks to be migrated. 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] Enhancing block/disk migration in libvirt
On Mon, Feb 16, 2015 at 01:42:13PM +1100, Tony Breeds wrote: Hello all, I'm new to both openstack and libvirt so I may get some of this slightly wrong[1]. Here is some context form the openstack world (which at least some of you are aware of). There are at least 2 open bug against openstack (nova) in the area of block/disk migration. 1) Live migration fails when the instance has a config-drive[2] Here openstack(nova) fails because a drive that nova expects to be migrated isn't migrated. 2) libvirt live_snapshot periodically explodes on libvirt 1.2.2 in the gate[3] Here openstack(nova) fails because a drive that nova expects NOT to be migrated is migrated. On a related note, some weeks ago, I captured some context and analysis (including some debugging done by Dan Berrange and Eric Blake on public IRC channel mid-last year) for the above bug, since the actual bug has too many comments: https://kashyapc.fedorapeople.org/virt/openstack/nova-live-snapshot-bug-1334398-context-and-analysis.txt The cause of it is almost narrowed down here by Dan in this comment https://bugs.launchpad.net/nova/+bug/1334398/comments/27 -- /kashyap -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] SRIOV NIC offload feature discovery
Adding functionality to libvirt that will allow it query the ethtool interface for the availability of certain NIC HW offload features --- src/conf/device_conf.h | 6 ++ src/conf/node_device_conf.c| 7 ++ src/conf/node_device_conf.h| 2 + src/libvirt_private.syms | 1 + src/node_device/node_device_udev.c | 4 ++ src/util/virnetdev.c | 134 + src/util/virnetdev.h | 12 +++- 7 files changed, 165 insertions(+), 1 deletion(-) diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index 7256cdc..091f2f0 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -62,6 +62,12 @@ struct _virInterfaceLink { unsigned int speed; /* link speed in Mbits per second */ }; +typedef struct _virDevFeature virDevFeature; +typedef virDevFeature *virDevFeaturePtr; +struct _virDevFeature { + char *name; /* device feature */ +}; + int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr); int virDevicePCIAddressParseXML(xmlNodePtr node, diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index a728a00..7f4dbfe 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -437,6 +437,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferEscapeString(buf, address%s/address\n, data-net.address); virInterfaceLinkFormat(buf, data-net.lnk); +if (data-net.features) { +for (i = 0; i data-net.nfeatures; i++) { +virBufferAsprintf(buf, feature name='%s'/\n, +data-net.features[i].name); +} +} if (data-net.subtype != VIR_NODE_DEV_CAP_NET_LAST) { const char *subtyp = virNodeDevNetCapTypeToString(data-net.subtype); @@ -1679,6 +1685,7 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case VIR_NODE_DEV_CAP_NET: VIR_FREE(data-net.ifname); VIR_FREE(data-net.address); +VIR_FREE(data-net.features); break; case VIR_NODE_DEV_CAP_SCSI_HOST: VIR_FREE(data-scsi_host.wwnn); diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index fd5d179..918523a 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -141,6 +141,8 @@ struct _virNodeDevCapsDef { char *ifname; virInterfaceLink lnk; virNodeDevNetCapType subtype; /* LAST - no subtype */ +size_t nfeatures; +virDevFeaturePtr features; } net; struct { unsigned int host; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c07a561..1d165a9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1659,6 +1659,7 @@ virNetDevAddRoute; virNetDevClearIPAddress; virNetDevDelMulti; virNetDevExists; +virNetDevGetFeatures; virNetDevGetIndex; virNetDevGetIPv4Address; virNetDevGetLinkInfo; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 03c7a0b..349733f 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -719,6 +719,10 @@ static int udevProcessNetworkInterface(struct udev_device *device, if (virNetDevGetLinkInfo(data-net.ifname, data-net.lnk) 0) goto out; +if (virNetDevGetFeatures(data-net.ifname, data-net.features, +data-net.nfeatures) 0) +goto out; + ret = 0; out: diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 2a0eb43..e513c85 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2728,3 +2728,137 @@ int virNetDevGetRxFilter(const char *ifname, *filter = fil; return ret; } + +#if defined(SIOCETHTOOL) defined(HAVE_STRUCT_IFREQ) + +/** + * _virNetDevFeatureAvailable + * This function checks for the availability of a network device feature + * + * @ifname: name of the interface + * @cmd: reference to an ethtool command structure + * + * Returns 0 on success, -1 on failure. + */ +int +_virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) +{ +int ret = -1; +int sock = -1; +virIfreq ifr; + +sock = socket(AF_LOCAL, SOCK_DGRAM, 0); +if (sock 0) { +virReportSystemError(errno, %s, _(Cannot open control socket)); +goto cleanup; +} + +strcpy(ifr.ifr_name, ifname); +ifr.ifr_data = (void*) cmd; + +if (ioctl(sock, SIOCETHTOOL, ifr) != 0) { +/* Privileged command, no error */ +if (errno == EPERM || errno == EINVAL) { +virReportSystemError(errno, %s, _(ioctl)); +/* Some kernels dont support named feature, no error */ +} else if (errno == EOPNOTSUPP) { +virReportSystemError(errno, %s, _(Warning)); +} else { +virReportSystemError(errno, %s, _(Error));
Re: [libvirt] [PATCH] SRIOV NIC offload feature discovery
On Mon, Feb 16, 2015 at 10:18:32AM +, James Chapman wrote: Adding functionality to libvirt that will allow it query the ethtool interface for the availability of certain NIC HW offload features --- src/conf/device_conf.h | 6 ++ src/conf/node_device_conf.c| 7 ++ src/conf/node_device_conf.h| 2 + src/libvirt_private.syms | 1 + src/node_device/node_device_udev.c | 4 ++ src/util/virnetdev.c | 134 + src/util/virnetdev.h | 12 +++- 7 files changed, 165 insertions(+), 1 deletion(-) If you're modifying the XML parser then you need to also change the XML schema (docs/schemas/nodedev.rng), add example XML for testing the schema (tests/nodedevschemadata) and finally extend the docs (docs/formatnodedev.html.in). Also it is good to include the proposed XML change in the commit message for reviewers. 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 2/2] docs: add page about virtlockd setup
Introduce some basic docs describing the virtlockd setup. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- docs/locking-lockd.html.in | 160 + docs/locking.html.in | 2 +- docs/sitemap.html.in | 4 ++ 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 docs/locking-lockd.html.in diff --git a/docs/locking-lockd.html.in b/docs/locking-lockd.html.in new file mode 100644 index 000..2b01ee5 --- /dev/null +++ b/docs/locking-lockd.html.in @@ -0,0 +1,160 @@ +?xml version=1.0 encoding=UTF-8? +!DOCTYPE html PUBLIC -//W3C//DTD XHTML 1.0 Strict//EN http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd; +html xmlns=http://www.w3.org/1999/xhtml; + body +h1Virtual machine lock manager, virtlockd plugin/h1 + +ul id=toc/ul + +p + This page describes use of the codevirtlockd/code + service as a a href=locking.htmllock driver/a + plugin for virtual machine disk mutual exclusion. +/p + +h2a name=backgroundvirtlockd background/a/h2 + +p + The virtlockd daemon is a single purpose binary which + focus exclusively on the task of acquiring and holding + locks on behalf of running virtual machines. It is + designed to offer a low overhead, portable locking + scheme can be used out of the box on virtualization + hosts with minimal configuration overheads. It makes + use of the POSIX fcntl advisory locking capability + to hold locks, which is supported by the majority of + commonly used filesystems +/p + +h2a name=sanlockvirtlockd daemon setup/a/h2 + +p + In most OS, the virtlockd daemon itself will not require + any upfront configuration work. It is installed by default + when libvirtd is present, and a systemd socket unit is + registered such that the daemon will be automatically + started when first required. With OS that predate systemd + though, it will be neccessary to start it at boot time, + prior to libvirtd being started. On RHEL/Fedora distros, + this can be achieved as follows +/p + +pre + # chkconfig virtlockd on + # service virtlockd start +/pre + +p + The above instructions apply to the instance of virtlockd + that runs privileged, and is used by the libvirtd daemon + that runs privileged. If running libvirtd as an unprivileged + user, it will always automatically spawn an instance of + the virtlockd daemon unprivileged too. This requires no + setup at all. +/p + +h2a name=lockdpluginlibvirt lockd plugin configuration/a/h2 + +p + Once the virtlockd daemon is running, or setup to autostart, + the next step is to configure the libvirt lockd plugin. + There is a separate configuration file for each libvirt + driver that is using virtlockd. For QEMU, we will edit + code/etc/libvirt/qemu-lockd.conf/code +/p + +p + The default behaviour of the lockd plugin is to acquire locks + directly on the virtual disk images associated with the guest + lt;diskgt; elements. This ensures it can run out of the box + with not configuration, providing locking for disk images on + shared filesystems such as NFS. It does not provide any cross + host protection for storage that is backed by block devices, + since locks acquired on device nodes in /dev only apply within + the host. It may also be the case that the filesystem holding + the disk images is not capable of supporting fcntl locks. +/p + +p + To address these problems it is possible to tell lockd to + acquire locks on an indirect file. Essentially lockd will + calculate the SHA256 checksum of the fully qualified path, + and create a zero length file in a given directory whose + filename is the checksum. It will then acquire a lock on + that file. Assuming the block devices assigned to the guest + are using stable paths (eg /dev/disk/by-path/XXX) then + this will allow for locks to apply across hosts. This + feature can be enabled by setting a configuration setting + that specifies the directory in which to create the lock + files. The directory referred to should of course be + placed on a shared filesystem (eg NFS) that is accessible + to all hosts which can see the shared block devices. +/p + +pre + $ su - root + # augtool -s set \ +/files/etc/libvirt/qemu-lockd.conf/file_lockspace_dir \ +/var/lib/libvirt/lockd/files +/pre + +p + If the guests are using either LVM and SCSI block devices + for their virtual disks, there is a unique identifier + associated with each device. It is possible to tell lockd + to use this UUID as the basis for acquiring locks, rather + than the SHA256 sum of the filename. The benefit of this + is that the locking protection will work even if
[libvirt] [PATCH 1/2] docs: split out sanlock setup docs
In preparation for adding docs about virtlockd, split out the sanlock setup docs into a separate page. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- docs/locking-sanlock.html.in | 247 ++ docs/locking.html.in | 277 ++- docs/sitemap.html.in | 6 + 3 files changed, 286 insertions(+), 244 deletions(-) create mode 100644 docs/locking-sanlock.html.in diff --git a/docs/locking-sanlock.html.in b/docs/locking-sanlock.html.in new file mode 100644 index 000..84b1fb3 --- /dev/null +++ b/docs/locking-sanlock.html.in @@ -0,0 +1,247 @@ +?xml version=1.0 encoding=UTF-8? +!DOCTYPE html PUBLIC -//W3C//DTD XHTML 1.0 Strict//EN http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd; +html xmlns=http://www.w3.org/1999/xhtml; + body +h1Virtual machine lock manager, sanlock plugin/h1 + +ul id=toc/ul + +p + This page describes use of the + a href=https://fedorahosted.org/sanlock/;sanlock/a + service as a a href=locking.htmllock driver/a + plugin for virtual machine disk mutual exclusion. +/p + +h2a name=sanlockSanlock daemon setup/a/h2 + +p + On many operating systems, the strongsanlock/strong plugin + is distributed in a sub-package which needs to be installed + separately from the main libvirt RPM. On a Fedora/RHEL host + this can be done with the codeyum/code command +/p + +pre + $ su - root + # yum install libvirt-lock-sanlock +/pre + +p + The next step is to start the sanlock daemon. For maximum + safety sanlock prefers to have a connection to a watchdog + daemon. This will cause the entire host to be rebooted in + the event that sanlock crashes / terminates abnormally. + To start the watchdog daemon on a Fedora/RHEL host + the following commands can be run: +/p + +pre + $ su - root + # chkconfig wdmd on + # service wdmd start +/pre + +p + Once the watchdog is running, sanlock can be started + as follows +/p + +pre + # chkconfig sanlock on + # service sanlock start +/pre + +p + emNote:/em if you wish to avoid the use of the + watchdog, add the following line to code/etc/sysconfig/sanlock/code + before starting it +/p + +pre + SANLOCKOPTS=-w 0 +/pre + +p + The sanlock daemon must be started on every single host + that will be running virtual machines. So repeat these + steps as necessary. +/p + +h2a name=sanlockpluginlibvirt sanlock plugin configuration/a/h2 + +p + Once the sanlock daemon is running, the next step is to + configure the libvirt sanlock plugin. There is a separate + configuration file for each libvirt driver that is using + sanlock. For QEMU, we will edit code/etc/libvirt/qemu-sanlock.conf/code + There is one mandatory parameter that needs to be set, + the codehost_id/code. This is a integer between + 1 and 2000, which must be set to a strongunique/strong + value on each host running virtual machines. +/p + +pre + $ su - root + # augtool -s set /files/etc/libvirt/qemu-sanlock.conf/host_id 1 +/pre + +p + Repeat this on every host, changing strong1/strong to a + unique value for the host. +/p + +h2a name=sanlockstoragelibvirt sanlock storage configuration/a/h2 + +p + The sanlock plugin needs to create leases in a directory + that is on a filesystem shared between all hosts running + virtual machines. Obvious choices for this include NFS + or GFS2. The libvirt sanlock plugin expects its lease + directory be at code/var/lib/libvirt/sanlock/code + so update the host's code/etc/fstab/code to mount + a suitable shared/cluster filesystem at that location +/p + +pre + $ su - root + # echo some.nfs.server:/export/sanlock /var/lib/libvirt/sanlock nfs hard,nointr 0 0 /etc/fstab + # mount /var/lib/libvirt/sanlock +/pre + +p + If your sanlock daemon happen to run under non-root + privileges, you need to tell this to libvirt so it + chowns created files correctly. This can be done by + setting codeuser/code and/or codegroup/code + variables in the configuration file. Accepted values + range is specified in description to the same + variables in code/etc/libvirt/qemu.conf/code. For + example: +/p + +pre + augtool -s set /files/etc/libvirt/qemu-sanlock.conf/user sanlock + augtool -s set /files/etc/libvirt/qemu-sanlock.conf/group sanlock +/pre + +p + But remember, that if this is NFS share, you need a + no_root_squash-ed one for chown (and chmod possibly) + to succeed. +/p + +p + In terms of storage requirements, if the filesystem + uses 512 byte sectors, you need to allow for code1MB/code + of storage for each
[libvirt] [PATCH 0/2] Document virtlockd
Just some docs additions wrt virtlockd Daniel P. Berrange (2): docs: split out sanlock setup docs docs: add page about virtlockd setup docs/locking-lockd.html.in | 160 + docs/locking-sanlock.html.in | 247 ++ docs/locking.html.in | 277 ++- docs/sitemap.html.in | 10 ++ 4 files changed, 450 insertions(+), 244 deletions(-) create mode 100644 docs/locking-lockd.html.in create mode 100644 docs/locking-sanlock.html.in -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list