date:20141004

Re: [libvirt] [PATCH v3 3/3] qemu: Build command line for ivshmem device

2014-10-04 Thread Martin Kletzander


On Fri, Oct 03, 2014 at 03:36:19PM -0600, Eric Blake wrote:

On 09/26/2014 04:43 AM, Martin Kletzander wrote:

This patch implements support for the ivshmem device in QEMU.

Signed-off-by: Maxime Leroy maxime.le...@6wind.com
Signed-off-by: Martin Kletzander mklet...@redhat.com
---



+virBufferAddLit(buf, ivshmem);
+if (shmem-size) {
+/*
+ * Thanks to our parsing code, we have a guarantee that the
+ * size is power of two and is at least a mebibyte in size.
+ * But because it may change inthe future, the checks are


s/inthe/in the/


+ * doubled in here.
+ */
+if (shmem-size  (shmem-size - 1)) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(shmem size must be a power of two));
+goto error;
+}
+if (shmem-size  1024 * 1024) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(shmem size must be at least 1 MiB));
+goto error;
+}
+virBufferAsprintf(buf, ,size=%llum,
+  VIR_DIV_UP(shmem-size, 1024 * 1024));


Similar comment as before; since you already validated sizing and
minimum value, you could use simpler 20 instead of making me guess
whether rounding up is occurring.



@@ -9897,7 +10014,6 @@ qemuBuildChrDeviceStr(char **deviceStr,
 return ret;
 }

-
 /*


Spurious line deletion; you might as well restore the two lines between
functions when doing the other touchups I've pointed out.



This wasn't pushed, I fixed it thanks to Levente's comment.  And in
this file the spaces are unequal, sometimes there are two lines,
sometimes one, so this would be a bigger follow-up unrelated to this
series.

All the other things are in a commit I'll push in a while.  Thanks for
the check on these.

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] minor shmem clean-ups

2014-10-04 Thread Martin Kletzander

Signed-off-by: Martin Kletzander mklet...@redhat.com
---

Notes:
Pushed as trivial.

This was pointed out by Eric in the shmem series after I've pushed it.

 docs/formatdomain.html.in | 2 +-
 src/conf/domain_conf.c| 5 ++---
 src/qemu/qemu_command.c   | 5 ++---
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8e50b8c..45b0f61 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5587,7 +5587,7 @@ qemu-kvm -net nic,model=? /dev/null
 p
   A shared memory device allows to share a memory region between
   different virtual machines and the host.
-  span class=sinceSince 1.2.9, QEMU and KVM only/span
+  span class=sinceSince 1.2.10, QEMU and KVM only/span
 /p

 pre
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c07ed6..a9c6f05 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17603,7 +17603,7 @@ virDomainShmemDefFormat(virBufferPtr buf,
 virDomainShmemDefPtr def,
 unsigned int flags)
 {
-virBufferAsprintf(buf, shmem name='%s', def-name);
+virBufferEscapeString(buf, shmem name='%s', def-name);

 if (!def-size 
 !def-server.enabled 
@@ -17618,8 +17618,7 @@ virDomainShmemDefFormat(virBufferPtr buf,
 virBufferAdjustIndent(buf, 2);

 if (def-size)
-virBufferAsprintf(buf, size unit='M'%llu/size\n,
-  VIR_DIV_UP(def-size, 1024 * 1024));
+virBufferAsprintf(buf, size unit='M'%llu/size\n, def-size  
20);

 if (def-server.enabled) {
 virBufferAddLit(buf, server);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6c22630..8cb0865 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7549,7 +7549,7 @@ qemuBuildShmemDevCmd(virCommandPtr cmd,
 /*
  * Thanks to our parsing code, we have a guarantee that the
  * size is power of two and is at least a mebibyte in size.
- * But because it may change inthe future, the checks are
+ * But because it may change in the future, the checks are
  * doubled in here.
  */
 if (shmem-size  (shmem-size - 1)) {
@@ -7562,8 +7562,7 @@ qemuBuildShmemDevCmd(virCommandPtr cmd,
_(shmem size must be at least 1 MiB));
 goto error;
 }
-virBufferAsprintf(buf, ,size=%llum,
-  VIR_DIV_UP(shmem-size, 1024 * 1024));
+virBufferAsprintf(buf, ,size=%llum, shmem-size  20);
 }

 if (!shmem-server.enabled) {
-- 
2.1.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm

2014-10-04 Thread Alex Bligh

Michael,

I'm about to post a nice simple v5 of this, but there are a couple of
your comments I am NOT addressing:

 
 diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
 index b72b34e..3c9da23 100644
 --- a/hw/acpi/piix4.c
 +++ b/hw/acpi/piix4.c
 @@ -200,12 +200,26 @@ static const VMStateDescription vmstate_pci_status = {
 }
 };
 
 +static const VMStateDescription vmstate_acpi_compat;
 +
 
 don't forward declare things, put them right here.

This is not addressable in v5; in v5 this is a forward declaration of
vmstate_acpi (not _compat). vmstate_acpi references acpi_load_old,
and acpi_load_old references vmstate_acpi, so we need a forward
reference for one of them in any case.

 +qemu_opt_get_bool(qemu_get_machine_opts(), qemu-kvm-migration,
 +  DEFAULT_QEMU_KVM_MIGRATION)) {
 +return vmstate_load_state(f, vmstate_acpi_compat,
 +  opaque, version_id);
 +}
 
 else if version_id == 2 return -EINVAL?

This is incorrect I think. A version_id of 2 with qemu-kvm_migration
off is permissible, and indicates an inbound migration from qemu-git
1.2, i.e. the old manual loading should be run.

 /* qemu-kvm 1.2 uses version 3 but advertised as 2
 - * To support incoming qemu-kvm 1.2 migration, change version_id
 - * and minimum_version_id to 2 below (which breaks migration from
 + * To support incoming qemu-kvm 1.2 migration, we support
 + * via a command line option a change to minimum_version_id
 + * of 2 in a _compat structure (which breaks migration from
  * qemu 1.2).
 
 Actually it's version 3 that breaks migration right?
 Pls say this explicitly: s/which/version 3 breaks migration from qemu 1.2/

It's changing the minimum version ID to 2 (from 3) which would break
migration from qemu (git) 1.2, because that uses a version ID of 2,
and we want the old loader to run in that case. I've made this clearer.

-- 
Alex Bligh





--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

2014-10-04 Thread Alex Bligh

Sorry that was v5 - git send-email fumble fingers as git-publish
doesn't seem to work well with single patches.

Alex

On 4 Oct 2014, at 17:37, Alex Bligh a...@alex.org.uk wrote:

 Add a machine type pc-1.0-qemu-kvm for live migrate compatibility
 with qemu-kvm version 1.0.
 
 This patch adds inbound migrate capability from qemu-kvm version
 1.0. The main ideas are those set out in Cole Robinson's patch here:
 http://pkgs.fedoraproject.org/cgit/qemu.git/tree/0001-Fix-migration-from-qemu-kvm.patch?h=f20
 however, rather than patching statically (and breaking inbound
 migration on existing machine types), I have added a new machine
 type (pc-1.0-qemu-kvm) without affecting any other machine types.
 The existing pc-1.0 machine type is renamed to pc-1.0-qemu-git,
 with pc-1.0 becoming an alias for one or another, as selected
 by a configure option (defaulting to pc-1.0-qemu-git, IE no
 change).
 
 Two aproaches are taken:
 
 * In hw/timer/i8254_common.c, the VMSTATE_UINT32_TEST macro
 is used to test the version for the irq_disable flags,
 allowing version 3 or more, or version 2 for an inbound
 migrate from qemu-kvm (only).
 
 * In hw/acpi/piix4.c, qemu-kvm incorrectly uses version 2 for
 a version 3 structure, causing acpi_load_old to be used.
 acpi_load_old detects this situation based on the machine type
 and restarts the attempt to load the vmstate using a
 customised VMStateDescription. The above cleaner approach is
 unavailable here.
 
 I developed this on qemu 2.0 but have forward ported it (trivially)
 to master. My testing has been on a VM live-migrated-to-file from
 Ubuntu Precise qemu-kvm 1.0.
 
 I have given this a moderate degree of testing but it could do
 with more.
 
 Note that certain hardware devices (including QXL) will not
 migrate properly due to a fundamental difference in their internal
 state between versions.
 
 Also note that (as expected) migration from qemu-2.x to qemu-1.0
 will not work, even if the machine types are the same.
 
 Changes from v4
 
 * Revert to using a machine type, but do not add alias machine types,
  configure options, or (potentially) change the meaning of
  pc-1.0 - leave this for distributions to ponder
 
 * Add compat_props for qemu-kvm-migration to the PIIX4_PM driver
  and the i8259 pit-common driver.
 
 Signed-off-by: Alex Bligh a...@alex.org.uk
 ---
 hw/acpi/piix4.c   |   26 +++---
 hw/i386/pc_piix.c |   27 +++
 hw/timer/i8254_common.c   |   18 +-
 include/hw/i386/pc.h  |8 
 include/hw/timer/i8254_internal.h |1 +
 5 files changed, 76 insertions(+), 4 deletions(-)
 
 diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
 index b72b34e..5c68d69 100644
 --- a/hw/acpi/piix4.c
 +++ b/hw/acpi/piix4.c
 @@ -86,6 +86,8 @@ typedef struct PIIX4PMState {
 Notifier cpu_added_notifier;
 
 MemHotplugState acpi_memory_hotplug;
 +
 +bool qemu_kvm_migration;
 } PIIX4PMState;
 
 #define TYPE_PIIX4_PM PIIX4_PM
 @@ -200,12 +202,26 @@ static const VMStateDescription vmstate_pci_status = {
 }
 };
 
 +static const VMStateDescription vmstate_acpi;
 +
 static int acpi_load_old(QEMUFile *f, void *opaque, int version_id)
 {
 PIIX4PMState *s = opaque;
 int ret, i;
 uint16_t temp;
 
 +/* If we are expecting the inbound migration to come from
 + * qemu-kvm 1.0, it will have a version_id of 2 but really
 + * be version 3, so call back the original vmstate_load_state
 + * with a different more tolerant vmstate descriptor
 + */
 +if (version_id == 2  s-qemu_kvm_migration) {
 +VMStateDescription vmstate_acpi_compat = vmstate_acpi;
 +vmstate_acpi_compat.minimum_version_id = 2;
 +return vmstate_load_state(f, vmstate_acpi_compat,
 +  opaque, version_id);
 +}
 +
 ret = pci_device_load(PCI_DEVICE(s), f);
 if (ret  0) {
 return ret;
 @@ -267,9 +283,11 @@ static const VMStateDescription vmstate_memhp_state = {
 };
 
 /* qemu-kvm 1.2 uses version 3 but advertised as 2
 - * To support incoming qemu-kvm 1.2 migration, change version_id
 - * and minimum_version_id to 2 below (which breaks migration from
 - * qemu 1.2).
 + * To support incoming qemu-kvm 1.2 migration, we support
 + * via a command line option a change to minimum_version_id
 + * of 2 in a _compat structure; we can't do this all the time
 + * as using a minimum_version_id of 2 (rather than 3) would
 + * break migration from qemu-git 1.2.
  *
  */
 static const VMStateDescription vmstate_acpi = {
 @@ -589,6 +607,8 @@ static Property piix4_pm_properties[] = {
  use_acpi_pci_hotplug, true),
 DEFINE_PROP_BOOL(memory-hotplug-support, PIIX4PMState,
  acpi_memory_hotplug.is_enabled, true),
 +DEFINE_PROP_BOOL(qemu-kvm-migration, PIIX4PMState,
 + qemu_kvm_migration, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
 diff --git

[libvirt] [PATCH] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

2014-10-04 Thread Alex Bligh

Add a machine type pc-1.0-qemu-kvm for live migrate compatibility
with qemu-kvm version 1.0.

This patch adds inbound migrate capability from qemu-kvm version
1.0. The main ideas are those set out in Cole Robinson's patch here:
http://pkgs.fedoraproject.org/cgit/qemu.git/tree/0001-Fix-migration-from-qemu-kvm.patch?h=f20
however, rather than patching statically (and breaking inbound
migration on existing machine types), I have added a new machine
type (pc-1.0-qemu-kvm) without affecting any other machine types.
The existing pc-1.0 machine type is renamed to pc-1.0-qemu-git,
with pc-1.0 becoming an alias for one or another, as selected
by a configure option (defaulting to pc-1.0-qemu-git, IE no
change).

Two aproaches are taken:

* In hw/timer/i8254_common.c, the VMSTATE_UINT32_TEST macro
 is used to test the version for the irq_disable flags,
 allowing version 3 or more, or version 2 for an inbound
 migrate from qemu-kvm (only).

* In hw/acpi/piix4.c, qemu-kvm incorrectly uses version 2 for
 a version 3 structure, causing acpi_load_old to be used.
 acpi_load_old detects this situation based on the machine type
 and restarts the attempt to load the vmstate using a
 customised VMStateDescription. The above cleaner approach is
 unavailable here.

I developed this on qemu 2.0 but have forward ported it (trivially)
to master. My testing has been on a VM live-migrated-to-file from
Ubuntu Precise qemu-kvm 1.0.

I have given this a moderate degree of testing but it could do
with more.

Note that certain hardware devices (including QXL) will not
migrate properly due to a fundamental difference in their internal
state between versions.

Also note that (as expected) migration from qemu-2.x to qemu-1.0
will not work, even if the machine types are the same.

Changes from v4

* Revert to using a machine type, but do not add alias machine types,
  configure options, or (potentially) change the meaning of
  pc-1.0 - leave this for distributions to ponder

* Add compat_props for qemu-kvm-migration to the PIIX4_PM driver
  and the i8259 pit-common driver.

Signed-off-by: Alex Bligh a...@alex.org.uk
---
 hw/acpi/piix4.c   |   26 +++---
 hw/i386/pc_piix.c |   27 +++
 hw/timer/i8254_common.c   |   18 +-
 include/hw/i386/pc.h  |8 
 include/hw/timer/i8254_internal.h |1 +
 5 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b72b34e..5c68d69 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -86,6 +86,8 @@ typedef struct PIIX4PMState {
 Notifier cpu_added_notifier;
 
 MemHotplugState acpi_memory_hotplug;
+
+bool qemu_kvm_migration;
 } PIIX4PMState;
 
 #define TYPE_PIIX4_PM PIIX4_PM
@@ -200,12 +202,26 @@ static const VMStateDescription vmstate_pci_status = {
 }
 };
 
+static const VMStateDescription vmstate_acpi;
+
 static int acpi_load_old(QEMUFile *f, void *opaque, int version_id)
 {
 PIIX4PMState *s = opaque;
 int ret, i;
 uint16_t temp;
 
+/* If we are expecting the inbound migration to come from
+ * qemu-kvm 1.0, it will have a version_id of 2 but really
+ * be version 3, so call back the original vmstate_load_state
+ * with a different more tolerant vmstate descriptor
+ */
+if (version_id == 2  s-qemu_kvm_migration) {
+VMStateDescription vmstate_acpi_compat = vmstate_acpi;
+vmstate_acpi_compat.minimum_version_id = 2;
+return vmstate_load_state(f, vmstate_acpi_compat,
+  opaque, version_id);
+}
+
 ret = pci_device_load(PCI_DEVICE(s), f);
 if (ret  0) {
 return ret;
@@ -267,9 +283,11 @@ static const VMStateDescription vmstate_memhp_state = {
 };
 
 /* qemu-kvm 1.2 uses version 3 but advertised as 2
- * To support incoming qemu-kvm 1.2 migration, change version_id
- * and minimum_version_id to 2 below (which breaks migration from
- * qemu 1.2).
+ * To support incoming qemu-kvm 1.2 migration, we support
+ * via a command line option a change to minimum_version_id
+ * of 2 in a _compat structure; we can't do this all the time
+ * as using a minimum_version_id of 2 (rather than 3) would
+ * break migration from qemu-git 1.2.
  *
  */
 static const VMStateDescription vmstate_acpi = {
@@ -589,6 +607,8 @@ static Property piix4_pm_properties[] = {
  use_acpi_pci_hotplug, true),
 DEFINE_PROP_BOOL(memory-hotplug-support, PIIX4PMState,
  acpi_memory_hotplug.is_enabled, true),
+DEFINE_PROP_BOOL(qemu-kvm-migration, PIIX4PMState,
+ qemu_kvm_migration, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7081c08..56555c1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -644,6 +644,32 @@ static QEMUMachine pc_machine_v1_0 = {
 .hw_version = 1.0,
 };
 
+#define PC_COMPAT_1_0_QEMU_KVM \
+PC_COMPAT_1_0,\

Re: [libvirt] [PATCH v3 3/3] qemu: Build command line for ivshmem device

[libvirt] [PATCH] minor shmem clean-ups

Re: [libvirt] [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm

Re: [libvirt] [Qemu-devel] [PATCH] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

[libvirt] [PATCH] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

5 matches

Site Navigation

Mail list logo

Footer information