Re: [libvirt] [PATCH v13] support offline migration
by li guang
在 2012-11-09五的 08:44 +0800,li guang写道:
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 978af57..6c2bf98 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -9796,7 +9796,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
> > asyncJob = QEMU_ASYNC_JOB_NONE;
> > }
> >
> > - if (!virDomainObjIsActive(vm)) {
> > + if (!virDomainObjIsActive(vm) && !(flags & VIR_MIGRATE_OFFLINE))
> {
> > virReportError(VIR_ERR_OPERATION_INVALID,
> > "%s", _("domain is not running"));
> > goto endjob;
> > @@ -9805,9 +9805,9 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
> > /* Check if there is any ejected media.
> > * We don't want to require them on the destination.
> > */
> > -
> > - if (qemuDomainCheckEjectableMedia(driver, vm, asyncJob) < 0)
> > - goto endjob;
> > + if (virDomainObjIsActive(vm))
> > + if (qemuDomainCheckEjectableMedia(driver, vm, asyncJob) < 0)
> > + goto endjob;
> >
> > if (!(xml = qemuMigrationBegin(driver, vm, xmlin, dname,
> > cookieout, cookieoutlen,
>
> What if you want to offline migrate a domain that is currently running?
> I think it's better to first move qemuDomainCheckEjectableMedia down to
> qemuMigrationBegin in a separate patch and then just use the code from
> v12 (without using the offline label).
sorry, I don't like to do offline migrate for a domain that's running.
>
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 5f8a9c5..66fbc02 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -72,6 +72,7 @@ enum qemuMigrationCookieFlags {
> > QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
> > QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
> > QEMU_MIGRATION_COOKIE_FLAG_NETWORK,
> > + QEMU_MIGRATION_COOKIE_FLAG_OFFLINE,
> >
> > QEMU_MIGRATION_COOKIE_FLAG_LAST
> > };
> > @@ -79,13 +80,14 @@ enum qemuMigrationCookieFlags {
> > VIR_ENUM_DECL(qemuMigrationCookieFlag);
> > VIR_ENUM_IMPL(qemuMigrationCookieFlag,
> > QEMU_MIGRATION_COOKIE_FLAG_LAST,
> > - "graphics", "lockstate", "persistent", "network");
> > + "graphics", "lockstate", "persistent", "network",
> "offline");
> >
> > enum qemuMigrationCookieFeatures {
> > QEMU_MIGRATION_COOKIE_GRAPHICS = (1 <<
> QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
> > QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 <<
> QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE),
> > QEMU_MIGRATION_COOKIE_PERSISTENT = (1 <<
> QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT),
> > QEMU_MIGRATION_COOKIE_NETWORK = (1 <<
> QEMU_MIGRATION_COOKIE_FLAG_NETWORK),
> > + QEMU_MIGRATION_COOKIE_OFFLINE = (1 <<
> QEMU_MIGRATION_COOKIE_FLAG_OFFLINE),
> > };
> >
> > typedef struct _qemuMigrationCookieGraphics
> qemuMigrationCookieGraphics;
> > @@ -594,6 +596,9 @@ qemuMigrationCookieXMLFormat(struct qemud_driver
> *driver,
> > if ((mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) && mig->network)
> > qemuMigrationCookieNetworkXMLFormat(buf, mig->network);
> >
> > + if (mig->flags & QEMU_MIGRATION_COOKIE_OFFLINE)
> > + virBufferAsprintf(buf, " <offline/>\n");
> > +
> > virBufferAddLit(buf, "</qemu-migration>\n");
> > return 0;
> > }
> > @@ -874,6 +879,11 @@
> qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
> > (!(mig->network = qemuMigrationCookieNetworkXMLParse(ctxt))))
> > goto error;
> >
> > + if ((flags & QEMU_MIGRATION_COOKIE_OFFLINE)) {
> > + if (virXPathBoolean("count(./offline) > 0", ctxt))
> > + mig->flags |= QEMU_MIGRATION_COOKIE_OFFLINE;
> > + }
> > +
> > return 0;
> >
> > error:
> > @@ -938,6 +948,10 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr
> mig,
> > return -1;
> > }
> >
> > + if (flags & QEMU_MIGRATION_COOKIE_OFFLINE) {
> > + mig->flags |= QEMU_MIGRATION_COOKIE_OFFLINE;
> > + }
> > +
> > if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
> > return -1;
> >
>
> Oh, I'm sorry for not noticing this earlier, but why exactly do we need
> this <offline/> element in migration cookie? It doesn't look like we need to
> store some additional data required for offline migration. I think just
> passing the VIR_MIGRATE_OFFLINE flag will be better. Not to mention that if an older
> libvirt gets a cookie with <offline/> element, it will just ignore it
> while passing a flag (which the code should already been doing anyway) should
> make it fail for unsupported flag.
without this how do you know you a offline migration at target side?
>
> > @@ -1443,6 +1457,24 @@ char *qemuMigrationBegin(struct qemud_driver
> *driver,
> > QEMU_MIGRATION_COOKIE_LOCKSTATE) < 0)
> > goto cleanup;
> >
> > + if (flags & VIR_MIGRATE_OFFLINE) {
> > + if (flags & (VIR_MIGRATE_NON_SHARED_DISK|
> > + VIR_MIGRATE_NON_SHARED_INC)) {
> > + virReportError(VIR_ERR_OPERATION_INVALID,
> > + "%s", _("offline migration cannot handle
> non-shared storage"));
> > + goto cleanup;
> > + }
> > + if (!(flags & VIR_MIGRATE_PERSIST_DEST)) {
> > + virReportError(VIR_ERR_OPERATION_INVALID,
> > + "%s", _("offline migration must be
> specified with the persistent flag set"));
> > + goto cleanup;
> > + }
> > + if (qemuMigrationBakeCookie(mig, driver, vm,
> > + cookieout, cookieoutlen,
> > + QEMU_MIGRATION_COOKIE_OFFLINE) <
> 0)
> > + goto cleanup;
> > + }
> > +
> > if (xmlin) {
> > if (!(def = virDomainDefParseString(driver->caps, xmlin,
> > QEMU_EXPECTED_VIRT_TYPES,
>
> Good, just wrap the two long lines and create a separate patch (which
> this one
> will depend on) to move qemuDomainCheckEjectableMedia here as well.
>
> > @@ -1607,6 +1639,15 @@ qemuMigrationPrepareAny(struct qemud_driver
> *driver,
> > goto endjob;
> > }
> >
> > + if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein,
> cookieinlen,
> > +
> QEMU_MIGRATION_COOKIE_OFFLINE)))
> > + return ret;
>
> AS I wrote in my last review, this needs to call goto endjob rather than
> returning directly.
right, will fix.
>
> > +
> > + if (mig->flags & QEMU_MIGRATION_COOKIE_OFFLINE) {
> > + ret = 0;
> > + goto done;
> > + }
> > +
> > /* Start the QEMU daemon, with the same command-line arguments
> plus
> > * -incoming $migrateFrom
> > */
> ...
> > @@ -2150,6 +2192,9 @@ qemuMigrationRun(struct qemud_driver *driver,
> > return -1;
> > }
> >
> > + if (flags & VIR_MIGRATE_OFFLINE)
> > + return 0;
> > +
> > if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein,
> cookieinlen,
> >
> QEMU_MIGRATION_COOKIE_GRAPHICS)))
> > goto cleanup;
>
> I still think we should not even get into qemuMigrationRun when doing
> offline migration.
No, it will get into here for I did not touch migrationPerform path,
initially I don't want to change any code at libvirt.c so I must keep
offline migration walk through whole path like normal, or there're maybe
lots of fixes, any will break up the migration path.
>
> > @@ -2665,7 +2710,12 @@ static int doPeer2PeerMigrate3(struct
> qemud_driver *driver,
> > uri, &uri_out, flags, dname, resource, dom_xml);
> > qemuDomainObjExitRemoteWithDriver(driver, vm);
> > }
> > +
> > VIR_FREE(dom_xml);
> > +
> > + if (flags & VIR_MIGRATE_OFFLINE)
> > + goto cleanup;
> > +
> > if (ret == -1)
> > goto cleanup;
> >
>
> Quoting from my previous review:
>
> This will skip not only Perform but also Finish phase in
> peer-to-peer migration. You want to jump to finish label and do that *after* the
> check for ret == 1.
OK, I will jump to finish label.
>
> > @@ -2771,7 +2821,7 @@ finish:
> > vm->def->name);
> >
> > cleanup:
> > - if (ddomain) {
> > + if (ddomain || (flags & VIR_MIGRATE_OFFLINE)) {
> > virObjectUnref(ddomain);
> > ret = 0;
> > } else {
>
> I think this should not be changed at all.
>
or you'll get an error for ret = -1.
> ...
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 393b67b..54ba63a 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -6644,6 +6644,7 @@ static const vshCmdInfo info_migrate[] = {
> >
> > static const vshCmdOptDef opts_migrate[] = {
> > {"live", VSH_OT_BOOL, 0, N_("live migration")},
> > + {"offline", VSH_OT_BOOL, 0, N_("offline (domain's inactive)
> migration")},
> > {"p2p", VSH_OT_BOOL, 0, N_("peer-2-peer migration")},
> > {"direct", VSH_OT_BOOL, 0, N_("direct migration")},
> > {"tunneled", VSH_OT_ALIAS, 0, "tunnelled"},
> > @@ -6729,6 +6730,15 @@ doMigrate(void *opaque)
> > if (vshCommandOptBool(cmd, "unsafe"))
> > flags |= VIR_MIGRATE_UNSAFE;
> >
> > + if (vshCommandOptBool(cmd, "offline")) {
> > + flags |= VIR_MIGRATE_OFFLINE;
> > + }
> > +
> > + if (virDomainIsActive(dom) && (flags & VIR_MIGRATE_OFFLINE)) {
> > + vshError(ctl, "%s", _("domain is active, offline migration
> for inactive domain only"));
> > + goto out;
> > + }
> > +
>
> Another thing I didn't notice last time :-( Is there any reason why
> offline migrating a running domain should be forbidden? But even if there was a
> reason, this check doesn't belong to virsh.
like I said before, running domain is active, so it's not offline,
if you want offline process to do job for an active domain, I think
an explicit flag is required.
> And --ofline flag for virsh migrate should also be documented in virsh
> man page.
>
got it, I'll add this to virsh man page.
> Jirka
>
>
--
li guang lig.fnst(a)cn.fujitsu.com
linux kernel team at FNST, china
12 years
[libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'
by Eric Blake
https://bugzilla.redhat.com/show_bug.cgi?id=873344 suggested that
the grouping 'boot', 'shutdown', 'reboot'; as well as the grouping
'start', 'stop', 'restart'; might be easier to remember than the
current mix of 'start', 'shutdown', 'reboot'.
* tools/virsh-domain.c (domManagementCmds): Add other command names.
* tools/virsh.pod (start, shutdown, reboot): Document the aliases.
---
This patch documents both spellings. An alternative would be to
leave the alternate spellings as hidden aliases (virsh has support
for that), but still mention them in virsh.pod (see how we did an
alias for nodedev-dettach, for reference).
tools/virsh-domain.c | 3 +++
tools/virsh.pod | 4 ++++
2 files changed, 7 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 393b67b..86ed4d3 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8298,6 +8298,7 @@ const vshCmdDef domManagementCmds[] = {
{"blockjob", cmdBlockJob, opts_block_job, info_block_job, 0},
{"blockpull", cmdBlockPull, opts_block_pull, info_block_pull, 0},
{"blockresize", cmdBlockResize, opts_block_resize, info_block_resize, 0},
+ {"boot", cmdStart, opts_start, info_start, 0},
{"change-media", cmdChangeMedia, opts_change_media, info_change_media, 0},
#ifndef WIN32
{"console", cmdConsole, opts_console, info_console, 0},
@@ -8351,6 +8352,7 @@ const vshCmdDef domManagementCmds[] = {
{"numatune", cmdNumatune, opts_numatune, info_numatune, 0},
{"reboot", cmdReboot, opts_reboot, info_reboot, 0},
{"reset", cmdReset, opts_reset, info_reset, 0},
+ {"restart", cmdReboot, opts_reboot, info_reboot, 0},
{"restore", cmdRestore, opts_restore, info_restore, 0},
{"resume", cmdResume, opts_resume, info_resume, 0},
{"save", cmdSave, opts_save, info_save, 0},
@@ -8367,6 +8369,7 @@ const vshCmdDef domManagementCmds[] = {
{"setvcpus", cmdSetvcpus, opts_setvcpus, info_setvcpus, 0},
{"shutdown", cmdShutdown, opts_shutdown, info_shutdown, 0},
{"start", cmdStart, opts_start, info_start, 0},
+ {"stop", cmdShutdown, opts_shutdown, info_shutdown, 0},
{"suspend", cmdSuspend, opts_suspend, info_suspend, 0},
{"ttyconsole", cmdTTYConsole, opts_ttyconsole, info_ttyconsole, 0},
{"undefine", cmdUndefine, opts_undefine, info_undefine, 0},
diff --git a/tools/virsh.pod b/tools/virsh.pod
index e0c6b42..7a3835b 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1124,6 +1124,7 @@ If I<--config> is specified, affect the next boot of a persistent guest.
If I<--current> is specified, affect the current guest state.
=item B<reboot> I<domain> [I<--mode acpi|agent>]
+=item B<restart> I<domain> [I<--mode acpi|agent>]
Reboot a domain. This acts just as if the domain had the B<reboot>
command run from the console. The command returns as soon as it has
@@ -1523,6 +1524,7 @@ be hot-plugged the next time the domain is booted. As such, it must only be
used with the I<--config> flag, and not with the I<--live> flag.
=item B<shutdown> I<domain> [I<--mode acpi|agent>]
+=item B<stop> I<domain> [I<--mode acpi|agent>]
Gracefully shuts down a domain. This coordinates with the domain OS
to perform graceful shutdown, so there is no guarantee that it will
@@ -1543,6 +1545,8 @@ can specify C<acpi> or C<agent>.
=item B<start> I<domain-name-or-uuid> [I<--console>] [I<--paused>]
[I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>]
+=item B<boot> I<domain-name-or-uuid> [I<--console>] [I<--paused>]
+[I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>]
Start a (previously defined) inactive domain, either from the last
B<managedsave> state, or via a fresh boot if no managedsave state is
--
1.7.11.7
12 years
[libvirt] [PATCH] qemu: allow larger discrepency between memory & currentMemory in domain xml
by Laine Stump
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=873134
The reported problem is that an attempt to restore a saved domain that
was configured with <currentMemory> and <memory> set to some (same for
both) number that's not a multiple of 4096KiB results in an error like
this:
error: Failed to start domain libvirt_test_api
error: XML error: current memory '4001792k' exceeds maximum '4000768k'
(in this case, currentMemory was set to 4000000KiB).
The reason for this failure is:
1) a saved image contains the "live xml" of the domain at the time of
the save.
2) the live xml of a running domain gets its currentMemory
(a.k.a. cur_balloon) directly from the qemu monitor rather than from
the configuration of the domain.
3) the value reported by qemu is (sometimes) not exactly what was
originally given to qemu when the domain was started, but is rounded
up to [some indeterminate granularity] - in some versions of qemu that
granularity is apparently 1MiB, and in others it is 4MiB.
4) When the XML is parsed to setup the state of the restored domain,
the XML parser for <currentMemory> compares it to <memory> (which is
the maximum allowed memory size for the domain) and if <currentMemory>
is greater than the next 1024KiB boundary above <memory>, it spits out
an error and fails.
For example (from the BZ) if you start qemu on RHEL6 with both
<currentMemory> and <memory> of 4000000 (this number is in KiB),
libvirt's dominfo or dumpxml will report "4001792" back (rounded up to
next 4MiB) for 10-20 seconds after the start, then revert to reporting
"4000000". On Fedora 16 (which uses qemu-1.0), it will instead report
"4000768" (rounded up to next 1MiB). On Fedora 17 (qemu-1.2), it seems
to always report "4000000". ("4000000" is of course okay, and
"4000768" is also okay since that's the next 1024KiB boundary above
"4000000" and the parser was already allowing for that. But "4001792
is *not* okay and produces the error message.)
This patch solves the problem by changing the allowed "fudge factor"
when parsing from 1024KiB to 4096KiB to match the maximum up-rounding
that could be done in qemu.
(I had earlier thought to fix this by up-rounding <memory> in the
dumpxml that's put into the saved image, but that wouldn't have fixed
the case where the save image was produced by an "unfixed"
libvirtd.)
---
src/conf/domain_conf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9f2029b..2dd4490 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8644,10 +8644,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
if (def->mem.cur_balloon > def->mem.max_balloon) {
/* Older libvirt could get into this situation due to
- * rounding; if the discrepancy is less than 1MiB, we silently
+ * rounding; if the discrepancy is less than 4MiB, we silently
* round down, otherwise we flag the issue. */
- if (VIR_DIV_UP(def->mem.cur_balloon, 1024) >
- VIR_DIV_UP(def->mem.max_balloon, 1024)) {
+ if (VIR_DIV_UP(def->mem.cur_balloon, 4096) >
+ VIR_DIV_UP(def->mem.max_balloon, 4096)) {
virReportError(VIR_ERR_XML_ERROR,
_("current memory '%lluk' exceeds "
"maximum '%lluk'"),
--
1.7.11.7
12 years
[libvirt] [PATCH] add ppc64 and s390x to arches where qemu-kvm exists
by Dan Horák
QEMU in Fedora >= 18 is configured with ppc64 and s390x as architectures
where KVM is enabled.
---
libvirt.spec.in | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 9aa2fb2..35c103b 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -53,7 +53,13 @@
%define with_qemu_tcg %{with_qemu}
# Change if we ever provide qemu-kvm binaries on non-x86 hosts
-%ifarch %{ix86} x86_64
+%if 0%{?fedora} >= 18
+%define qemu_kvm_arches %{ix86} x86_64 ppc64 s390x
+%else
+%define qemu_kvm_arches %{ix86} x86_64
+%endif
+
+%ifarch %{qemu_kvm_arches}
%define with_qemu_kvm %{with_qemu}
%else
%define with_qemu_kvm 0
--
1.7.7.6
12 years
[libvirt] [ANNOUNCE] libvirt-glib 0.1.4 release
by Daniel P. Berrange
I am pleased to announce that a new release of the libvirt-glib package,
version 0.1.4, is now available from
ftp://libvirt.org/libvirt/glib/
The packages are GPG signed with
Key fingerprint: DAF3 A6FD B26B 6291 2D0E 8E3F BE86 EBB4 1510 4FDF (4096R)
New in this release:
- Add support for configuring power management config
- Add API binding for updating device config
- Avoid SEGV when querying storage volune with NULL name
- Avoid reference count leak when constructing wrapped libvirt objects
- Avoid calling g_type_init for newer glib
- Add flags for domain reboot API
- Fix leak with GMutex compatibility wrappers
- Don't use storage volumes before the pool refresh finishes
- Add binding for destroying storage pools
- Add binding for defining storage pools
libvirt-glib comprises three distinct libraries:
- libvirt-glib - Integrate with the GLib event loop and error handling
- libvirt-gconfig - Representation of libvirt XML documents as GObjects
- libvirt-gobject - Mapping of libvirt APIs into the GObject type system
NB: While libvirt aims to be API/ABI stable forever, with libvirt-glib
we are not yet guaranteeing that libvirt-glib libraries are API/ABI
permanently stable. As of the 0.0.8 release, we have tentatively frozen
the API/ABI with the intent of being longterm stable hereafter, but
there is still a small chance we might find flaws requiring an API/ABI
change. The likelihood of this is low, however, and we will strive to
avoid it.
Follow up comments about libvirt-glib should be directed to the regular
libvir-list(a)redhat.com development list.
Thanks to all the people involved in contributing to this release.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12 years
[libvirt] [PATCHv3 0/4] Use nodeGetCPUCount/virNodeGetCPUMap where appropriate
by Viktor Mihajlovski
This series concludes the introduction of the virNodeGetCPUMap API
by replacing the usage of virNodeGetInfo for computing the maximum number
of node CPUs with either nodeGetCPUCount or virNodeGetCPUMap.
It's basically a resend of the original 3-patch series plus the
nodeGetCPUCount fix. Event in the light of Peter Krempa's recent
CPU topology fix this series is still relevant.
1/4 is needed to support older kernels
2/4 is not strictly required but is more efficient
3/4 and 4/4 are required for correct function of vcpu pinning calls
of python and virsh clients when talking to libvirtd 1.0.0.
V3 Changes:
- pulled in the nodeGetCPUCount patch for older kernels
- changed commit message of 2/4
V2 Changes:
Rework based on Eric's feedback:
- Use nodeGetCPUCount instead of nodeGetCPUMap
- Avoid code bloat by computing node CPU count in a helper function
Viktor Mihajlovski (4):
nodeinfo: enable nodeGetCPUCount for older kernels
qemu, lxc: Change host CPU number detection logic.
python: Use virNodeGetCPUMap where possible
virsh: Use virNodeGetCPUMap if possible
python/libvirt-override.c | 87 ++++++++++++++++++++++++++++-------------------
src/lxc/lxc_controller.c | 8 ++---
src/nodeinfo.c | 33 +++++++++++++++---
src/qemu/qemu_driver.c | 14 +++-----
src/qemu/qemu_process.c | 8 ++---
tools/virsh-domain.c | 32 ++++++++++++-----
6 files changed, 114 insertions(+), 68 deletions(-)
--
1.7.12.4
12 years
[libvirt] [PATCH] snapshot: require user to supply external memory file name
by Eric Blake
For disk snapshots, the user could request an external snapshot
but not supply a filename; later on, we would check this condition
and generate a suitable name if possible, or gracefully error out
when not possible (such as when the original file was a block
device). But unless we come up with a suitable way to generate
external memory file names, we have no later code point that was
checking for NULL, so we should forbid this up front.
* src/conf/snapshot_conf.c (virDomainSnapshotDefParseString):
Avoid NULL deref, since we don't generate names yet.
---
src/conf/snapshot_conf.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 1ee4017..72bdd30 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -293,6 +293,12 @@ virDomainSnapshotDefParseString(const char *xmlStr,
memoryFile);
goto cleanup;
}
+ if (!memoryFile &&
+ def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("external memory snapshots require a filename"));
+ goto cleanup;
+ }
} else if (memoryFile) {
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
} else if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
--
1.7.11.7
12 years
[libvirt] [PATCH] qemu: add bootindex for usb-host and usb-redir devices
by Ján Tomko
Allow bootindex to be specified for redirected USB devices and host USB
devices.
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=805414
---
Bootindex for these devices is supported since QEMU 1.1.0 (or commit 65bb3a5c)
but the actual booting from usb-host devices is broken since a844ed84
(before 1.2.0).
I haven't tested it with usb-redir yet.
---
docs/schemas/domaincommon.rng | 3 +++
src/conf/domain_conf.h | 4 ++--
src/qemu/qemu_capabilities.c | 10 ++++++++++
src/qemu/qemu_capabilities.h | 2 ++
src/qemu/qemu_command.c | 37 ++++++++++++++++++++++++++++++-------
5 files changed, 47 insertions(+), 9 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 2beb035..02ad477 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2779,6 +2779,9 @@
<optional>
<ref name="address"/>
</optional>
+ <optional>
+ <ref name="deviceBoot"/>
+ </optional>
</element>
</define>
<define name="redirfilter">
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6539281..091879e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -279,8 +279,8 @@ struct _virDomainDeviceInfo {
* devices. */
int rombar; /* enum virDomainPciRombarMode */
char *romfile;
- /* bootIndex is only user for disk, network interface, and
- * hostdev devices. */
+ /* bootIndex is only used for disk, network interface, hostdev
+ * and redirdev devices */
int bootIndex;
};
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5ce93f2..6ce2638 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -191,6 +191,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"vnc",
"drive-mirror", /* 115 */
+ "usb-redir.bootindex",
+ "usb-host.bootindex",
);
struct _qemuCaps {
@@ -1325,6 +1327,11 @@ static struct qemuCapsStringFlags qemuCapsObjectPropsPixx4PM[] = {
static struct qemuCapsStringFlags qemuCapsObjectPropsUsbRedir[] = {
{ "filter", QEMU_CAPS_USB_REDIR_FILTER },
+ { "bootindex", QEMU_CAPS_USB_REDIR_BOOTINDEX },
+};
+
+static struct qemuCapsStringFlags qemuCapsObjectPropsUsbHost[] = {
+ { "bootindex", QEMU_CAPS_USB_HOST_BOOTINDEX },
};
struct qemuCapsObjectTypeProps {
@@ -1350,6 +1357,8 @@ static struct qemuCapsObjectTypeProps qemuCapsObjectProps[] = {
ARRAY_CARDINALITY(qemuCapsObjectPropsPixx4PM) },
{ "usb-redir", qemuCapsObjectPropsUsbRedir,
ARRAY_CARDINALITY(qemuCapsObjectPropsUsbRedir) },
+ { "usb-host", qemuCapsObjectPropsUsbHost,
+ ARRAY_CARDINALITY(qemuCapsObjectPropsUsbHost) },
};
@@ -1545,6 +1554,7 @@ qemuCapsExtractDeviceStr(const char *qemu,
"-device", "PIIX4_PM,?",
"-device", "usb-redir,?",
"-device", "ide-drive,?",
+ "-device", "usb-host,?",
NULL);
/* qemu -help goes to stdout, but qemu -device ? goes to stderr. */
virCommandSetErrorBuffer(cmd, &output);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index fb88aa1..751b3ec 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -153,6 +153,8 @@ enum qemuCapsFlags {
QEMU_CAPS_BLOCK_COMMIT = 113, /* block-commit */
QEMU_CAPS_VNC = 114, /* Is -vnc available? */
QEMU_CAPS_DRIVE_MIRROR = 115, /* drive-mirror monitor command */
+ QEMU_CAPS_USB_REDIR_BOOTINDEX = 116, /* usb-redir.bootindex */
+ QEMU_CAPS_USB_HOST_BOOTINDEX = 117, /* usb-host.bootindex */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 440fd62..93079f3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3504,6 +3504,16 @@ qemuBuildRedirdevDevStr(virDomainDefPtr def,
}
}
+ if (dev->info.bootIndex) {
+ if (!qemuCapsGet(caps, QEMU_CAPS_USB_REDIR_BOOTINDEX)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("USB redirection booting is not "
+ "supported by this version of QEMU"));
+ goto error;
+ }
+ virBufferAsprintf(&buf, ",bootindex=%d", dev->info.bootIndex);
+ }
+
if (qemuBuildDeviceAddressStr(&buf, &dev->info, caps) < 0)
goto error;
@@ -3540,6 +3550,8 @@ qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev,
dev->source.subsys.u.usb.device);
}
virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
+ if (dev->info->bootIndex)
+ virBufferAsprintf(&buf, ",bootindex=%d", dev->info->bootIndex);
if (qemuBuildDeviceAddressStr(&buf, dev->info, caps) < 0)
goto error;
@@ -6439,16 +6451,27 @@ qemuBuildCommandLine(virConnectPtr conn,
if (hostdev->info->bootIndex) {
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
- hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+ (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI &&
+ hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("booting from assigned devices is only"
- " supported for PCI devices"));
- goto error;
- } else if (!qemuCapsGet(caps, QEMU_CAPS_PCI_BOOTINDEX)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("booting from assigned PCI devices is not"
- " supported with this version of qemu"));
+ " supported for PCI and USB devices"));
goto error;
+ } else {
+ if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI &&
+ !qemuCapsGet(caps, QEMU_CAPS_PCI_BOOTINDEX)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("booting from assigned PCI devices is not"
+ " supported with this version of qemu"));
+ goto error;
+ }
+ if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB &&
+ !qemuCapsGet(caps, QEMU_CAPS_USB_HOST_BOOTINDEX)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("booting from assigned USB devices is not"
+ " supported with this version of qemu"));
+ goto error;
+ }
}
}
--
1.7.8.6
12 years