[libvirt] vCPU hotplug roadmap (was: Minutes for KVM call 2013-01-15)
by Andreas Färber
Am 15.01.2013 17:16, schrieb Juan Quintela:
>
> * cpu hot plug
> - use qdev propierties conected to a set of socket objects (anthony)
> - cpusets are the wrong interface (anthony)
> - make a link between cpu <-> socket instead of a propierty?
> - how far are we from being able to describe a cpu with -device?
> (didn't heare the answer, andreas?)
> - perhaps the best approach?
> - After soft-freeze, exceptions depend on the maintainer
> - After hard-freeze, no exceptions
> -device don't require a bus, just an implementation detail, we can change that
> - use cpuset as an intermediate step until full vision is implemented
> - several approaches from where we are now, to have something before
> we get a full solution
>
>
> At this point, Andreas agreed to write a better summary of the
> discussion and suggestions O:-)
Got buried, here we go:
== vCPU hot-plug user interfaces ==
=== cpu_set ===
Previously available in qemu-kvm.git:
`cpu_set <n+1> online` via HMP
Pros:
* Hides QOM/qdev implementation details (afaerber)
* Thus: Doesn't depend on QOM CPUState refactoring (imammedo)
* Opens a fast route to implementing vCPU unplug in KVM (imammedo)
* Unintrusive to add and easy to obsolete/remove in future (imammedo)
* Existing virt-test cases (afaerber)
* Supported by libvirt (imammedo)
* Prevents confusing guests by hot-plugging random mix of CPUs (agraf)
Cons:
* Cannot express topologies (ehabkost)
=== device_add ===
`device_add driver=Haswell-x86_64-cpu id=qdevid`
[You can try this today and see it failing / not working.]
Pros:
* QMP/HMP command available today and known to users (afaerber)
* Unified command for device and CPU hot-plug (imammedo)
* Would allow first doing thread-level vCPU hotplug (imammedo)
* Could be extended to support socket-level hot-plug (aliguori/imammedo)
Cons:
* Operates on raw QOM type name unlike -cpu (afaerber)
* Needs support in libvirt for device_add driver=<CPU> (imammedo)
* libvirt needs means to enumerate CPU types (imammedo) => QMP? (AF)
Challenges:
* No CPU qbus (afaerber)
=> "should work" without (aliguori)
* CPU subclasses needed for identifying type name (afaerber/imammedo)
=> "Haswell-x86_64-cpu" does not exist yet, just "x86_64-cpu"
* CPU class_init for -cpu host requires KVM init (imammedo)
[suggestion by ehabkost to use kvm_arch_vcpu_init, WIP by afaerber]
* Conversion of CPU features to static properties needed (imammedo)
=> device_add driver=foo,level=x,xlevel=y,...
* Alternatively conversion to global properties (imammedo)
* Cements type names - rename for 1.4? (afaerber) => permissable (alig.)
[patches for arm, m68k, openrisc, unicore32 on list]
=== qom-set ===
`qom-set` via QMP w/ link<CPUSocket> property (aliguori)
Topology represented in QOM:
CPUSocket has-a CPUCore has-a CPUThread a.k.a. CPUState, or
CPUSocket links-to CPUCore links-to CPUThread a.k.a. CPUState
Challenges (afaerber):
* No CPUSocket/CPUCore objects yet and may take a while to get there...
topology fields being moved to CPUState for 1.4 [done, more WIP]
* No decisions on canonical paths for CPUs: CPU? machine? unassigned?
* Duality of thread-level device types and socket-level? (afaerber)
=> fine to have, e.g., quad-core Xeon 500 device (aliguori)
* CPUState is no_user (afaerber)
=> need to generally drop no_user for QOM (aliguori)
=== libvirt ===
libvirt's XML topology modelling is closer to today's -smp than to the
desired QOM modelling:
http://www.libvirt.org/formatcaps.html
`virsh setvcpus <domain> <n>`
http://libvirt.org/sources/virshcmdref/html/sect-setvcpus.html
== qom-cpu course of action (afaerber) ==
It was requested to have vCPU hot-plug in v1.5.
For device_add we need to move code from cpu_init() into QOM facilities.
=> QOM realize support would help [applied by aliguori]
=> cleanups piggy-backed onto CPU realizefn [applied to qom-cpu-next]
Agreement on goal of X86CPU subclasses, but conflicts how to get there:
* Refactor x86_def_t to X86CPUInfo for X86CPUClass class_init? (AF 2012)
* Refactor x86_def_t to X86CPU instance_init as done for arm?
* Refactor x86_def_t to class_inits? (afaerber)
-> heavy merge conflicts due to bug fixes / cleanups
Pro: We can get things into a consistent QOM'ish state across targets.
Con: We will refactor again on top for machine-compat properties.
* Keep x86_def_t within X86CPUClass as done for ppc? (WIP: afaerber)
=> smallest common denominator, separates x86 from cross-target work
APIC ID topology fixes are being reviewed for 1.4. [merged]
X86CPU wave 4 cleanups by Igor are being reviewed for 1.4. [merged]
Rename CPU types according to unified <name>-<arch>-cpu scheme for 1.4?
(aliguori: permissable) [patches on list]
VMState series by Juan being rebased - subset for 1.4, rest for 1.5.
[1.4 part on list, WIP for 1.5]
Remainder is considered 1.5 material, qom-cpu-next avail. during Freeze.
== Common issues (imammedo) ==
- back-port CPU hot-plug ACPI notification
- hot-plug is not allowed on SysBus:
- APIC that is created by CPU on hot-plug is a SysBus device,
so essentially APIC is hot-plugged to SysBus anyway which causes
abort in qdev_try_create() -> qdev_set_parent_bus(SysBus) ->
bus_add_child() [no hot-plug on SysBus], which is hard to fix
without allowing hot-plug on SysBus.
[AF: change the APIC to not be a SysBusDevice (like CPUState) then?]
- qdev_device_add() and qdev_try_create() assign SysBus by default to
device if no explicit bus was provided.
- qbus_find_recursive() in qdev_device_add() always returns SysBus
if last 2 args are NULL, which is case with busless CPU
- qdev_try_create() adds device SysBus if no bus was provided in
1st arg
- vapic is not working with hot-plug in current master
(do not know why yet)
Experimental CPU hot-add tree using device_add implemented only for
qemu64 CPU:
https://github.com/imammedo/qemu/tree/x86-cpu-hot-add.WIP
---
That completes Igor's and my notes, amended by status updates of mine.
Sorry for the delay, I did try to address some of the discussed issues
in form of patches in the meantime though. ;)
CC'ing s390x, ppc and arm KVM folks that were not on the call for
feedback on their requirements and thoughts.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
12 years, 1 month
[libvirt] [PATCH] snapshot: Mention disk-only snapshots in error message
by Peter Krempa
When a disk-only snapshot is requested the domain is treated as if it
was offline. This forbids to mix memory checkpoints with the DISK_ONLY
flag.
This patch improves the error message and mentions the restriction in
the virsh man page.
---
Note: this is techicaly a v2 as I've redone it after checking the code
again after Eric's review. Difference to the previous version:
- The check for --memspec and --disk-only together was removed from virsh
- Error message while parsing the xml was improved
src/conf/snapshot_conf.c | 3 ++-
tools/virsh.pod | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 0c5b005..fe77bd0 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -309,7 +309,8 @@ virDomainSnapshotDefParseString(const char *xmlStr,
if (offline && def->memory &&
def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("memory state cannot be saved with offline snapshot"));
+ _("memory state cannot be saved with offline or "
+ "disk-only snapshot"));
goto cleanup;
}
def->file = memoryFile;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index fd0f762..ec1772d 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2749,7 +2749,8 @@ The I<--memspec> option can be used to control whether a checkpoint
is internal or external. The I<--memspec> flag is mandatory, followed
by a B<memspec> of the form B<[file=]name[,snapshot=type]>, where
type can be B<none>, B<internal>, or B<external>. To include a literal
-comma in B<file=name>, escape it with a second comma.
+comma in B<file=name>, escape it with a second comma. I<--memspec> cannot
+be used together with I<--disk-only>.
The I<--diskspec> option can be used to control how I<--disk-only> and
external checkpoints create external files. This option can occur
--
1.8.1.1
12 years, 1 month
[libvirt] [PATCH] Sync HACKING and hacking.html.in
by Martin Kletzander
After one last-minute change, only HACKING was updated and not
hacking.html.in, so this patch synchronizes that.
---
Pushed under the 'trivial' rule.
---
docs/hacking.html.in | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 78620fc..fe4324a 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -241,7 +241,7 @@
<p>
The keywords <code>if</code>, <code>for</code>, <code>while</code>,
and <code>switch</code> must have a single space following them
- before the opening bracket. e.g.
+ before the opening bracket. E.g.
</p>
<pre>
if(foo) // Bad
@@ -250,7 +250,7 @@
<p>
Function implementations must <strong>not</strong> have any whitespace
- between the function name and the opening bracket. e.g.
+ between the function name and the opening bracket. E.g.
</p>
<pre>
int foo (int wizz) // Bad
@@ -259,7 +259,7 @@
<p>
Function calls must <strong>not</strong> have any whitespace
- between the function name and the opening bracket. e.g.
+ between the function name and the opening bracket. E.g.
</p>
<pre>
bar = foo (wizz); // Bad
@@ -269,7 +269,7 @@
<p>
Function typedefs must <strong>not</strong> have any whitespace
between the closing bracket of the function name and opening
- bracket of the arg list. e.g.
+ bracket of the arg list. E.g.
</p>
<pre>
typedef int (*foo) (int wizz); // Bad
@@ -278,7 +278,7 @@
<p>
There must not be any whitespace immediately following any
- opening bracket, or immediately prior to any closing bracket. e.g.
+ opening bracket, or immediately prior to any closing bracket. E.g.
</p>
<pre>
int foo( int wizz ); // Bad
--
1.8.1.2
12 years, 1 month
[libvirt] [PATCH v2 0/3] Miscellaneous cleanups
by Martin Kletzander
This is basically v2 of long lost [1] with fixes for what Osier found
out and with one one-liner patch added.
[1] http://www.redhat.com/archives/libvir-list/2012-December/msg01043.html
Martin Kletzander (3):
conf: Don't format cputune element when not needed
docs: aesthetical cleanups
Ignore '.trs' files
.gitignore | 1 +
HACKING | 10 +++++-----
docs/firewall.html.in | 2 +-
docs/formatdomain.html.in | 4 ++--
docs/hacking.html.in | 10 +++++-----
src/conf/domain_conf.c | 12 ++++++------
src/locking/lock_driver.h | 6 +++---
7 files changed, 23 insertions(+), 22 deletions(-)
--
1.8.1.2
12 years, 1 month
[libvirt] [PATCH] docs: fix missed documentation for virterror functions
by Claudio Bley
Commit f24404a3244b50c1d6d3a735445b02450ac40981 renamed virterror.c to
virerror.c but forgot to change the reference in this file.
Signed-off-by: Claudio Bley <cbley(a)av-test.de>
---
docs/apibuild.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/apibuild.py b/docs/apibuild.py
index 9a29c42..91dabf8 100755
--- a/docs/apibuild.py
+++ b/docs/apibuild.py
@@ -24,7 +24,7 @@ included_files = {
"libvirt.h": "header with general libvirt API definitions",
"virterror.h": "header with error specific API definitions",
"libvirt.c": "Main interfaces for the libvirt library",
- "virterror.c": "implements error handling and reporting code for libvirt",
+ "virerror.c": "implements error handling and reporting code for libvirt",
"virevent.c": "event loop for monitoring file handles",
"virtypedparam.c": "virTypedParameters APIs",
}
--
1.7.9.5
12 years, 1 month
[libvirt] [PATCH v2] LXC: create directory /dev/shm automatically
by Gao feng
Now we mount /dev as tmpfs and haven't created directory
/dev/shm,so the glibc api such as shm_open/sem_open will
create files under dir /dev.(since /dev is mounted as tmpfs)
Through these api still useable in container,but this cause
directory /dev looks a little chaos.
We already have filesystem type "ram",if user want to limit
the memory usage of /dev/shm,he should configure the XML,so
we only need to create directroy /dev/shm.If user configure
the filesystem type on /dev/shm, we will mount it on /dev/shm
then.
This patch create directory /dev/shm automatically,the files
created by shm_open/sem_open will stay in this directroy.
This patch also enlarge the size of directroy /dev to the
half of container's memory,64k may be too small to contain
POSIX shared memory and named semphore.
Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com>
---
src/lxc/lxc_container.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 497539c..eba3e68 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -504,7 +504,8 @@ err:
static int lxcContainerMountBasicFS(bool pivotRoot,
- char *sec_mount_options)
+ char *sec_mount_options,
+ virDomainDefPtr vmDef)
{
const struct {
const char *src;
@@ -567,12 +568,22 @@ static int lxcContainerMountBasicFS(bool pivotRoot,
if (pivotRoot) {
/*
- * tmpfs is limited to 64kb, since we only have device nodes in there
- * and don't want to DOS the entire OS RAM usage
+ * tmpfs is limited to half of container's memory,
+ * since we not only have device nodes in there,we
+ * may have POSIX shared memory object and named
+ * semphore in there too.
*/
- ignore_value(virAsprintf(&opts,
- "mode=755,size=65536%s", sec_mount_options));
+ if (vmDef->mem.max_balloon != VIR_DOMAIN_MEMORY_PARAM_UNLIMITED) {
+ unsigned long long size = 0;
+ size = vmDef->mem.max_balloon << 9;
+ ignore_value(virAsprintf(&opts,
+ "mode=755,size=%llu%s", size, sec_mount_options));
+ } else {
+ ignore_value(virAsprintf(&opts,
+ "mode=755,%s", sec_mount_options));
+ }
+
if (!opts) {
virReportOOMError();
goto cleanup;
@@ -586,6 +597,13 @@ static int lxcContainerMountBasicFS(bool pivotRoot,
"devfs", "/dev", "tmpfs", opts);
goto cleanup;
}
+
+ VIR_DEBUG("create directory /dev/shm for POSIX shared memory and named semphore");
+ if (virFileMakePath("/dev/shm") < 0) {
+ virReportSystemError(errno, "%s",
+ _("Failed to mkdir /dev/shm"));
+ goto cleanup;
+ }
}
rc = 0;
@@ -1947,7 +1965,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef,
goto cleanup;
/* Mounts the core /proc, /sys, etc filesystems */
- if (lxcContainerMountBasicFS(true, sec_mount_options) < 0)
+ if (lxcContainerMountBasicFS(true, sec_mount_options, vmDef) < 0)
goto cleanup;
/* Mounts /proc/meminfo etc sysinfo */
@@ -2055,7 +2073,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef,
goto cleanup;
/* Mounts the core /proc, /sys, etc filesystems */
- if (lxcContainerMountBasicFS(false, sec_mount_options) < 0)
+ if (lxcContainerMountBasicFS(false, sec_mount_options, vmDef) < 0)
goto cleanup;
/* Mounts /proc/meminfo etc sysinfo */
--
1.7.11.7
12 years, 1 month
[libvirt] [PATCH 0/9] Address Coverity false positives
by John Ferlan
As the Coverity work winds down, I've bundled together a series of
patches that only add a comment tag to the offending line of code
that Coverity believes the error to be on. Each submit has a commit
message indicating the reason for the tag.
John Ferlan (9):
xend: Address some Coverity false positives
lxc: Add coverity[dead_error_begin] tag in switch stmts
nodeinfo: Add coverity[dead_error_begin] and [returned_null] tags
xen: Add coverity[ptr_arith] and [sign_extension] tags
rpc: Add coverity[dead_error_begin] tag
qemu: Add coverity[negative_returns] tag
remote: Avoid coverity[leaked_storage] message.
network: Add coverity[leaked_handle] to ignore error
storage: Add coverity[dead_error_condition] to avoid error
src/lxc/lxc_driver.c | 3 +++
src/network/bridge_driver.c | 1 +
src/nodeinfo.c | 3 +++
src/qemu/qemu_hotplug.c | 1 +
src/remote/remote_driver.c | 1 +
src/rpc/virnetclient.c | 1 +
src/storage/parthelper.c | 2 ++
src/xen/xen_hypervisor.c | 5 ++++-
src/xen/xend_internal.c | 4 ++++
9 files changed, 20 insertions(+), 1 deletion(-)
--
1.7.11.7
12 years, 1 month
[libvirt] Add support for QEMU file descriptor sets
by Stefan Berger
Add two API calls to virCommand for generating the parameters necessary
for passing to the QEMU -add-fd option, for example "set=10,fd=20",
and the file descriptor set for the path= option, such as for example
"/dev/fdset/10"
Regards,
Stefan
---
src/libvirt_private.syms | 2 +
src/util/vircommand.c | 65 +++++++++++++++++++++++++++++++++++++++++++----
src/util/vircommand.h | 6 ++++
3 files changed, 68 insertions(+), 5 deletions(-)
Index: libvirt/src/libvirt_private.syms
===================================================================
--- libvirt.orig/src/libvirt_private.syms
+++ libvirt/src/libvirt_private.syms
@@ -165,6 +165,8 @@ virCommandSetPreExecHook;
virCommandSetWorkingDirectory;
virCommandToString;
virCommandTransferFD;
+virCommandTransferFDGetDevSet;
+virCommandTransferFDGetFDSet;
virCommandWait;
virCommandWriteArgLog;
virFork;
Index: libvirt/src/util/vircommand.c
===================================================================
--- libvirt.orig/src/util/vircommand.c
+++ libvirt/src/util/vircommand.c
@@ -109,7 +109,7 @@ struct _virCommand {
* Returns true if @set contains @fd,
* false otherwise.
*/
-static bool
+static int
virCommandFDIsSet(int fd,
const int *set,
int set_size)
@@ -118,9 +118,9 @@ virCommandFDIsSet(int fd,
while (i < set_size)
if (set[i++] == fd)
- return true;
+ return i--;
- return false;
+ return -1;
}
/*
@@ -145,7 +145,7 @@ virCommandFDSet(int fd,
if (fd < 0 || !set || !set_size)
return -1;
- if (virCommandFDIsSet(fd, *set, *set_size))
+ if (virCommandFDIsSet(fd, *set, *set_size) >= 0)
return 0;
if (VIR_REALLOC_N(*set, *set_size + 1) < 0) {
@@ -523,7 +523,7 @@ virExecWithHook(const char *const*argv,
for (i = 3; i < openmax; i++) {
if (i == infd || i == childout || i == childerr)
continue;
- if (!keepfd || !virCommandFDIsSet(i, keepfd, keepfd_size)) {
+ if (!keepfd || virCommandFDIsSet(i, keepfd, keepfd_size) < 0) {
tmpfd = i;
VIR_MASS_CLOSE(tmpfd);
} else if (virSetInherit(i, true) < 0) {
@@ -896,6 +896,61 @@ virCommandTransferFD(virCommandPtr cmd,
VIR_FORCE_CLOSE(fd);
}
+/**
+ * virCommandTransferFDGetFDSet:
+ * @cmd: the command to modify
+ * @fd: fd to reassign to the child
+ *
+ * Get the parameters for the the QEMU -add-fd command line option
+ * for the given file descriptor. The file descriptor must previously
+ * have been 'transferred' in a virCommandTransfer() call.
+ * This function for example returns "set=10,fd=20" for file descriptor 20.
+ */
+char *
+virCommandTransferFDGetFDSet(virCommandPtr cmd, int fd)
+{
+ char *result = NULL;
+ int idx = virCommandFDIsSet(fd, cmd->transfer, cmd->transfer_size);
+
+ if (idx >= 0) {
+ if (virAsprintf(&result, "set=%d,fd=%d", idx, fd) < 0) {
+ virReportOOMError();
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("file descriptor %d has not been transferred"), fd);
+ }
+
+ return result;
+}
+
+/**
+ * virCommandTransferFDGetDevSet:
+ * @cmd: the command to modify
+ * @fd: fd to reassign to the child
+ *
+ * Get the parameters for the the QEMU path= parameter where a file
+ * descriptor is accessed via a file descriptor set, for example
+ * /dev/fdset/10. The file descriptor must previously have been
+ * 'transferred' in a virCommandTransfer() call.
+ */
+char *
+virCommandTransferFDGetDevSet(virCommandPtr cmd, int fd)
+{
+ char *result = NULL;
+ int idx = virCommandFDIsSet(fd, cmd->transfer, cmd->transfer_size);
+
+ if (idx >= 0) {
+ if (virAsprintf(&result, "/dev/fdset/%d", idx) < 0) {
+ virReportOOMError();
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("file descriptor %d has not been transferred"), fd);
+ }
+ return result;
+}
+
/**
* virCommandSetPidFile:
Index: libvirt/src/util/vircommand.h
===================================================================
--- libvirt.orig/src/util/vircommand.h
+++ libvirt/src/util/vircommand.h
@@ -58,6 +58,12 @@ void virCommandPreserveFD(virCommandPtr
void virCommandTransferFD(virCommandPtr cmd,
int fd);
+char *virCommandTransferFDGetFDSet(virCommandPtr cmd,
+ int fd);
+
+char *virCommandTransferFDGetDevSet(virCommandPtr cmd,
+ int fd);
+
void virCommandSetPidFile(virCommandPtr cmd,
const char *pidfile) ATTRIBUTE_NONNULL(2);
12 years, 1 month
[libvirt] [PATCH v2 0/7] Introduce virCommandDoAsyncIO
by Michal Privoznik
Currently, if we want to use IO with asynchronous command we have
to copy code from virFileWrapperFd to misuse our event loop for
reading and writing to the command. However, we can extend our
virCommand implementation to automatically set things up.
All patches but the first has been ACKed already.
diff to v1:
-drop usleep(100) while waiting for the event loop to process our string IOs
and do it ourselves instead.
Michal Privoznik (7):
virCommand: Introduce virCommandDoAsyncIO
Introduce event loop to commandtest
tests: Create test for virCommandDoAsyncIO
virFileWrapperFd: Switch to new virCommandDoAsyncIO
qemu: Catch stderr of image decompression binary
qemu: Catch stderr of image compression binary
qemu: Destroy domain on decompression binary error
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 9 +-
src/qemu/qemu_migration.c | 9 +-
src/util/vircommand.c | 279 ++++++++++++++++++++++++++++++++++++++++++--
src/util/vircommand.h | 1 +
src/util/virfile.c | 82 +------------
tests/commanddata/test3.log | 2 +-
tests/commandtest.c | 136 +++++++++++++++++++++
8 files changed, 430 insertions(+), 89 deletions(-)
--
1.8.0.2
12 years, 1 month
