Re: [libvirt] [PATCH] spec: Update polkit dependencies for CVE-2013-4311
On Tue, Jul 15, 2014 at 17:09:29 -0600, Eric Blake wrote:
On 07/15/2014 07:23 AM, Jiri Denemark wrote:
Use secured polkit on distros which provide it. However, RHEL-6 will
still allow for older polkit-0.93 rather than forcing polkit-0.96-5
which is not available in all RHEL-6 releases.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
libvirt.spec.in | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8d1acfa..f32ab00 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -535,7 +535,9 @@ BuildRequires: module-init-tools
BuildRequires: cyrus-sasl-devel
%endif
%if %{with_polkit}
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} = 21 || 0%{?rhel} = 7
+BuildRequires: polkit-devel = 0.112
+%elif 0%{?fedora} = 12 || 0%{?rhel} = 6
BuildRequires: polkit-devel = 0.93
Ouch - make rpm now complains:
error: line 519: Unknown tag: %elif (020) || 0 = 6
I don't think %elif is a valid spec file construct (too much shell
programming for you lately?)
No, I just blindly copied your suggestion and thought that trying make
rpm even without dependencies and most features turned off would be
enough to check the spec file syntax. Which was apparently wrong.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew
On Tue, Jul 15, 2014 at 10:47:04PM -0400, Jincheng Miao wrote:
- Original Message -
I'm not sure errno is set when using our virMutexInit(). Most of the
code uses virReportError instead, I suggest using that. This should
Actually, the implement of virMutexInit() already set errno when failure:
int virMutexInit(virMutexPtr m)
{
int ret;
pthread_mutexattr_t attr;
pthread_mutexattr_init(attr);
pthread_mutexattr_settype(attr, PTHREAD_MUTEX_NORMAL);
ret = pthread_mutex_init(m-lock, attr);
pthread_mutexattr_destroy(attr);
if (ret != 0) {
errno = ret;
return -1;
}
return 0;
}
Oh, sorry; /me facepalms...
You're right then, but if you want to change it everywhere in the code
(which I don't require), it would be better to create a
virMutexInitInternal that takes 2 parameters; the second being a bool
that controls error reporting and virMutexInit and virMutexInitQuiet
would be two macros. Similarly to virVasprintf for example.
be changed everywhere in the code. Rough idea of the places could be
I think it worthy of adding after all virMutexInit, I will include it in
my V2 patchset.
gotten by the following command:
git grep -nA5 virMutexInit | grep SystemError
but as I said, only rough idea :)
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew
- Original Message - Oh, sorry; /me facepalms... You're right then, but if you want to change it everywhere in the code (which I don't require), it would be better to create a virMutexInitInternal that takes 2 parameters; the second being a bool that controls error reporting and virMutexInit and virMutexInitQuiet would be two macros. Similarly to virVasprintf for example. Good idea :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] leaseshelper: add enhancements to support all events
On Fri, Jul 11, 2014 at 5:49 AM, Nehal J Wani nehaljw.k...@gmail.com wrote: This patch enables the helper program to detect event(s) triggered when there is a change in lease length or expiry and client-id. This transfers complete control of leases database to libvirt and suppresses use of the lease database file (network-name.leases). That file will not be created, read, or written. This is achieved by adding the option --leasefile-ro to dnsmasq and applying the symlink technique, which helps us map events related to leases with their corresponding network bridges. Example: /var/lib/libvirt/dnsmasq/virbr0.helper - /home/wani/libvirt/src/libvirt_leaseshelper /var/lib/libvirt/dnsmasq/virbr3.helper - /home/wani/libvirt/src/libvirt_leaseshelper Also, this requires the addition of a new non-lease entry in our custom lease database: server-duid. It is required to identify a DHCPv6 server. Now that dnsmasq doesn't maintain its own leases database, it relies on our helper program to tell it about previous leases and server duid. Thus, this patch makes our leases program honor an extra action: init, in which it sends the known info in a particular format to dnsmasq by printing it to stdout. --- src/network/bridge_driver.c | 43 +++- src/network/leaseshelper.c | 156 +--- 2 files changed, 175 insertions(+), 24 deletions(-) Ping! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew
- Original Message - - Original Message - Oh, sorry; /me facepalms... You're right then, but if you want to change it everywhere in the code (which I don't require), it would be better to create a virMutexInitInternal that takes 2 parameters; the second being a bool that controls error reporting and virMutexInit and virMutexInitQuiet would be two macros. Similarly to virVasprintf for example. Good idea :) I will remove this patch from this patchset on V2, and send a separate one for virMutexInit* implement. -- 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] [PATCHv2 1/2] Introduce virTristateBool enum type
Replace all three-state (default/yes/no) enums with it:
virDomainBootMenu
virDomainPMState
virDomainGraphicsSpiceClipboardCopypaste
virDomainGraphicsSpiceAgentFileTransfer
virNetworkDNSForwardPlainNames
---
src/conf/domain_conf.c | 57 -
src/conf/domain_conf.h | 43 --
src/conf/network_conf.c | 11 ++---
src/conf/network_conf.h | 15 +---
src/libvirt_private.syms| 10 ++--
src/network/bridge_driver.c | 3 +--
src/qemu/qemu_command.c | 22 -
src/qemu/qemu_driver.c | 4 ++--
src/util/virutil.c | 5
src/util/virutil.h | 11 +
10 files changed, 50 insertions(+), 131 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1d83f13..e374604 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -134,11 +134,6 @@ VIR_ENUM_IMPL(virDomainBoot, VIR_DOMAIN_BOOT_LAST,
hd,
network)
-VIR_ENUM_IMPL(virDomainBootMenu, VIR_DOMAIN_BOOT_MENU_LAST,
- default,
- yes,
- no)
-
VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
acpi,
apic,
@@ -180,11 +175,6 @@ VIR_ENUM_IMPL(virDomainLockFailure,
VIR_DOMAIN_LOCK_FAILURE_LAST,
pause,
ignore)
-VIR_ENUM_IMPL(virDomainPMState, VIR_DOMAIN_PM_STATE_LAST,
- default,
- yes,
- no)
-
VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
none,
disk,
@@ -576,18 +566,6 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceStreamingMode,
all,
off);
-VIR_ENUM_IMPL(virDomainGraphicsSpiceClipboardCopypaste,
- VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_LAST,
- default,
- yes,
- no);
-
-VIR_ENUM_IMPL(virDomainGraphicsSpiceAgentFileTransfer,
- VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_LAST,
- default,
- yes,
- no);
-
VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
subsystem,
capabilities)
@@ -8789,7 +8767,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
}
if ((copypasteVal =
-
virDomainGraphicsSpiceClipboardCopypasteTypeFromString(copypaste)) = 0) {
+ virTristateBoolTypeFromString(copypaste)) = 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(unknown copypaste value '%s'),
copypaste);
VIR_FREE(copypaste);
@@ -8809,7 +8787,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
}
if ((enableVal =
-
virDomainGraphicsSpiceAgentFileTransferTypeFromString(enable)) = 0) {
+ virTristateBoolTypeFromString(enable)) = 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(unknown enable value '%s'), enable);
VIR_FREE(enable);
@@ -9916,7 +9894,7 @@ virDomainPMStateParseXML(xmlXPathContextPtr ctxt,
int ret = -1;
char *tmp = virXPathString(xpath, ctxt);
if (tmp) {
-*val = virDomainPMStateTypeFromString(tmp);
+*val = virTristateBoolTypeFromString(tmp);
if (*val 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(unknown PM state value %s), tmp);
@@ -10879,14 +10857,14 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
tmp = virXPathString(string(./os/bootmenu[1]/@enable), ctxt);
if (tmp) {
-def-os.bootmenu = virDomainBootMenuTypeFromString(tmp);
+def-os.bootmenu = virTristateBoolTypeFromString(tmp);
if (def-os.bootmenu = 0) {
/* In order not to break misconfigured machines, this
* should not emit an error, but rather set the bootmenu
* to disabled */
VIR_WARN(disabling bootmenu due to unknown option '%s',
tmp);
-def-os.bootmenu = VIR_DOMAIN_BOOT_MENU_DISABLED;
+def-os.bootmenu = VIR_TRISTATE_BOOL_NO;
}
VIR_FREE(tmp);
}
@@ -10901,9 +10879,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
for useserial));
goto cleanup;
}
-def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES;
+def-os.bios.useserial = VIR_TRISTATE_BOOL_YES;
} else {
-def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO;
+def-os.bios.useserial = VIR_TRISTATE_BOOL_NO;
}
VIR_FREE(tmp);
}
@@ -16943,10 +16921,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
[libvirt] [PATCHv2 0/2] Tri-state bool enum cleanups
v2: better names move to virutil and use them in network_conf and device_conf too Ján Tomko (2): Introduce virTristateBool enum type Introduce virTristateSwitch enum src/conf/device_conf.c | 8 +- src/conf/device_conf.h | 10 --- src/conf/domain_conf.c | 202 +++- src/conf/domain_conf.h | 116 ++--- src/conf/network_conf.c | 11 +-- src/conf/network_conf.h | 15 +--- src/libvirt_private.syms| 28 +- src/libxl/libxl_conf.c | 6 +- src/lxc/lxc_container.c | 4 +- src/lxc/lxc_native.c| 2 +- src/network/bridge_driver.c | 3 +- src/qemu/qemu_command.c | 90 ++-- src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_process.c | 2 +- src/util/virutil.c | 11 +++ src/util/virutil.h | 21 + src/vbox/vbox_tmpl.c| 22 ++--- src/xenxs/xen_sxpr.c| 20 ++--- src/xenxs/xen_xm.c | 20 ++--- 19 files changed, 200 insertions(+), 395 deletions(-) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] util: forbid freeing const pointers
On 07/15/2014 10:04 PM, Eric Blake wrote:
Now that we've finally fixed all the violators, it's time to
enforce that any pointer to a const object is never freed (it
is aliasing some other memory, where the non-const original
should be freed instead). Alas, the code still needs a normal
vs. Coverity version, but at least we are still guaranteeing
that the macro call evaluates its argument exactly once.
I verified that we still get the following compiler warnings,
which in turn halts the build thanks to -Werror on gcc (hmm,
gcc 4.8.3's placement of the ^ for ?: type mismatch is a bit
off, but that's not our problem):
int oops1 = 0;
VIR_FREE(oops1);
const char *oops2 = NULL;
VIR_FREE(oops2);
struct blah { int dummy; } oops3;
VIR_FREE(oops3);
util/virauthconfig.c:159:35: error: pointer/integer type mismatch in
conditional expression [-Werror]
VIR_FREE(oops1);
^
util/virauthconfig.c:161:5: error: passing argument 1 of 'virFree' discards
'const' qualifier from pointer target type [-Werror]
VIR_FREE(oops2);
^
In file included from util/virauthconfig.c:28:0:
util/viralloc.h:79:6: note: expected 'void *' but argument is of type 'const
void *'
void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
^
util/virauthconfig.c:163:35: error: type mismatch in conditional expression
VIR_FREE(oops3);
^
* src/util/viralloc.h (VIR_FREE): No longer cast away const.
* src/xenapi/xenapi_utils.c (xenSessionFree): Work around bogus
header.
Signed-off-by: Eric Blake ebl...@redhat.com
---
v2: this depends on the existing 1/4, while being a replacement
to all of 2-4/4 at once.
https://www.redhat.com/archives/libvir-list/2014-July/msg00716.html
src/util/viralloc.h | 11 +--
src/xenapi/xenapi_utils.c | 4 +++-
2 files changed, 8 insertions(+), 7 deletions(-)
This patch compiles for me with clang 3.4.1 and all the three VIR_FREEs above
cause a warning.
ACK
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/2] virsh: Reject negative numbers in vshCommandOptUInt
On 07/15/14 00:51, John Ferlan wrote: Bumping again - I found a related bz in my backlog... On 05/29/2014 01:15 PM, Eric Blake wrote: On 05/29/2014 05:54 AM, Peter Krempa wrote: Use virStrToLong_uip instead of virStrToLong_ui to reject negative numbers in the helper. None of the callers expects the wraparound feature for negative numbers. I had to audit all callers, and found the following (fortunately the list is fairly small): snip vol-upload, vol-download [offset, length]: Can't a negative offset be treated as offset from the end of the file? And doesn't -1 as implying unlimited length make sense? Here, allowing -1 might make sense BZ - https://bugzilla.redhat.com/show_bug.cgi?id=1087104 has a different take on whether negative values should be allowed. In particular: Additional Info: In manual page: vol-download [--pool pool-or-uuid] [--offset bytes] [--length bytes] --offset is the position in the storage volume at which to start reading the data. --length is an upper bound of the amount of data to be downloaded. It is said that offset is the position in the storage volume at which to start reading the data, so I think the value of offset should be no smaller than 0, option length as well. So - do we adjust the man page to indicate that using a -1 is OK and what it would do? Probably similar type action for the changes made (commit id's 0e2d73051 c62125395)? Or does a negative really make sense for offset? Sure -1 makes sense and works because 'lseek()' allows it, but other negative numbers I just tried get an error: Well it might make sense semantically but it certainly isn't coded up. We'd need to wrap the number sensibly according to the length of the file so that it would mean the offset from the end of the file as Eric suggested. Also we might add the docs about how the offset is wrapped to virsh but reject those numbers in the API. This said, I'm not a big fan of the negative offset as with the theoretically unlimited file sizes (2^64bytes) you might want to have the full number space of the value we are passing as the offset available for very long offsets. Granted, that will not happen for a while so we might want to sacrifice half of the numbers for negative offsets, but we should've gone with signed types in that case. TL;DR; Taking negative portions of the --offset as offset from the back might not play well with very large files. virsh vol-download --pool default qcow3-vol2 /home/vm-images/raw --offset -2 --length -1000 error: cannot download from volume qcow3-vol2 error: Unable to seek /home/vm-images/qcow3-vol2 to 18446744073709551614: Invalid argument Not even sure what a negative length file is... Is that the definition of a sparse file? Is the suggestion that we'd be downloading (or Eric thought of lenght of -1 as full file lenght. uploading) from end of file backwards some number of bytes? Not quite sure that's what's happening as the negative value is turned positive and it seems means everything. So while, yes, -1 for both makes sense as a sort of pseudonym for maximum - other values don't, but how does one go about distinguishing that? (eg, that a -1 was supplied and it's OK, although other negative values are not). We should have designed the APIs with signed types if we wanted to take signed numbers. I still think of parsing -1 into a unsigned type as a quirk that we shouldn't abuse very much. John Peter 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 6/6] storage: fs: Don't fail volume update if backing store isn't accessible
On 07/15/14 22:29, Eric Blake wrote: On 07/15/2014 07:25 AM, Peter Krempa wrote: When the backing store of a volume wasn't accessible while updating the volume definition the call would fail alltogether. In cases where we s/alltogether/altogether/ currently (incorrectly) treat remote backing stores as local one this might lead to strange errors. Ignore the opening errors until we figure out how to track proper volume metadata. --- src/storage/storage_backend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index f5bfdee..fdcaada 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1465,7 +1465,8 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, (ret = virStorageBackendUpdateVolTargetInfo(vol-target.backingStore, updateCapacity, withBlockVolFormat, - VIR_STORAGE_VOL_OPEN_DEFAULT)) 0) + VIR_STORAGE_VOL_OPEN_DEFAULT | + VIR_STORAGE_VOL_OPEN_NOERROR) 0)) return ret; Works for me as a stopgap. (On IRC, we were discussing that we probably want to update the storage volume XML for backingStore to look a bit more like domain disk backingstore, at least for cases where we know the backing store is not in the same pool as the source - but that's a bigger project so worth later patches). ACK. I've fixed the problems pointed out in the individual reviews and pushed this series. Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] add support for --config in setmaxmem command
Chen Hanxiao (2): LXC: add support for --config in setmaxmem command LXC: use lxcDomainSetMemoryFlags to do lxcDomainSetMaxMemory's work src/lxc/lxc_driver.c | 105 +-- 1 file changed, 51 insertions(+), 54 deletions(-) -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] LXC: use lxcDomainSetMemoryFlags to do lxcDomainSetMaxMemory's work
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
src/lxc/lxc_driver.c | 36 +---
1 file changed, 5 insertions(+), 31 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index be6ee19..9f974eb 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -680,37 +680,6 @@ lxcDomainGetMaxMemory(virDomainPtr dom)
return ret;
}
-static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax)
-{
-virDomainObjPtr vm;
-int ret = -1;
-
-if (!(vm = lxcDomObjFromDomain(dom)))
-goto cleanup;
-
-if (virDomainSetMaxMemoryEnsureACL(dom-conn, vm-def) 0)
-goto cleanup;
-
-if (newmax vm-def-mem.cur_balloon) {
-if (!virDomainObjIsActive(vm)) {
-vm-def-mem.cur_balloon = newmax;
-} else {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(Cannot set max memory lower than current
- memory for an active domain));
-goto cleanup;
-}
-}
-
-vm-def-mem.max_balloon = newmax;
-ret = 0;
-
- cleanup:
-if (vm)
-virObjectUnlock(vm);
-return ret;
-}
-
static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
unsigned int flags)
{
@@ -809,6 +778,11 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned
long newmem)
return lxcDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_LIVE);
}
+static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long memory)
+{
+return lxcDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM);
+}
+
static int
lxcDomainSetMemoryParameters(virDomainPtr dom,
virTypedParameterPtr params,
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] LXC: add support for --config in setmaxmem command
In lxc, we could not use setmaxmem command
with --config options.
This patch will add support for this.
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
src/lxc/lxc_driver.c | 69 ++--
1 file changed, 46 insertions(+), 23 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index b7b4b02..be6ee19 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -721,10 +721,10 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
virLXCDomainObjPrivatePtr priv;
virLXCDriverPtr driver = dom-conn-privateData;
virLXCDriverConfigPtr cfg = NULL;
-unsigned long oldmax = 0;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
- VIR_DOMAIN_AFFECT_CONFIG, -1);
+ VIR_DOMAIN_AFFECT_CONFIG |
+ VIR_DOMAIN_MEM_MAXIMUM, -1);
if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
@@ -743,32 +743,55 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
persistentDef) 0)
goto cleanup;
-if (flags VIR_DOMAIN_AFFECT_LIVE)
-oldmax = vm-def-mem.max_balloon;
-if (flags VIR_DOMAIN_AFFECT_CONFIG) {
-if (!oldmax || oldmax persistentDef-mem.max_balloon)
-oldmax = persistentDef-mem.max_balloon;
-}
+if (flags VIR_DOMAIN_MEM_MAXIMUM) {
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+if (newmem vm-def-mem.cur_balloon) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Cannot resize the max memory less than current
+ memory for an active domain));
+goto cleanup;
+}
+vm-def-mem.max_balloon = newmem;
+}
-if (newmem oldmax) {
-virReportError(VIR_ERR_INVALID_ARG,
- %s, _(Cannot set memory higher than max memory));
-goto cleanup;
-}
+if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+sa_assert(persistentDef);
+persistentDef-mem.max_balloon = newmem;
+if (persistentDef-mem.cur_balloon newmem)
+persistentDef-mem.cur_balloon = newmem;
+if (virDomainSaveConfig(cfg-configDir, persistentDef) 0)
+goto cleanup;
+}
+} else {
+unsigned long oldmax = 0;
-if (flags VIR_DOMAIN_AFFECT_LIVE) {
-if (virCgroupSetMemory(priv-cgroup, newmem) 0) {
-virReportError(VIR_ERR_OPERATION_FAILED,
- %s, _(Failed to set memory for domain));
-goto cleanup;
+if (flags VIR_DOMAIN_AFFECT_LIVE)
+oldmax = vm-def-mem.max_balloon;
+if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+if (!oldmax || oldmax persistentDef-mem.max_balloon)
+oldmax = persistentDef-mem.max_balloon;
}
-}
-if (flags VIR_DOMAIN_AFFECT_CONFIG) {
-sa_assert(persistentDef);
-persistentDef-mem.cur_balloon = newmem;
-if (virDomainSaveConfig(cfg-configDir, persistentDef) 0)
+if (newmem oldmax) {
+virReportError(VIR_ERR_INVALID_ARG,
+ %s, _(Cannot set memory higher than max
memory));
goto cleanup;
+}
+
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+if (virCgroupSetMemory(priv-cgroup, newmem) 0) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ %s, _(Failed to set memory for domain));
+goto cleanup;
+}
+}
+
+if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+sa_assert(persistentDef);
+persistentDef-mem.cur_balloon = newmem;
+if (virDomainSaveConfig(cfg-configDir, persistentDef) 0)
+goto cleanup;
+}
}
ret = 0;
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: reconnect to domains after libvirtd restart
Roman Bogorodskiy wrote: Roman Bogorodskiy wrote: Roman Bogorodskiy wrote: Try to reconnect to the running domains after libvirtd restart. To achieve that, do: ping? ping? Roman Bogorodskiy -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-tck] bhyve: reconnect to domains after libvirtd restart
On 06/29/2014 06:06 PM, Roman Bogorodskiy wrote:
Try to reconnect to the running domains after libvirtd restart. To
achieve that, do:
* Save domain state
- Modify virBhyveProcessStart() to save domain state to the state
dir
- Modify virBhyveProcessStop() to cleanup the pidfile and the state
* Detect if the state information loaded from the driver's state
dir matches the actual state. Consider domain active if:
- PID it points to exist
- Process title of this PID matches the expected one with the
domain name
Otherwise, mark the domain as shut off.
Note: earlier development bhyve versions before FreeBSD 10.0-RELEASE
didn't set proctitle we expect, so the current code will not detect
it. I don't plan adding support for this unless somebody requests
this.
---
src/bhyve/bhyve_driver.c | 18 ++
src/bhyve/bhyve_process.c | 91
+++
src/bhyve/bhyve_process.h | 2 ++
3 files changed, 111 insertions(+)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index eb5fc95..4c7596e 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1151,6 +1151,8 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
void *opaque ATTRIBUTE_UNUSED)
{
+virConnectPtr conn = NULL;
+
if (!priveleged) {
VIR_INFO(Not running priveleged, disabling driver);
return 0;
@@ -1199,6 +1201,15 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED,
}
if (virDomainObjListLoadAllConfigs(bhyve_driver-domains,
+ BHYVE_STATE_DIR,
+ NULL, 1,
+ bhyve_driver-caps,
+ bhyve_driver-xmlopt,
+ 1 VIR_DOMAIN_VIRT_BHYVE,
+ NULL, NULL) 0)
+goto cleanup;
+
+if (virDomainObjListLoadAllConfigs(bhyve_driver-domains,
BHYVE_CONFIG_DIR,
BHYVE_AUTOSTART_DIR, 0,
bhyve_driver-caps,
@@ -1207,9 +1218,16 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED,
NULL, NULL) 0)
goto cleanup;
+conn = virConnectOpen(bhyve:///system);
The connection does not seem to be used anywhere.
QEMU driver uses it for:
qemuTranslateDiskSourcePool
qemuProcessFiltersInstantiate
qemuProcessRecoverJob
Neither of which is done in the bhyve driver.
ACK with the conn removed.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3 0/2] storagevol: add 'nocow' option to vol xml
On 07/15/2014 10:49 AM, Chunyan Liu wrote: Add 'nocow' option to vol xml, so that user can have a chance to create a volume with NOCOW flag set on btrfs. It improves performance when the volume is a file image on btrfs and is used in VM environment. --- Changes: * fix typo in V2 * add test case for 'nocow', in separate patch V2 is here: http://www.redhat.com/archives/libvir-list/2014-July/msg00361.html Chunyan Liu (2): storagevol: add nocow to vol xml add nocow test case docs/formatstorage.html.in | 7 + docs/schemas/storagevol.rng| 5 src/conf/storage_conf.c| 3 ++ src/storage/storage_backend.c | 22 +++ src/util/virstoragefile.h | 1 + .../storagevolxml2argvdata/qcow2-nocow-compat.argv | 3 ++ tests/storagevolxml2argvdata/qcow2-nocow.argv | 3 ++ tests/storagevolxml2argvtest.c | 6 tests/storagevolxml2xmlin/vol-qcow2-nocow.xml | 32 ++ tests/storagevolxml2xmlout/vol-qcow2-nocow.xml | 31 + 10 files changed, 113 insertions(+) create mode 100644 tests/storagevolxml2argvdata/qcow2-nocow-compat.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nocow.argv create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-nocow.xml create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-nocow.xml ACK I have fixed the 'since' libvirt version in the documentation and pushed the series. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/2] virsh: Reject negative numbers in vshCommandOptUInt
On 07/16/2014 03:18 AM, Peter Krempa wrote: So - do we adjust the man page to indicate that using a -1 is OK and what it would do? Probably similar type action for the changes made (commit id's 0e2d73051 c62125395)? Or does a negative really make sense for offset? Sure -1 makes sense and works because 'lseek()' allows it, but other negative numbers I just tried get an error: Well it might make sense semantically but it certainly isn't coded up. We'd need to wrap the number sensibly according to the length of the file so that it would mean the offset from the end of the file as Eric suggested. Also we might add the docs about how the offset is wrapped to virsh but reject those numbers in the API. This said, I'm not a big fan of the negative offset as with the theoretically unlimited file sizes (2^64bytes) you might want to have the full number space of the value we are passing as the offset Unlimited file size is 2^63, not 2^64 (off_t is signed; so there is no way the kernel can provide you access to more than 8 exabytes. Anyone that really has 16 exabytes of data to manage [hah!] has to split it into multiple files. Treating a negative value as distance from the end of the file is always possible. Whether or not it is practical and worth coding up is another matter. available for very long offsets. Granted, that will not happen for a while so we might want to sacrifice half of the numbers for negative offsets, but we should've gone with signed types in that case. off_t is already signed; the kernel has already sacrificed half the numbers, and lseek() already has a mode to do negative offsets from the end of an arbitrarily large file, up to the 2^63 limit imposed by off_t. TL;DR; Taking negative portions of the --offset as offset from the back might not play well with very large files. Not true. virsh vol-download --pool default qcow3-vol2 /home/vm-images/raw --offset -2 --length -1000 error: cannot download from volume qcow3-vol2 error: Unable to seek /home/vm-images/qcow3-vol2 to 18446744073709551614: Invalid argument Not even sure what a negative length file is... Is that the definition of a sparse file? Is the suggestion that we'd be downloading (or Eric thought of lenght of -1 as full file lenght. Another possibility is special-casing length 0 (not -1) as full file length, since it transferring 0 bytes is already a weird thing to do. uploading) from end of file backwards some number of bytes? Not quite sure that's what's happening as the negative value is turned positive and it seems means everything. So while, yes, -1 for both makes sense as a sort of pseudonym for maximum - other values don't, but how does one go about distinguishing that? (eg, that a -1 was supplied and it's OK, although other negative values are not). We should have designed the APIs with signed types if we wanted to take signed numbers. I still think of parsing -1 into a unsigned type as a quirk that we shouldn't abuse very much. It's a nice quirk for anyone familiar with 2s-complement. But I can also agree that not abusing it means fewer corner cases to test. -- 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 v2] util: forbid freeing const pointers
On 07/16/2014 03:06 AM, Ján Tomko wrote: On 07/15/2014 10:04 PM, Eric Blake wrote: Now that we've finally fixed all the violators, it's time to enforce that any pointer to a const object is never freed (it is aliasing some other memory, where the non-const original should be freed instead). Alas, the code still needs a normal vs. Coverity version, but at least we are still guaranteeing that the macro call evaluates its argument exactly once. This patch compiles for me with clang 3.4.1 and all the three VIR_FREEs above cause a warning. Good to know that clang will also flag violations of safe usage. ACK Pushed. -- 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] blockjob: wait for pivot to complete
On 07/14/14 18:18, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1119173 documents that commit eaba79d was flawed in the implementation of the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag when it comes to completing a blockcopy. Basically, the qemu pivot action is async (the QMP command returns immediately, but the user must wait for the BLOCK_JOB_COMPLETE event to know that all I/O related to the job has finally been flushed), but the libvirt command was documented as synchronous by default. As active block commit will also be using this code, it is worth fixing now. * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Don't skip wait loop after pivot. Signed-off-by: Eric Blake ebl...@redhat.com --- Shown here with extra context, to hopefully aid the review. src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) ACK, Peter 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] blockjob: wait for pivot to complete
On 07/16/2014 07:19 AM, Peter Krempa wrote: On 07/14/14 18:18, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1119173 documents that commit eaba79d was flawed in the implementation of the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag when it comes to completing a blockcopy. Basically, the qemu pivot action is async (the QMP command returns immediately, but the user must wait for the BLOCK_JOB_COMPLETE event to know that all I/O related to the job has finally been flushed), but the libvirt command was documented as synchronous by default. As active block commit will also be using this code, it is worth fixing now. * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Don't skip wait loop after pivot. Signed-off-by: Eric Blake ebl...@redhat.com --- Shown here with extra context, to hopefully aid the review. src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) ACK, Thanks; pushed. -- 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] [PREPOST 05/17] src/xenxs:Refactor PCI parsing code
Looking at both of your comments, I go with VIR_WARN
On Wed, Jul 16, 2014 at 12:26 AM, Eric Blake ebl...@redhat.com wrote:
On 07/11/2014 11:09 AM, Jim Fehlig wrote:
David Kiarie wrote:
+
+/* pci=[':00:1b.0',':00:13.0'] */
+if (!(key = list-str))
+goto skippci;
+if (!(nextkey = strchr(key, ':')))
+goto skippci;
+
+if (virStrncpy(domain, key, (nextkey - key), sizeof(domain))
== NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Domain %s too big for destination),
key);
Pre-existing, but while we are touching the code, I wonder if the use of
virReportError here is correct? The device is skipped if there is a
problem parsing it. I think these errors should be logged via VIR_WARN,
but would like confirmation from another libvirt dev before asking you
to change them. At the very least, the error should be changed to
VIR_ERR_CONF_SYNTAX.
How easy is it to trigger this error path via user input? If the string
that we are splitting is normally provided from a sane source, using
VIR_ERR_INTERNAL_ERROR is okay; if the string we are splitting can come
from a user that can easily try to fuzz things to confuse us, then
VIR_ERR_CONF_SYNTAX is indeed nicer.
As for whether to skip a device we can't parse, or outright fail, I'm
not sure which is better. If you are just skipping the device, then
using VIR_WARN instead of virReportError will avoid the odd case of
returning success to the user while still having an error set.
Don't know if I helped or just made it more confusing.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] leaseshelper: add enhancements to support all events
On 07/11/14 02:19, Nehal J Wani wrote: This patch enables the helper
program to detect event(s) triggered when there
is a change in lease length or expiry and client-id. This transfers complete
control of leases database to libvirt and suppresses use of the lease database
file (network-name.leases). That file will not be created, read, or written.
This is achieved by adding the option --leasefile-ro to dnsmasq and applying
the symlink technique, which helps us map events related to leases with their
corresponding network bridges.
Example:
/var/lib/libvirt/dnsmasq/virbr0.helper -
/home/wani/libvirt/src/libvirt_leaseshelper
/var/lib/libvirt/dnsmasq/virbr3.helper -
/home/wani/libvirt/src/libvirt_leaseshelper
Also, this requires the addition of a new non-lease entry in our custom lease
database: server-duid. It is required to identify a DHCPv6 server.
Now that dnsmasq doesn't maintain its own leases database, it relies on our
helper program to tell it about previous leases and server duid. Thus, this
patch makes our leases program honor an extra action: init, in which it
sends
the known info in a particular format to dnsmasq by printing it to stdout.
---
src/network/bridge_driver.c | 43 +++-
src/network/leaseshelper.c | 156
+---
2 files changed, 175 insertions(+), 24 deletions(-)
Note this message is not a full review, just a few questions before I
carry on.
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 6a2e760..1e1e53f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1287,9 +1303,30 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr
network,
LIBEXECDIR)))
goto cleanup;
+/* Symlink technique for dnsmasq lease file is needed so that libvirt
+ * can have complete control over the handling of leases database */
+
+/* Remove unwanted, old symlink */
+if (unlink(pseudo_leaseshelper_path) 0
+errno != ENOENT errno != ENOTDIR) {
+virReportSystemError(errno,
+ _(Failed to delete symlink '%s'),
+ pseudo_leaseshelper_path);
+goto cleanup;
+}
+
+/* Create a new symlink */
+if (symlink(leaseshelper_path, pseudo_leaseshelper_path) 0) {
+virReportSystemError(errno,
+ _(Failed to create symlink '%s' to '%s'),
+ leaseshelper_path, pseudo_leaseshelper_path);
+goto cleanup;
+}
+
cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
virCommandAddArgFormat(cmd, --conf-file=%s, configfile);
-virCommandAddArgFormat(cmd, --dhcp-script=%s, leaseshelper_path);
+virCommandAddArgFormat(cmd, --dhcp-script=%s,
pseudo_leaseshelper_path);
+virCommandAddArgFormat(cmd, --leasefile-ro);
Does dnsmasq pass through the rest of the environment we pass to it at
this point? The leaseshelper extracts quite a lot of stuff from the
environment variables that are passed by dnsmasq. In case it does we
could use an env variable to pass the interface name instead of linking
the helper and using different names.
A second issue that comes into my mind is compatibility with already
existing files. Does this work when you already have a lease file? (I
didn't try it, I'm just asking).
Peter
main(int argc, char **argv)
@@ -105,6 +113,7 @@ main(int argc, char **argv)
char *pid_file = NULL;
char *lease_entries = NULL;
char *custom_lease_file = NULL;
+char *server_duid = NULL;
const char *ip = NULL;
const char *mac = NULL;
const char *iaid = virGetEnvAllowSUID(DNSMASQ_IAID);
@@ -112,20 +121,26 @@ main(int argc, char **argv)
const char *interface = virGetEnvAllowSUID(DNSMASQ_INTERFACE);
const char *exptime_tmp = virGetEnvAllowSUID(DNSMASQ_LEASE_EXPIRES);
const char *hostname = virGetEnvAllowSUID(DNSMASQ_SUPPLIED_HOSTNAME);
+const char *server_duid_env = virGetEnvAllowSUID(DNSMASQ_SERVER_DUID);
Here we extract a lot of stuff ...
const char *leases_str = NULL;
long long currtime = 0;
long long expirytime = 0;
size_t i = 0;
+size_t count_ipv6 = 0;
+size_t count_ipv4 = 0;
int action = -1;
int pid_file_fd = -1;
int rv = EXIT_FAILURE;
int custom_lease_file_len = 0;
bool add = false;
+bool init = false;
bool delete = false;
virJSONValuePtr lease_new = NULL;
virJSONValuePtr lease_tmp = NULL;
virJSONValuePtr leases_array = NULL;
virJSONValuePtr leases_array_new = NULL;
+virJSONValuePtr *leases_ipv4 = NULL;
+virJSONValuePtr *leases_ipv6 = NULL;
virSetErrorFunc(NULL, NULL);
virSetErrorLogPriorityFunc(NULL);
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
Re: [libvirt] [PREPOST 03/17] src/xenxm:Refactor network config parsing code
I think you comments on PCI parsing code could also apply here
On Wed, Jul 16, 2014 at 12:19 AM, Jim Fehlig jfeh...@suse.com wrote:
Jim Fehlig wrote:
David Kiarie wrote:
From: Kiarie Kahurani davidkiar...@gmail.com
I introduced the function
xenFormatXMVif(virConfPtr conf, ..);
which parses network configuration
signed-off-by: David Kiarie davidkiar...@gmail.com
---
src/xenxs/xen_xm.c | 298
-
1 file changed, 155 insertions(+), 143 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index dc840e5..26ebd5a 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -309,6 +309,159 @@ static int xenParseXMMem(virConfPtr conf,
virDomainDefPtr def)
return 0;
}
+static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
Same comment here about blank lines between functions. I won't bore you
with repeating myself in all the patches. Also, remember Eric's comment
about function return type on one line and name and params on another.
E.g.
static int
xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
{
...
}
+{
+char *script = NULL;
+virDomainNetDefPtr net = NULL;
+virConfValuePtr list = virConfGetValue(conf, vif);
Style is to have a blank line after local variable declarations.
I think you should also define a local variable ret, initialized to -1.
+if (list list-type == VIR_CONF_LIST) {
+list = list-list;
+while (list) {
+char model[10];
+char type[10];
+char ip[16];
+char mac[18];
+char bridge[50];
+char vifname[50];
+char *key;
+
+bridge[0] = '\0';
+mac[0] = '\0';
+ip[0] = '\0';
+model[0] = '\0';
+type[0] = '\0';
+vifname[0] = '\0';
+
+if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
+goto skipnic;
+
+key = list-str;
+while (key) {
+char *data;
+char *nextkey = strchr(key, ',');
+
+if (!(data = strchr(key, '=')))
+goto skipnic;
+data++;
+
+if (STRPREFIX(key, mac=)) {
+int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
+if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(MAC address %s too big for
destination),
+ data);
+goto skipnic;
here
+}
+} else if (STRPREFIX(key, bridge=)) {
+int len = nextkey ? (nextkey - data) : sizeof(bridge)
- 1;
+if (virStrncpy(bridge, data, len, sizeof(bridge)) ==
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Bridge %s too big for
destination),
+ data);
+goto skipnic;
here
+}
+} else if (STRPREFIX(key, script=)) {
+int len = nextkey ? (nextkey - data) : strlen(data);
+VIR_FREE(script);
+if (VIR_STRNDUP(script, data, len) 0)
+goto cleanup;
+} else if (STRPREFIX(key, model=)) {
+int len = nextkey ? (nextkey - data) : sizeof(model) -
1;
+if (virStrncpy(model, data, len, sizeof(model)) ==
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Model %s too big for
destination), data);
+goto skipnic;
here
+}
+} else if (STRPREFIX(key, type=)) {
+int len = nextkey ? (nextkey - data) : sizeof(type) -
1;
+if (virStrncpy(type, data, len, sizeof(type)) == NULL)
{
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Type %s too big for
destination), data);
+goto skipnic;
here
+}
+} else if (STRPREFIX(key, vifname=)) {
+int len = nextkey ? (nextkey - data) : sizeof(vifname)
- 1;
+if (virStrncpy(vifname, data, len, sizeof(vifname)) ==
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Vifname %s too big for
destination),
+ data);
+goto skipnic;
here
+}
+} else if (STRPREFIX(key, ip=)) {
+int len = nextkey ? (nextkey - data) : sizeof(ip) - 1;
Re: [libvirt] [PREPOST 03/17] src/xenxm:Refactor network config parsing code
The use of virReportError,
On Wed, Jul 16, 2014 at 4:33 PM, David kiarie davidkiar...@gmail.com wrote:
I think you comments on PCI parsing code could also apply here
On Wed, Jul 16, 2014 at 12:19 AM, Jim Fehlig jfeh...@suse.com wrote:
Jim Fehlig wrote:
David Kiarie wrote:
From: Kiarie Kahurani davidkiar...@gmail.com
I introduced the function
xenFormatXMVif(virConfPtr conf, ..);
which parses network configuration
signed-off-by: David Kiarie davidkiar...@gmail.com
---
src/xenxs/xen_xm.c | 298
-
1 file changed, 155 insertions(+), 143 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index dc840e5..26ebd5a 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -309,6 +309,159 @@ static int xenParseXMMem(virConfPtr conf,
virDomainDefPtr def)
return 0;
}
+static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
Same comment here about blank lines between functions. I won't bore you
with repeating myself in all the patches. Also, remember Eric's comment
about function return type on one line and name and params on another.
E.g.
static int
xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
{
...
}
+{
+char *script = NULL;
+virDomainNetDefPtr net = NULL;
+virConfValuePtr list = virConfGetValue(conf, vif);
Style is to have a blank line after local variable declarations.
I think you should also define a local variable ret, initialized to -1.
+if (list list-type == VIR_CONF_LIST) {
+list = list-list;
+while (list) {
+char model[10];
+char type[10];
+char ip[16];
+char mac[18];
+char bridge[50];
+char vifname[50];
+char *key;
+
+bridge[0] = '\0';
+mac[0] = '\0';
+ip[0] = '\0';
+model[0] = '\0';
+type[0] = '\0';
+vifname[0] = '\0';
+
+if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
+goto skipnic;
+
+key = list-str;
+while (key) {
+char *data;
+char *nextkey = strchr(key, ',');
+
+if (!(data = strchr(key, '=')))
+goto skipnic;
+data++;
+
+if (STRPREFIX(key, mac=)) {
+int len = nextkey ? (nextkey - data) : sizeof(mac) -
1;
+if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(MAC address %s too big for
destination),
+ data);
+goto skipnic;
here
+}
+} else if (STRPREFIX(key, bridge=)) {
+int len = nextkey ? (nextkey - data) : sizeof(bridge)
- 1;
+if (virStrncpy(bridge, data, len, sizeof(bridge)) ==
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Bridge %s too big for
destination),
+ data);
+goto skipnic;
here
+}
+} else if (STRPREFIX(key, script=)) {
+int len = nextkey ? (nextkey - data) : strlen(data);
+VIR_FREE(script);
+if (VIR_STRNDUP(script, data, len) 0)
+goto cleanup;
+} else if (STRPREFIX(key, model=)) {
+int len = nextkey ? (nextkey - data) : sizeof(model)
- 1;
+if (virStrncpy(model, data, len, sizeof(model)) ==
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Model %s too big for
destination), data);
+goto skipnic;
here
+}
+} else if (STRPREFIX(key, type=)) {
+int len = nextkey ? (nextkey - data) : sizeof(type) -
1;
+if (virStrncpy(type, data, len, sizeof(type)) ==
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Type %s too big for
destination), data);
+goto skipnic;
here
+}
+} else if (STRPREFIX(key, vifname=)) {
+int len = nextkey ? (nextkey - data) :
sizeof(vifname) - 1;
+if (virStrncpy(vifname, data, len, sizeof(vifname))
== NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Vifname %s too big for
destination),
+ data);
+goto skipnic;
here
+}
+
Re: [libvirt] [PATCHv3 1/2] virsh: Reject negative numbers in vshCommandOptUInt
On 07/16/2014 08:47 AM, Eric Blake wrote:
On 07/16/2014 03:18 AM, Peter Krempa wrote:
So - do we adjust the man page to indicate that using a -1 is OK and
what it would do? Probably similar type action for the changes made
(commit id's 0e2d73051 c62125395)?
Or does a negative really make sense for offset? Sure -1 makes sense
and works because 'lseek()' allows it, but other negative numbers I just
tried get an error:
Well it might make sense semantically but it certainly isn't coded up.
We'd need to wrap the number sensibly according to the length of the
file so that it would mean the offset from the end of the file as Eric
suggested.
Also we might add the docs about how the offset is wrapped to virsh but
reject those numbers in the API.
This said, I'm not a big fan of the negative offset as with the
theoretically unlimited file sizes (2^64bytes) you might want to have
the full number space of the value we are passing as the offset
Unlimited file size is 2^63, not 2^64 (off_t is signed; so there is no
way the kernel can provide you access to more than 8 exabytes. Anyone
that really has 16 exabytes of data to manage [hah!] has to split it
into multiple files. Treating a negative value as distance from the end
of the file is always possible. Whether or not it is practical and
worth coding up is another matter.
But yet vol-{upload|download} has explicitly chosen to define offset as
an unsigned unsigned long - so is the claim then that is incorrect?
Should it be be an 'off_t'? thus allowing for a negative offset?
If so, that's *a lot* of code impacted compared to perhaps disallowing a
negative value for offset.
available for very long offsets. Granted, that will not happen for a
while so we might want to sacrifice half of the numbers for negative
offsets, but we should've gone with signed types in that case.
off_t is already signed; the kernel has already sacrificed half the
numbers, and lseek() already has a mode to do negative offsets from the
end of an arbitrarily large file, up to the 2^63 limit imposed by off_t.
In the scope of the vol-{upload|download} does it make sense to lseek to
some offset from the (unknown?) end of file in order to copy some set
number of bytes (or perhaps everything if length was negative)?
TL;DR;
Taking negative portions of the --offset as offset from the back might
not play well with very large files.
Not true.
virsh vol-download --pool default qcow3-vol2 /home/vm-images/raw
--offset -2 --length -1000
error: cannot download from volume qcow3-vol2
error: Unable to seek /home/vm-images/qcow3-vol2 to
18446744073709551614: Invalid argument
Not even sure what a negative length file is... Is that the definition
of a sparse file? Is the suggestion that we'd be downloading (or
Eric thought of lenght of -1 as full file lenght.
My intended sarcastic comment didn't translate well...
Another possibility is special-casing length 0 (not -1) as full file
length, since it transferring 0 bytes is already a weird thing to do.
I agree a length of 0 means essentially nothing unless you consider some
long running program that periodically uses vol-{upload|download} to
copy the difference in bytes/length of the file since the previous run.
If the previous run had no change, then copying over the entire file
because the difference in length was 0 probably wouldn't go over well. I
feel I've been down that path before...
I think allowing negative length's as the feature to mean essentially
everything is more reasonable. The bugger is documenting things.
uploading) from end of file backwards some number of bytes? Not quite
sure that's what's happening as the negative value is turned positive
and it seems means everything.
So while, yes, -1 for both makes sense as a sort of pseudonym for
maximum - other values don't, but how does one go about distinguishing
that? (eg, that a -1 was supplied and it's OK, although other negative
values are not).
We should have designed the APIs with signed types if we wanted to take
signed numbers. I still think of parsing -1 into a unsigned type as a
quirk that we shouldn't abuse very much.
It's a nice quirk for anyone familiar with 2s-complement. But I can
also agree that not abusing it means fewer corner cases to test.
Quirks keep us employed :-)
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] examples: Introduce domtop
There's this question on the list that is asked over and over again.
How do I get {cpu, memory, ...} usage in percentage? Or its modified
version: How do I plot nice graphs like virt-manager does?
It would be nice if we have an example to inspire people. And that's
what domtop should do. Yes, it could be written in different ways, but
I've chosen this one as I think it show explicitly what users need to
implement in order to imitate virt-manager's graphing.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
.gitignore | 1 +
Makefile.am | 2 +-
cfg.mk | 2 +-
configure.ac| 1 +
examples/domtop/Makefile.am | 27 +++
examples/domtop/domtop.c| 388
libvirt.spec.in | 2 +-
7 files changed, 420 insertions(+), 3 deletions(-)
create mode 100644 examples/domtop/Makefile.am
create mode 100644 examples/domtop/domtop.c
diff --git a/.gitignore b/.gitignore
index 2d4d401..90fee91 100644
--- a/.gitignore
+++ b/.gitignore
@@ -75,6 +75,7 @@
/examples/dominfo/info1
/examples/domsuspend/suspend
/examples/dommigrate/dommigrate
+/examples/domtop/domtop
/examples/hellolibvirt/hellolibvirt
/examples/openauth/openauth
/gnulib/lib/*
diff --git a/Makefile.am b/Makefile.am
index a374e1a..4aafe94 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -24,7 +24,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs
gnulib/tests \
examples/dominfo examples/domsuspend examples/apparmor \
examples/xml/nwfilter examples/openauth examples/systemtap \
tools/wireshark examples/dommigrate \
- examples/lxcconvert
+ examples/lxcconvert examples/domtop
ACLOCAL_AMFLAGS = -I m4
diff --git a/cfg.mk b/cfg.mk
index baaab71..9880704 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1078,7 +1078,7 @@ exclude_file_name_regexp--sc_prohibit_sprintf = \
exclude_file_name_regexp--sc_prohibit_strncpy = ^src/util/virstring\.c$$
exclude_file_name_regexp--sc_prohibit_strtol = \
-
^(src/(util/virsexpr|(vbox|xen|xenxs)/.*)\.c)|(examples/domsuspend/suspend.c)$$
+
^(src/(util/virsexpr|(vbox|xen|xenxs)/.*)\.c)|(examples/domsuspend/suspend.c)|(examples/domtop/domtop.c)$$
exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/virxml\.c$$
diff --git a/configure.ac b/configure.ac
index 8001e24..f37c716 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2755,6 +2755,7 @@ AC_CONFIG_FILES([\
examples/domsuspend/Makefile \
examples/dominfo/Makefile \
examples/dommigrate/Makefile \
+examples/domtop/Makefile \
examples/openauth/Makefile \
examples/hellolibvirt/Makefile \
examples/systemtap/Makefile \
diff --git a/examples/domtop/Makefile.am b/examples/domtop/Makefile.am
new file mode 100644
index 000..c5cb6c7
--- /dev/null
+++ b/examples/domtop/Makefile.am
@@ -0,0 +1,27 @@
+## Process this file with automake to produce Makefile.in
+
+## Copyright (C) 2014 Red Hat, Inc.
+##
+## This library is free software; you can redistribute it and/or
+## modify it under the terms of the GNU Lesser General Public
+## License as published by the Free Software Foundation; either
+## version 2.1 of the License, or (at your option) any later version.
+##
+## This library is distributed in the hope that it will be useful,
+## but WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+## Lesser General Public License for more details.
+##
+## You should have received a copy of the GNU Lesser General Public
+## License along with this library. If not, see
+## http://www.gnu.org/licenses/.
+
+INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include
+LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la \
+ $(COVERAGE_LDFLAGS)
+
+noinst_PROGRAMS=domtop
+
+domtop_SOURCES=domtop.c
+domtop_LDFLAGS=
+domtop_LDADD= $(LDADDS)
diff --git a/examples/domtop/domtop.c b/examples/domtop/domtop.c
new file mode 100644
index 000..fcdcc66
--- /dev/null
+++ b/examples/domtop/domtop.c
@@ -0,0 +1,388 @@
+/*
+ * domtop.c: Demo program showing how to calculate CPU usage
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Michal Privoznik mpriv...@redhat.com
+ */
+
+#include errno.h
+#include
Re: [libvirt] [PATCH 0/8] Add support for setting DAC permissions on remote filesystems
On 07/10/14 16:22, Peter Krempa wrote: Peter Krempa (8): storage: Implement storage driver helper to chown disk images storage: Add witness for checking storage volume use in security driver security: DAC: Remove superfluous link resolution security: DAC: Introduce callback to perform image chown security: DAC: Plumb usage of chown callback qemu: Implement DAC driver chown callback to co-operate with storage drv storage: Implement virStorageFileCreate for local and gluster files qemu: snapshot: Use storage driver to pre-create snapshot file Ping, could somebody please have a look :) Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix reporting of i/o errors by iohelper process
From: Jason J. Herne jjhe...@us.ibm.com libvirt_iohelper is a helper process that is exec'ed and used to handle I/O during a Qemu managed save operation. Due to a missing call to virFileWrapperFdClose, all I/O error messages reported by iohelper are lost. This patch adds a call to virFileWrapperFdClose to the cleanup phase of qemuDomainSaveMemory. This patch also modifies virFileWrapperFdClose such that errors are only reported when the length of the err_msg buffer is 0. Before now, the existence of the buffer would trigger error reporting in virFileWrapperFdClose. Signed-off-by: Jason J. Herne jjhe...@us.ibm.com --- src/qemu/qemu_driver.c | 1 + src/util/virfile.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ecccf6c..8d78805 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3015,6 +3015,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, cleanup: VIR_FORCE_CLOSE(fd); +virFileWrapperFdClose(wrapperFd); virFileWrapperFdFree(wrapperFd); VIR_FREE(xml); diff --git a/src/util/virfile.c b/src/util/virfile.c index 463064c..813b4f5 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -322,7 +322,7 @@ virFileWrapperFdClose(virFileWrapperFdPtr wfd) return 0; ret = virCommandWait(wfd-cmd, NULL); -if (wfd-err_msg) +if (wfd-err_msg strlen(wfd-err_msg)) VIR_WARN(iohelper reports: %s, wfd-err_msg); return ret; -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] examples: Introduce domtop
On 07/16/2014 07:53 AM, Michal Privoznik wrote:
There's this question on the list that is asked over and over again.
How do I get {cpu, memory, ...} usage in percentage? Or its modified
version: How do I plot nice graphs like virt-manager does?
It would be nice if we have an example to inspire people. And that's
what domtop should do. Yes, it could be written in different ways, but
I've chosen this one as I think it show explicitly what users need to
implement in order to imitate virt-manager's graphing.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
.gitignore | 1 +
Makefile.am | 2 +-
cfg.mk | 2 +-
configure.ac| 1 +
examples/domtop/Makefile.am | 27 +++
examples/domtop/domtop.c| 388
libvirt.spec.in | 2 +-
7 files changed, 420 insertions(+), 3 deletions(-)
create mode 100644 examples/domtop/Makefile.am
create mode 100644 examples/domtop/domtop.c
[first round review - I still plan to compile and examine what happens
when actually running the program, which may result in more comments...]
+++ b/cfg.mk
@@ -1078,7 +1078,7 @@ exclude_file_name_regexp--sc_prohibit_sprintf = \
exclude_file_name_regexp--sc_prohibit_strncpy = ^src/util/virstring\.c$$
exclude_file_name_regexp--sc_prohibit_strtol = \
-
^(src/(util/virsexpr|(vbox|xen|xenxs)/.*)\.c)|(examples/domsuspend/suspend.c)$$
+
^(src/(util/virsexpr|(vbox|xen|xenxs)/.*)\.c)|(examples/domsuspend/suspend.c)|(examples/domtop/domtop.c)$$
Long line. I'd be happy with the shorter equivalent:
^(src/(util/virsexpr|(vbox|xen|xenxs)/.*)|examples/dom.*/.*)\.c$$
[side question - why are we allowing strtol in vbox, xen, and xenxs?
Probably worth an independent cleanup there]
+++ b/examples/domtop/domtop.c
@@ -0,0 +1,388 @@
+
+static int debug;
+static int run_top = 1;
Worth including stdbool.h and making these bool?
+
+printf(\n%s [options] [domain name]\n\n
+ options:\n
+ -d | --debugenable debug printings\n
s/printings/messages/
+ -h | --help print this help\n
+ -c | --connect=URI hypervisor connection URI\n
+ -D | --delay=X delay between updates in miliseconds\n,
s/miliseconds/milliseconds/
+ unified_progname);
+}
+
+static int
+parse_argv(int argc, char *argv[],
+ const char **uri,
+ const char **dom_name,
+ unsigned int *mili_seconds)
s/mili_seconds/milliseconds/
+
+while ((arg = getopt_long(argc, argv, +:dhc:D:, opt, NULL)) != -1) {
+switch (arg) {
+case 'd':
+debug = 1;
again, bool might be nicer here.
+
+printf(Running domains:\n);
+printf(\n);
Personal preference - I like puts() rather than printf() when there is
no % in the format string.
+
+static int
+do_top(virConnectPtr conn,
+ const char *dom_name,
+ unsigned int mili_seconds)
s/mili_seconds/milliseconds/
Overall looks fairly useful.
--
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] Fix reporting of i/o errors by iohelper process
On 07/16/2014 08:12 AM, Jason J. Herne wrote: From: Jason J. Herne jjhe...@us.ibm.com libvirt_iohelper is a helper process that is exec'ed and used to handle I/O during a Qemu managed save operation. Due to a missing call to virFileWrapperFdClose, all I/O error messages reported by iohelper are lost. This patch adds a call to virFileWrapperFdClose to the cleanup phase of qemuDomainSaveMemory. This patch also modifies virFileWrapperFdClose such that errors are only reported when the length of the err_msg buffer is 0. Before now, the existence of the buffer would trigger error reporting in virFileWrapperFdClose. Signed-off-by: Jason J. Herne jjhe...@us.ibm.com --- src/qemu/qemu_driver.c | 1 + src/util/virfile.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) Nice! Thanks for persevering on this one. Congrats on your first libvirt patch. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ecccf6c..8d78805 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3015,6 +3015,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, cleanup: VIR_FORCE_CLOSE(fd); +virFileWrapperFdClose(wrapperFd); virFileWrapperFdFree(wrapperFd); VIR_FREE(xml); diff --git a/src/util/virfile.c b/src/util/virfile.c index 463064c..813b4f5 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -322,7 +322,7 @@ virFileWrapperFdClose(virFileWrapperFdPtr wfd) return 0; ret = virCommandWait(wfd-cmd, NULL); -if (wfd-err_msg) +if (wfd-err_msg strlen(wfd-err_msg)) Micro-optimization: more efficient when written: if (wfd-err_msg *wfd-err_msg) since then you don't have to waste time of crawling the entire string. ACK based on looking at the code, and I'll push with that modification once I've also tested it (or reply again if something turns up). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 00/16] Support for per-guest-node binding
v3 of https://www.redhat.com/archives/libvir-list/2014-July/msg00372.html v3: - Michal's suggestions worked in - rebased on current master Martin Kletzander (16): qemu: purely a code movement qemu: remove useless error check conf: purely a code movement conf, schema: add 'id' field for cells numatune: create new module for numatune numatune: unify numatune struct and enum names numatune: Encapsulate numatune configuration in order to unify results conf, schema: add support for memnode elements numatune: add support for per-node memory bindings in private APIs qemu: allow qmp probing for cmdline options without params qemu: memory-backend-ram capability probing qemu: newer -numa parameter capability probing qemu: enable disjoint numa cpu ranges qemu: pass numa node binding preferences to qemu qemu: split out cpuset.mems setting qemu: leave restricting cpuset.mems after initialization docs/formatdomain.html.in | 32 +- docs/schemas/domaincommon.rng | 22 + po/POTFILES.in | 1 + src/Makefile.am| 3 +- src/conf/cpu_conf.c| 41 +- src/conf/cpu_conf.h| 3 +- src/conf/domain_conf.c | 203 ++- src/conf/domain_conf.h | 10 +- src/conf/numatune_conf.c | 595 + src/conf/numatune_conf.h | 108 src/libvirt_private.syms | 24 +- src/lxc/lxc_cgroup.c | 20 +- src/lxc/lxc_controller.c | 5 +- src/lxc/lxc_native.c | 15 +- src/parallels/parallels_driver.c | 7 +- src/qemu/qemu_capabilities.c | 16 +- src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_cgroup.c | 52 +- src/qemu/qemu_cgroup.h | 4 +- src/qemu/qemu_command.c| 98 +++- src/qemu/qemu_driver.c | 84 ++- src/qemu/qemu_monitor.c| 6 +- src/qemu/qemu_monitor.h| 3 +- src/qemu/qemu_monitor_json.c | 8 +- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_process.c| 12 +- src/util/virnuma.c | 61 +-- src/util/virnuma.h | 28 +- tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 5 + tests/qemumonitorjsontest.c| 20 +- .../qemuxml2argv-cpu-numa-disjoint.args| 6 + .../qemuxml2argv-cpu-numa-disjoint.xml | 28 + tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 + .../qemuxml2argv-numatune-auto-prefer.xml | 29 + .../qemuxml2argv-numatune-memnode-no-memory.args | 8 + .../qemuxml2argv-numatune-memnode-no-memory.xml| 30 ++ .../qemuxml2argv-numatune-memnode-nocpu.xml| 25 + .../qemuxml2argv-numatune-memnode.args | 11 + .../qemuxml2argv-numatune-memnode.xml | 33 ++ .../qemuxml2argv-numatune-memnodes-problematic.xml | 31 ++ tests/qemuxml2argvtest.c | 12 + .../qemuxml2xmlout-cpu-numa1.xml | 28 + .../qemuxml2xmlout-cpu-numa2.xml | 28 + .../qemuxml2xmlout-numatune-auto-prefer.xml| 29 + .../qemuxml2xmlout-numatune-memnode.xml| 33 ++ tests/qemuxml2xmltest.c| 8 + 49 files changed, 1479 insertions(+), 389 deletions(-) create mode 100644 src/conf/numatune_conf.c create mode 100644 src/conf/numatune_conf.h create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml create mode 100644
[libvirt] [PATCH v3 04/16] conf, schema: add 'id' field for cells
In XML format, by definition, order of fields should not matter, so
order of parsing the elements doesn't affect the end result. When
specifying guest NUMA cells, we depend only on the order of the 'cell'
elements. With this patch all older domain XMLs are parsed as before,
but with the 'id' attribute they are parsed and formatted according to
that field. This will be useful when we have tuning settings for
particular guest NUMA node.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 17 +
docs/schemas/domaincommon.rng | 5 +++
src/conf/cpu_conf.c| 41 +++---
src/conf/cpu_conf.h| 3 +-
src/qemu/qemu_command.c| 2 +-
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 +
tests/qemuxml2argvtest.c | 1 +
.../qemuxml2xmlout-cpu-numa1.xml | 28 +++
.../qemuxml2xmlout-cpu-numa2.xml | 28 +++
tests/qemuxml2xmltest.c| 3 ++
12 files changed, 145 insertions(+), 20 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b69da4c..9f1082b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1030,8 +1030,8 @@
lt;cpugt;
...
lt;numagt;
- lt;cell cpus='0-3' memory='512000'/gt;
- lt;cell cpus='4-7' memory='512000'/gt;
+ lt;cell id='0' cpus='0-3' memory='512000'/gt;
+ lt;cell id='1' cpus='4-7' memory='512000'/gt;
lt;/numagt;
...
lt;/cpugt;
@@ -1039,10 +1039,15 @@
p
Each codecell/code element specifies a NUMA cell or a NUMA node.
- codecpus/code specifies the CPU or range of CPUs that are part of
- the node. codememory/code specifies the node memory in kibibytes
- (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
- or nodeid in the increasing order starting from 0.
+ codecpus/code specifies the CPU or range of CPUs that are
+ part of the node. codememory/code specifies the node memory
+ in kibibytes (i.e. blocks of 1024 bytes).
+ span class=sinceSince 1.2.7/span all cells should
+ have codeid/code attribute in case referring to some cell is
+ necessary in the code, otherwise the cells are
+ assigned codeid/codes in the increasing order starting from
+ 0. Mixing cells with and without the codeid/code attribute
+ is not recommended as it may result in unwanted behaviour.
/p
p
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7be028d..155a33e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3902,6 +3902,11 @@
define name=numaCell
element name=cell
+ optional
+attribute name=id
+ ref name=unsignedInt/
+/attribute
+ /optional
attribute name=cpus
ref name=cpuset/
/attribute
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 811893d..5003cf1 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -152,7 +152,6 @@ virCPUDefCopy(const virCPUDef *cpu)
copy-ncells_max = copy-ncells = cpu-ncells;
for (i = 0; i cpu-ncells; i++) {
-copy-cells[i].cellid = cpu-cells[i].cellid;
copy-cells[i].mem = cpu-cells[i].mem;
copy-cells[i].cpumask = virBitmapNewCopy(cpu-cells[i].cpumask);
@@ -438,17 +437,48 @@ virCPUDefParseXML(xmlNodePtr node,
for (i = 0; i n; i++) {
char *cpus, *memory;
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
[libvirt] [PATCH v3 03/16] conf: purely a code movement
to ease the review of commits to follow.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c | 52 +-
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1d83f13..315ea6a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11692,6 +11692,32 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(nodes);
+/* analysis of cpu handling */
+if ((node = virXPathNode(./cpu[1], ctxt)) != NULL) {
+xmlNodePtr oldnode = ctxt-node;
+ctxt-node = node;
+def-cpu = virCPUDefParseXML(node, ctxt, VIR_CPU_TYPE_GUEST);
+ctxt-node = oldnode;
+
+if (def-cpu == NULL)
+goto error;
+
+if (def-cpu-sockets
+def-maxvcpus
+def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
+virReportError(VIR_ERR_XML_DETAIL, %s,
+ _(Maximum CPUs greater than topology limit));
+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;
+}
+}
+
/* Extract numatune if exists. */
if ((n = virXPathNodeSet(./numatune, ctxt, nodes)) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -12889,32 +12915,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
-/* analysis of cpu handling */
-if ((node = virXPathNode(./cpu[1], ctxt)) != NULL) {
-xmlNodePtr oldnode = ctxt-node;
-ctxt-node = node;
-def-cpu = virCPUDefParseXML(node, ctxt, VIR_CPU_TYPE_GUEST);
-ctxt-node = oldnode;
-
-if (def-cpu == NULL)
-goto error;
-
-if (def-cpu-sockets
-def-maxvcpus
-def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
-virReportError(VIR_ERR_XML_DETAIL, %s,
- _(Maximum CPUs greater than topology limit));
-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 ((node = virXPathNode(./sysinfo[1], ctxt)) != NULL) {
xmlNodePtr oldnode = ctxt-node;
ctxt-node = node;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 10/16] qemu: allow qmp probing for cmdline options without params
That can be lately achieved with by having .param == NULL in the
virQEMUCapsCommandLineProps struct.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 10 --
src/qemu/qemu_monitor.c | 6 --
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 8 +++-
src/qemu/qemu_monitor_json.h | 3 ++-
tests/qemumonitorjsontest.c | 20 +---
6 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c5fef53..a8d4648 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2429,6 +2429,7 @@ static int
virQEMUCapsProbeQMPCommandLine(virQEMUCapsPtr qemuCaps,
qemuMonitorPtr mon)
{
+bool found = false;
int nvalues;
char **values;
size_t i, j;
@@ -2436,10 +2437,15 @@ virQEMUCapsProbeQMPCommandLine(virQEMUCapsPtr qemuCaps,
for (i = 0; i ARRAY_CARDINALITY(virQEMUCapsCommandLine); i++) {
if ((nvalues = qemuMonitorGetCommandLineOptionParameters(mon,
virQEMUCapsCommandLine[i].option,
- values)) 0)
+ values,
+ found)) 0)
return -1;
+
+if (found !virQEMUCapsCommandLine[i].param)
+virQEMUCapsSet(qemuCaps, virQEMUCapsCommandLine[i].flag);
+
for (j = 0; j nvalues; j++) {
-if (STREQ(virQEMUCapsCommandLine[i].param, values[j])) {
+if (STREQ_NULLABLE(virQEMUCapsCommandLine[i].param, values[j])) {
virQEMUCapsSet(qemuCaps, virQEMUCapsCommandLine[i].flag);
break;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index db3dd73..3d9f87b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3659,7 +3659,8 @@ int qemuMonitorGetEvents(qemuMonitorPtr mon,
int
qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr mon,
const char *option,
- char ***params)
+ char ***params,
+ bool *found)
{
VIR_DEBUG(mon=%p option=%s params=%p, mon, option, params);
@@ -3675,7 +3676,8 @@ qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr
mon,
return -1;
}
-return qemuMonitorJSONGetCommandLineOptionParameters(mon, option, params);
+return qemuMonitorJSONGetCommandLineOptionParameters(mon, option,
+ params, found);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8a23267..c3695f2 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -748,7 +748,8 @@ int qemuMonitorGetEvents(qemuMonitorPtr mon,
char ***events);
int qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr mon,
const char *option,
- char ***params);
+ char ***params,
+ bool *found);
int qemuMonitorGetKVMState(qemuMonitorPtr mon,
bool *enabled,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bef6a14..a62c02f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4504,7 +4504,8 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr mon,
int
qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
const char *option,
- char ***params)
+ char ***params,
+ bool *found)
{
int ret;
virJSONValuePtr cmd = NULL;
@@ -4516,6 +4517,8 @@
qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
size_t i;
*params = NULL;
+if (found)
+*found = false;
/* query-command-line-options has fixed output for a given qemu
* binary; but since callers want to query parameters for one
@@ -4576,6 +4579,9 @@
qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
goto cleanup;
}
+if (found)
+*found = true;
+
if ((n = virJSONValueArraySize(data)) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(query-command-line-options parameter data was not
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 385211c..5f6c846 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -332,7 +332,8 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr
[libvirt] [PATCH v3 02/16] qemu: remove useless error check
Excerpt from the virCommandAddArgBuffer() description: Correctly transfers memory errors or contents from buf to cmd. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_command.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4307f1f..9fc0778 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6412,9 +6412,6 @@ qemuBuildNumaArgStr(const virDomainDef *def, virCommandPtr cmd) virBufferAdd(buf, cpumask, -1); virBufferAsprintf(buf, ,mem=%d, cellmem); -if (virBufferCheckError(buf) 0) -goto cleanup; - virCommandAddArgBuffer(cmd, buf); } ret = 0; -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 11/16] qemu: memory-backend-ram capability probing
The numa patch series in qemu adds memory-backend-ram object type by
which we can tell whether we can use such objects.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a8d4648..c1a8947 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -260,6 +260,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
msg-timestamp,
active-commit,
change-backing-file,
+
+ memory-backend-ram, /* 170 */
);
@@ -1477,6 +1479,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ ich9-intel-hda, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA },
{ pvpanic, QEMU_CAPS_DEVICE_PANIC },
{ usb-kbd, QEMU_CAPS_DEVICE_USB_KBD },
+{ memory-backend-ram, QEMU_CAPS_OBJECT_MEMORY_RAM },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 99cf9ed..c661d5a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -209,6 +209,7 @@ typedef enum {
QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */
QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */
QEMU_CAPS_CHANGE_BACKING_FILE = 169, /* change name of backing file in
metadata */
+QEMU_CAPS_OBJECT_MEMORY_RAM = 170, /* -object memory-backend-ram */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 12/16] qemu: newer -numa parameter capability probing
When qemu switched to using OptsVisitor for -numa parameter, it did
two things in the same patch. One of them is that the numa parameter
is now visible in query-command-line-options, the second one is that
it enabled using disjoint cpu ranges for -numa specification. This
will be used in later patch.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps| 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 5 +
4 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c1a8947..07306e5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -262,6 +262,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
change-backing-file,
memory-backend-ram, /* 170 */
+ numa,
);
@@ -2426,6 +2427,7 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
{ boot-opts, reboot-timeout, QEMU_CAPS_REBOOT_TIMEOUT },
{ spice, disable-agent-file-xfer, QEMU_CAPS_SPICE_FILE_XFER_DISABLE },
{ msg, timestamp, QEMU_CAPS_MSG_TIMESTAMP },
+{ numa, NULL, QEMU_CAPS_NUMA },
};
static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c661d5a..4332633 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -210,6 +210,7 @@ typedef enum {
QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */
QEMU_CAPS_CHANGE_BACKING_FILE = 169, /* change name of backing file in
metadata */
QEMU_CAPS_OBJECT_MEMORY_RAM = 170, /* -object memory-backend-ram */
+QEMU_CAPS_NUMA = 171, /* newer -numa handling with disjoint
cpu ranges */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
index 32bccdb..4b9f693 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
@@ -142,4 +142,5 @@
flag name='usb-kbd'/
flag name='host-pci-multidomain'/
flag name='msg-timestamp'/
+flag name='numa'/
/qemuCaps
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
b/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
index a60542a..ec7451f 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
@@ -2389,6 +2389,11 @@
{
parameters: [
],
+option: numa
+},
+{
+parameters: [
+],
option: device
},
{
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 05/16] numatune: create new module for numatune
There are many places with numatune-related code that should be put
into special numatune_conf and this patch creates a basis for that.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/Makefile.am | 3 ++-
src/conf/domain_conf.h | 2 +-
src/conf/numatune_conf.c | 37 ++
src/conf/numatune_conf.h | 54
src/libvirt_private.syms | 12 ++
src/qemu/qemu_capabilities.c | 1 +
src/util/virnuma.c | 13 +--
src/util/virnuma.h | 26 ++---
8 files changed, 106 insertions(+), 42 deletions(-)
create mode 100644 src/conf/numatune_conf.c
create mode 100644 src/conf/numatune_conf.h
diff --git a/src/Makefile.am b/src/Makefile.am
index a40e63f..982f63d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -252,7 +252,8 @@ DOMAIN_CONF_SOURCES =
\
conf/domain_conf.c conf/domain_conf.h \
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/snapshot_conf.c conf/snapshot_conf.h \
+ conf/numatune_conf.c conf/numatune_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 32674e0..fc7680c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -36,6 +36,7 @@
# include virhash.h
# include virsocketaddr.h
# include nwfilter_params.h
+# include numatune_conf.h
# include virnetdevmacvlan.h
# include virsysinfo.h
# include virnetdevvportprofile.h
@@ -46,7 +47,6 @@
# include device_conf.h
# include virbitmap.h
# include virstoragefile.h
-# include virnuma.h
# include virseclabel.h
/* forward declarations of all device types, required by
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
new file mode 100644
index 000..e9be040
--- /dev/null
+++ b/src/conf/numatune_conf.c
@@ -0,0 +1,37 @@
+/*
+ * numatune_conf.c
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Martin Kletzander mklet...@redhat.com
+ */
+
+#include config.h
+
+#include numatune_conf.h
+
+VIR_ENUM_IMPL(virDomainNumatuneMemMode,
+ VIR_DOMAIN_NUMATUNE_MEM_LAST,
+ strict,
+ preferred,
+ interleave);
+
+VIR_ENUM_IMPL(virNumaTuneMemPlacementMode,
+ VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_LAST,
+ default,
+ static,
+ auto);
diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
new file mode 100644
index 000..6bdfdc0
--- /dev/null
+++ b/src/conf/numatune_conf.h
@@ -0,0 +1,54 @@
+/*
+ * numatune_conf.h
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Martin Kletzander mklet...@redhat.com
+ */
+
+#ifndef __NUMATUNE_CONF_H__
+# define __NUMATUNE_CONF_H__
+
+# include internal.h
+# include virutil.h
+# include virbitmap.h
+
+typedef enum {
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_DEFAULT = 0,
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC,
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO,
+
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_LAST
+} virDomainNumaTuneMemPlacementMode;
+
+VIR_ENUM_DECL(virNumaTuneMemPlacementMode)
+
+VIR_ENUM_DECL(virDomainNumatuneMemMode)
+
+typedef struct _virNumaTuneDef virNumaTuneDef;
+typedef virNumaTuneDef *virNumaTuneDefPtr;
+struct _virNumaTuneDef {
+struct {
+virBitmapPtr nodemask;
+
[libvirt] [PATCH v3 09/16] numatune: add support for per-node memory bindings in private APIs
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/numatune_conf.c | 111 ---
src/conf/numatune_conf.h | 14 --
src/libvirt_private.syms | 1 +
src/lxc/lxc_cgroup.c | 3 +-
src/qemu/qemu_cgroup.c | 2 +-
src/qemu/qemu_driver.c | 10 ++---
src/util/virnuma.c | 6 +--
7 files changed, 117 insertions(+), 30 deletions(-)
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index a39c028..82418aa 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -63,6 +63,18 @@ struct _virDomainNumatune {
};
+static inline bool
+virDomainNumatuneNodeSpecified(virDomainNumatunePtr numatune,
+ int cellid)
+{
+if (numatune
+cellid = 0
+cellid numatune-nmem_nodes)
+return numatune-mem_nodes[cellid].nodeset;
+
+return false;
+}
+
static int
virDomainNumatuneNodeParseXML(virDomainDefPtr def,
xmlXPathContextPtr ctxt)
@@ -330,26 +342,37 @@ virDomainNumatuneFree(virDomainNumatunePtr numatune)
}
virDomainNumatuneMemMode
-virDomainNumatuneGetMode(virDomainNumatunePtr numatune)
+virDomainNumatuneGetMode(virDomainNumatunePtr numatune,
+ int cellid)
{
-return (numatune numatune-memory.specified) ? numatune-memory.mode :
0;
+if (!numatune)
+return 0;
+
+if (virDomainNumatuneNodeSpecified(numatune, cellid))
+return numatune-mem_nodes[cellid].mode;
+
+if (numatune-memory.specified)
+return numatune-memory.mode;
+
+return 0;
}
virBitmapPtr
virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune,
-virBitmapPtr auto_nodeset)
+virBitmapPtr auto_nodeset,
+int cellid)
{
if (!numatune)
return NULL;
-if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
+if (numatune-memory.specified
+numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
return auto_nodeset;
-/*
- * This weird logic has the same meaning as switch with
- * auto/static/default, but can be more readably changed later.
- */
-if (numatune-memory.placement != VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC)
+if (virDomainNumatuneNodeSpecified(numatune, cellid))
+return numatune-mem_nodes[cellid].nodeset;
+
+if (!numatune-memory.specified)
return NULL;
return numatune-memory.nodeset;
@@ -357,23 +380,31 @@ virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune,
char *
virDomainNumatuneFormatNodeset(virDomainNumatunePtr numatune,
- virBitmapPtr auto_nodeset)
+ virBitmapPtr auto_nodeset,
+ int cellid)
{
return virBitmapFormat(virDomainNumatuneGetNodeset(numatune,
- auto_nodeset));
+ auto_nodeset,
+ cellid));
}
int
virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune,
virBitmapPtr auto_nodeset,
-char **mask)
+char **mask,
+int cellid)
{
*mask = NULL;
if (!numatune)
return 0;
-if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO
+if (!virDomainNumatuneNodeSpecified(numatune, cellid)
+!numatune-memory.specified)
+return 0;
+
+if (numatune-memory.specified
+numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO
!auto_nodeset) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Advice from numad is needed in case of
@@ -381,7 +412,7 @@ virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr
numatune,
return -1;
}
-*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset);
+*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset, cellid);
if (!*mask)
return -1;
@@ -475,6 +506,35 @@ virDomainNumatuneSet(virDomainDefPtr def,
return ret;
}
+static bool
+virDomainNumatuneNodesEqual(virDomainNumatunePtr n1,
+virDomainNumatunePtr n2)
+{
+size_t i = 0;
+
+if (n1-nmem_nodes != n2-nmem_nodes)
+return false;
+
+for (i = 0; i n1-nmem_nodes; i++) {
+virDomainNumatuneNodePtr nd1 = n1-mem_nodes[i];
+virDomainNumatuneNodePtr nd2 = n2-mem_nodes[i];
+
+if (!nd1-nodeset !nd2-nodeset)
+continue;
+
+if (!nd1-nodeset || !nd2-nodeset)
+return false;
+
+if (nd1-mode != nd2-mode)
+return false;
+
+if (!virBitmapEqual(nd1-nodeset, nd2-nodeset))
+return false;
+}
+
+return true;
+}
+
bool
[libvirt] [PATCH v3 08/16] conf, schema: add support for memnode elements
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 15 ++
docs/schemas/domaincommon.rng | 17 ++
src/conf/numatune_conf.c | 187 +++--
.../qemuxml2argv-numatune-memnode-no-memory.xml| 30
.../qemuxml2argv-numatune-memnode-nocpu.xml| 25 +++
.../qemuxml2argv-numatune-memnode.xml | 33
.../qemuxml2argv-numatune-memnodes-problematic.xml | 31
tests/qemuxml2argvtest.c | 2 +
.../qemuxml2xmlout-numatune-memnode.xml| 33
tests/qemuxml2xmltest.c| 2 +
10 files changed, 362 insertions(+), 13 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9f1082b..1301639 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -709,6 +709,8 @@
...
lt;numatunegt;
lt;memory mode=strict nodeset=1-4,^3/gt;
+lt;memnode cellid=0 mode=strict nodeset=1/gt;
+lt;memnode cellid=2 mode=preferred nodeset=2/gt;
lt;/numatunegt;
...
lt;/domaingt;
@@ -745,6 +747,19 @@
span class='since'Since 0.9.3/span
/dd
+ dtcodememnode/code/dt
+ dd
+Optional codememnode/code elements can specify memory allocation
+policies per each guest NUMA node. For those nodes having no
+corresponding codememnode/code element, the default from
+element codememory/code will be used. Attribute
codecellid/code
+addresses guest NUMA node for which the settings are applied.
+Attributes codemode/code and codenodeset/code have the same
+meaning and syntax as in codememory/code element.
+
+This setting is not compatible with automatic placement.
+span class='since'QEMU Since 1.2.7/span
+ /dd
/dl
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 155a33e..0b31261 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -789,6 +789,23 @@
/choice
/element
/optional
+ zeroOrMore
+element name=memnode
+ attribute name=cellid
+ref name=unsignedInt/
+ /attribute
+ attribute name=mode
+choice
+ valuestrict/value
+ valuepreferred/value
+ valueinterleave/value
+/choice
+ /attribute
+ attribute name='nodeset'
+ref name='cpuset'/
+ /attribute
+/element
+ /zeroOrMore
/element
/define
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 6ce1e2d..a39c028 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -42,17 +42,140 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement,
static,
auto);
+typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
+typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
+
struct _virDomainNumatune {
struct {
+bool specified;
virBitmapPtr nodeset;
virDomainNumatuneMemMode mode;
virDomainNumatunePlacement placement;
} memory; /* pinning for all the memory */
+struct _virDomainNumatuneNode {
+virBitmapPtr nodeset;
+virDomainNumatuneMemMode mode;
+} *mem_nodes; /* fine tuning per guest node */
+size_t nmem_nodes;
+
/* Future NUMA tuning related stuff should go here. */
};
+static int
+virDomainNumatuneNodeParseXML(virDomainDefPtr def,
+ xmlXPathContextPtr ctxt)
+{
+char *tmp = NULL;
+int n = 0;;
+int ret = -1;
+size_t i = 0;
+xmlNodePtr *nodes = NULL;
+
+if ((n = virXPathNodeSet(./numatune/memnode, ctxt, nodes)) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot extract memnode nodes));
+goto cleanup;
+}
+
+if (!n)
+return 0;
+
+if (def-numatune def-numatune-memory.specified
+def-numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Per-node binding is not compatible with
+ automatic NUMA placement.));
+goto cleanup;
+}
+
+if (!def-cpu || !def-cpu-ncells) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Element 'memnode' is invalid without
+ any guest NUMA cells));
+goto
[libvirt] [PATCH v3 01/16] qemu: purely a code movement
to ease the review of commits to follow.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2185ef4..4307f1f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6384,12 +6384,24 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virCommandPtr cmd)
int ret = -1;
for (i = 0; i def-cpu-ncells; i++) {
+int cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024);
+def-cpu-cells[i].mem = cellmem * 1024;
+
VIR_FREE(cpumask);
+
+if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
+goto cleanup;
+
+if (strchr(cpumask, ',')) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(disjoint NUMA cpu ranges are not supported
+ with this QEMU));
+goto cleanup;
+}
+
virCommandAddArg(cmd, -numa);
virBufferAsprintf(buf, node,nodeid=%d, def-cpu-cells[i].cellid);
virBufferAddLit(buf, ,cpus=);
-if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
-goto cleanup;
/* Up through qemu 1.4, -numa does not accept a cpus
* argument any more complex than start-stop.
@@ -6397,16 +6409,8 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virCommandPtr cmd)
* XXX For qemu 1.5, the syntax has not yet been decided;
* but when it is, we need a capability bit and
* translation of our cpumask into the qemu syntax. */
-if (strchr(cpumask, ',')) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(disjoint NUMA cpu ranges are not supported
- with this QEMU));
-goto cleanup;
-}
virBufferAdd(buf, cpumask, -1);
-def-cpu-cells[i].mem = VIR_DIV_UP(def-cpu-cells[i].mem,
-1024) * 1024;
-virBufferAsprintf(buf, ,mem=%d, def-cpu-cells[i].mem / 1024);
+virBufferAsprintf(buf, ,mem=%d, cellmem);
if (virBufferCheckError(buf) 0)
goto cleanup;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 13/16] qemu: enable disjoint numa cpu ranges
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c| 26 ++--
.../qemuxml2argv-cpu-numa-disjoint.args| 6 +
.../qemuxml2argv-cpu-numa-disjoint.xml | 28 ++
tests/qemuxml2argvtest.c | 2 ++
tests/qemuxml2xmltest.c| 1 +
5 files changed, 51 insertions(+), 12 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bf0f56e..f1dbb34 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6376,11 +6376,13 @@ qemuBuildSmpArgStr(const virDomainDef *def,
}
static int
-qemuBuildNumaArgStr(const virDomainDef *def, virCommandPtr cmd)
+qemuBuildNumaArgStr(const virDomainDef *def,
+virCommandPtr cmd,
+virQEMUCapsPtr qemuCaps)
{
size_t i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
-char *cpumask = NULL;
+char *cpumask = NULL, *tmpmask = NULL, *next = NULL;
int ret = -1;
for (i = 0; i def-cpu-ncells; i++) {
@@ -6392,7 +6394,8 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virCommandPtr cmd)
if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
goto cleanup;
-if (strchr(cpumask, ',')) {
+if (strchr(cpumask, ',')
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(disjoint NUMA cpu ranges are not supported
with this QEMU));
@@ -6401,15 +6404,14 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virCommandPtr cmd)
virCommandAddArg(cmd, -numa);
virBufferAsprintf(buf, node,nodeid=%zu, i);
-virBufferAddLit(buf, ,cpus=);
-/* Up through qemu 1.4, -numa does not accept a cpus
- * argument any more complex than start-stop.
- *
- * XXX For qemu 1.5, the syntax has not yet been decided;
- * but when it is, we need a capability bit and
- * translation of our cpumask into the qemu syntax. */
-virBufferAdd(buf, cpumask, -1);
+for (tmpmask = cpumask; tmpmask; tmpmask = next) {
+if ((next = strchr(tmpmask, ',')))
+*(next++) = '\0';
+virBufferAddLit(buf, ,cpus=);
+virBufferAdd(buf, tmpmask, -1);
+}
+
virBufferAsprintf(buf, ,mem=%d, cellmem);
virCommandAddArgBuffer(cmd, buf);
@@ -7240,7 +7242,7 @@ qemuBuildCommandLine(virConnectPtr conn,
VIR_FREE(smp);
if (def-cpu def-cpu-ncells)
-if (qemuBuildNumaArgStr(def, cmd) 0)
+if (qemuBuildNumaArgStr(def, cmd, qemuCaps) 0)
goto error;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_UUID))
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args
new file mode 100644
index 000..073e84b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args
@@ -0,0 +1,6 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc \
+-m 214 -smp 16 -numa node,nodeid=0,cpus=0-3,cpus=8-11,mem=107 \
+-numa node,nodeid=1,cpus=4-7,cpus=12-15,mem=107 -nographic -monitor \
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -usb -net none -serial
none \
+-parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
new file mode 100644
index 000..474a238
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
@@ -0,0 +1,28 @@
+domain type='qemu'
+ nameQEMUGuest1/name
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+ memory unit='KiB'219100/memory
+ currentMemory unit='KiB'219100/currentMemory
+ vcpu placement='static'16/vcpu
+ os
+type arch='x86_64' machine='pc'hvm/type
+boot dev='network'/
+ /os
+ cpu
+topology sockets='2' cores='4' threads='2'/
+numa
+ cell id='0' cpus='0-3,8-11' memory='109550'/
+ cell id='1' cpus='4-7,12-15' memory='109550'/
+/numa
+ /cpu
+ clock offset='utc'/
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashdestroy/on_crash
+ devices
+emulator/usr/bin/qemu/emulator
+controller type='usb' index='0'/
+controller type='pci' index='0' model='pci-root'/
+memballoon model='virtio'/
+ /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 308a376..56d3fd5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1178,6 +1178,8 @@ mymain(void)
DO_TEST(cpu-numa1, NONE);
DO_TEST(cpu-numa2, QEMU_CAPS_SMP_TOPOLOGY);
DO_TEST_PARSE_ERROR(cpu-numa3, NONE);
+
[libvirt] [PATCH v3 14/16] qemu: pass numa node binding preferences to qemu
Currently, we only bind the whole QEMU domain to memory nodes
specified in nodemask altogether. That, however, doesn't make much
sense when one wants to control from where the memory for particular
guest nodes should be allocated. QEMU allows us to do that by
specifying 'host-nodes' parameter for the 'memory-backend-ram' object,
so let's use that.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c| 59 +-
.../qemuxml2argv-numatune-memnode-no-memory.args | 8 +++
.../qemuxml2argv-numatune-memnode.args | 11
tests/qemuxml2argvtest.c | 7 +++
4 files changed, 84 insertions(+), 1 deletion(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f1dbb34..6235a74 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -150,6 +150,11 @@ VIR_ENUM_IMPL(qemuDomainFSDriver,
VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
NULL,
NULL);
+VIR_ENUM_DECL(qemuNumaPolicy)
+VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST,
+ bind,
+ preferred,
+ interleave);
/**
* qemuPhysIfaceConnect:
@@ -6383,13 +6388,23 @@ qemuBuildNumaArgStr(const virDomainDef *def,
size_t i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *cpumask = NULL, *tmpmask = NULL, *next = NULL;
+char *nodemask = NULL;
int ret = -1;
+if (virDomainNumatuneHasPerNodeBinding(def-numatune)
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Per-node memory binding is not supported
+ with this QEMU));
+goto cleanup;
+}
+
for (i = 0; i def-cpu-ncells; i++) {
int cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024);
def-cpu-cells[i].mem = cellmem * 1024;
VIR_FREE(cpumask);
+VIR_FREE(nodemask);
if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
goto cleanup;
@@ -6402,6 +6417,43 @@ qemuBuildNumaArgStr(const virDomainDef *def,
goto cleanup;
}
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virDomainNumatuneMemMode mode;
+const char *policy = NULL;
+
+mode = virDomainNumatuneGetMode(def-numatune, i);
+policy = qemuNumaPolicyTypeToString(mode);
+
+virBufferAsprintf(buf,
memory-backend-ram,size=%dM,id=ram-node%zu,
+ cellmem, i);
+
+if (virDomainNumatuneMaybeFormatNodeset(def-numatune, NULL,
+nodemask, i) 0)
+goto cleanup;
+
+if (nodemask) {
+if (strchr(nodemask, ',')
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(disjoint NUMA node ranges are not
supported
+ with this QEMU));
+goto cleanup;
+}
+
+for (tmpmask = nodemask; tmpmask; tmpmask = next) {
+if ((next = strchr(tmpmask, ',')))
+*(next++) = '\0';
+virBufferAddLit(buf, ,host-nodes=);
+virBufferAdd(buf, tmpmask, -1);
+}
+
+virBufferAsprintf(buf, ,policy=%s, policy);
+}
+
+virCommandAddArg(cmd, -object);
+virCommandAddArgBuffer(cmd, buf);
+}
+
virCommandAddArg(cmd, -numa);
virBufferAsprintf(buf, node,nodeid=%zu, i);
@@ -6412,7 +6464,11 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virBufferAdd(buf, tmpmask, -1);
}
-virBufferAsprintf(buf, ,mem=%d, cellmem);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virBufferAsprintf(buf, ,memdev=ram-node%zu, i);
+} else {
+virBufferAsprintf(buf, ,mem=%d, cellmem);
+}
virCommandAddArgBuffer(cmd, buf);
}
@@ -6420,6 +6476,7 @@ qemuBuildNumaArgStr(const virDomainDef *def,
cleanup:
VIR_FREE(cpumask);
+VIR_FREE(nodemask);
virBufferFreeAndReset(buf);
return ret;
}
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
new file mode 100644
index 000..b0e274c
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
@@ -0,0 +1,8 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/kvm -S -M pc -m 64 -smp 2 \
+-object
[libvirt] [PATCH v3 16/16] qemu: leave restricting cpuset.mems after initialization
When domain is started with numatune memory mode strict and the
nodeset does not include host NUMA node with DMA and DMA32 zones, KVM
initialization fails. This is because cgroup restrict even kernel
allocations. We are already doing numa_set_membind() which does the
same thing, only it does not restrict kernel allocations.
This patch leaves the userspace numa_set_membind() in place and moves
the cpuset.mems setting after the point where monitor comes up, but
before vcpu and emulator sub-groups are created.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
Another approach would be not using cgroups for this at all; it should
still work as expected.
src/qemu/qemu_cgroup.c | 10 +++---
src/qemu/qemu_cgroup.h | 4 +++-
src/qemu/qemu_process.c | 4
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e95ad17..62a8f81 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -627,9 +627,6 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm,
if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
return 0;
-if (qemuSetupCpusetMems(vm, nodemask) 0)
-goto cleanup;
-
if (vm-def-cpumask ||
(vm-def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)) {
@@ -826,6 +823,13 @@ qemuSetupCgroup(virQEMUDriverPtr driver,
}
int
+qemuSetupCgroupPostInit(virDomainObjPtr vm,
+virBitmapPtr nodemask)
+{
+return qemuSetupCpusetMems(vm, nodemask);
+}
+
+int
qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
unsigned long long period,
long long quota)
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 732860e..7394969 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -1,7 +1,7 @@
/*
* qemu_cgroup.h: QEMU cgroup management
*
- * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -47,6 +47,8 @@ int qemuConnectCgroup(virQEMUDriverPtr driver,
int qemuSetupCgroup(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virBitmapPtr nodemask);
+int qemuSetupCgroupPostInit(virDomainObjPtr vm,
+virBitmapPtr nodemask);
int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
unsigned long long period,
long long quota);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4c57f15..8a6b384 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4155,6 +4155,10 @@ int qemuProcessStart(virConnectPtr conn,
if (!qemuProcessVerifyGuestCPU(driver, vm))
goto cleanup;
+VIR_DEBUG(Setting up post-init cgroup restrictions);
+if (qemuSetupCgroupPostInit(vm, nodemask) 0)
+goto cleanup;
+
VIR_DEBUG(Detecting VCPU PIDs);
if (qemuProcessDetectVcpuPIDs(driver, vm) 0)
goto cleanup;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 15/16] qemu: split out cpuset.mems setting
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_cgroup.c | 29 -
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 40fe448..e95ad17 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -589,13 +589,11 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver,
static int
-qemuSetupCpusetCgroup(virDomainObjPtr vm,
- virBitmapPtr nodemask,
- virCapsPtr caps)
+qemuSetupCpusetMems(virDomainObjPtr vm,
+virBitmapPtr nodemask)
{
qemuDomainObjPrivatePtr priv = vm-privateData;
char *mem_mask = NULL;
-char *cpu_mask = NULL;
int ret = -1;
if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
@@ -610,6 +608,28 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm,
virCgroupSetCpusetMems(priv-cgroup, mem_mask) 0)
goto cleanup;
+ret = 0;
+ cleanup:
+VIR_FREE(mem_mask);
+return ret;
+}
+
+
+static int
+qemuSetupCpusetCgroup(virDomainObjPtr vm,
+ virBitmapPtr nodemask,
+ virCapsPtr caps)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+char *cpu_mask = NULL;
+int ret = -1;
+
+if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+return 0;
+
+if (qemuSetupCpusetMems(vm, nodemask) 0)
+goto cleanup;
+
if (vm-def-cpumask ||
(vm-def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)) {
@@ -632,7 +652,6 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm,
ret = 0;
cleanup:
-VIR_FREE(mem_mask);
VIR_FREE(cpu_mask);
return ret;
}
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 06/16] numatune: unify numatune struct and enum names
Since there was already public virDomainNumatune*, I changed the
private virNumaTune to match the same, so all the uses are unified and
public API is kept:
s/vir\(Domain\)\?Numa[tT]une/virDomainNumatune/g
then shrunk long lines, and mainly functions, that were created after
that:
sed -i 's/virDomainNumatuneMemPlacementMode/virDomainNumatunePlacement/g'
And to cope with the enum name, I haad to change the constants as
well:
s/VIR_NUMA_TUNE_MEM_PLACEMENT_MODE/VIR_DOMAIN_NUMATUNE_PLACEMENT/g
Last thing I did was at least a little shortening of already long
name:
s/virDomainNumatuneDef/virDomainNumatune/g
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c | 20 ++--
src/conf/domain_conf.h | 2 +-
src/conf/numatune_conf.c | 4 ++--
src/conf/numatune_conf.h | 20 ++--
src/libvirt_private.syms | 4 ++--
src/lxc/lxc_cgroup.c | 4 ++--
src/lxc/lxc_controller.c | 2 +-
src/lxc/lxc_native.c | 2 +-
src/qemu/qemu_cgroup.c | 4 ++--
src/qemu/qemu_driver.c | 10 +-
src/qemu/qemu_process.c | 2 +-
src/util/virnuma.c | 8
src/util/virnuma.h | 2 +-
13 files changed, 42 insertions(+), 42 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 315ea6a..f83050d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11773,7 +11773,7 @@ virDomainDefParseXML(xmlDocPtr xml,
int placement_mode = 0;
if (placement) {
if ((placement_mode =
-
virNumaTuneMemPlacementModeTypeFromString(placement)) 0) {
+
virDomainNumatunePlacementTypeFromString(placement)) 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Unsupported memory placement
mode '%s'), placement);
@@ -11783,18 +11783,18 @@ virDomainDefParseXML(xmlDocPtr xml,
VIR_FREE(placement);
} else if (def-numatune.memory.nodemask) {
/* Defaults to static if nodeset is specified. */
-placement_mode =
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC;
+placement_mode = VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC;
} else {
/* Defaults to placement of vcpu if nodeset is
* not specified.
*/
if (def-placement_mode ==
VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC)
-placement_mode =
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC;
+placement_mode =
VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC;
else
-placement_mode =
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO;
+placement_mode =
VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO;
}
-if (placement_mode ==
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC
+if (placement_mode == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC
!def-numatune.memory.nodemask) {
virReportError(VIR_ERR_XML_ERROR, %s,
_(nodeset for NUMA memory tuning must
be set
@@ -11803,7 +11803,7 @@ virDomainDefParseXML(xmlDocPtr xml,
}
/* Ignore 'nodeset' if 'placement' is 'auto' finally */
-if (placement_mode ==
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO) {
+if (placement_mode == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) {
virBitmapFree(def-numatune.memory.nodemask);
def-numatune.memory.nodemask = NULL;
}
@@ -11811,7 +11811,7 @@ virDomainDefParseXML(xmlDocPtr xml,
/* Copy 'placement' of numatune to vcpu if its
'placement'
* is not specified and 'placement' of numatune is
specified.
*/
-if (placement_mode ==
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO
+if (placement_mode == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO
!def-cpumask)
def-placement_mode =
VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO;
@@ -11830,7 +11830,7 @@ virDomainDefParseXML(xmlDocPtr xml,
* and 'placement' of vcpu is 'auto'.
*/
if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
-def-numatune.memory.placement_mode =
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO;
+def-numatune.memory.placement_mode =
VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO;
def-numatune.memory.mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
}
}
@@ -17452,13 +17452,13 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAsprintf(buf,
[libvirt] [PATCH v3 07/16] numatune: Encapsulate numatune configuration in order to unify results
There were numerous places where numatune configuration (and thus
domain config as well) was changed in different ways. On some
places this even resulted in persistent domain definition not to be
stable (it would change with daemon's restart).
In order to uniformly change how numatune config is dealt with, all
the internals are now accessible directly only in numatune_conf.c and
outside this file accessors must be used.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
po/POTFILES.in | 1 +
src/conf/domain_conf.c | 159 ++-
src/conf/domain_conf.h | 8 +-
src/conf/numatune_conf.c | 318 +
src/conf/numatune_conf.h | 72 -
src/libvirt_private.syms | 11 +
src/lxc/lxc_cgroup.c | 19 +-
src/lxc/lxc_controller.c | 5 +-
src/lxc/lxc_native.c | 15 +-
src/parallels/parallels_driver.c | 7 +-
src/qemu/qemu_cgroup.c | 23 +-
src/qemu/qemu_driver.c | 84 +++---
src/qemu/qemu_process.c| 8 +-
src/util/virnuma.c | 48 ++--
src/util/virnuma.h | 2 +-
.../qemuxml2argv-numatune-auto-prefer.xml | 29 ++
.../qemuxml2xmlout-numatune-auto-prefer.xml| 29 ++
tests/qemuxml2xmltest.c| 2 +
18 files changed, 555 insertions(+), 285 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml
diff --git a/po/POTFILES.in b/po/POTFILES.in
index fd4b56e..d10e892 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -25,6 +25,7 @@ src/conf/netdev_vlan_conf.c
src/conf/netdev_vport_profile_conf.c
src/conf/network_conf.c
src/conf/node_device_conf.c
+src/conf/numatune_conf.c
src/conf/nwfilter_conf.c
src/conf/nwfilter_params.c
src/conf/object_event.c
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f83050d..d34d94c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2097,7 +2097,7 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainVcpuPinDefFree(def-cputune.emulatorpin);
-virBitmapFree(def-numatune.memory.nodemask);
+virDomainNumatuneFree(def-numatune);
virSysinfoDefFree(def-sysinfo);
@@ -11256,7 +11256,6 @@ virDomainDefParseXML(xmlDocPtr xml,
unsigned long count;
bool uuid_generated = false;
virHashTablePtr bootHash = NULL;
-xmlNodePtr cur;
bool usb_none = false;
bool usb_other = false;
bool usb_master = false;
@@ -11718,123 +11717,8 @@ virDomainDefParseXML(xmlDocPtr xml,
}
}
-/* Extract numatune if exists. */
-if ((n = virXPathNodeSet(./numatune, ctxt, nodes)) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- %s, _(cannot extract numatune nodes));
+if (virDomainNumatuneParseXML(def, ctxt) 0)
goto error;
-}
-
-if (n 1) {
-virReportError(VIR_ERR_XML_ERROR, %s,
- _(only one numatune is supported));
-VIR_FREE(nodes);
-goto error;
-}
-
-if (n) {
-cur = nodes[0]-children;
-while (cur != NULL) {
-if (cur-type == XML_ELEMENT_NODE) {
-if (xmlStrEqual(cur-name, BAD_CAST memory)) {
-char *mode = NULL;
-char *placement = NULL;
-char *nodeset = NULL;
-
-mode = virXMLPropString(cur, mode);
-if (mode) {
-if ((def-numatune.memory.mode =
- virDomainNumatuneMemModeTypeFromString(mode))
0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(Unsupported NUMA memory
- tuning mode '%s'),
- mode);
-VIR_FREE(mode);
-goto error;
-}
-VIR_FREE(mode);
-} else {
-def-numatune.memory.mode =
VIR_DOMAIN_NUMATUNE_MEM_STRICT;
-}
-
-nodeset = virXMLPropString(cur, nodeset);
-if (nodeset) {
-if (virBitmapParse(nodeset,
- 0,
- def-numatune.memory.nodemask,
- VIR_DOMAIN_CPUMASK_LEN) 0) {
-VIR_FREE(nodeset);
-goto error;
-}
-
Re: [libvirt] [PATCH 2/3] lxc conf2xml: convert lxc.network.name for veth networks
On 02.07.2014 15:57, Cédric Bosdonnat wrote:
---
src/lxc/lxc_native.c | 22 --
.../lxcconf2xmldata/lxcconf2xml-physnetwork.config | 1 +
tests/lxcconf2xmldata/lxcconf2xml-simple.xml | 1 +
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index f4c4556..e14face 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -338,7 +338,8 @@ lxcCreateNetDef(const char *type,
const char *linkdev,
const char *mac,
const char *flag,
-const char *macvlanmode)
+const char *macvlanmode,
+const char *name)
{
virDomainNetDefPtr net = NULL;
virMacAddr macAddr;
@@ -353,6 +354,8 @@ lxcCreateNetDef(const char *type,
net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN;
}
+if (name VIR_STRDUP(net-ifname_guest, name) 0)
+goto error;
One of the requirements when I introduced VIR_STRDUP was, that it's NULL
safe so we don't have to do this. s/name //
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] lxc network configuration allows setting target container NIC name
On 02.07.2014 15:57, Cédric Bosdonnat wrote:
LXC network devices can now be assigned a custom NIC device name on the
container side. For example, this is configured with:
interface type='network'
source network='default'/
guest dev=eth1/
/interface
In this example the network card will appear as eth1 in the guest.
---
docs/schemas/domaincommon.rng | 17 +
src/conf/domain_conf.c | 27 +++
src/conf/domain_conf.h | 2 ++
src/lxc/lxc_container.c| 29 +
src/lxc/lxc_process.c | 25 +
tests/lxcxml2xmldata/lxc-idmap.xml | 1 +
6 files changed, 97 insertions(+), 4 deletions(-)
The 3/3 should be merged with this so any element addition goes with RNG
schema adjustment and is documented.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 33d0308..e7ca992 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2165,6 +2165,23 @@
/element
/optional
optional
+element name=guest
+ interleave
+optional
+ attribute name=dev
+ref name=deviceName/
+ /attribute
+/optional
+optional
+ attribute name=actual
+ref name=deviceName/
+ /attribute
+/optional
+ /interleave
+ empty/
+/element
+ /optional
+ optional
element name=mac
attribute name=address
ref name=uniMacAddr/
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b7aa4f5..5cd6ae6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1383,6 +1383,8 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
VIR_FREE(def-virtPortProfile);
VIR_FREE(def-script);
VIR_FREE(def-ifname);
+VIR_FREE(def-ifname_guest);
+VIR_FREE(def-ifname_guest_actual);
virDomainDeviceInfoClear(def-info);
@@ -6618,6 +6620,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
char *bridge = NULL;
char *dev = NULL;
char *ifname = NULL;
+char *ifname_guest = NULL;
+char *ifname_guest_actual = NULL;
char *script = NULL;
char *address = NULL;
char *port = NULL;
@@ -6723,6 +6727,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
/* An auto-generated target name, blank it out */
VIR_FREE(ifname);
}
+} else if ((!ifname_guest || !ifname_guest_actual)
+ xmlStrEqual(cur-name, BAD_CAST guest)) {
+ifname_guest = virXMLPropString(cur, dev);
+ifname_guest_actual = virXMLPropString(cur, actual);
} else if (!linkstate
xmlStrEqual(cur-name, BAD_CAST link)) {
linkstate = virXMLPropString(cur, state);
@@ -6964,6 +6972,14 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
def-ifname = ifname;
ifname = NULL;
}
+if (ifname_guest != NULL) {
+def-ifname_guest = ifname_guest;
+ifname_guest = NULL;
+}
+if (ifname_guest_actual != NULL) {
+def-ifname_guest_actual = ifname_guest_actual;
+ifname_guest_actual = NULL;
+}
/* NIC model (see -net nic,model=?). We only check that it looks
* reasonable, not that it is a supported NIC type. FWIW kvm
@@ -15883,6 +15899,17 @@ virDomainNetDefFormat(virBufferPtr buf,
/* Skip auto-generated target names for inactive config. */
virBufferEscapeString(buf, target dev='%s'/\n, def-ifname);
}
+if (def-ifname_guest || def-ifname_guest_actual) {
+virBufferAddLit(buf, guest);
+/* Skip auto-generated target names for inactive config. */
+if (def-ifname_guest)
+virBufferEscapeString(buf, dev='%s', def-ifname_guest);
+
+/* Only set if the host is running, so shouldn't pollute output */
+if (def-ifname_guest_actual)
+virBufferEscapeString(buf, actual='%s',
def-ifname_guest_actual);
+virBufferAddLit(buf, /\n);
+}
if (def-model) {
virBufferEscapeString(buf, model type='%s'/\n,
def-model);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1122eb2..60aa491 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -915,6 +915,8 @@ struct _virDomainNetDef {
} tune;
char *script;
char *ifname;
+char *ifname_guest;
+char *ifname_guest_actual;
virDomainDeviceInfo info;
char *filter;
virNWFilterHashTablePtr filterparams;
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index fd8ab16..c7423db 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -464,6 +464,21 @@ static int
Re: [libvirt] [PATCH] leaseshelper: add enhancements to support all events
cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps)); virCommandAddArgFormat(cmd, --conf-file=%s, configfile); -virCommandAddArgFormat(cmd, --dhcp-script=%s, leaseshelper_path); +virCommandAddArgFormat(cmd, --dhcp-script=%s, pseudo_leaseshelper_path); +virCommandAddArgFormat(cmd, --leasefile-ro); Does dnsmasq pass through the rest of the environment we pass to it at this point? The leaseshelper extracts quite a lot of stuff from the environment variables that are passed by dnsmasq. In case it does we could use an env variable to pass the interface name instead of linking the helper and using different names. If I use the following line of code in the function networkBuildDhcpDaemonCommandLine ... setenv(VIR_BRIDGE_NAME, network-def-bridge, 1); .. then yes, the helper program invoked by dnsmasq does get to see this variable. (It does so in Fedora20). (Looks like I acted in haste, should've waited for more replies on RFC, sigh) So, in the next version that I will send, should I first make a wrapper by the name virSetEnvBlockSUID for setenv, just like we have virGetEnvBlockSUID for getenv? A second issue that comes into my mind is compatibility with already existing files. Does this work when you already have a lease file? (I didn't try it, I'm just asking). If we use the --leasefile-ro option, then although this method will work, but no, the lease file will *not* be read/written at all. So even if an old one exists, its of no use. But then again, the use of --leasefile-ro is mandatory, so that all events are captured by our helper program. For example, renew of a lease. Regards, Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Changing the default qemu path in libvirt
On 07/16/2014 01:41 AM, satheesh wrote: Hi All, I would like to change the default qemu path in libvirt xml and the changed path has to be used while installing VMs using virt-install. There was a similar question rasied some years back, I could not find the answer to the same. http://www.redhat.com/archives/libvir-list/2011-December/msg01100.html Is that possible right now?, If we have the support already, please point to the same. You can manually specify the emulator path with virt-install nowadays, since version 1.0.0: virt-install --boot emulator=/my/emulator/path But there isn't any way to explicitly change the default emulator, it's still the same behavior as Dan describes in that above mail. - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] support for QEMU vhost-user
On 11.07.2014 19:47, Michele Paolino wrote: This patch adds support for the QEMU vhost-user feature to libvirt. vhost-user enables the communication between a QEMU virtual machine and other userspace process using the Virtio transport protocol. It uses a char dev (e.g. Unix socket) for the control plane, while the data plane based on shared memory. The XML looks like: interface type='vhostuser' source type='unix' path='/tmp/vhost.sock' mode='server'/ mac address='52:54:00:3b:83:1a'/ model type='virtio'/ /interface changes from v1: * addressed comments * removed unnecessary checks * series merged in a single patch We tend to write the diff to previous versions into notes not in the commit message as it pollutes git log. BTW: I didn't ask the whole patchset to be merged into a single patch, but it doesn't hurt in this specific case either (the diff stat seems reasonably big). The previous version of this patch can be found at: http://www.redhat.com/archives/libvir-list/2014-July/msg00111.html Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- docs/formatdomain.html.in | 34 + docs/schemas/domaincommon.rng | 25 +++ src/conf/domain_conf.c | 87 ++ src/conf/domain_conf.h | 10 ++- src/libxl/libxl_conf.c | 1 + src/lxc/lxc_process.c | 1 + src/qemu/qemu_command.c| 63 src/uml/uml_conf.c | 5 ++ src/xenxs/xen_sxpr.c | 1 + .../qemuxml2argv-net-vhostuser.args| 7 ++ .../qemuxml2argv-net-vhostuser.xml | 33 tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c| 1 + 13 files changed, 267 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3f8bbee..606b7d4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3927,6 +3927,40 @@ qemu-kvm -net nic,model=? /dev/null span class=sinceSince 0.9.5/span /p +h5a name=elementVhostuservhost-user interface/a/h5 + +p + vhost-user enables the communication between a QEMU virtual machine + and other userspace process using the Virtio transport protocol. + A char dev (e.g. Unix socket) is used for the control plane, while + the data plane is based on shared memory. +/p + +pre + ... + lt;devicesgt; +lt;interface type='vhostuser'gt; + lt;source type='unix' path='/tmp/vhost.sock' mode='server'gt; + lt;/sourcegt; + lt;mac address='52:54:00:3b:83:1a'gt; + lt;/macgt; + lt;model type='virtio'gt; + lt;/modelgt; I don't think so. Empty bodies elements are written as elem/. And that's how libvirt formats them too. And if I were to be really picky, mac/ is formated before source/. +lt;/interfacegt; + lt;/devicesgt; + .../pre + +p + The codelt;sourcegt;/code element has to be specified + along with the type of char device. + Currently, only type='unix' is supported, where the path (the + directory path of the socket) and mode attributes are required. + Both codemode='server'/code and codemode='client'/code + are supported. + vhost-user requires the virtio model type, thus the + codelt;modelgt;/code element is mandatory. +/p I think it would be useful for users reading the documentation to know what version of libvirt was this introduced in. + h4a name=elementsInputInput devices/a/h4 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af51eee..c9c02b6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1968,6 +1968,31 @@ /group group attribute name=type +valuevhostuser/value + /attribute + interleave + element name=source +attribute name=type + choice +valueunix/value + /choice +/attribute +attribute name=path + ref name=absFilePath/ +/attribute +attribute name=mode + choice +valueserver/value +valueclient/value + /choice +/attribute +empty/ + /element +ref name=interface-options/ + /interleave +/group +group
Re: [libvirt] [PATCHv2 0/2] storage: Don't wipe volumes on remote filesystems with local tools
On 07/11/2014 08:20 AM, Peter Krempa wrote: Version 2 now splits the stuff into separate driver backend funcs. Peter Krempa (2): storage: wipe: Move helper code into storage backend storage: Split out volume wiping as separate backend function src/storage/storage_backend.c | 203 + src/storage/storage_backend.h | 12 ++ src/storage/storage_backend_disk.c| 1 + src/storage/storage_backend_fs.c | 3 + src/storage/storage_backend_iscsi.c | 1 + src/storage/storage_backend_logical.c | 1 + src/storage/storage_backend_mpath.c | 1 + src/storage/storage_backend_scsi.c| 1 + src/storage/storage_driver.c | 205 +- 9 files changed, 229 insertions(+), 199 deletions(-) ACK both as they seem to fulfill the changes requested from http://www.redhat.com/archives/libvir-list/2014-July/msg00556.html John BTW: Since I was looking at a bug (bz 1091866) in this area - this caught my attention... Not that these changes fix the problem, but perhaps they'll help since as it seems sparse snapshot logical volumes are problematic... -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 00/16] Support for per-guest-node binding
On 16.07.2014 16:42, Martin Kletzander wrote: v3 of https://www.redhat.com/archives/libvir-list/2014-July/msg00372.html v3: - Michal's suggestions worked in - rebased on current master Martin Kletzander (16): qemu: purely a code movement qemu: remove useless error check conf: purely a code movement conf, schema: add 'id' field for cells numatune: create new module for numatune numatune: unify numatune struct and enum names numatune: Encapsulate numatune configuration in order to unify results conf, schema: add support for memnode elements numatune: add support for per-node memory bindings in private APIs qemu: allow qmp probing for cmdline options without params qemu: memory-backend-ram capability probing qemu: newer -numa parameter capability probing qemu: enable disjoint numa cpu ranges qemu: pass numa node binding preferences to qemu qemu: split out cpuset.mems setting qemu: leave restricting cpuset.mems after initialization docs/formatdomain.html.in | 32 +- docs/schemas/domaincommon.rng | 22 + po/POTFILES.in | 1 + src/Makefile.am| 3 +- src/conf/cpu_conf.c| 41 +- src/conf/cpu_conf.h| 3 +- src/conf/domain_conf.c | 203 ++- src/conf/domain_conf.h | 10 +- src/conf/numatune_conf.c | 595 + src/conf/numatune_conf.h | 108 src/libvirt_private.syms | 24 +- src/lxc/lxc_cgroup.c | 20 +- src/lxc/lxc_controller.c | 5 +- src/lxc/lxc_native.c | 15 +- src/parallels/parallels_driver.c | 7 +- src/qemu/qemu_capabilities.c | 16 +- src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_cgroup.c | 52 +- src/qemu/qemu_cgroup.h | 4 +- src/qemu/qemu_command.c| 98 +++- src/qemu/qemu_driver.c | 84 ++- src/qemu/qemu_monitor.c| 6 +- src/qemu/qemu_monitor.h| 3 +- src/qemu/qemu_monitor_json.c | 8 +- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_process.c| 12 +- src/util/virnuma.c | 61 +-- src/util/virnuma.h | 28 +- tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 5 + tests/qemumonitorjsontest.c| 20 +- .../qemuxml2argv-cpu-numa-disjoint.args| 6 + .../qemuxml2argv-cpu-numa-disjoint.xml | 28 + tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 + .../qemuxml2argv-numatune-auto-prefer.xml | 29 + .../qemuxml2argv-numatune-memnode-no-memory.args | 8 + .../qemuxml2argv-numatune-memnode-no-memory.xml| 30 ++ .../qemuxml2argv-numatune-memnode-nocpu.xml| 25 + .../qemuxml2argv-numatune-memnode.args | 11 + .../qemuxml2argv-numatune-memnode.xml | 33 ++ .../qemuxml2argv-numatune-memnodes-problematic.xml | 31 ++ tests/qemuxml2argvtest.c | 12 + .../qemuxml2xmlout-cpu-numa1.xml | 28 + .../qemuxml2xmlout-cpu-numa2.xml | 28 + .../qemuxml2xmlout-numatune-auto-prefer.xml| 29 + .../qemuxml2xmlout-numatune-memnode.xml| 33 ++ tests/qemuxml2xmltest.c| 8 + 49 files changed, 1479 insertions(+), 389 deletions(-) create mode 100644 src/conf/numatune_conf.c create mode 100644 src/conf/numatune_conf.h create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml create mode 100644
Re: [libvirt] [PATCH v3 04/16] conf, schema: add 'id' field for cells
On 07/16/2014 08:42 AM, Martin Kletzander wrote: In XML format, by definition, order of fields should not matter, so order of parsing the elements doesn't affect the end result. When specifying guest NUMA cells, we depend only on the order of the 'cell' elements. With this patch all older domain XMLs are parsed as before, but with the 'id' attribute they are parsed and formatted according to that field. This will be useful when we have tuning settings for particular guest NUMA node. It's not always true that order doesn't matter - for the longest time, the order of disk under devices determined boot order. But I agree that the flexibility of using an id to allow arbitrary ordering can be nicer than enforcing constant ordering. Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/formatdomain.html.in | 17 + docs/schemas/domaincommon.rng | 5 +++ Looks okay to me on the schema side. -- 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 v3 04/16] conf, schema: add 'id' field for cells
On Wed, Jul 16, 2014 at 12:14:35PM -0600, Eric Blake wrote: On 07/16/2014 08:42 AM, Martin Kletzander wrote: In XML format, by definition, order of fields should not matter, so order of parsing the elements doesn't affect the end result. When specifying guest NUMA cells, we depend only on the order of the 'cell' elements. With this patch all older domain XMLs are parsed as before, but with the 'id' attribute they are parsed and formatted according to that field. This will be useful when we have tuning settings for particular guest NUMA node. It's not always true that order doesn't matter - for the longest time, the order of disk under devices determined boot order. But I agree that the flexibility of using an id to allow arbitrary ordering can be nicer than enforcing constant ordering. I was under the impression that it's determined by the buses the disks are connected to. Anyway even if that matters, I'm not sure we guarantee that and if we do, it should be fixed from my POV. But that's a discussion for another day. Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/formatdomain.html.in | 17 + docs/schemas/domaincommon.rng | 5 +++ Looks okay to me on the schema side. Thanks for checking it. signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 00/16] Support for per-guest-node binding
On Wed, Jul 16, 2014 at 07:48:45PM +0200, Michal Privoznik wrote: On 16.07.2014 16:42, Martin Kletzander wrote: v3 of https://www.redhat.com/archives/libvir-list/2014-July/msg00372.html v3: - Michal's suggestions worked in - rebased on current master [...] ACK series. Although, if anybody has objections to the XML schema, speak now! I pushed it and if anyone has an objection, there's still time till the release comes ;-) Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 08/16] conf, schema: add support for memnode elements
On 07/16/2014 08:42 AM, Martin Kletzander wrote: Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/formatdomain.html.in | 15 ++ docs/schemas/domaincommon.rng | 17 ++ src/conf/numatune_conf.c | 187 +++-- .../qemuxml2argv-numatune-memnode-no-memory.xml| 30 .../qemuxml2argv-numatune-memnode-nocpu.xml| 25 +++ .../qemuxml2argv-numatune-memnode.xml | 33 .../qemuxml2argv-numatune-memnodes-problematic.xml | 31 tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-numatune-memnode.xml| 33 tests/qemuxml2xmltest.c| 2 + 10 files changed, 362 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9f1082b..1301639 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -709,6 +709,8 @@ ... lt;numatunegt; lt;memory mode=strict nodeset=1-4,^3/gt; +lt;memnode cellid=0 mode=strict nodeset=1/gt; +lt;memnode cellid=2 mode=preferred nodeset=2/gt; Ah, the ability to skip cellid's (for the cells that inherit the default policies from memory makes it essential to have ids for correlation in the earlier patch in this series. lt;/numatunegt; ... lt;/domaingt; @@ -745,6 +747,19 @@ span class='since'Since 0.9.3/span /dd + dtcodememnode/code/dt + dd +Optional codememnode/code elements can specify memory allocation +policies per each guest NUMA node. For those nodes having no +corresponding codememnode/code element, the default from +element codememory/code will be used. Attribute codecellid/code +addresses guest NUMA node for which the settings are applied. +Attributes codemode/code and codenodeset/code have the same +meaning and syntax as in codememory/code element. + +This setting is not compatible with automatic placement. +span class='since'QEMU Since 1.2.7/span Again, schema documentation looks okay to me. +++ b/docs/schemas/domaincommon.rng @@ -789,6 +789,23 @@ /choice /element /optional + zeroOrMore +element name=memnode + attribute name=cellid +ref name=unsignedInt/ + /attribute + attribute name=mode +choice + valuestrict/value + valuepreferred/value + valueinterleave/value +/choice + /attribute + attribute name='nodeset' +ref name='cpuset'/ + /attribute +/element + /zeroOrMore /element Missing an interleave here (memory and memnode should be swappable with one another). -- 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 v3 00/16] Support for per-guest-node binding
On 07/16/2014 12:21 PM, Martin Kletzander wrote: On Wed, Jul 16, 2014 at 07:48:45PM +0200, Michal Privoznik wrote: On 16.07.2014 16:42, Martin Kletzander wrote: v3 of https://www.redhat.com/archives/libvir-list/2014-July/msg00372.html v3: - Michal's suggestions worked in - rebased on current master [...] ACK series. Although, if anybody has objections to the XML schema, speak now! I pushed it and if anyone has an objection, there's still time till the release comes ;-) I just missed you; I pointed out good idea for a followup to the RNG schema in 8/16. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 7/8] util: add virCommandPassListenFDs() function
That sets a new flag, but that flag does mean the child will get
LISTEN_FDS and LISTEN_PID environment variables properly set and
passed FDs reordered so that it corresponds with LISTEN_FDS (they must
start right after STDERR_FILENO).
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/vircommand.c| 99
src/util/vircommand.h| 4 +-
tests/commanddata/test24.log | 7
tests/commandtest.c | 56 +
5 files changed, 166 insertions(+), 1 deletion(-)
create mode 100644 tests/commanddata/test24.log
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 138a9fa..3332952 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1165,6 +1165,7 @@ virCommandNewArgList;
virCommandNewArgs;
virCommandNonblockingFDs;
virCommandPassFD;
+virCommandPassListenFDs;
virCommandRawStatus;
virCommandRequireHandshake;
virCommandRun;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index e775ba6..3b3e6f5 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -66,6 +66,7 @@ enum {
VIR_EXEC_CLEAR_CAPS = (1 2),
VIR_EXEC_RUN_SYNC = (1 3),
VIR_EXEC_ASYNC_IO = (1 4),
+VIR_EXEC_LISTEN_FDS = (1 5),
};
typedef struct _virCommandFD virCommandFD;
@@ -200,6 +201,78 @@ virCommandFDSet(virCommandPtr cmd,
return 0;
}
+static void
+virCommandReorderFDs(virCommandPtr cmd)
+{
+int maxfd = 0;
+int openmax = 0;
+size_t i = 0;
+
+if (!cmd || cmd-has_error || !cmd-npassfd)
+return;
+
+for (i = 0; i cmd-npassfd; i++)
+maxfd = MAX(cmd-passfd[i].fd, maxfd);
+
+openmax = sysconf(_SC_OPEN_MAX);
+if (openmax 0 ||
+maxfd + cmd-npassfd openmax)
+goto error;
+
+/*
+ * Simple two-pass sort, nothing fancy. This is not designed for
+ * anything else than passing around 2 FDs into the child.
+ *
+ * So first dup2() them somewhere else.
+ */
+for (i = 0; i cmd-npassfd; i++) {
+int newfd = maxfd + i + 1;
+int oldfd = cmd-passfd[i].fd;
+if (dup2(oldfd, newfd) != newfd) {
+virReportSystemError(errno,
+ _(Cannot dup2() fd %d before
+ passing it to the child),
+ oldfd);
+goto error;
+}
+VIR_FORCE_CLOSE(cmd-passfd[i].fd);
+}
+
+VIR_DEBUG(First reorder pass done);
+
+/*
+ * And then dup2() them in orderly manner.
+ */
+for (i = 0; i cmd-npassfd; i++) {
+int newfd = STDERR_FILENO + i + 1;
+int oldfd = maxfd + i + 1;
+if (dup2(oldfd, newfd) != newfd) {
+virReportSystemError(errno,
+ _(Cannot dup2() fd %d before
+ passing it to the child),
+ oldfd);
+goto error;
+}
+if (virSetInherit(newfd, true) 0) {
+virReportSystemError(errno,
+ _(Cannot set O_CLOEXEC on fd %d before
+ passing it to the child),
+ newfd);
+goto error;
+}
+VIR_FORCE_CLOSE(oldfd);
+cmd-passfd[i].fd = newfd;
+}
+
+VIR_DEBUG(Second reorder pass done);
+
+return;
+
+ error:
+cmd-has_error = -1;
+return;
+}
+
#ifndef WIN32
/**
@@ -678,6 +751,15 @@ virExec(virCommandPtr cmd)
goto fork_error;
}
+if (cmd-flags VIR_EXEC_LISTEN_FDS) {
+virCommandReorderFDs(cmd);
+virCommandAddEnvFormat(cmd, LISTEN_PID=%u, getpid());
+virCommandAddEnvFormat(cmd, LISTEN_FDS=%zu, cmd-npassfd);
+
+if (cmd-has_error)
+goto fork_error;
+}
+
/* Close logging again to ensure no FDs leak to child */
virLogReset();
@@ -919,6 +1001,23 @@ virCommandPassFD(virCommandPtr cmd, int fd, unsigned int
flags)
}
/**
+ * virCommandPassListenFDs:
+ * @cmd: the command to modify
+ *
+ * Pass LISTEN_FDS and LISTEN_PID environment variables into the
+ * child. LISTEN_PID has the value of the child's PID and LISTEN_FDS
+ * is a number of passed file descriptors starting from 3.
+ */
+void
+virCommandPassListenFDs(virCommandPtr cmd)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-flags |= VIR_EXEC_LISTEN_FDS;
+}
+
+/**
* virCommandSetPidFile:
* @cmd: the command to modify
* @pidfile: filename to use
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 8cdb31c..d3b286d 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -1,7 +1,7 @@
/*
* vircommand.h: Child command execution
*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General
[libvirt] [PATCH v2 3/8] rpc: set listen backlog on FDs as well as on other sockets
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/locking/lock_daemon.c | 2 +-
src/rpc/virnetserverservice.c | 5 +
src/rpc/virnetserverservice.h | 1 +
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index e9219d5..02d77e3 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -614,7 +614,7 @@ virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)
#if WITH_GNUTLS
NULL,
#endif
- false, 1)))
+ false, 0, 1)))
return -1;
if (virNetServerAddService(srv, svc, NULL) 0) {
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index b50f57e..7892a95 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -131,6 +131,7 @@ virNetServerServiceNewFDOrUNIX(const char *path,
tls,
#endif
readonly,
+max_queued_clients,
nrequests_client_max);
}
}
@@ -263,6 +264,7 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
virNetTLSContextPtr tls,
#endif
bool readonly,
+size_t max_queued_clients,
size_t nrequests_client_max)
{
virNetServerServicePtr svc;
@@ -290,6 +292,9 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
goto error;
for (i = 0; i svc-nsocks; i++) {
+if (virNetSocketListen(svc-socks[i], max_queued_clients) 0)
+goto error;
+
/* IO callback is initially disabled, until we're ready
* to deal with incoming clients */
if (virNetSocketAddIOCallback(svc-socks[i],
diff --git a/src/rpc/virnetserverservice.h b/src/rpc/virnetserverservice.h
index a1c8960..b1d6c2d 100644
--- a/src/rpc/virnetserverservice.h
+++ b/src/rpc/virnetserverservice.h
@@ -74,6 +74,7 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
virNetTLSContextPtr tls,
# endif
bool readonly,
+size_t max_queued_clients,
size_t nrequests_client_max);
virNetServerServicePtr virNetServerServiceNewPostExecRestart(virJSONValuePtr
object);
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 6/8] tests: support dynamic prefixes in commandtest
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
tests/commandtest.c | 49 ++---
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 7d2161c..ba823f7 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -62,7 +62,8 @@ main(void)
#else
-static int checkoutput(const char *testname)
+static int checkoutput(const char *testname,
+ char *prefix)
{
int ret = -1;
char *expectname = NULL;
@@ -86,6 +87,16 @@ static int checkoutput(const char *testname)
goto cleanup;
}
+if (prefix) {
+char *tmp = NULL;
+
+if (virAsprintf(tmp, %s%s, prefix, expectlog) 0)
+goto cleanup;
+
+VIR_FREE(expectlog);
+expectlog = tmp;
+}
+
if (STRNEQ(expectlog, actuallog)) {
virtTestDifference(stderr, expectlog, actuallog);
goto cleanup;
@@ -173,7 +184,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED)
return -1;
}
-if ((ret = checkoutput(test2)) != 0) {
+if ((ret = checkoutput(test2, NULL)) != 0) {
virCommandFree(cmd);
return ret;
}
@@ -187,7 +198,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test2);
+return checkoutput(test2, NULL);
}
/*
@@ -219,7 +230,7 @@ static int test3(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test3);
+ret = checkoutput(test3, NULL);
cleanup:
virCommandFree(cmd);
@@ -261,7 +272,7 @@ static int test4(const void *unused ATTRIBUTE_UNUSED)
while (kill(pid, 0) != -1)
usleep(100*1000);
-ret = checkoutput(test4);
+ret = checkoutput(test4, NULL);
cleanup:
virCommandFree(cmd);
@@ -291,7 +302,7 @@ static int test5(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test5);
+return checkoutput(test5, NULL);
}
@@ -315,7 +326,7 @@ static int test6(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test6);
+return checkoutput(test6, NULL);
}
@@ -340,7 +351,7 @@ static int test7(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test7);
+return checkoutput(test7, NULL);
}
/*
@@ -365,7 +376,7 @@ static int test8(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test8);
+return checkoutput(test8, NULL);
}
@@ -403,7 +414,7 @@ static int test9(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test9);
+return checkoutput(test9, NULL);
}
@@ -429,7 +440,7 @@ static int test10(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test10);
+return checkoutput(test10, NULL);
}
/*
@@ -453,7 +464,7 @@ static int test11(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test11);
+return checkoutput(test11, NULL);
}
/*
@@ -475,7 +486,7 @@ static int test12(const void *unused ATTRIBUTE_UNUSED)
virCommandFree(cmd);
-return checkoutput(test12);
+return checkoutput(test12, NULL);
}
/*
@@ -510,7 +521,7 @@ static int test13(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test13);
+ret = checkoutput(test13, NULL);
cleanup:
virCommandFree(cmd);
@@ -582,7 +593,7 @@ static int test14(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test14);
+ret = checkoutput(test14, NULL);
cleanup:
virCommandFree(cmd);
@@ -613,7 +624,7 @@ static int test15(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test15);
+ret = checkoutput(test15, NULL);
cleanup:
VIR_FREE(cwd);
@@ -659,7 +670,7 @@ static int test16(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test16);
+ret = checkoutput(test16, NULL);
cleanup:
virCommandFree(cmd);
@@ -841,7 +852,7 @@ static int test20(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test20);
+ret = checkoutput(test20, NULL);
cleanup:
virCommandFree(cmd);
VIR_FREE(buf);
@@ -900,7 +911,7 @@ static int test21(const void *unused ATTRIBUTE_UNUSED)
goto cleanup;
}
-ret = checkoutput(test21);
+ret = checkoutput(test21, NULL);
cleanup:
VIR_FREE(outbuf);
VIR_FREE(errbuf);
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/8] cfg.mk: allow integers to be assigned a value computed with i|j|k
Even line like this: int asdf = i - somevar; was matched by the regex, so this patch adds '=' to the list of characters that break the regexp. Signed-off-by: Martin Kletzander mklet...@redhat.com --- cfg.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfg.mk b/cfg.mk index baaab71..85a7060 100644 --- a/cfg.mk +++ b/cfg.mk @@ -568,7 +568,7 @@ sc_avoid_attribute_unused_in_header: $(_sc_search_regexp) sc_prohibit_int_ijk: - @prohibit='\(int|unsigned) ([^(]* )*(i|j|k)\(\s|,|;)' \ + @prohibit='\(int|unsigned) ([^(=]* )*(i|j|k)\(\s|,|;)'\ halt='use size_t, not int/unsigned int for loop vars i, j, k' \ $(_sc_search_regexp) -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/8] remote: create virNetServerServiceNewFDOrUNIX() wrapper
It's just a wrapper around NewFD and NewUNIX that selects the right
option and increments the number of used FDs.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/libvirt_remote.syms | 1 +
src/rpc/virnetserverservice.c | 48 ++-
src/rpc/virnetserverservice.h | 14 -
3 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index d482a55..6b520b5 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -159,6 +159,7 @@ virNetServerServiceGetMaxRequests;
virNetServerServiceGetPort;
virNetServerServiceIsReadonly;
virNetServerServiceNewFD;
+virNetServerServiceNewFDOrUNIX;
virNetServerServiceNewPostExecRestart;
virNetServerServiceNewTCP;
virNetServerServiceNewUNIX;
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index 320a02c..b50f57e 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -1,7 +1,7 @@
/*
* virnetserverservice.c: generic network RPC server service
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2012, 2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -90,6 +90,52 @@ static void virNetServerServiceAccept(virNetSocketPtr sock,
}
+virNetServerServicePtr
+virNetServerServiceNewFDOrUNIX(const char *path,
+ mode_t mask,
+ gid_t grp,
+ int auth,
+#if WITH_GNUTLS
+ virNetTLSContextPtr tls,
+#endif
+ bool readonly,
+ size_t max_queued_clients,
+ size_t nrequests_client_max,
+ unsigned int nfds,
+ unsigned int *cur_fd)
+{
+if (*cur_fd - 2 nfds) {
+/*
+ * There are no more file descriptors to use, so we have to
+ * fallback to UNIX socket.
+ */
+return virNetServerServiceNewUNIX(path,
+ mask,
+ grp,
+ auth,
+#if WITH_GNUTLS
+ tls,
+#endif
+ readonly,
+ max_queued_clients,
+ nrequests_client_max);
+
+} else {
+/*
+ * There's still enough file descriptors. In this case we'll
+ * use the current one and increment it afterwards.
+ */
+return virNetServerServiceNewFD(*cur_fd++,
+auth,
+#if WITH_GNUTLS
+tls,
+#endif
+readonly,
+nrequests_client_max);
+}
+}
+
+
virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
const char *service,
int auth,
diff --git a/src/rpc/virnetserverservice.h b/src/rpc/virnetserverservice.h
index eb31abf..a1c8960 100644
--- a/src/rpc/virnetserverservice.h
+++ b/src/rpc/virnetserverservice.h
@@ -1,7 +1,7 @@
/*
* virnetserverservice.h: generic network RPC server service
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2011, 2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -37,6 +37,18 @@ typedef int
(*virNetServerServiceDispatchFunc)(virNetServerServicePtr svc,
virNetSocketPtr sock,
void *opaque);
+virNetServerServicePtr virNetServerServiceNewFDOrUNIX(const char *path,
+ mode_t mask,
+ gid_t grp,
+ int auth,
+# if WITH_GNUTLS
+ virNetTLSContextPtr tls,
+# endif
+ bool readonly,
+ size_t
max_queued_clients,
+ size_t
nrequests_client_max,
+ unsigned int nfds,
+ unsigned int *cur_fd);
virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
const char *service,
int auth,
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/8] util: abstract parsing of passed FDs into virGetListenFDs()
Since not only systemd can do this (we'll be doing it as well few
patches later), change 'systemd' to 'caller' and fix LISTEN_FDS to
LISTEN_PID where applicable.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/libvirt_private.syms | 1 +
src/locking/lock_daemon.c | 45 -
src/util/virutil.c| 51 +++
src/util/virutil.h| 2 ++
4 files changed, 58 insertions(+), 41 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8d3671c..138a9fa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2123,6 +2123,7 @@ virGetGroupID;
virGetGroupList;
virGetGroupName;
virGetHostname;
+virGetListenFDs;
virGetSelfLastChanged;
virGetUnprivSGIOSysfsPath;
virGetUserCacheDirectory;
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 3379f29..e9219d5 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -600,50 +600,13 @@ static int
virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)
{
virNetServerServicePtr svc;
-const char *pidstr;
-const char *fdstr;
-unsigned long long procid;
unsigned int nfds;
-VIR_DEBUG(Setting up networking from systemd);
-
-if (!(pidstr = virGetEnvAllowSUID(LISTEN_PID))) {
-VIR_DEBUG(No LISTEN_FDS from systemd);
-return 0;
-}
-
-if (virStrToLong_ull(pidstr, NULL, 10, procid) 0) {
-VIR_DEBUG(Malformed LISTEN_PID from systemd %s, pidstr);
-return 0;
-}
-
-if ((pid_t)procid != getpid()) {
-VIR_DEBUG(LISTEN_PID %s is not for us %llu,
- pidstr, (unsigned long long)getpid());
-return 0;
-}
-
-if (!(fdstr = virGetEnvAllowSUID(LISTEN_FDS))) {
-VIR_DEBUG(No LISTEN_FDS from systemd);
-return 0;
-}
-
-if (virStrToLong_ui(fdstr, NULL, 10, nfds) 0) {
-VIR_DEBUG(Malformed LISTEN_FDS from systemd %s, fdstr);
-return 0;
-}
-
-if (nfds 1) {
-VIR_DEBUG(Too many (%d) file descriptors from systemd,
- nfds);
-nfds = 1;
-}
-
-unsetenv(LISTEN_PID);
-unsetenv(LISTEN_FDS);
-
-if (nfds == 0)
+if ((nfds = virGetListenFDs()) == 0)
return 0;
+if (nfds 1)
+VIR_DEBUG(Too many (%d) file descriptors from systemd, nfds);
+nfds = 1;
/* Systemd passes FDs, starting immediately after stderr,
* so the first FD we'll get is '3'. */
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 95d1ff9..6f3f411 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2227,3 +2227,54 @@ void virUpdateSelfLastChanged(const char *path)
selfLastChanged = sb.st_ctime;
}
}
+
+/*
+ * virGetListenFDs:
+ *
+ * Parse LISTEN_PID and LISTEN_FDS passed from caller.
+ *
+ * Returns number of passed FDs.
+ */
+unsigned int
+virGetListenFDs(void)
+{
+const char *pidstr;
+const char *fdstr;
+unsigned long long procid;
+unsigned int nfds;
+
+VIR_DEBUG(Setting up networking from caller);
+
+if (!(pidstr = virGetEnvAllowSUID(LISTEN_PID))) {
+VIR_DEBUG(No LISTEN_PID from caller);
+return 0;
+}
+
+if (virStrToLong_ull(pidstr, NULL, 10, procid) 0) {
+VIR_DEBUG(Malformed LISTEN_PID from caller %s, pidstr);
+return 0;
+}
+
+if ((pid_t)procid != getpid()) {
+VIR_DEBUG(LISTEN_PID %s is not for us %llu,
+ pidstr, (unsigned long long)getpid());
+return 0;
+}
+
+if (!(fdstr = virGetEnvAllowSUID(LISTEN_FDS))) {
+VIR_DEBUG(No LISTEN_FDS from caller);
+return 0;
+}
+
+if (virStrToLong_ui(fdstr, NULL, 10, nfds) 0) {
+VIR_DEBUG(Malformed LISTEN_FDS from caller %s, fdstr);
+return 0;
+}
+
+unsetenv(LISTEN_PID);
+unsetenv(LISTEN_FDS);
+
+VIR_DEBUG(Got %u file descriptors, nfds);
+
+return nfds;
+}
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 2bb74e2..76c1ef3 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -203,4 +203,6 @@ bool virIsSUID(void);
time_t virGetSelfLastChanged(void);
void virUpdateSelfLastChanged(const char *path);
+unsigned int virGetListenFDs(void);
+
#endif /* __VIR_UTIL_H__ */
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 8/8] rpc: pass listen FD to the daemon being started
This eliminates the need for active waiting.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/rpc/virnetsocket.c | 58 +-
1 file changed, 39 insertions(+), 19 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index a94b2bc..c00209c 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1,7 +1,7 @@
/*
* virnetsocket.c: generic network socket handling
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -121,7 +121,7 @@ VIR_ONCE_GLOBAL_INIT(virNetSocket)
#ifndef WIN32
-static int virNetSocketForkDaemon(const char *binary)
+static int virNetSocketForkDaemon(const char *binary, int passfd)
{
int ret;
virCommandPtr cmd = virCommandNewArgList(binary,
@@ -134,6 +134,10 @@ static int virNetSocketForkDaemon(const char *binary)
virCommandAddEnvPassBlockSUID(cmd, XDG_RUNTIME_DIR, NULL);
virCommandClearCaps(cmd);
virCommandDaemonize(cmd);
+if (passfd) {
+virCommandPassFD(cmd, passfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+virCommandPassListenFDs(cmd);
+}
ret = virCommandRun(cmd, NULL);
virCommandFree(cmd);
return ret;
@@ -542,8 +546,7 @@ int virNetSocketNewConnectUNIX(const char *path,
{
virSocketAddr localAddr;
virSocketAddr remoteAddr;
-int fd;
-int retries = 0;
+int fd, passfd;
memset(localAddr, 0, sizeof(localAddr));
memset(remoteAddr, 0, sizeof(remoteAddr));
@@ -569,28 +572,45 @@ int virNetSocketNewConnectUNIX(const char *path,
if (remoteAddr.data.un.sun_path[0] == '@')
remoteAddr.data.un.sun_path[0] = '\0';
- retry:
-if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
-if ((errno == ECONNREFUSED ||
- errno == ENOENT)
-spawnDaemon retries 20) {
-VIR_DEBUG(Connection refused for %s, trying to spawn %s,
- path, binary);
-if (retries == 0
-virNetSocketForkDaemon(binary) 0)
-goto error;
+if (spawnDaemon) {
+if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) 0) {
+virReportSystemError(errno, %s, _(Failed to create socket));
+goto error;
+}
-retries++;
-usleep(1000 * 100 * retries);
-goto retry;
+/*
+ * We cannot do the umask() trick here because that's not
+ * thread-safe. fchmod(), however, is not guaranteed to work on
+ * some BSD favours, but *should* work on Linux before the socket
+ * is bound. POSIX says the behaviour of fchmod() called on
+ * socket is unspecified, though.
+ */
+if (fchmod(passfd, 0700) 0) {
+virReportSystemError(errno, %s,
+ _(Failed to change permissions on socket));
+goto error;
}
-virReportSystemError(errno,
- _(Failed to connect socket to '%s'),
+if (bind(passfd, remoteAddr.data.sa, remoteAddr.len) 0) {
+virReportSystemError(errno, _(Failed to bind socket to %s),
path);
+goto error;
+}
+
+if (listen(passfd, 0) 0) {
+virReportSystemError(errno, %s, _(Failed to listen on socket
to));
+goto error;
+}
+}
+
+if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
+virReportSystemError(errno, _(Failed to connect socket to '%s'),
path);
goto error;
}
+if (spawnDaemon virNetSocketForkDaemon(binary, passfd) 0)
+goto error;
+
localAddr.len = sizeof(localAddr.data);
if (getsockname(fd, localAddr.data.sa, localAddr.len) 0) {
virReportSystemError(errno, %s, _(Unable to get local socket
name));
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/8] Speed up waiting for the session daemon
This is complete rework of: http://www.redhat.com/archives/libvir-list/2013-April/msg01351.html where Daniel suggested we use systemd-like passing of socket fd in combination with the LISTEN_FDS environment variable: http://www.redhat.com/archives/libvir-list/2013-April/msg01356.html Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369 Martin Kletzander (8): util: abstract parsing of passed FDs into virGetListenFDs() remote: create virNetServerServiceNewFDOrUNIX() wrapper rpc: set listen backlog on FDs as well as on other sockets daemon: support passing FDs from the calling process cfg.mk: allow integers to be assigned a value computed with i|j|k tests: support dynamic prefixes in commandtest util: add virCommandPassListenFDs() function rpc: pass listen FD to the daemon being started cfg.mk| 2 +- daemon/libvirtd.c | 45 ++ src/libvirt_private.syms | 2 + src/libvirt_remote.syms | 1 + src/locking/lock_daemon.c | 47 ++- src/rpc/virnetserverservice.c | 53 - src/rpc/virnetserverservice.h | 15 +- src/rpc/virnetsocket.c| 58 +++ src/util/vircommand.c | 99 +++ src/util/vircommand.h | 4 +- src/util/virutil.c| 51 src/util/virutil.h| 2 + tests/commanddata/test24.log | 7 +++ tests/commandtest.c | 105 ++ 14 files changed, 389 insertions(+), 102 deletions(-) create mode 100644 tests/commanddata/test24.log -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 08/16] conf, schema: add support for memnode elements
On Wed, Jul 16, 2014 at 12:21:24PM -0600, Eric Blake wrote: On 07/16/2014 08:42 AM, Martin Kletzander wrote: Signed-off-by: Martin Kletzander mklet...@redhat.com --- [...] +++ b/docs/schemas/domaincommon.rng @@ -789,6 +789,23 @@ /choice /element /optional + zeroOrMore +element name=memnode + attribute name=cellid +ref name=unsignedInt/ + /attribute + attribute name=mode +choice + valuestrict/value + valuepreferred/value + valueinterleave/value +/choice + /attribute + attribute name='nodeset' +ref name='cpuset'/ + /attribute +/element + /zeroOrMore /element Missing an interleave here (memory and memnode should be swappable with one another). Oh! My bad. So sorry :-( Good point, is this OK to push as trivial (git diff -w): diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng index a0ea300..fb5bdb3 100644 --- i/docs/schemas/domaincommon.rng +++ w/docs/schemas/domaincommon.rng @@ -790,6 +790,7 @@ /element /optional zeroOrMore +interleave element name=memnode attribute name=cellid ref name=unsignedInt/ @@ -805,6 +806,7 @@ ref name='cpuset'/ /attribute /element +/interleave /zeroOrMore /element /define -- Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/8] daemon: support passing FDs from the calling process
First FD is the RW unix socket to listen on, second one (if
applicable) is the RO unix socket.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
daemon/libvirtd.c | 45 +++--
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 4c926b3..d20aeae 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -56,6 +56,7 @@
#include virstring.h
#include locking/lock_manager.h
#include viraccessmanager.h
+#include virutil.h
#ifdef WITH_DRIVER_MODULES
# include driver.h
@@ -476,11 +477,19 @@ static int daemonSetupNetworking(virNetServerPtr srv,
int unix_sock_ro_mask = 0;
int unix_sock_rw_mask = 0;
+unsigned int cur_fd = STDIN_FILENO + 1;
+unsigned int nfds = virGetListenFDs();
+
if (config-unix_sock_group) {
if (virGetGroupID(config-unix_sock_group, unix_sock_gid) 0)
return -1;
}
+if (nfds nfds ((int)!!sock_path + (int)!!sock_path_ro)) {
+VIR_ERROR(_(Too many (%u) FDs passed from caller), nfds);
+return -1;
+}
+
if (virStrToLong_i(config-unix_sock_ro_perms, NULL, 8,
unix_sock_ro_mask) != 0) {
VIR_ERROR(_(Failed to parse mode '%s'), config-unix_sock_ro_perms);
goto error;
@@ -491,30 +500,30 @@ static int daemonSetupNetworking(virNetServerPtr srv,
goto error;
}
-VIR_DEBUG(Registering unix socket %s, sock_path);
-if (!(svc = virNetServerServiceNewUNIX(sock_path,
- unix_sock_rw_mask,
- unix_sock_gid,
- config-auth_unix_rw,
+if (!(svc = virNetServerServiceNewFDOrUNIX(sock_path,
+ unix_sock_rw_mask,
+ unix_sock_gid,
+ config-auth_unix_rw,
#if WITH_GNUTLS
- NULL,
+ NULL,
#endif
- false,
- config-max_queued_clients,
- config-max_client_requests)))
+ false,
+ config-max_queued_clients,
+ config-max_client_requests,
+ nfds, cur_fd)))
goto error;
if (sock_path_ro) {
-VIR_DEBUG(Registering unix socket %s, sock_path_ro);
-if (!(svcRO = virNetServerServiceNewUNIX(sock_path_ro,
- unix_sock_ro_mask,
- unix_sock_gid,
- config-auth_unix_ro,
+if (!(svcRO = virNetServerServiceNewFDOrUNIX(sock_path_ro,
+ unix_sock_ro_mask,
+ unix_sock_gid,
+ config-auth_unix_ro,
#if WITH_GNUTLS
- NULL,
+ NULL,
#endif
- true,
- config-max_queued_clients,
- config-max_client_requests)))
+ true,
+
config-max_queued_clients,
+
config-max_client_requests,
+ nfds, cur_fd)))
goto error;
}
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 08/16] conf, schema: add support for memnode elements
On 07/16/2014 12:33 PM, Martin Kletzander wrote: On Wed, Jul 16, 2014 at 12:21:24PM -0600, Eric Blake wrote: On 07/16/2014 08:42 AM, Martin Kletzander wrote: Signed-off-by: Martin Kletzander mklet...@redhat.com --- [...] Missing an interleave here (memory and memnode should be swappable with one another). Oh! My bad. So sorry :-( Good point, is this OK to push as trivial (git diff -w): Count this as my ACK :) diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng index a0ea300..fb5bdb3 100644 --- i/docs/schemas/domaincommon.rng +++ w/docs/schemas/domaincommon.rng @@ -790,6 +790,7 @@ /element /optional zeroOrMore +interleave element name=memnode I'm assuming the odd spacing here is due to pasting into the email body, not how it actually looked in the diff. That, and diff -w already plays games with spacing. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virsh capabilities vs. domcapabilities
We have some inconsistencies in the node capabilities (which shows guest capabilities for some default binaries) and domcapabilities (which shows guest capabilities for a specified binary). It might be nicer for client applications if the two XML components share a common schema and output layout, so that the same client code can be used to parse either (sub-tree of) XML, modulo the name of the top-most element containing the tree. Furthermore, I'm trying to figure out how to advertise whether a given domain will support active commit (and similarly, Peter's patches for relative backing name preservation). Advertising the feature just through 'virsh capabilities' is insufficient, because that only covers the default binary; so it seems like the sort of thing that should also be in 'virsh domcapabilities'. Since virConnectGetDomainCapabilities is brand new to 1.2.7, we still have time to change its XML. But I want consensus on whether we need things to match, or whether we intentionally want people to rely only on the newer XML format of the new API call (that is, don't bloat 'virsh capabilities'/virConnectGetCapabilities any further, and learning whether active commit is supported will have to be done via 'virsh domcapabilities'/virConnectGetDomainCapabilities). That is, I'm wondering if domainCapabilities should use capabilities/guest as its starting point, rather than completely inventing new XML. I'm also finding 'virsh domcapabilities' a bit hard to use - even though it allows all its arguments to be optional at the RPC level, the qemu implementation isn't so happy: # tools/virsh domcapabilities error: failed to get emulator capabilities error: virttype_str in qemuConnectGetDomainCapabilities must not be NULL but how am I supposed to know what --virttype strings are valid? # tools/virsh domcapabilities --virttype kvm error: failed to get emulator capabilities error: invalid argument: at least one of emulatorbin or architecture fields must be present Would it be nicer to behave the same as 'virsh capabilities' and give the details of the default binary in this case? Now, for a comparison between the two XML per binary: 'virsh capabilities' gives me this segment: guest os_typehvm/os_type arch name='alpha' wordsize64/wordsize emulator/usr/bin/qemu-system-alpha/emulator machine maxCpus='4'clipper/machine domain type='qemu' /domain /arch features deviceboot/ disksnapshot default='on' toggle='no'/ /features /guest while 'virsh domcapabilities --emulatorbin /usr/bin/qemu-system-alpha --virttype kvm' gives this: domainCapabilities path/usr/bin/qemu-system-alpha/path domainkvm/domain machineclipper/machine archalpha/arch vcpu max='4'/ devices disk supported='yes' enum name='diskDevice' valuedisk/value valuecdrom/value valuefloppy/value valuelun/value /enum enum name='bus' valueide/value valuefdc/value valuescsi/value valuevirtio/value valueusb/value /enum /disk hostdev supported='yes' enum name='mode' valuesubsystem/value /enum enum name='startupPolicy' valuedefault/value valuemandatory/value valuerequisite/value valueoptional/value /enum enum name='subsysType' valueusb/value valuepci/value valuescsi/value /enum enum name='capsType'/ enum name='pciBackend'/ /hostdev /devices /domainCapabilities I'm okay that the domcapabilites output is longer, and don't think we need to backport any of the new stuff to the older API. But any information present in the old API should be easily retrieved using the new API, so that the information isn't lost, and so that a client can learn the same amount of detail about a non-default binary as they can about the defaults. Look at the difference in XPath to get to some of the same information: /guest/os_type vs. ? - where is os_type in domcapabilities? /guest/arch@name vs. /domainCapabilities/arch - why is one an attribute and the other an element? /guest/arch/wordsize vs. nothing - where is wordsize in domcapabilities? /guest/arch/emulator vs. /domainCapabilities/path - why 3 levels vs. 2, and different naming? /guest/arch/machine@maxCpus vs. /domainCapabilities/vcpu@max - why 3 levels vs. 2, with different naming? /guest/arch/machine vs. /domainCapabilities/machine - why 3 levels vs. 2? /guest/arch/domain@type vs. /domainCapabilities/domain - why attribute of 3 levels vs. element at 2 levels? And did I expose an error when I passed --virrtype kvm, even though qemu-system-alpha is only a qemu emulator on my machine (since kvm emulators is only for hardware-assit emulation, which is not possible when I'm doing cross architecture)? /guest/features vs. ? - where are the features in domcapabilities? -- Eric Blake eblake redhat com
Re: [libvirt] [PATCH 0/2] (for 1.2.7) network: Bring netdevs online later
On 07/01/2014 02:00 PM, Matthew Rosato wrote: The following patchset introduces code to defer setting netdevs online (and therefore registering MACs) until right before beginning guest CPU execution. The first patch introduces some infrastructure changes in preparation of the actual function added in the 2nd patch. Associated BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1081461 Ping... Changes since RFC: * Add a separate patch to introduce a flags field for macvlan/macvtap creation. * Use macvlan/tap IFUP flags to skip virNetDevSetOnline (for qemu only). * Add hotplug support. * For macvlan, save the current virNetDevVPortProfileOp in virDomainNetDef during qemuPhysIfaceConnect. As Laine mentioned, could use this field in a future patch to eliminate passing virNetDevVPortProfileOp everywhere. * Add qemu_interface.c and qemu_interface.h to encapsulate new functions. Matthew Rosato (2): util: Introduce flags field for macvtap creation network: Bring netdevs online later src/Makefile.am |1 + src/conf/domain_conf.h |2 ++ src/lxc/lxc_process.c |3 +- src/qemu/qemu_command.c |9 -- src/qemu/qemu_hotplug.c |7 + src/qemu/qemu_interface.c | 65 +++ src/qemu/qemu_interface.h | 33 ++ src/qemu/qemu_process.c |4 +++ src/util/virnetdevmacvlan.c | 32 + src/util/virnetdevmacvlan.h | 10 ++- 10 files changed, 150 insertions(+), 16 deletions(-) create mode 100644 src/qemu/qemu_interface.c create mode 100644 src/qemu/qemu_interface.h -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 08/16] conf, schema: add support for memnode elements
On Wed, Jul 16, 2014 at 12:43:07PM -0600, Eric Blake wrote: On 07/16/2014 12:33 PM, Martin Kletzander wrote: On Wed, Jul 16, 2014 at 12:21:24PM -0600, Eric Blake wrote: On 07/16/2014 08:42 AM, Martin Kletzander wrote: Signed-off-by: Martin Kletzander mklet...@redhat.com --- [...] Missing an interleave here (memory and memnode should be swappable with one another). Oh! My bad. So sorry :-( Good point, is this OK to push as trivial (git diff -w): Count this as my ACK :) I pushed it then, thank you. diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng index a0ea300..fb5bdb3 100644 --- i/docs/schemas/domaincommon.rng +++ w/docs/schemas/domaincommon.rng @@ -790,6 +790,7 @@ /element /optional zeroOrMore +interleave element name=memnode I'm assuming the odd spacing here is due to pasting into the email body, not how it actually looked in the diff. That, and diff -w already plays games with spacing. diff -w looked OK when I pasted it in the mail body, but somewhere on the way it got smudged. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 08/16] conf, schema: add support for memnode elements
On 07/16/2014 02:43 PM, Martin Kletzander wrote: Good point, is this OK to push as trivial (git diff -w): Count this as my ACK :) I pushed it then, thank you. Not my day. I was so focused on the 'diff -w' aspect that I completely overlooked another aspect. The patch is wrong: diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng index a0ea300..fb5bdb3 100644 --- i/docs/schemas/domaincommon.rng +++ w/docs/schemas/domaincommon.rng @@ -790,6 +790,7 @@ /element /optional zeroOrMore +interleave interleave makes no difference here. As the zeroOrMore has only one element child, there is nothing to be interleaved. I meant for it to go one level higher, outside the zeroOrMore, where it can also interleave with memory. element name=memnode I'm assuming the odd spacing here is due to pasting into the email body, not how it actually looked in the diff. That, and diff -w already plays games with spacing. diff -w looked OK when I pasted it in the mail body, but somewhere on the way it got smudged. As penance, I'm proposing this followup: diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng index fb5bdb3..2caeef9 100644 --- i/docs/schemas/domaincommon.rng +++ w/docs/schemas/domaincommon.rng @@ -759,6 +759,7 @@ !-- All the NUMA related tunables would go in the numatune -- define name=numatune element name=numatune + interleave optional element name=memory optional @@ -790,7 +791,6 @@ /element /optional zeroOrMore -interleave element name=memnode attribute name=cellid ref name=unsignedInt/ @@ -806,8 +806,8 @@ ref name='cpuset'/ /attribute /element -/interleave /zeroOrMore + /interleave /element /define -- 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 v3 08/16] conf, schema: add support for memnode elements
On 07/16/2014 02:53 PM, Eric Blake wrote: zeroOrMore +interleave interleave makes no difference here. As the zeroOrMore has only one element child, there is nothing to be interleaved. I meant for it to go one level higher, outside the zeroOrMore, where it can also interleave with memory. element name=memnode I'm assuming the odd spacing here is due to pasting into the email body, not how it actually looked in the diff. That, and diff -w already plays games with spacing. diff -w looked OK when I pasted it in the mail body, but somewhere on the way it got smudged. As penance, I'm proposing this followup: and also squashing in this change to the testsuite, to expose the difference: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml index 49b328c..440413b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml @@ -5,8 +5,8 @@ currentMemory unit='KiB'24682468/currentMemory vcpu placement='static'32/vcpu numatune -memory mode='strict' nodeset='0-7'/ memnode cellid='0' mode='preferred' nodeset='3'/ +memory mode='strict' nodeset='0-7'/ memnode cellid='2' mode='strict' nodeset='1-2,5-7,^6'/ /numatune os -- 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] libxl: Implement basic video device selection
Stefan Bader wrote:
being as bad with timely responses. Ok, so how about the following?
One note: it could be the STRDUP's are not strictly needed. But
to me it felt wrong to have two places refer to the same strings
(as MakeVFB copies the struct containing the pointers).
Agreed. Without the STRDUP's, seems there is a potential for double
free when libxl_device_vfb and libxl_domain_config objects are disposed.
If this
is not needed, then all changes now in MakeVFB probably can be
dropped (except setting the keyboard layout, maybe; which I
might miss ;)).
-Stefan
From a95db265fa4c1a231e7c2d70baa360c6a0500e3b Mon Sep 17 00:00:00 2001
From: Stefan Bader stefan.ba...@canonical.com
Date: Thu, 27 Mar 2014 16:01:18 +0100
Subject: [PATCH] libxl: Implement basic video device selection
This started as an investigation into an issue where libvirt (using the
libxl driver) and the Xen host, like an old couple, could not agree on
who is responsible for selecting the VNC port to use.
Things usually (and a bit surprisingly) did work because, just like that
old couple, they had the same idea on what to do by default. However it
was possible that this ended up in a big argument.
The problem is that display information exists in two different places:
in the vfbs list and in the build info. And for launching the device model,
only the latter is used. But that never gets initialized from libvirt. So
Xen allows the device model to select a default port while libvirt thinks
it has told Xen that this is done by libvirt (though the vfbs config).
While fixing that, I made a stab at actually evaluating the configuration
of the video device. So that it is now possible to at least decide between
a Cirrus or standard VGA emulation and to modify the VRAM within certain
limits using libvirt.
[v2: Check return code of VIR_STRDUP and fix indentation]
[v3: Split out VRAM fixup and return error for unsupported video type]
[v4: Re-arrange code and move VFB setup into libxlMakeVfbList]
[meta-comment]
libvirt prefers patch version history like this to be below the '---'
following your Signed-off-by, so as to not pollute the commit message.
Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
src/libxl/libxl_conf.c | 63
++--
1 file changed, 61 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 8eeaf82..43cabcf 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1098,10 +1098,21 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports,
libxl_domain_build_info *b_info = d_config-b_info;
libxl_device_vfb vfb = d_config-vfbs[0];
-if (libxl_defbool_val(vfb.vnc.enable))
+if (libxl_defbool_val(vfb.vnc.enable)) {
memcpy(b_info-u.hvm.vnc, vfb.vnc, sizeof(libxl_vnc_info));
-else if (libxl_defbool_val(vfb.sdl.enable))
+if (VIR_STRDUP(b_info-u.hvm.vnc.listen, vfb.vnc.listen) 0)
+goto error;
+if (VIR_STRDUP(b_info-u.hvm.vnc.passwd, vfb.vnc.passwd) 0)
+goto error;
+} else if (libxl_defbool_val(vfb.sdl.enable)) {
memcpy(b_info-u.hvm.sdl, vfb.sdl, sizeof(libxl_sdl_info));
+if (VIR_STRDUP(b_info-u.hvm.sdl.display, vfb.sdl.display) 0)
+goto error;
+if (VIR_STRDUP(b_info-u.hvm.sdl.xauthority, vfb.sdl.xauthority)
0)
+goto error;
+}
+if (VIR_STRDUP(b_info-u.hvm.keymap, vfb.keymap) 0)
+goto error;
}
return 0;
@@ -1363,6 +1374,45 @@ libxlMakeCapabilities(libxl_ctx *ctx)
return NULL;
}
+static int
+libxlMakeVideo(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+libxl_domain_build_info *b_info = d_config-b_info;
+
+if (d_config-c_info.type != LIBXL_DOMAIN_TYPE_HVM)
+return 0;
+
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+break;
+case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s,
+ _(video type not supported by libxl));
+return -1;
+}
+b_info-video_memkb = def-videos[0]-vram ?
+ def-videos[0]-vram :
+
Re: [libvirt] [PATCH] libxl: Implement basic video device selection
On 07/16/2014 03:05 PM, Jim Fehlig wrote: +b_info-video_memkb = def-videos[0]-vram ? + def-videos[0]-vram : + LIBXL_MEMKB_DEFAULT; While testing this, I noticed that libvirt will set vram to 9216 if not specified. E.g. The 9216 default for qemu is absolutely stupid. No real hardware has a limit of 9M (8M or 16M are more likely). Please feel free to not perpetuate that stupidity into libxl. With type='vga', libxl will then fail to start the domain libxl: error: libxl_create.c:253:libxl__domain_build_info_setdefault: videoram must be at least 16 MB for STDVGA on QEMU_XEN This could be handled in libxlDomainDeviceDefPostParse(), where we can check for sane vram values for the various VIR_DOMAIN_VIDEO_TYPE_*, or set sane defaults if vram is not specified. Sounds like for libxl, a sane default is 16M. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Fix a bug in the XML code for storage pools
For ESX, the code that builds XML descriptions for attached storage pools was not setting the host count to anything when it returned a host name. --- src/esx/esx_storage_backend_vmfs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 6bed3ce..cf0da84 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -488,6 +488,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) if (VIR_ALLOC_N(def.source.hosts, 1) 0) goto cleanup; def.type = VIR_STORAGE_POOL_NETFS; +def.source.nhost = 1; def.source.hosts[0].name = nasInfo-nas-remoteHost; def.source.dir = nasInfo-nas-remotePath; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix reporting of i/o errors by iohelper process
On 07/16/2014 08:30 AM, Eric Blake wrote: On 07/16/2014 08:12 AM, Jason J. Herne wrote: From: Jason J. Herne jjhe...@us.ibm.com libvirt_iohelper is a helper process that is exec'ed and used to handle I/O during a Qemu managed save operation. Due to a missing call to virFileWrapperFdClose, all I/O error messages reported by iohelper are lost. This patch adds a call to virFileWrapperFdClose to the cleanup phase of qemuDomainSaveMemory. This patch also modifies virFileWrapperFdClose such that errors are only reported when the length of the err_msg buffer is 0. Before now, the existence of the buffer would trigger error reporting in virFileWrapperFdClose. Signed-off-by: Jason J. Herne jjhe...@us.ibm.com --- src/qemu/qemu_driver.c | 1 + src/util/virfile.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) Nice! Thanks for persevering on this one. Congrats on your first libvirt patch. Sadly, the more I look at this, the more I wonder how it can fix anything. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ecccf6c..8d78805 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3015,6 +3015,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, cleanup: VIR_FORCE_CLOSE(fd); +virFileWrapperFdClose(wrapperFd); virFileWrapperFdFree(wrapperFd); VIR_FREE(xml); But qemuDomainSaveMemory() already has a call to virFileWrapperFdClose() earlier on; worse, the current implementation of virFileWrapperFdClose() is not designed to be called twice if an error occurred (rather, if there is an error, two calls will end up logging the error twice, when once would have been sufficient). How are you getting to a point where the cleanup label is reached without going through the earlier close? Is there some easy setup you used in testing this patch, so that I can reproduce a scenario where iohelper will reliably fail? -- 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] esx: Fix a bug in the XML code for storage pools
On 07/16/2014 03:50 PM, Geoff Hickey wrote: For ESX, the code that builds XML descriptions for attached storage pools was not setting the host count to anything when it returned a host name. --- src/esx/esx_storage_backend_vmfs.c | 1 + 1 file changed, 1 insertion(+) ACK; will push shortly. diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 6bed3ce..cf0da84 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -488,6 +488,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) if (VIR_ALLOC_N(def.source.hosts, 1) 0) goto cleanup; def.type = VIR_STORAGE_POOL_NETFS; +def.source.nhost = 1; def.source.hosts[0].name = nasInfo-nas-remoteHost; def.source.dir = nasInfo-nas-remotePath; -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Fix a comment about VSphere versions
Update the VSphere version comment in esx_vi.c for ESX 5.1 and 5.5.
---
src/esx/esx_vi.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 3f5becb..c02a293 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -4556,12 +4556,14 @@
esxVI_ProductVersionToDefaultVirtualHWVersion(esxVI_ProductVersion productVersio
/*
* virtualHW.version compatibility matrix:
*
- * 4 7 8 API
- * ESX 3.5+ 2.5
- * ESX 4.0+ + 4.0
- * ESX 4.1+ + 4.1
- * ESX 5.0+ + + 5.0
- * GSX 2.0+ + 2.5
+ * 4 7 8 9 10 API
+ * ESX 3.5+2.5
+ * ESX 4.0+ + 4.0
+ * ESX 4.1+ + 4.1
+ * ESX 5.0+ + +5.0
+ * ESX 5.1+ + + + 5.1
+ * ESX 5.5+ + + + +5.5
+ * GSX 2.0+ + 2.5
*/
switch (productVersion) {
case esxVI_ProductVersion_ESX35:
--
1.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix reporting of i/o errors by iohelper process
On 07/16/2014 03:56 PM, Eric Blake wrote: On 07/16/2014 08:30 AM, Eric Blake wrote: On 07/16/2014 08:12 AM, Jason J. Herne wrote: From: Jason J. Herne jjhe...@us.ibm.com libvirt_iohelper is a helper process that is exec'ed and used to handle I/O during a Qemu managed save operation. Due to a missing call to virFileWrapperFdClose, all I/O error messages reported by iohelper are lost. +++ b/src/qemu/qemu_driver.c @@ -3015,6 +3015,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, cleanup: VIR_FORCE_CLOSE(fd); +virFileWrapperFdClose(wrapperFd); virFileWrapperFdFree(wrapperFd); VIR_FREE(xml); But qemuDomainSaveMemory() already has a call to virFileWrapperFdClose() earlier on; worse, the current implementation of virFileWrapperFdClose() is not designed to be called twice if an error occurred (rather, if there is an error, two calls will end up logging the error twice, when once would have been sufficient). How are you getting to a point where the cleanup label is reached without going through the earlier close? Is there some easy setup you used in testing this patch, so that I can reproduce a scenario where iohelper will reliably fail? Okay, looking into this a bit more: I made the following change: diff --git i/src/util/iohelper.c w/src/util/iohelper.c index 8a3c377..dfb45e1 100644 --- i/src/util/iohelper.c +++ w/src/util/iohelper.c @@ -307,6 +307,7 @@ main(int argc, char **argv) if (delete) unlink(path); +fprintf(stderr, _(goodbye world\n)); goto error; return 0; error: then tried to do a 'virsh save testvm1 /tmp/save'. Even without your patch, I get a very nice error: error: Failed to save domain testvm1 to /tmp/save error: internal error: Child process (LIBVIRT_LOG_OUTPUTS=1:stderr /home/eblake/libvirt/src/libvirt_iohelper /tmp/save 0 1) unexpected exit status 1: goodbye world /home/eblake/libvirt/src/libvirt_iohelper: unknown failure with /tmp/save But if I then rework the iohelper patch: diff --git i/src/util/iohelper.c w/src/util/iohelper.c index 8a3c377..efb1366 100644 --- i/src/util/iohelper.c +++ w/src/util/iohelper.c @@ -301,6 +301,7 @@ main(int argc, char **argv) exit(EXIT_FAILURE); } +fprintf(stderr, _(goodbye world\n)); goto error; if (fd 0 || runIO(path, fd, oflags, length) 0) goto error; the error is now: error: Failed to save domain testvm1 to /tmp/save error: operation failed: domain save job: unexpectedly failed So the problem is that we have _two_ possible sources of errors (qemu reporting failure because it can't write to iohelper, and iohelper reporting an error from whatever other reason, such as disk full). If qemu finishes, we have only the iohelper message and properly report it; but if we have both failures, then the qemu error takes priority, and in this case it is lower priority. There are also cases where qemu will error out but iohelper succeeds (such as if qemu refuses to migrate because the guest has hostdev passthrough). So I _think_ what we want to do is always check BOTH places for error; if only one of the two fails, use that message. If both fail, then I don't know whether it is possible to have a heuristic for which failure message is more meaningful, or whether it is better to report both errors (even though it will often be the case that one error was a knock-on effect from the other). But I'm a bit stuck on the best way to implement that. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] virsh: Document bandwidth maximum more clearly
Commit id '0e2d7305' modified the code to allow a negative value to be supplied for the bandwidth argument of the various block virsh commands and the migrate-setspeed; however, it failed to update the man page to describe the feature whereby a very large value could be interpreted by the hypervisor to mean maximum value allowed. Although initially designed to handle a -1 value, the reality is just about any negative value could be provided and essentially perform the same feature. Signed-off-by: John Ferlan jfer...@redhat.com --- tools/virsh.pod | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index e02ff5d..849ae31 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -865,7 +865,10 @@ to a unique target name (target dev='name'/) or source file (source file='name'/) for one of the disk devices attached to Idomain (see also Bdomblklist for listing these names). Ibandwidth specifies copying bandwidth limit in MiB/s, although for -qemu, it may be non-zero only for an online domain. +qemu, it may be non-zero only for an online domain. Specifying a negative +value is interpreted as an unsigned long long value or essentially +unlimited. The hypervisor can choose whether to reject the value or +convert it to the maximum value allowed. =item Bblockcopy Idomain Ipath Idest [Ibandwidth] [I--shallow] [I--reuse-external] [I--raw] [I--wait [I--async] [I--verbose]] @@ -908,7 +911,10 @@ as fast as possible, otherwise the command may continue to block a little while longer until the job has actually cancelled. Ipath specifies fully-qualified path of the disk. -Ibandwidth specifies copying bandwidth limit in MiB/s. +Ibandwidth specifies copying bandwidth limit in MiB/s. Specifying a negative +value is interpreted as an unsigned long long value or essentially +unlimited. The hypervisor can choose whether to reject the value or +convert it to the maximum value allowed. =item Bblockpull Idomain Ipath [Ibandwidth] [Ibase] [I--wait [I--verbose] [I--timeout Bseconds] [I--async]] @@ -939,7 +945,10 @@ Ipath specifies fully-qualified path of the disk; it corresponds to a unique target name (target dev='name'/) or source file (source file='name'/) for one of the disk devices attached to Idomain (see also Bdomblklist for listing these names). -Ibandwidth specifies copying bandwidth limit in MiB/s. +Ibandwidth specifies copying bandwidth limit in MiB/s. Specifying a negative +value is interpreted as an unsigned long long value or essentially +unlimited. The hypervisor can choose whether to reject the value or +convert it to the maximum value allowed. =item Bblkdeviotune Idomain Idevice [[I--config] [I--live] | [I--current]] @@ -995,6 +1004,9 @@ commit job be pivoted over to the new image. If I--info is specified, the active job information on the specified disk will be printed. Ibandwidth can be used to set bandwidth limit for the active job. +Specifying a negative value is interpreted as an unsigned long long +value or essentially unlimited. The hypervisor can choose whether to +reject the value or convert it to the maximum value allowed. =item Bblockresize Idomain Ipath Isize @@ -1390,7 +1402,10 @@ obtained from domjobinfo. =item Bmigrate-setspeed Idomain Ibandwidth Set the maximum migration bandwidth (in MiB/s) for a domain which is being -migrated to another host. +migrated to another host. Ibandwidth is interpreted as an unsigned long +long value. Specifying a negative value results in an essentially unlimited +value being provided to the hypervisor. The hypervisor can choose whether to +reject the value or convert it to the maximum value allowed. =item Bmigrate-getspeed Idomain -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] virsh vol-upload/download disallow negative offset
https://bugzilla.redhat.com/show_bug.cgi?id=1087104
Commit id 'c6212539' explicitly allowed a negative value to be used for
offset and length as a shorthand for the largest value after commit id
'f18c02ec' modified virStrToLong_ui() to essentially disallow a negative
value.
However, allowing a negative value for offset ONLY worked if the negative
value was -1 since the eventual lseek() does allow a -1 to mean the end
of the file. Providing other negative values resulted in errors such as:
$ virsh vol-download --pool default qcow3-vol2 /home/vm-images/raw \
--offset -2 --length -1000
error: cannot download from volume qcow3-vol2
error: Unable to seek /home/vm-images/qcow3-vol2 to 18446744073709551614:
Invalid argument
$
Thus, it seems unreasonable to expect or allow a negative value for offset
since the only benefit is to lseek() to the end of the file and then only
take advantage of how the OS would handle such a seek. For the purposes of
upload or download of volume data, that seems to be a no-op. Therefore,
disallow a negative value for offset.
Additionally, modify the man page for vol-upload and vol-download to provide
more details regarding the valid values for both offset and length.
Signed-off-by: John Ferlan jfer...@redhat.com
---
tools/virsh-volume.c | 6 +++---
tools/virsh.pod | 10 --
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 724a86b..b528928 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -787,13 +787,13 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
unsigned long long offset = 0, length = 0;
bool created = false;
-if (vshCommandOptULongLongWrap(cmd, offset, offset) 0) {
-vshError(ctl, _(Unable to parse integer));
+if (vshCommandOptULongLong(cmd, offset, offset) 0) {
+vshError(ctl, _(Unable to parse offset value));
return false;
}
if (vshCommandOptULongLongWrap(cmd, length, length) 0) {
-vshError(ctl, _(Unable to parse integer));
+vshError(ctl, _(Unable to parse length value));
return false;
}
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 1a2b01f..e02ff5d 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2961,7 +2961,10 @@ is in.
Ivol-name-or-key-or-path is the name or key or path of the volume where the
file will be uploaded.
I--offset is the position in the storage volume at which to start writing
-the data. I--length is an upper bound of the amount of data to be uploaded.
+the data. The value must be 0 or larger. I--length is an upper bound
+of the amount of data to be uploaded. A negative value is interpreted
+as an unsigned long long value to essentially include everything from
+the offset to the end of the volume.
An error will occur if the Ilocal-file is greater than the specified length.
=item Bvol-download [I--pool Ipool-or-uuid] [I--offset Ibytes]
@@ -2972,7 +2975,10 @@ I--pool Ipool-or-uuid is the name or UUID of the
storage pool the volume
is in.
Ivol-name-or-key-or-path is the name or key or path of the volume to
download.
I--offset is the position in the storage volume at which to start reading
-the data. I--length is an upper bound of the amount of data to be downloaded.
+the data. The value must be 0 or larger. I--length is an upper bound of
+the amount of data to be downloaded. A negative value is interpreted as
+an unsigned long long value to essentially include everything from the
+offset to the end of the volume.
=item Bvol-wipe [I--pool Ipool-or-uuid] [I--algorithm Ialgorithm]
Ivol-name-or-key-or-path
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] virsh: negative numbers for specific commands
Following up to the recently restarted discussion:
http://www.redhat.com/archives/libvir-list/2014-July/msg00686.html
regarding negative values for certain virsh commands - these patches
will document the feature of using a negative value to indicate
the largest value *and* for the vol-{upload|download} change 'offset'
to not accept a negative value.
John Ferlan (2):
virsh vol-upload/download disallow negative offset
virsh: Document bandwidth maximum more clearly
tools/virsh-volume.c | 6 +++---
tools/virsh.pod | 33 +++--
2 files changed, 30 insertions(+), 9 deletions(-)
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix reporting of i/o errors by iohelper process
On 07/16/2014 05:20 PM, Eric Blake wrote: But if I then rework the iohelper patch: diff --git i/src/util/iohelper.c w/src/util/iohelper.c index 8a3c377..efb1366 100644 --- i/src/util/iohelper.c +++ w/src/util/iohelper.c @@ -301,6 +301,7 @@ main(int argc, char **argv) exit(EXIT_FAILURE); } +fprintf(stderr, _(goodbye world\n)); goto error; if (fd 0 || runIO(path, fd, oflags, length) 0) goto error; the error is now: error: Failed to save domain testvm1 to /tmp/save error: operation failed: domain save job: unexpectedly failed and with your patch, I see: error: Failed to save domain testvm1 to /tmp/save error: internal error: Child process (LIBVIRT_LOG_OUTPUTS=1:stderr /home/eblake/libvirt/src/libvirt_iohelper /tmp/save 0 1) unexpected exit status 1: goodbye world /home/eblake/libvirt/src/libvirt_iohelper: unknown failure with /tmp/save on the console, but this longer spew in libvirt's log: 2014-07-16 23:34:23.855+: 25406: error : qemuMigrationUpdateJobStatus:1788 : operation failed: domain save job: unexpectedly failed 2014-07-16 23:34:23.857+: 25406: error : virCommandWait:2423 : internal error: Child process (LIBVIRT_LOG_OUTPUTS=1:stderr /home/eblake/libvirt/src/libvirt_iohelper /tmp/save 0 1) unexpected exit status 1: goodbye world /home/eblake/libvirt/src/libvirt_iohelper: unknown failure with /tmp/save 2014-07-16 23:34:23.857+: 25406: warning : virFileWrapperFdClose:326 : iohelper reports: goodbye world /home/eblake/libvirt/src/libvirt_iohelper: unknown failure with /tmp/save so the act of closing the wrapperfd is losing the earlier error message from being reported to the user (seems okay in this case, but might not always be), AND logging the stderr contents twice (once via the error reported to the user, and again due to a VIR_WARN). So the problem is that we have _two_ possible sources of errors (qemu reporting failure because it can't write to iohelper, and iohelper reporting an error from whatever other reason, such as disk full). If qemu finishes, we have only the iohelper message and properly report it; but if we have both failures, then the qemu error takes priority, and in this case it is lower priority. There are also cases where qemu will error out but iohelper succeeds (such as if qemu refuses to migrate because the guest has hostdev passthrough). So I _think_ what we want to do is always check BOTH places for error; if only one of the two fails, use that message. If both fail, then I don't know whether it is possible to have a heuristic for which failure message is more meaningful, or whether it is better to report both errors (even though it will often be the case that one error was a knock-on effect from the other). But I'm a bit stuck on the best way to implement that. I'm still thinking about the best solution -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Syslog nested job is dangerous message
Hello everyone,
After performing a migration, the syslog often contains several messages like
this:
This thread seems to be the async job owner; entering monitor without asking
for a nested job is dangerous
The message makes it sound as if the user has done something dangerous but,
after looking at the code that produces the message, it appears to be more to
do with how the libvirt job code is used.
The commit that added the warning (6eede368bc8e3df2c94c2ec1a913885ce08e99db)
doesn't explain why it might be dangerous...
qemu: Warn on possibly incorrect usage of EnterMonitor*
qemuDomainObjEnterMonitor{,WithDriver} should not be called from async
jobs, only EnterMonitorAsync variant is allowed.
... so I would appreciate some advice from people who understand that area.
Would it be appropriate to re-word the message, or perhaps change it to a debug
message so that it's not normally seen by users?
Is it important to track down the cases that are generating the warning and fix
them? (Could it cause some kind of significant problem?)
Cheers,
Sam.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Syslog nested job is dangerous message
On 07/16/2014 07:52 PM, Sam Bobroff wrote: Hello everyone, [Can you configure your mailer to wrap long lines?] After performing a migration, the syslog often contains several messages like this: This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous The sign of a bug that we need to fix. What version of libvirt are you using? We may have already fixed it. The message makes it sound as if the user has done something dangerous but, after looking at the code that produces the message, it appears to be more to do with how the libvirt job code is used. Rather, libvirt itself has done something dangerous. The commit that added the warning (6eede368bc8e3df2c94c2ec1a913885ce08e99db) doesn't explain why it might be dangerous... Jiri can probably explain better, but it means there is a race condition where libvirt can lose track of a long-running job and cause memory corruption in its management of the guest. Would it be appropriate to re-word the message, or perhaps change it to a debug message so that it's not normally seen by users? No. Is it important to track down the cases that are generating the warning and fix them? (Could it cause some kind of significant problem?) Yes. -- 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] Syslog nested job is dangerous message
On 17/07/14 12:50, Eric Blake wrote: On 07/16/2014 07:52 PM, Sam Bobroff wrote: Hello everyone, [Can you configure your mailer to wrap long lines?] [No problem, done.] After performing a migration, the syslog often contains several messages like this: This thread seems to be the async job owner; entering monitor without asking for a nested job is dangerous The sign of a bug that we need to fix. What version of libvirt are you using? We may have already fixed it. I've been testing on 1.2.6, but I will re-test on the latest code from git. The message makes it sound as if the user has done something dangerous but, after looking at the code that produces the message, it appears to be more to do with how the libvirt job code is used. Rather, libvirt itself has done something dangerous. The commit that added the warning (6eede368bc8e3df2c94c2ec1a913885ce08e99db) doesn't explain why it might be dangerous... Jiri can probably explain better, but it means there is a race condition where libvirt can lose track of a long-running job and cause memory corruption in its management of the guest. Would it be appropriate to re-word the message, or perhaps change it to a debug message so that it's not normally seen by users? No. Is it important to track down the cases that are generating the warning and fix them? (Could it cause some kind of significant problem?) Yes. Acknowledged. If they still occur on the latest code I'll start tracking them down :-) Cheers, Sam. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 08/16] conf, schema: add support for memnode elements
On Wed, Jul 16, 2014 at 02:53:09PM -0600, Eric Blake wrote: On 07/16/2014 02:43 PM, Martin Kletzander wrote: Good point, is this OK to push as trivial (git diff -w): Count this as my ACK :) I pushed it then, thank you. Not my day. I was so focused on the 'diff -w' aspect that I completely overlooked another aspect. The patch is wrong: diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng index a0ea300..fb5bdb3 100644 --- i/docs/schemas/domaincommon.rng +++ w/docs/schemas/domaincommon.rng @@ -790,6 +790,7 @@ /element /optional zeroOrMore +interleave interleave makes no difference here. As the zeroOrMore has only one element child, there is nothing to be interleaved. I meant for it to go one level higher, outside the zeroOrMore, where it can also interleave with memory. element name=memnode I'm assuming the odd spacing here is due to pasting into the email body, not how it actually looked in the diff. That, and diff -w already plays games with spacing. diff -w looked OK when I pasted it in the mail body, but somewhere on the way it got smudged. As penance, I'm proposing this followup: diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng index fb5bdb3..2caeef9 100644 --- i/docs/schemas/domaincommon.rng +++ w/docs/schemas/domaincommon.rng @@ -759,6 +759,7 @@ !-- All the NUMA related tunables would go in the numatune -- define name=numatune element name=numatune + interleave optional element name=memory optional @@ -790,7 +791,6 @@ /element /optional zeroOrMore -interleave element name=memnode attribute name=cellid ref name=unsignedInt/ @@ -806,8 +806,8 @@ ref name='cpuset'/ /attribute /element -/interleave /zeroOrMore + /interleave /element /define Seeing this diff I see what I did wrong. Completely wrong to be accurate. It wasn't my day either, hopefully today will be better. ACK from me. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 08/16] conf, schema: add support for memnode elements
On 07/16/2014 09:38 PM, Martin Kletzander wrote: interleave makes no difference here. As the zeroOrMore has only one element child, there is nothing to be interleaved. I meant for it to go one level higher, outside the zeroOrMore, where it can also interleave with memory. As penance, I'm proposing this followup: Seeing this diff I see what I did wrong. Completely wrong to be accurate. It wasn't my day either, hopefully today will be better. ACK from me. Pushed, including the testsuite enhancement. -- 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
