[libvirt] [PATCH 00/21] qemu_hotplug: refactor device detach functions to fix one serious and other minor bugs
by Laine Stump
This all started with the observation that we were sending the
DEVICE_REMOVED events for unplugged devices too soon (reported in
https://bugzilla.redhat.com/1658198 ). While noticing that *all* of
the device types send their DEVICE_REMOVED event too early, I also
noticed how eerily similar all the different detach functions are
(they have converged over time, as legacy code supporting old qemus
with different methods of deleting different types of devices has been
removed - pkrempa and mprivozn committed patches just in the last week
that contributed to this convergence and removed some duplicated
code).
I also saw a few other definite bugs, as well as a couple that I think
are bugs, but am not enough informed about zPCI to be certain (I've
Cc'ed the author and reviewers of the original patch adding that code
to the patches so that they can verify, or negate, my suspicions).
Many/most of these patches are either very simple / easily verifiable
code movement / removal / renaming of variables. Patch 20 is a bit
more involved; viewing the new versions of the functions along with
the patch would probably help it go much quicker.
The first two patches in the series eliminate what I'm 99% certain is
erroneous calling of the qemuDomainDetachExtensionDevice() when
detaching a PCI device on S390. The commit log details my reasons for
believing this code should be removed.
The rest of the series eliminates small differences iteratively,
arriving at a set of detach functions which are completely identical
at the end (and only called from one place), and then replaces all
those duplicates with a single block of common code in a higher level
function that calls all of the individual functions (it was their only
caller anyway). This eliminated the inconsistent application of
checking for multifunction PCI devices and for the existence of a
device alias. It also eliminated individual calls to
qemuDomainRemove*Device() calls, and replaced them with a single call
to qemuDomainRemoveDevice().
Eliminating all the calls (except one) to the individual
qemuDomainRemove*Device() functions in turn made it possible to move
the code that builds and queues the DEVICE_REMOVED event into a single
place in the one remaining higher level caller -
qemuDomainRemoveDevice() - thus eliminating the "premature event" for *all*
device types at once.
Laine Stump (21):
qemu_hotplug: remove erroneous call to
qemuDomainDetachExtensionDevice()
qemu_hotplug: remove another erroneous
qemuDomainDetachExtensionDevice() call
qemu_hotplug: remove unnecessary check for valid PCI address
qemu_hotplug: rename a virDomainDeviceInfoPtr to avoid confusion
qemu_hotplug: eliminate multiple identical
qemuDomainDetachHost*Device() functions
qemu_hotplug: eliminate unnecessary call to
qemuDomainDetachNetDevice()
qemu_hotplug: refactor qemuDomainDetachDiskLive and
qemuDomainDetachDiskDevice
qemu_hotplug: don't call DetachThisHostDevice for hostdev network
devices
qemu_hotplug: merge qemuDomainDetachThisHostDevice into
qemuDomainDetachHostDevice
qemu_hotplug: move qemuDomainChangeGraphicsPasswords()
qemu_hotplug: move (almost) all qemuDomainDetach*() functions together
qemu_hotplug: move (Attach|Detach)Lease functions with others of same
type
qemu_hotplug: move qemuDomainDetachDeviceLive() to qemu_hotplug.c
qemu_hotplug: remove extra function in middle of DetachController call
chain
qemu_hotplug: pull qemuDomainUpdateDeviceList out of
qemuDomainDetachDeviceLive
test: replace calls to individual detach functions with one call to
main detach
qemu_hotplug: rename dev to match in qemuDomainDetachDeviceLive
qemu_hotplug: standardize the names/args/calling of
qemuDomainDetach*()
qemu_hotplug: new function qemuDomainRemoveAuditDevice()
qemu_hotplug: consolidate all common detach code in
qemuDomainDetachDeviceLive
qemu_hotplug: delay sending DEVICE_REMOVED event until after *all*
teardown
src/qemu/qemu_driver.c | 102 +--
src/qemu/qemu_hotplug.c | 1407 ++++++++++++++++++---------------------
src/qemu/qemu_hotplug.h | 56 +-
tests/qemuhotplugtest.c | 8 +-
4 files changed, 655 insertions(+), 918 deletions(-)
Yay! jtomko would be proud - more deletions than insertions!!!
--
2.20.1
6 years, 2 months
[libvirt] [PATCH] virDomainMomentAssignDef: Don't dereference a NULL pointer
by Michal Privoznik
This functions tries to add a domain moment (love the name!) onto
a list of domain moments. Firstly, it checks if another moment
with the same name already exists. Then, it creates an empty
moment (without initializing its definition) and tries to add the
moment onto the list dereferencing moment definition in that
process. If it succeeds (which it never can), only after that it
sets moment->def.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/virdomainmomentobjlist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c
index b2122e7292..01e50ae1d4 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -227,7 +227,7 @@ virDomainMomentAssignDef(virDomainMomentObjListPtr moments,
if (!(moment = virDomainMomentObjNew()))
return NULL;
- if (virHashAddEntry(moments->objs, moment->def->name, moment) < 0) {
+ if (virHashAddEntry(moments->objs, def->name, moment) < 0) {
VIR_FREE(moment);
return NULL;
}
--
2.19.2
6 years, 2 months
[libvirt] [PATCH] nwfilter: fix adding std MAC and IP values to filter binding
by Nikolay Shirokovskiy
Commit d1a7c08eb changed filter instantiation code to ignore MAC and IP
variables explicitly specified for filter binding. It just replaces
explicit values with values associated with the binding. Before the
commit virNWFilterCreateVarsFrom was used so that explicit value
take precedence. Let's bring old behavior back.
This is useful. For example if domain has two interfaces it makes
sense to list both mac adresses in MAC var of every interface
filterref. So that if guest make a bond of these interfaces
and start sending frames with one of the mac adresses from
both interfaces we can pass outgress traffic from both
interfaces too.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/nwfilter/nwfilter_gentech_driver.c | 92 ++++++++++++----------------------
1 file changed, 32 insertions(+), 60 deletions(-)
diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index 655f088..6d68189 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -127,60 +127,6 @@ virNWFilterRuleInstFree(virNWFilterRuleInstPtr inst)
/**
- * virNWFilterVarHashmapAddStdValues:
- * @tables: pointer to hash tabel to add values to
- * @macaddr: The string of the MAC address to add to the hash table,
- * may be NULL
- * @ipaddr: The string of the IP address to add to the hash table;
- * may be NULL
- *
- * Returns 0 in case of success, -1 in case an error happened with
- * error having been reported.
- *
- * Adds a couple of standard keys (MAC, IP) to the hash table.
- */
-static int
-virNWFilterVarHashmapAddStdValues(virHashTablePtr table,
- const char *macaddr,
- const virNWFilterVarValue *ipaddr)
-{
- virNWFilterVarValue *val;
-
- if (macaddr) {
- val = virNWFilterVarValueCreateSimpleCopyValue(macaddr);
- if (!val)
- return -1;
-
- if (virHashUpdateEntry(table,
- NWFILTER_STD_VAR_MAC,
- val) < 0) {
- virNWFilterVarValueFree(val);
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Could not add variable 'MAC' to hashmap"));
- return -1;
- }
- }
-
- if (ipaddr) {
- val = virNWFilterVarValueCopy(ipaddr);
- if (!val)
- return -1;
-
- if (virHashUpdateEntry(table,
- NWFILTER_STD_VAR_IP,
- val) < 0) {
- virNWFilterVarValueFree(val);
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Could not add variable 'IP' to hashmap"));
- return -1;
- }
- }
-
- return 0;
-}
-
-
-/**
* Convert a virHashTable into a string of comma-separated
* variable names.
*/
@@ -705,6 +651,28 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
}
+static int
+virNWFilterVarHashmapAddStdValue(virHashTablePtr table,
+ const char *var,
+ const char *value)
+{
+ virNWFilterVarValue *val;
+
+ if (virHashLookup(table, var))
+ return 0;
+
+ if (!(val = virNWFilterVarValueCreateSimpleCopyValue(value)))
+ return -1;
+
+ if (virHashAddEntry(table, var, val) < 0) {
+ virNWFilterVarValueFree(val);
+ return -1;
+ }
+
+ return 0;
+}
+
+
/*
* Call this function while holding the NWFilter filter update lock
*/
@@ -717,7 +685,7 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
bool forceWithPendingReq,
bool *foundNewFilter)
{
- int rc;
+ int rc = -1;
const char *drvname = EBIPTABLES_DRIVER_ID;
virNWFilterTechDriverPtr techdriver;
virNWFilterObjPtr obj;
@@ -743,14 +711,18 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
return -1;
virMacAddrFormat(&binding->mac, vmmacaddr);
+ if (virNWFilterVarHashmapAddStdValue(binding->filterparams,
+ NWFILTER_STD_VAR_MAC,
+ vmmacaddr) < 0)
+ goto err_exit;
ipaddr = virNWFilterIPAddrMapGetIPAddr(binding->portdevname);
-
- if (virNWFilterVarHashmapAddStdValues(binding->filterparams,
- vmmacaddr, ipaddr) < 0) {
- rc = -1;
+ if (ipaddr &&
+ virNWFilterVarHashmapAddStdValue(binding->filterparams,
+ NWFILTER_STD_VAR_IP,
+ virNWFilterVarValueGetSimple(ipaddr)) < 0)
goto err_exit;
- }
+
filter = virNWFilterObjGetDef(obj);
--
1.8.3.1
6 years, 2 months
[libvirt] [PATCH] xml: nodedev: make pci capability class element optional
by Nikolay Shirokovskiy
Commit 3bd4ed46 introduced this element as required which
breaks backcompat for test driver. Let's make the element optional.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
docs/formatnode.html.in | 2 +-
docs/schemas/nodedev.rng | 12 ++++++-----
src/conf/node_device_conf.c | 23 +++++++++++-----------
src/conf/node_device_conf.h | 2 +-
src/node_device/node_device_udev.c | 2 +-
.../pci_0000_02_10_7_sriov_pf_vfs_all.xml | 1 -
6 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 5095b97..c2a8f8f 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -71,7 +71,7 @@
include:
<dl>
<dt><code>class</code></dt>
- <dd>Combined class, subclass and
+ <dd>Optional element for combined class, subclass and
programming interface codes as 6-digit hexadecimal number.
<span class="since">Since 5.2.0</span></dd>
<dt><code>domain</code></dt>
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 0f45d79..fe6ffa0 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -133,11 +133,13 @@
<value>pci</value>
</attribute>
- <element name='class'>
- <data type="string">
- <param name="pattern">0x[0-9a-fA-F]{6}</param>
- </data>
- </element>
+ <optional>
+ <element name='class'>
+ <data type="string">
+ <param name="pattern">0x[0-9a-fA-F]{6}</param>
+ </data>
+ </element>
+ </optional>
<element name='domain'>
<ref name='unsignedLong'/>
</element>
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 19c601a..b96d10c 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -208,7 +208,8 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf,
{
size_t i;
- virBufferAsprintf(buf, "<class>0x%.6x</class>\n", data->pci_dev.klass);
+ if (data->pci_dev.klass >= 0)
+ virBufferAsprintf(buf, "<class>0x%.6x</class>\n", data->pci_dev.klass);
virBufferAsprintf(buf, "<domain>%d</domain>\n",
data->pci_dev.domain);
virBufferAsprintf(buf, "<bus>%d</bus>\n", data->pci_dev.bus);
@@ -1645,16 +1646,16 @@ virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt,
orignode = ctxt->node;
ctxt->node = node;
- if (virNodeDevCapsDefParseHexId("string(./class[1])", ctxt,
- &pci_dev->klass, def,
- _("no PCI class supplied for '%s'"),
- _("invalid PCI class supplied for '%s'")) < 0)
- goto out;
-
- if (pci_dev->klass > 0xffffff) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("invalid PCI class supplied for '%s'"), def->name);
- goto out;
+ if ((tmp = virXPathString("string(./class[1])", ctxt))) {
+ if (virStrToLong_i(tmp, NULL, 16, &pci_dev->klass) < 0 ||
+ pci_dev->klass > 0xffffff) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid PCI class supplied for '%s'"), def->name);
+ goto out;
+ }
+ VIR_FREE(tmp);
+ } else {
+ pci_dev->klass = -1;
}
if (virNodeDevCapsDefParseULong("number(./domain[1])", ctxt,
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index b13bc13..e8cb315 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -153,7 +153,7 @@ struct _virNodeDevCapPCIDev {
unsigned int function;
unsigned int product;
unsigned int vendor;
- unsigned int klass;
+ int klass;
char *product_name;
char *vendor_name;
virPCIDeviceAddressPtr physical_function;
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index f0e61e4..5ed3463 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -402,7 +402,7 @@ udevProcessPCI(struct udev_device *device,
privileged = driver->privileged;
nodeDeviceUnlock();
- if (udevGetUintProperty(device, "PCI_CLASS", &pci_dev->klass, 16) < 0)
+ if (udevGetIntProperty(device, "PCI_CLASS", &pci_dev->klass, 16) < 0)
goto cleanup;
if ((p = strrchr(def->sysfs_path, '/')) == NULL ||
diff --git a/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all.xml b/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all.xml
index 74036a6..9e8dace 100644
--- a/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all.xml
+++ b/tests/nodedevschemadata/pci_0000_02_10_7_sriov_pf_vfs_all.xml
@@ -2,7 +2,6 @@
<name>pci_0000_02_10_7</name>
<parent>pci_0000_00_04_0</parent>
<capability type='pci'>
- <class>0xffffff</class>
<domain>0</domain>
<bus>2</bus>
<slot>16</slot>
--
1.8.3.1
6 years, 2 months
[libvirt] [PATCH 00/21] tests: qemuxml2argv: support optional arguments
by Cole Robinson
Right now in qemuxml2argv we have a proliferation of DO_*TEST* macros.
They essentially fill in data in the testInfo struct and invoke the
test function.
There's several bits of data that we want to specify for a small
subset of tests: flags, parseFlags, migrateFrom/migrateFd, gic. The
base macros need to handle these in their argument lists, and we
provide many convenience macros that fill in default values for the
less common parameters. Any time we want to add a new bit of data
though, the base macros are extended, and every caller of those
needs to be adjusted, and we are faced with the question of whether
to extend the combinatorial explosion of convenience macros which
have only a subset of options exposed.
This series adds a testInfoSetArgs which uses va_args to make these
mandatory parameters optional. The general format is:
DO_TEST_FULL(...
ARG_FOO, foovalue,
ARG_BAR, barvalue, ...)
Specifically, one of the migrate tests went from:
DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0, GIC_NONE, NONE);
to:
DO_TEST_FULL("restore-v2",
ARG_MIGRATE_FROM, "exec:cat",
ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
Which may not seem directly compelling, but adding new testInfo fields
will be much easier. This will also be a base for a later series to
share the VIR_TEST_CAPS infrastructure with qemuxml2xmltest
Cole Robinson (21):
qemu: add virQEMUCapsSetVList
tests: qemuxml2argv: add testInfoSetArgs
tests: qemuxml2argv: add va_arg enum handling
tests: qemuxml2argv: push ARG_QEMU_CAPS to callers
tests: qemuxml2argv: break apart testInitQEMUCaps
tests: qemuxml2argv: handle gic with vaargs
tests: qemuxml2argv: handle migrate* with varargs
tests: qemuxml2argv: handle flags with varargs
tests: qemuxml2argv: handle parseFlags with varargs
tests: qemuxml2argv: remove DO_TEST_PARSE_FLAGS_ERROR
tests: qemuxml2argv: remove unused DO_TEST_CAPS* macros
tests: qemuxml2argv: add a comment separating DO_TEST* macros
tests: qemuxml2argv: remove unused CAPS migrateFrom
tests: qemuxml2argv: use varargs for CAPS flags
tests: qemuxml2argv: remove full testInfo initialization
tests: qemuxml2argv: centralize CAPS suffix building
tests: qemuxml2argv: build capsfile in DO_TEST_CAPS_INTERNAL
tests: qemuxml2argv: add testInfoClear
tests: qemuxml2argv: move DO_TEST qemuCaps init
tests: qemuxml2argv: move DO_CAPS_TEST* qemuCaps init
tests: qemuxml2argv: add TEST_INTERNAL
src/qemu/qemu_capabilities.c | 14 +-
src/qemu/qemu_capabilities.h | 2 +
tests/qemuxml2argvtest.c | 273 +++++++++++++++++++++++------------
3 files changed, 193 insertions(+), 96 deletions(-)
--
2.20.1
6 years, 2 months
Re: [libvirt] [Qemu-devel] [PULL v3 00/54] Kconfig conversion, excluding ARM and MIPS
by Laszlo Ersek
Hi Paolo,
(+libvir-list)
I'd like to report a regression introduced by this patch set:
On 03/07/19 21:58, Paolo Bonzini wrote:
> The following changes since commit
> 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6:
>
> Merge remote-tracking branch
> 'remotes/cleber/tags/python-next-pull-request' into staging
> (2019-03-07 16:16:02 +0000)
>
> are available in the Git repository at:
>
> git://github.com/bonzini/qemu.git tags/for-upstream-kconfig
>
> for you to fetch changes up to
> 576c3f2f16e7392e28cc9fe10d9a920d67d3645b:
>
> kconfig: add documentation (2019-03-07 21:54:22 +0100)
>
> ----------------------------------------------------------------
> Initial Kconfig work, excluding ARM and MIPS
>
> ----------------------------------------------------------------
I use "libvirt-daemon-4.5.0-10.el7.x86_64" at the moment. Here are the
symptoms:
(1) the output of the following command changes across this series:
virsh domcapabilities qemu /opt/qemu-installed-optimized/bin/qemu-system-aarch64 aarch64 virt-4.0
as shown by the diff
> @@ -92,6 +92,8 @@
> <video supported='yes'>
> <enum name='modelType'>
> <value>vga</value>
> + <value>cirrus</value>
> + <value>vmvga</value>
> <value>virtio</value>
> </enum>
> </video>
> @@ -111,10 +113,7 @@
> <value>scsi</value>
> </enum>
> <enum name='capsType'/>
> - <enum name='pciBackend'>
> - <value>default</value>
> - <value>vfio</value>
> - </enum>
> + <enum name='pciBackend'/>
> </hostdev>
> </devices>
> <features>
(2) given the following domain XML snippet, in a qemu (not KVM) aarch64
guest that I have,
> <video>
> <model type='virtio' heads='1' primary='yes'/>
> <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/>
> </video>
the QEMU command line generated by libvirt changes:
> - -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.4,addr=0x0 \
> + -device virtio-vga,id=video0,max_outputs=1,bus=pci.4,addr=0x0 \
Perhaps this series causes (1), which in turn causes (2), or else this
series causes both (1) and (2) independently of each other. I'm not sure
about that. Either way, (2) is a problem, because virtio-vga is
inappropriate for aarch64 guests (and the ArmVirtQemu platform firmware
in edk2 won't even try to drive it -- a blank screen is the result).
Now, I can't pinpoint an exact commit, because (after a *brutal*
bisection) I get:
> There are only 'skip'ped commits left to test.
> The first bad commit could be any of:
> a02c0edb35662de38d400f42b68845540669ca3d
> 03b348bdcbd1eda4ce097b2b84527dec774d80c4
> d6e9c470fc91f75db1785f17a9d3567d5a27953d
> a7e23159074c9d774fb1e88357b778994a0c9365
> bcb129b3154ba743f8e52c21c331a0dfcaee7c38
> 02017ee385ef574133c4a978d368640772978178
> 7c28b925b7e176b4e44ed05d23cf883561000546
> 1550b0e6bfe3ab6985e5ad75df1c528a0ca39468
> e9947d18df97e6c6584f020cf9cc995404cc8061
> 8f01b41e1098d8cb9491fa3ea7bd59cf187a5bd7
> 9533dcdd416530a0d72140c122bf90517b6c81eb
> 32690c8bed5762518272876dcb6dd39a54f54fd1
> c3a98aa54c734dcb7a36d193c6330d8f04d4bf8e
> ccf222a816d59af1318a7efb59c6b9c5578d1abf
> f349474920d80838ecea3d421531fdb0660b8740
> 2ac041c2c3d89691cda1657981c41fe4bc20244b
> e0e312f3525ad6ac18ba6633af29190dd9620cbc
> b42075bb77672616127c9452c0f836e757e9ce1a
> We cannot bisect more!
Here's how I built each step of the bisection:
- "git clean -ffdx" and "git submodule deinit --all --force" on the
source directory
- create a new, pristine build directory
- configure QEMU for out-of-tree build, in that directory
- select the i386, x86_64, arm, and aarch64 targets (all softmmu)
- report "skip" to git-bisect iff the build breaks, otherwise test the
aarch64 emulator through libvirt, and report good/bad dependent on the
test result.
The bisection log is attached -- "git bisect replay" and "git bisect
visualize" should tell the story better than the above dump of commit
hashes. Basically:
- The tree builds up to and including commit 82f5181777eb ("kconfig:
introduce kconfig files", 2019-03-07), and that commit works fine as
well.
- Then the build breaks (starting with commit e0e312f3525a, "build:
switch to Kconfig", 2019-03-07).
- The tree becomes buildable again at commit b42075bb7767 ("virtio:
express virtio dependencies with Kconfig", 2019-03-07), but at that
point, the functionality has been regressed.
Thanks,
Laszlo
6 years, 2 months
[libvirt] [PATCH v2] numa: warn if numa 'mem' option or default RAM splitting between nodes is used.
by Igor Mammedov
Amend -numa option docs and print warnings if 'mem' option or default RAM
splitting between nodes is used. It's intended to discourage users from using
configuration that allows only to fake NUMA on guest side while leading
to reduced performance of the guest due to inability to properly configure
VM's RAM on the host.
In NUMA case, it's recommended to always explicitly configure guest RAM
using -numa node,memdev={backend-id} option.
Signed-off-by: Igor Mammedov <imammedo(a)redhat.com>
---
numa.c | 5 +++++
qemu-options.hx | 12 ++++++++----
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/numa.c b/numa.c
index 3875e1e..42838f9 100644
--- a/numa.c
+++ b/numa.c
@@ -121,6 +121,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
if (node->has_mem) {
numa_info[nodenr].node_mem = node->mem;
+ warn_report("Parameter -numa node,mem is obsolete,"
+ " use -numa node,memdev instead");
}
if (node->has_memdev) {
Object *o;
@@ -407,6 +409,9 @@ void numa_complete_configuration(MachineState *ms)
if (i == nb_numa_nodes) {
assert(mc->numa_auto_assign_ram);
mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size);
+ warn_report("Default splitting of RAM between nodes is obsolete,"
+ " Use '-numa node,memdev' to explicitly define RAM"
+ " allocation per node");
}
numa_total = 0;
diff --git a/qemu-options.hx b/qemu-options.hx
index 1cf9aac..61035cb 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -206,10 +206,14 @@ For example:
-numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=1,socket-id=1
@end example
-@samp{mem} assigns a given RAM amount to a node. @samp{memdev}
-assigns RAM from a given memory backend device to a node. If
-@samp{mem} and @samp{memdev} are omitted in all nodes, RAM is
-split equally between them.
+@samp{memdev} assigns RAM from a given memory backend device to a node.
+
+Legacy options/behaviour: @samp{mem} assigns a given RAM amount to a node.
+If @samp{mem} and @samp{memdev} are omitted in all nodes, RAM is split equally
+between them. Option @samp{mem} and default RAM splitting are obsolete as they
+do not provide means to manage RAM on the host side and only allow QEMU to fake
+NUMA support which in practice could degrade VM performance.
+It's advised to always explicitly configure NUMA RAM by using the @samp{memdev} option.
@samp{mem} and @samp{memdev} are mutually exclusive. Furthermore,
if one node uses @samp{memdev}, all of them have to use it.
--
2.7.4
6 years, 2 months
[libvirt] [PATCH 1/2] snapshots: allow create --redefine to force a new configuration
by Christian Ehrhardt
Currently there is no means to permanently modify the metadata stored
within a snapshot if it does not pass the ABI compat checker.
That is pretty much intentional but there are use cases where users
verified that the change will work according to their needs and
don't want to: snapshot-revert -> edit -> start, but instead store
the modified domain definition within the metadata.
To achive that this adds a --force argument to snapshot-create
--redefine which will make it skip the check.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
include/libvirt/libvirt-domain-snapshot.h | 3 +++
src/conf/snapshot_conf.c | 3 ++-
src/qemu/qemu_driver.c | 7 ++++++-
tools/virsh-snapshot.c | 6 ++++++
tools/virsh.pod | 8 +++++++-
5 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/include/libvirt/libvirt-domain-snapshot.h b/include/libvirt/libvirt-domain-snapshot.h
index 602e5def59..dc8ba2b4a9 100644
--- a/include/libvirt/libvirt-domain-snapshot.h
+++ b/include/libvirt/libvirt-domain-snapshot.h
@@ -71,6 +71,9 @@ typedef enum {
VIR_DOMAIN_SNAPSHOT_CREATE_LIVE = (1 << 8), /* create the snapshot
while the guest is
running */
+ VIR_DOMAIN_SNAPSHOT_CREATE_NOCHECK = (1 << 9), /* take definition as
+ provided without
+ ABI compat check */
} virDomainSnapshotCreateFlags;
/* Take a snapshot of the current VM state */
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index ffb1313c89..f95b104ad5 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -474,7 +474,8 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def,
if (other->def->dom) {
if (def->dom) {
- if (!virDomainDefCheckABIStability(other->def->dom,
+ if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NOCHECK) &&
+ !virDomainDefCheckABIStability(other->def->dom,
def->dom, xmlopt))
return -1;
} else {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a16eab5467..f3d032f633 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15680,7 +15680,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT |
VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE |
VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC |
- VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, NULL);
+ VIR_DOMAIN_SNAPSHOT_CREATE_LIVE |
+ VIR_DOMAIN_SNAPSHOT_CREATE_NOCHECK, NULL);
VIR_REQUIRE_FLAG_RET(VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE,
VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY,
@@ -15689,6 +15690,10 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE,
NULL);
+ VIR_REQUIRE_FLAG_RET(VIR_DOMAIN_SNAPSHOT_CREATE_NOCHECK,
+ VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE,
+ NULL);
+
if ((redefine && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) ||
(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA))
update_current = false;
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 6ea6e2744a..a680d2b410 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -146,6 +146,10 @@ static const vshCmdOptDef opts_snapshot_create[] = {
.type = VSH_OT_BOOL,
.help = N_("require atomic operation")
},
+ {.name = "force",
+ .type = VSH_OT_BOOL,
+ .help = N_("do not check XML for ABI compatibility")
+ },
VIRSH_COMMON_OPT_LIVE(N_("take a live snapshot")),
{.name = NULL}
};
@@ -177,6 +181,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
if (vshCommandOptBool(cmd, "live"))
flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE;
+ if (vshCommandOptBool(cmd, "force"))
+ flags |= VIR_DOMAIN_SNAPSHOT_CREATE_NOCHECK;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index db72343159..a8cd15bad2 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -4570,7 +4570,7 @@ used to represent properties of snapshots.
=item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]]
| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external>]
-[I<--quiesce>] [I<--atomic>] [I<--live>]}
+[I<--quiesce>] [I<--atomic>] [I<--live>] [I<--force>]}
Create a snapshot for domain I<domain> with the properties specified in
I<xmlfile>. Normally, the only properties settable for a domain snapshot
@@ -4629,6 +4629,12 @@ the guest is running. Both disk snapshot and domain memory snapshot are
taken. This increases the size of the memory image of the external
snapshot. This is currently supported only for full system external snapshots.
+If I<--force> is specified, then the usual ABI compatibility check that
+would be done on I<--redefine> will be skipped allowing to update the
+domain definition stored in the snapshot metadata arbitrarily.
+Note: this obviously involves extra risk, supplying I<--force> assures
+libvirt that the snapshot will be compatible with the new configuration.
+
Existence of snapshot metadata will prevent attempts to B<undefine>
a persistent domain. However, for transient domains, snapshot
metadata is silently lost when the domain quits running (whether
--
2.17.1
6 years, 2 months
