date:20140804

On Tue, Jul 29, 2014 at 08:31:28AM +0100, Alex Bligh wrote:
 Serge,
 
  I don't think that is in any way a problem.  Is migrating to older
  versions ever actually expected to work?  In either case I don't
  think for this particular case it's a problem.
 
 Good; no; and good - respectively.
 
  (The how to handle this in libvirt question is more interesting)
 
 I've been giving this some thought. However rococo we make this,
 I think the caller is often going to need to modify the command
 line anyway, i.e. is going to need to be aware of the migration
 problem.
 
 For instance, taking Ubuntu as an example, 12.04 shipped with
 qemu-kvm-1.0, and a pxe-virtio.rom of just under 64k, giving
 a 64k ROM slot. 14.04 ships with qemu-2.0 and a pxe-virtio.rom
 of over 80k, giving a 128k ROM slot. So however we fix the
 machine types, when migrating in a 12.04 initiated VM, qemu
 will need
  -global virtio-net-pci.romfile=/path/to/pxe-virtio.rom.12.04
 on the command line (or, if you don't much care about PXE
 working on a soft reboot, a blank file of the same size),
 whereas when migrating in a 14.04 initiated VM, that must
 not be on the command line.
 
 Fixing this properly would entail requiring that the ROMs are
 (effectively) distributed with qemu or at least that all
 ROM sizes become part of the machine type standard. This
 would have the advantage that loading a larger ROM than
 the machine type specifies would fail unless the ROM
 size was explicitly configured on the command line. But
 this is a subject wider than this patch.
 
 So the long and the short of it is that libvirt (sadly) like
 anything else starting qemu machines needs to know a bit about
 different versions of qemu, and be able to replace a machine
 type option with a machine type option and more on the
 command line.
 
 My previous suggestion doesn't help much because qemu will
 still need to be passed something on the command line.
 
 I think the best way to go with this patch would be something
 like:
 
 * Add pc-1.0-qemu-kvm as a machine type (done)
 
 * Rename pc-1.0 to pc-1.0-qemu-git
 
 * Add an alias for pc-1.0 to either pc-1.0-qemu-git or
   pc-1.0-qemu-kvm, configurable at build time with
   a ./configure option. The distro can then set this
   appropriately. This would default to pc-1.0-qemu-git
   (i.e. the current behaviour).
 
 On distros that only every used one of the above,
 ./configure will sort things out, +/- self-inflicted
 injuries relating to ROM size changes. If life is
 more complicated, libvirt (and other callers) will
 need to be aware. pc-1.0-qemu-git and pc-1.0-qemu-kvm
 can be used to unambiguously mean the relevant machine
 type, which will fix things going forward for that
 machine type.
 
 WDYT?


This just means we perpetuate the broken-ness.

I would rather we teach libvirt to do the right thing
unconditionally.


 -- 
 Alex Bligh
 
 
 

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

Re: [libvirt] [PATCH v2 2/2] Add configure option --enable-pc-1-0-qemu-kvm

On Fri, Aug 01, 2014 at 08:12:13PM +0100, Alex Bligh wrote:
 Add a configure option --enable-pc-1-0-qemu-kvm and the
 corresponding --disable-pc-1-0-qemu-kvm, defaulting
 to disabled.
 
 Rename machine type pc-1.0 to pc-1.0-qemu-git.
 
 Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm
 or pc-1.0-qemu-git depending on the value of the config
 option.
 
 Signed-off-by: Alex Bligh a...@alex.org.uk

Nah that's too messy.
qemu-kvm should have used a different machine type,
it didn't but we should not perpetuate this mistake.
Please add a work around for this in management.

 ---
  configure |   12 
  hw/i386/pc_piix.c |8 +++-
  2 files changed, 19 insertions(+), 1 deletion(-)
 
 diff --git a/configure b/configure
 index f7685b5..b143302 100755
 --- a/configure
 +++ b/configure
 @@ -335,6 +335,7 @@ libssh2=
  vhdx=
  quorum=
  numa=
 +pc_1_0_qemu_kvm=no
  
  # parse CC options first
  for opt do
 @@ -1125,6 +1126,10 @@ for opt do
;;
--enable-numa) numa=yes
;;
 +  --disable-pc-1-0-qemu-kvm) pc_1_0_qemu_kvm=no
 +  ;;
 +  --enable-pc-1-0-qemu-kvm) pc_1_0_qemu_kvm=yes
 +  ;;
*)
echo ERROR: unknown option $opt
echo Try '$0 --help' for more information
 @@ -1394,6 +1399,8 @@ Advanced options (experts only):
--enable-quorum  enable quorum block filter support
--disable-numa   disable libnuma support
--enable-numaenable libnuma support
 +  --disable-pc-1-0-qemu-kvm disable pc-1.0 machine type reflecting qemu-kvm
 +  --enable-pc-1-0-qemu-kvm enable pc-1.0 machine type reflecting qemu-kvm
  
  NOTE: The object files are built at the place where configure is launched
  EOF
 @@ -4262,6 +4269,7 @@ echo Quorum$quorum
  echo lzo support   $lzo
  echo snappy support$snappy
  echo NUMA host support $numa
 +echo pc-1.0 qemu-kvm   $pc_1_0_qemu_kvm
  
  if test $sdl_too_old = yes; then
  echo - Your SDL version is too old - please upgrade to have SDL support
 @@ -5241,6 +5249,10 @@ if test $numa = yes; then
echo CONFIG_NUMA=y  $config_host_mak
  fi
  
 +if test $pc_1_0_qemu_kvm = yes; then
 +  echo CONFIG_PC_1_0_QEMU_KVM=y  $config_host_mak
 +fi
 +
  # build tree in object directory in case the source is not in the current 
 directory
  DIRS=tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos 
 tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests
  DIRS=$DIRS fsdev
 diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
 index e400ea6..7a3db34 100644
 --- a/hw/i386/pc_piix.c
 +++ b/hw/i386/pc_piix.c
 @@ -647,7 +647,10 @@ static QEMUMachine pc_machine_v1_1 = {
  
  static QEMUMachine pc_machine_v1_0 = {
  PC_I440FX_1_2_MACHINE_OPTIONS,
 -.name = pc-1.0,
 +.name = pc-1.0-qemu-git,
 +#ifndef CONFIG_PC_1_0_QEMU_KVM
 +.alias = pc-1.0,
 +#endif
  .compat_props = (GlobalProperty[]) {
  PC_COMPAT_1_0,
  { /* end of list */ }
 @@ -666,6 +669,9 @@ static QEMUMachine pc_machine_v1_0 = {
  static QEMUMachine pc_machine_v1_0_qemu_kvm = {
  PC_I440FX_1_2_MACHINE_OPTIONS,
  .name = pc-1.0-qemu-kvm,
 +#ifdef CONFIG_PC_1_0_QEMU_KVM
 +.alias = pc-1.0,
 +#endif
  .init = pc_init_pci_1_2_qemu_kvm,
  .compat_props = (GlobalProperty[]) {
  PC_COMPAT_1_0_QEMU_KVM,
 -- 
 1.7.9.5

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

Re: [libvirt] [PATCH v2 0/2] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

On Fri, Aug 01, 2014 at 08:12:11PM +0100, Alex Bligh wrote:
This patch series 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).

This requires 'hot patching' the VMStateDescription in a couple of
places, which in turn is less than obvious as there may be (indeed
are for i8259) derived classes. Whilst pretty nausea-inducing, this
approach has the benefit of being entirely self-contained.

Ow come on.
Just add a flag and select the appropriate format based on it, using
field_exists.

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) migratino from qemu-2.x to qemu-1.0
will not work, even if the machine types are the same.

Alex Bligh (2):
Add machine type pc-1.0-qemu-kvm for live migrate compatibility with
qemu-kvm
Add configure option --enable-pc-1-0-qemu-kvm

--
1.7.9.5

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

[libvirt] [PATCH] Post-release version bump for new dev cycle

Signed-off-by: Eric Blake ebl...@redhat.com
---

Pushing under the trivial rule.

 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index f37c716..9dd07d2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -16,7 +16,7 @@ dnl You should have received a copy of the GNU Lesser General 
Public
 dnl License along with this library.  If not, see
 dnl http://www.gnu.org/licenses/.

-AC_INIT([libvirt], [1.2.7], [libvir-list@redhat.com], [], [http://libvirt.org])
+AC_INIT([libvirt], [1.2.8], [libvir-list@redhat.com], [], [http://libvirt.org])
 AC_CONFIG_SRCDIR([src/libvirt.c])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
-- 
1.9.3

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

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


On 4 Aug 2014, at 14:31, Michael S. Tsirkin m...@redhat.com wrote:

 On Fri, Aug 01, 2014 at 08:12:11PM +0100, Alex Bligh wrote:
 This patch series 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).
 
 This requires 'hot patching' the VMStateDescription in a couple of
 places, which in turn is less than obvious as there may be (indeed
 are for i8259) derived classes. Whilst pretty nausea-inducing, this
 approach has the benefit of being entirely self-contained.
 
 Ow come on.
 Just add a flag and select the appropriate format based on it, using
 field_exists.

I don't think it is that simple. All those things are initialised well
before the command line is parsed.

Unless I'm missing what you are saying?

-- 
Alex Bligh





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

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

On Mon, Aug 04, 2014 at 02:51:01PM +0100, Alex Bligh wrote:

On 4 Aug 2014, at 14:31, Michael S. Tsirkin m...@redhat.com wrote:

Ow come on.
Just add a flag and select the appropriate format based on it, using
field_exists.

I don't think it is that simple. All those things are initialised well
before the command line is parsed.

You initialize both and select the correct one at migration time.

Unless I'm missing what you are saying?

I think you are: check how vmstate_test_use_acpi_pci_hotplug
and vmstate_test_no_use_acpi_pci_hotplug are used
in vmstate_acpi.

--
Alex Bligh

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

Re: [libvirt] [PATCH v2] storage: Refresh storage pool after upload

2014-08-04 Thread John Ferlan




  src/libvirt.c|   8 
  src/libvirt_private.syms |   1 +
  src/storage/storage_driver.c | 103 
 +++
  tools/virsh.pod  |   3 ++
  4 files changed, 115 insertions(+)
 
 ACK
 
 
 Regards,
 Daniel
 

I have pushed this now that 1.2.7 is out.

Tks,

John

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

Re: [libvirt] [libvirt-glib] [PATCH v6 1/3] libvirt-gobject-domain: Add _fetch_snapshots

2014-08-04 Thread Christophe Fergeau

Series looks good to me now, ACK! Thanks for going through all these
iterations.

Christophe

On Thu, Jul 31, 2014 at 10:16:21PM +0200, m...@baedert.org wrote:
 From: Timm Bäder m...@baedert.org
 
 This function can be used to fetch the snapshots of a domain (according
 to the given GVirDomainSnapshotListFlags) and save them in a
 domain-internal GHashTable. A function to access them from outside will
 be added in a later patch.
 ---
  libvirt-gobject/libvirt-gobject-domain.c | 85 
 
  libvirt-gobject/libvirt-gobject-domain.h | 37 ++
  libvirt-gobject/libvirt-gobject.sym  |  2 +
  3 files changed, 124 insertions(+)
 
 diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
 b/libvirt-gobject/libvirt-gobject-domain.c
 index c6e30e5..708cb3b 100644
 --- a/libvirt-gobject/libvirt-gobject-domain.c
 +++ b/libvirt-gobject/libvirt-gobject-domain.c
 @@ -38,6 +38,8 @@ struct _GVirDomainPrivate
  {
  virDomainPtr handle;
  gchar uuid[VIR_UUID_STRING_BUFLEN];
 +GHashTable *snapshots;
 +GMutex *lock;
  };
  
  G_DEFINE_TYPE(GVirDomain, gvir_domain, G_TYPE_OBJECT);
 @@ -121,6 +123,11 @@ static void gvir_domain_finalize(GObject *object)
  
  g_debug(Finalize GVirDomain=%p, domain);
  
 +if (priv-snapshots) {
 +g_hash_table_unref(priv-snapshots);
 +}
 +g_mutex_free(priv-lock);
 +
  virDomainFree(priv-handle);
  
  G_OBJECT_CLASS(gvir_domain_parent_class)-finalize(object);
 @@ -237,6 +244,7 @@ static void gvir_domain_init(GVirDomain *domain)
  g_debug(Init GVirDomain=%p, domain);
  
  domain-priv = GVIR_DOMAIN_GET_PRIVATE(domain);
 +domain-priv-lock = g_mutex_new();
  }
  
  typedef struct virDomain GVirDomainHandle;
 @@ -1514,3 +1522,80 @@ gvir_domain_create_snapshot(GVirDomain *dom,
  g_free(custom_xml);
  return dom_snapshot;
  }
 +
 +
 +
 +/**
 + * gvir_domain_fetch_snapshots:
 + * @dom: The domain
 + * @list_flags: bitwise-OR of #GVirDomainSnapshotListFlags
 + * @cancellable: (allow-none)(transfer-none): cancellation object
 + * @error: (allow-none): Place-holder for error or NULL
 + *
 + * Returns: TRUE on success, FALSE otherwise.
 + */
 +gboolean gvir_domain_fetch_snapshots(GVirDomain *dom,
 + guint list_flags,
 + GCancellable *cancellable,
 + GError **error)
 +{
 +GVirDomainPrivate *priv;
 +virDomainSnapshotPtr *snapshots = NULL;
 +GVirDomainSnapshot *snap;
 +GHashTable *snap_table;
 +int n_snaps = 0;
 +int i;
 +gboolean ret = FALSE;
 +
 +g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
 +g_return_val_if_fail((error == NULL) || (*error == NULL), FALSE);
 +
 +priv = dom-priv;
 +
 +snap_table = g_hash_table_new_full(g_str_hash,
 +   g_str_equal,
 +   NULL,
 +   g_object_unref);
 +
 +
 +n_snaps = virDomainListAllSnapshots(priv-handle, snapshots, 
 list_flags);
 +
 +if (g_cancellable_set_error_if_cancelled(cancellable, error)) {
 +goto cleanup;
 +}
 +
 +if (n_snaps  0) {
 +gvir_set_error(error, GVIR_DOMAIN_ERROR, 0,
 +   Unable to fetch snapshots of %s,
 +   gvir_domain_get_name(dom));
 +goto cleanup;
 +}
 +
 +for (i = 0; i  n_snaps; i ++) {
 +if (g_cancellable_set_error_if_cancelled(cancellable, error)) {
 +goto cleanup;
 +}
 +snap = GVIR_DOMAIN_SNAPSHOT(g_object_new(GVIR_TYPE_DOMAIN_SNAPSHOT,
 + handle, snapshots[i],
 + NULL));
 +g_hash_table_insert(snap_table,
 +(gpointer)gvir_domain_snapshot_get_name(snap),
 +snap);
 +}
 +
 +
 +g_mutex_lock(priv-lock);
 +if (priv-snapshots != NULL)
 +g_hash_table_unref(priv-snapshots);
 +priv-snapshots = snap_table;
 +snap_table = NULL;
 +g_mutex_unlock(priv-lock);
 +
 +ret = TRUE;
 +
 +cleanup:
 +free(snapshots);
 +if (snap_table != NULL)
 +g_hash_table_unref(snap_table);
 +return ret;
 +}
 diff --git a/libvirt-gobject/libvirt-gobject-domain.h 
 b/libvirt-gobject/libvirt-gobject-domain.h
 index 38d3458..8c1a8e5 100644
 --- a/libvirt-gobject/libvirt-gobject-domain.h
 +++ b/libvirt-gobject/libvirt-gobject-domain.h
 @@ -183,6 +183,39 @@ typedef enum {
  GVIR_DOMAIN_REBOOT_GUEST_AGENT= VIR_DOMAIN_REBOOT_GUEST_AGENT,
  } GVirDomainRebootFlags;
  
 +/**
 + * GVirDomainSnapshotListFlags:
 + * @GVIR_DOMAIN_SNAPSHOT_LIST_ALL: List all snapshots
 + * @GVIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS: List all descendants, not just
 + * children, when listing a snapshot.
 + * For historical reasons, groups do

[libvirt] [PATCH] Perform disk config validity checking for attach-device config

2014-08-04 Thread John Ferlan

https://bugzilla.redhat.com/show_bug.cgi?id=1078126

Using 'virsh attach-device --config' (or --persistent) to attach a
file backed lun device will succeed; however, subsequent domain restarts
will result in failure because the configuration of a file backed lun
is not supported.

Although allowing 'illegal configurations' is something that can be
allowed, it may not be practical in this case. Generally, when attaching
a device to a domain means the domain must be running. A way around
this is using the --config (or --persistent) option. When an attach
is done to a running domain, a temporary configuration is modified
first followed by the live update. The live update will make a number
of disk validity checks when building the qemu command to attach the
disk. If any fail, then change is rejected.

Rather than allow a potentially illegal combination, adjust the code
in the configuration path to make the same checks as the running path
will make with respect to disk validity checks. This way we avoid
having the potential for some subsequent start/reboot to fail because
an illegal combination was allowed.

NB: The live checking remains since it is possible to just to --live
configuration modification.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_command.c | 138 ++--
 src/qemu/qemu_command.h |   3 ++
 src/qemu/qemu_driver.c  |   2 +
 3 files changed, 81 insertions(+), 62 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4e074e5..8e56151 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3234,6 +3234,81 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
 }
 
 
+int
+qemuBuildCheckDisk(virDomainDiskDefPtr disk,
+   virQEMUCapsPtr qemuCaps)
+{
+if (virDiskNameToIndex(disk-dst)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(unsupported disk type '%s'), disk-dst);
+goto error;
+}
+
+if (disk-wwn) {
+if ((disk-bus != VIR_DOMAIN_DISK_BUS_IDE) 
+(disk-bus != VIR_DOMAIN_DISK_BUS_SCSI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(Only ide and scsi disk support wwn));
+goto error;
+}
+}
+
+if ((disk-vendor || disk-product) 
+disk-bus != VIR_DOMAIN_DISK_BUS_SCSI) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(Only scsi disk supports vendor and product));
+goto error;
+}
+
+if (disk-device == VIR_DOMAIN_DISK_DEVICE_LUN) {
+/* make sure that both the bus and the qemu binary support
+ *  type='lun' (SG_IO).
+ */
+if (disk-bus != VIR_DOMAIN_DISK_BUS_VIRTIO 
+disk-bus != VIR_DOMAIN_DISK_BUS_SCSI) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(disk device='lun' is not supported for 
bus='%s'),
+   virDomainDiskQEMUBusTypeToString(disk-bus));
+goto error;
+}
+if (disk-src-type == VIR_STORAGE_TYPE_NETWORK) {
+if (disk-src-protocol != VIR_STORAGE_NET_PROTOCOL_ISCSI) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(disk device='lun' is not supported 
+ for protocol='%s'),
+   
virStorageNetProtocolTypeToString(disk-src-protocol));
+goto error;
+}
+} else if (!virDomainDiskSourceIsBlockType(disk)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(disk device='lun' is only valid for block 
+ type disk source));
+goto error;
+}
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(disk device='lun' is not supported by 
+ this QEMU));
+goto error;
+}
+if (disk-wwn) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(Setting wwn is not supported for lun device));
+goto error;
+}
+if (disk-vendor || disk-product) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(Setting vendor or product is not supported 
+ for lun device));
+goto error;
+}
+}
+return 0;
+ error:
+return -1;
+}
+
+
 char *
 qemuBuildDriveStr(virConnectPtr conn,
   virDomainDiskDefPtr disk,
@@ -3620,71 +3695,10 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
 {
 virBuffer opt = VIR_BUFFER_INITIALIZER;
 const char *bus = virDomainDiskQEMUBusTypeToString(disk-bus);
-int idx = virDiskNameToIndex(disk-dst);
 int controllerModel;
 
-if (idx  0) {
-

Re: [libvirt] [PATCH v2 0/2] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

2014-08-04 Thread Serge Hallyn

Quoting Alex Bligh (a...@alex.org.uk):
This patch series 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).

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) migratino from qemu-2.x to qemu-1.0
will not work, even if the machine types are the same.

Alex Bligh (2):
Add machine type pc-1.0-qemu-kvm for live migrate compatibility with
qemu-kvm
Add configure option --enable-pc-1-0-qemu-kvm

Thanks, Alex, I hope to set up a test of this this afternoon.

--
1.7.9.5

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

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

2014-08-04 Thread Serge Hallyn

Quoting Michael S. Tsirkin (m...@redhat.com):
 On Tue, Jul 29, 2014 at 08:31:28AM +0100, Alex Bligh wrote:
  Serge,
  
   I don't think that is in any way a problem.  Is migrating to older
   versions ever actually expected to work?  In either case I don't
   think for this particular case it's a problem.
  
  Good; no; and good - respectively.
  
   (The how to handle this in libvirt question is more interesting)
  
  I've been giving this some thought. However rococo we make this,
  I think the caller is often going to need to modify the command
  line anyway, i.e. is going to need to be aware of the migration
  problem.
  
  For instance, taking Ubuntu as an example, 12.04 shipped with
  qemu-kvm-1.0, and a pxe-virtio.rom of just under 64k, giving
  a 64k ROM slot. 14.04 ships with qemu-2.0 and a pxe-virtio.rom
  of over 80k, giving a 128k ROM slot. So however we fix the
  machine types, when migrating in a 12.04 initiated VM, qemu
  will need
   -global virtio-net-pci.romfile=/path/to/pxe-virtio.rom.12.04
  on the command line (or, if you don't much care about PXE
  working on a soft reboot, a blank file of the same size),
  whereas when migrating in a 14.04 initiated VM, that must
  not be on the command line.
  
  Fixing this properly would entail requiring that the ROMs are
  (effectively) distributed with qemu or at least that all
  ROM sizes become part of the machine type standard. This
  would have the advantage that loading a larger ROM than
  the machine type specifies would fail unless the ROM
  size was explicitly configured on the command line. But
  this is a subject wider than this patch.
  
  So the long and the short of it is that libvirt (sadly) like
  anything else starting qemu machines needs to know a bit about
  different versions of qemu, and be able to replace a machine
  type option with a machine type option and more on the
  command line.
  
  My previous suggestion doesn't help much because qemu will
  still need to be passed something on the command line.
  
  I think the best way to go with this patch would be something
  like:
  
  * Add pc-1.0-qemu-kvm as a machine type (done)
  
  * Rename pc-1.0 to pc-1.0-qemu-git
  
  * Add an alias for pc-1.0 to either pc-1.0-qemu-git or
pc-1.0-qemu-kvm, configurable at build time with
a ./configure option. The distro can then set this
appropriately. This would default to pc-1.0-qemu-git
(i.e. the current behaviour).
  
  On distros that only every used one of the above,
  ./configure will sort things out, +/- self-inflicted
  injuries relating to ROM size changes. If life is
  more complicated, libvirt (and other callers) will
  need to be aware. pc-1.0-qemu-git and pc-1.0-qemu-kvm
  can be used to unambiguously mean the relevant machine
  type, which will fix things going forward for that
  machine type.
  
  WDYT?
 
 
 This just means we perpetuate the broken-ness.
 
 I would rather we teach libvirt to do the right thing
 unconditionally.

Well, now, here's a thought - can we hot-patch qemu to
change the machine type while it is running before the
migrate?  Just s/pc-1.0/pc-x.0/ or something?

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

Re: [libvirt] [PATCH v2 7/8] domain_conf: Common routine to handle network storage host xml def

On 07/21/2014 02:47 PM, John Ferlan wrote:
 In preparation for hostdev support for iSCSI and a virStorageNetHostDefPtr,
 split out the network disk storage parsing of the 'host' element into a
 separate routine.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/conf/domain_conf.c | 137 
 +++--
  1 file changed, 76 insertions(+), 61 deletions(-)
 

ACK

-- 
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] [Qemu-devel] [PATCH] [RFC] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

On Mon, Aug 04, 2014 at 03:08:31PM +, Serge Hallyn wrote:
 Quoting Michael S. Tsirkin (m...@redhat.com):
  On Tue, Jul 29, 2014 at 08:31:28AM +0100, Alex Bligh wrote:
   Serge,
   
I don't think that is in any way a problem.  Is migrating to older
versions ever actually expected to work?  In either case I don't
think for this particular case it's a problem.
   
   Good; no; and good - respectively.
   
(The how to handle this in libvirt question is more interesting)
   
   I've been giving this some thought. However rococo we make this,
   I think the caller is often going to need to modify the command
   line anyway, i.e. is going to need to be aware of the migration
   problem.
   
   For instance, taking Ubuntu as an example, 12.04 shipped with
   qemu-kvm-1.0, and a pxe-virtio.rom of just under 64k, giving
   a 64k ROM slot. 14.04 ships with qemu-2.0 and a pxe-virtio.rom
   of over 80k, giving a 128k ROM slot. So however we fix the
   machine types, when migrating in a 12.04 initiated VM, qemu
   will need
-global virtio-net-pci.romfile=/path/to/pxe-virtio.rom.12.04
   on the command line (or, if you don't much care about PXE
   working on a soft reboot, a blank file of the same size),
   whereas when migrating in a 14.04 initiated VM, that must
   not be on the command line.
   
   Fixing this properly would entail requiring that the ROMs are
   (effectively) distributed with qemu or at least that all
   ROM sizes become part of the machine type standard. This
   would have the advantage that loading a larger ROM than
   the machine type specifies would fail unless the ROM
   size was explicitly configured on the command line. But
   this is a subject wider than this patch.
   
   So the long and the short of it is that libvirt (sadly) like
   anything else starting qemu machines needs to know a bit about
   different versions of qemu, and be able to replace a machine
   type option with a machine type option and more on the
   command line.
   
   My previous suggestion doesn't help much because qemu will
   still need to be passed something on the command line.
   
   I think the best way to go with this patch would be something
   like:
   
   * Add pc-1.0-qemu-kvm as a machine type (done)
   
   * Rename pc-1.0 to pc-1.0-qemu-git
   
   * Add an alias for pc-1.0 to either pc-1.0-qemu-git or
 pc-1.0-qemu-kvm, configurable at build time with
 a ./configure option. The distro can then set this
 appropriately. This would default to pc-1.0-qemu-git
 (i.e. the current behaviour).
   
   On distros that only every used one of the above,
   ./configure will sort things out, +/- self-inflicted
   injuries relating to ROM size changes. If life is
   more complicated, libvirt (and other callers) will
   need to be aware. pc-1.0-qemu-git and pc-1.0-qemu-kvm
   can be used to unambiguously mean the relevant machine
   type, which will fix things going forward for that
   machine type.
   
   WDYT?
  
  
  This just means we perpetuate the broken-ness.
  
  I would rather we teach libvirt to do the right thing
  unconditionally.
 
 Well, now, here's a thought - can we hot-patch qemu to
 change the machine type while it is running before the
 migrate?  Just s/pc-1.0/pc-x.0/ or something?

Frankly I don't see what will this accomplish.

If you really want it to be called pc-1.0, you
can make it a machine property instead.
E.g. qemu-kvm-compatibility.
Teach management to set it if remote is qemu-kvm:
-machine pc-1.0,qemu-kvm-compatibility=on


-- 
MST

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

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

2014-08-04 Thread Serge Hallyn

Quoting Michael S. Tsirkin (m...@redhat.com):
 On Mon, Aug 04, 2014 at 03:08:31PM +, Serge Hallyn wrote:
  Quoting Michael S. Tsirkin (m...@redhat.com):
   On Tue, Jul 29, 2014 at 08:31:28AM +0100, Alex Bligh wrote:
Serge,

 I don't think that is in any way a problem.  Is migrating to older
 versions ever actually expected to work?  In either case I don't
 think for this particular case it's a problem.

Good; no; and good - respectively.

 (The how to handle this in libvirt question is more interesting)

I've been giving this some thought. However rococo we make this,
I think the caller is often going to need to modify the command
line anyway, i.e. is going to need to be aware of the migration
problem.

For instance, taking Ubuntu as an example, 12.04 shipped with
qemu-kvm-1.0, and a pxe-virtio.rom of just under 64k, giving
a 64k ROM slot. 14.04 ships with qemu-2.0 and a pxe-virtio.rom
of over 80k, giving a 128k ROM slot. So however we fix the
machine types, when migrating in a 12.04 initiated VM, qemu
will need
 -global virtio-net-pci.romfile=/path/to/pxe-virtio.rom.12.04
on the command line (or, if you don't much care about PXE
working on a soft reboot, a blank file of the same size),
whereas when migrating in a 14.04 initiated VM, that must
not be on the command line.

Fixing this properly would entail requiring that the ROMs are
(effectively) distributed with qemu or at least that all
ROM sizes become part of the machine type standard. This
would have the advantage that loading a larger ROM than
the machine type specifies would fail unless the ROM
size was explicitly configured on the command line. But
this is a subject wider than this patch.

So the long and the short of it is that libvirt (sadly) like
anything else starting qemu machines needs to know a bit about
different versions of qemu, and be able to replace a machine
type option with a machine type option and more on the
command line.

My previous suggestion doesn't help much because qemu will
still need to be passed something on the command line.

I think the best way to go with this patch would be something
like:

* Add pc-1.0-qemu-kvm as a machine type (done)

* Rename pc-1.0 to pc-1.0-qemu-git

* Add an alias for pc-1.0 to either pc-1.0-qemu-git or
  pc-1.0-qemu-kvm, configurable at build time with
  a ./configure option. The distro can then set this
  appropriately. This would default to pc-1.0-qemu-git
  (i.e. the current behaviour).

On distros that only every used one of the above,
./configure will sort things out, +/- self-inflicted
injuries relating to ROM size changes. If life is
more complicated, libvirt (and other callers) will
need to be aware. pc-1.0-qemu-git and pc-1.0-qemu-kvm
can be used to unambiguously mean the relevant machine
type, which will fix things going forward for that
machine type.

WDYT?
   
   
   This just means we perpetuate the broken-ness.
   
   I would rather we teach libvirt to do the right thing
   unconditionally.
  
  Well, now, here's a thought - can we hot-patch qemu to
  change the machine type while it is running before the
  migrate?  Just s/pc-1.0/pc-x.0/ or something?
 
 Frankly I don't see what will this accomplish.
 
 If you really want it to be called pc-1.0, you
 can make it a machine property instead.
 E.g. qemu-kvm-compatibility.
 Teach management to set it if remote is qemu-kvm:
   -machine pc-1.0,qemu-kvm-compatibility=on

That sounds nice - Alex, what do you think?

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

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


On 4 Aug 2014, at 16:38, Serge Hallyn serge.hal...@ubuntu.com wrote:

 
 If you really want it to be called pc-1.0, you
 can make it a machine property instead.
 E.g. qemu-kvm-compatibility.
 Teach management to set it if remote is qemu-kvm:
  -machine pc-1.0,qemu-kvm-compatibility=on
 
 That sounds nice - Alex, what do you think?

Not having used the machine property stuff before,
or played with libvirt much, I'm not sure how this
helps libvirt.

I thought the issue here was that migrating from
1.0-qemu-kvm to 2.x OR 1.0-qemu-git to 2.x, libvirt
is going to to supply the same command line. As
libvirt doesn't know what the sender is (and
it's not possible to detect this automatically -
at least not without a far more intrusive patch),
one has to make a choice at build time as to what
'pc-1.0' represents. This is what patch #2 does.
I fully agree it is not pretty.

So I am not sure why
-machine pc-1.0,qemu-kvm-compatibility=on
is any easier for libvirt than
-machine pc-1.0-qemu-kvm

IE what does using a machine property rather than
a machine type buy us?

-- 
Alex Bligh




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

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

Michael,

On 4 Aug 2014, at 15:26, Michael S. Tsirkin m...@redhat.com wrote:

 
 Unless I'm missing what you are saying?
 
 I think you are: check how vmstate_test_use_acpi_pci_hotplug
 and vmstate_test_no_use_acpi_pci_hotplug are used
 in vmstate_acpi.

I /think/ you are talking about using the VMSTATE_FOO_TEST
macros.

These are capable of modifying fields within the
VMStateDescription of the relevant object.

However, the PIIX4 change modifies the minimum_version_id
(outside fields); I don't quite see how that would work.
Can you help here?

The i8254 change modifies a field which is marked with
a minimum version to be a field with no versioning; I
guess the route there would be a test function that
(somehow) accesses the version of the inbound migration
inside it, does the comparison manually, and returns
1 if EITHER the inbound version =3 or the compatibility
thing is set.

Is that what you meant? I'm rather new to this so
you may have to lead me a little - apologies.

I was trying to produce code which would be 'obviously
correct' in the sense of not breaking the existing pc-1.0
migrations; if playing around with the existing vmstate
fields is permissible then clearly I have a few more
degrees of freedom.

I did try modifying the objects live after the command
line has been parsed; this doesn't work because the
QOM inheritance memcpy's the structs for classes derived
from i8254 etc.

-- 
Alex Bligh




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

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

On Mon, Aug 04, 2014 at 04:47:11PM +0100, Alex Bligh wrote:
 
 On 4 Aug 2014, at 16:38, Serge Hallyn serge.hal...@ubuntu.com wrote:
 
  
  If you really want it to be called pc-1.0, you
  can make it a machine property instead.
  E.g. qemu-kvm-compatibility.
  Teach management to set it if remote is qemu-kvm:
 -machine pc-1.0,qemu-kvm-compatibility=on
  
  That sounds nice - Alex, what do you think?
 
 Not having used the machine property stuff before,
 or played with libvirt much, I'm not sure how this
 helps libvirt.
 
 I thought the issue here was that migrating from
 1.0-qemu-kvm to 2.x OR 1.0-qemu-git to 2.x, libvirt
 is going to to supply the same command line.
 As
 libvirt doesn't know what the sender is (and
 it's not possible to detect this automatically -
 at least not without a far more intrusive patch),

Yes, this is up to higher level user.
At libvirt xml level, you would just specify
something like legacy qemu-kvm compatibility in the xml.

 one has to make a choice at build time as to what
 'pc-1.0' represents.

There's no choice really. Downstreams must make sure
their machine types are distinct from upstream ones.
qemu-kvm as a downstream violated this rule but
I don't think this means upstream should violate it.

 This is what patch #2 does.
 I fully agree it is not pretty.

The problem is not prettyness.
The problem is, it creates a situation where two instances
of qemu have different ideas about what a specific
machine type is.


 So I am not sure why
   -machine pc-1.0,qemu-kvm-compatibility=on
 is any easier for libvirt than
   -machine pc-1.0-qemu-kvm
 
 IE what does using a machine property rather than
 a machine type buy us?

Seems to be easier to understand that it maps to pc-1.0
on the other side.

 -- 
 Alex Bligh
 
 
 

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

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

On Mon, Aug 04, 2014 at 05:11:11PM +0100, Alex Bligh wrote:
 Michael,
 
 On 4 Aug 2014, at 15:26, Michael S. Tsirkin m...@redhat.com wrote:
 
  
  Unless I'm missing what you are saying?
  
  I think you are: check how vmstate_test_use_acpi_pci_hotplug
  and vmstate_test_no_use_acpi_pci_hotplug are used
  in vmstate_acpi.
 
 I /think/ you are talking about using the VMSTATE_FOO_TEST
 macros.

These use field_exists internally.

 These are capable of modifying fields within the
 VMStateDescription of the relevant object.
 
 However, the PIIX4 change modifies the minimum_version_id
 (outside fields); I don't quite see how that would work.
 Can you help here?

If you want to support lower version IDs,
you can just decrease minimum_version_id.

field_exists gets the version ID so you can
parse different fields depending on the
version.


 The i8254 change modifies a field which is marked with
 a minimum version to be a field with no versioning; I
 guess the route there would be a test function that
 (somehow) accesses the version of the inbound migration
 inside it,

Yes, field_exists gets the version value so no problem here.

 does the comparison manually, and returns
 1 if EITHER the inbound version =3 or the compatibility
 thing is set.
 
 Is that what you meant?

I think so, yes.

 I'm rather new to this so
 you may have to lead me a little - apologies.
 
 I was trying to produce code which would be 'obviously
 correct' in the sense of not breaking the existing pc-1.0
 migrations; if playing around with the existing vmstate
 fields is permissible then clearly I have a few more
 degrees of freedom.

Yes I think is should be simple but not to the
exclusion of maintainability.

 I did try modifying the objects live after the command
 line has been parsed; this doesn't work because the
 QOM inheritance memcpy's the structs for classes derived
 from i8254 etc.

Right, just add a new flag for this thing.

 -- 
 Alex Bligh
 
 
 

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

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

Michael,

On 4 Aug 2014, at 17:22, Michael S. Tsirkin m...@redhat.com wrote:

 
 These are capable of modifying fields within the
 VMStateDescription of the relevant object.
 
 However, the PIIX4 change modifies the minimum_version_id
 (outside fields); I don't quite see how that would work.
 Can you help here?
 
 If you want to support lower version IDs,
 you can just decrease minimum_version_id.
 
 field_exists gets the version ID so you can
 parse different fields depending on the
 version.

The issue is that qemu-kvm 1.0 used version ID 2
but is actually sending a version 3 structure.

I don't think I can just reduce the minimum version
ID. As per the comment in the original patch from
Cole Robinson:

http://pkgs.fedoraproject.org/cgit/qemu.git/tree/0001-Fix-migration-from-qemu-kvm.patch?h=f20

--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -289,7 +289,13 @@ static int acpi_load_old(QEMUFile *f, void *opaque, int 
version_id)
 static const VMStateDescription vmstate_acpi = {
 .name = piix4_pm,
 .version_id = 3,
-.minimum_version_id = 3,
+/*
+ * qemu-kvm 1.2 uses qemu.git version 3 format, but advertised as 2.
+ * This allows incoming migration from qemu-kvm, but breaks incoming
+ * migration from qemu  1.3.
+ */
+//minimum_version_id = 3,
+.minimum_version_id = 2,
 .minimum_version_id_old = 1,
 .load_state_old = acpi_load_old,
 .post_load = vmstate_acpi_post_load,

An inbound migration from qemu-1.0-git, qemu-1.1 or
qemu-1.2 will have version ID 2 and actually mean version
2; currently (i.e. with minimum_version_id = 3), these
use the minimum_version_id_old field (1) and
acpi_load_old routine.

If I decrease minimum_version_id to 2, as far as I can
tell this will break inbound migration from the 'good'
earlier versions, whilst fixing qemu-1.0-kvm (which uses
version 2 to mean version 3).

So as far as I can tell, the minimum version ID needs
to be dependent upon inbound machine type (or machine
parameters). By the time the command line is parsed,
these structures have already been built. That's why
I took the approach I did, but again I'm new to this
so may be missing something.

Or were you suggesting I introduced a
  test_force_use_new_load_vm_state
type field into VMStateDescription so it can dynamically
choose not to use the load_state_old but rather to
use the new method, irrespective of the minimum version
mismatch?

-- 
Alex Bligh





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

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

On Mon, Aug 04, 2014 at 05:46:58PM +0100, Alex Bligh wrote:
 Michael,
 
 On 4 Aug 2014, at 17:22, Michael S. Tsirkin m...@redhat.com wrote:
 
  
  These are capable of modifying fields within the
  VMStateDescription of the relevant object.
  
  However, the PIIX4 change modifies the minimum_version_id
  (outside fields); I don't quite see how that would work.
  Can you help here?
  
  If you want to support lower version IDs,
  you can just decrease minimum_version_id.
  
  field_exists gets the version ID so you can
  parse different fields depending on the
  version.
 
 The issue is that qemu-kvm 1.0 used version ID 2
 but is actually sending a version 3 structure.
 
 I don't think I can just reduce the minimum version
 ID.



 As per the comment in the original patch from
 Cole Robinson:
 
 http://pkgs.fedoraproject.org/cgit/qemu.git/tree/0001-Fix-migration-from-qemu-kvm.patch?h=f20
 
 --- a/hw/acpi/piix4.c
 +++ b/hw/acpi/piix4.c
 @@ -289,7 +289,13 @@ static int acpi_load_old(QEMUFile *f, void *opaque, int 
 version_id)
  static const VMStateDescription vmstate_acpi = {
  .name = piix4_pm,
  .version_id = 3,
 -.minimum_version_id = 3,
 +/*
 + * qemu-kvm 1.2 uses qemu.git version 3 format, but advertised as 2.
 + * This allows incoming migration from qemu-kvm, but breaks incoming
 + * migration from qemu  1.3.
 + */
 +//minimum_version_id = 3,
 +.minimum_version_id = 2,
  .minimum_version_id_old = 1,
  .load_state_old = acpi_load_old,
  .post_load = vmstate_acpi_post_load,
 
 An inbound migration from qemu-1.0-git, qemu-1.1 or
 qemu-1.2 will have version ID 2 and actually mean version
 2; currently (i.e. with minimum_version_id = 3), these
 use the minimum_version_id_old field (1) and
 acpi_load_old routine.
 
 If I decrease minimum_version_id to 2, as far as I can
 tell this will break inbound migration from the 'good'
 earlier versions, whilst fixing qemu-1.0-kvm (which uses
 version 2 to mean version 3).
 
 So as far as I can tell, the minimum version ID needs
 to be dependent upon inbound machine type (or machine
 parameters). By the time the command line is parsed,
 these structures have already been built. That's why
 I took the approach I did, but again I'm new to this
 so may be missing something.
 
 Or were you suggesting I introduced a
   test_force_use_new_load_vm_state
 type field into VMStateDescription so it can dynamically
 choose not to use the load_state_old but rather to
 use the new method, irrespective of the minimum version
 mismatch?

I was merely suggesting changing acpi_load_old to
detect the new flag and parse the qemu-kvm format.


 -- 
 Alex Bligh
 
 
 

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

[libvirt] [python PATCH] Bump version to 1.2.8 for new dev cycle

Signed-off-by: Eric Blake ebl...@redhat.com
---

Pushing under the trivial rule.

 setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index 334d7b2..f5c6542 100755
--- a/setup.py
+++ b/setup.py
@@ -309,7 +309,7 @@ class my_clean(clean):
 _c_modules, _py_modules = get_module_lists()

 setup(name = 'libvirt-python',
-  version = '1.2.7',
+  version = '1.2.8',
   url = 'http://www.libvirt.org',
   maintainer = 'Libvirt Maintainers',
   maintainer_email = 'libvir-list@redhat.com',
-- 
1.9.3

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

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


On 4 Aug 2014, at 17:59, Michael S. Tsirkin m...@redhat.com wrote:

 I was merely suggesting changing acpi_load_old to
 detect the new flag and parse the qemu-kvm format.

Oh OK - far simpler.

If the machine is subsequently migrated to another
qemu-2.x device, I take it that will still write out
a proper version 3 object?

-- 
Alex Bligh




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

[libvirt] [python PATCH] maint: document use of libvirt's run script

Ever since libvirt commit 78c09530, it's easier to just use the
run script.

* README: Mention run script.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 README | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/README b/README
index df1de59..96082f0 100644
--- a/README
+++ b/README
@@ -32,10 +32,11 @@ A makefile shim is provided so that you can do
 rather than directly invoking setup.py.

 As of libvirt 1.2.6, it is possible to develop against an uninstalled
-libvirt.git checkout, by setting some environment variables:
+libvirt.git checkout, by setting PKG_CONFIG_PATH and LD_LIBRARY_PATH
+environment variables to point into that libvirt tree; you can even
+automate this by using libvirt's run script:

-  export PKG_CONFIG_PATH=/path/to/libvirt/git/src/
-  export LD_LIBRARY_PATH=/path/to/libvirt/git/src/.libs/
+  /path/to/libvirt/run python setup.py build

 Patches for this code should be sent to the main libvirt
 development mailing list
-- 
1.9.3

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

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

On Mon, Aug 04, 2014 at 06:08:05PM +0100, Alex Bligh wrote:
 
 On 4 Aug 2014, at 17:59, Michael S. Tsirkin m...@redhat.com wrote:
 
  I was merely suggesting changing acpi_load_old to
  detect the new flag and parse the qemu-kvm format.
 
 Oh OK - far simpler.
 
 If the machine is subsequently migrated to another
 qemu-2.x device, I take it that will still write out
 a proper version 3 object?


I think so, yes.

 -- 
 Alex Bligh
 
 
 

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

Re: [libvirt] [python PATCH] Bump version to 1.2.8 for new dev cycle

Eric Blake wrote:
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---

 Pushing under the trivial rule.

  setup.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/setup.py b/setup.py
 index 334d7b2..f5c6542 100755
 --- a/setup.py
 +++ b/setup.py
 @@ -309,7 +309,7 @@ class my_clean(clean):
  _c_modules, _py_modules = get_module_lists()

  setup(name = 'libvirt-python',
 -  version = '1.2.7',
 +  version = '1.2.8',
   

Seems the 1.2.7 bindings were not released.  At least I'm not seeing a
tarball on http://libvirt.org/sources/python/

Regards,
Jim

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

Re: [libvirt] [PATCH v2 8/8] hostdev: Add iSCSI hostdev XML

On 07/21/2014 02:47 PM, John Ferlan wrote:
 Introduce a new structure to handle an iSCSI host device based on the
 existing virDomainHostdevSubsysSCSI by adding a protocol='iscsi' to
 the source/ element.  The hostdev structure mimics the existing
 disk/ element for an iSCSI device (network) device. New XML is:
 
   hostdev mode='subsystem' type='scsi' managed='yes'
 auth username='myname'
   secret type='iscsi' usage='mycluster_myname'/
 /auth
 source protocol='iscsi' name='iqn.1992-01.com.example'
   host name='example.org' port='3260'/
 /source
 address type='drive' controller='0' bus='0' target='2' unit='5'/
   /hostdev
 
 The controller element will mimic the existing scsi_host code insomuch
 as when 'lsi' and 'virtio-scsi' are used.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  docs/formatdomain.html.in  | 142 ---
  docs/schemas/domaincommon.rng  |  46 ++-
  src/conf/domain_conf.c | 152 
 ++---
  .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args  |  14 ++
  .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml   |  46 +++
  .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args   |  14 ++
  .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml|  40 ++
  ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args |  16 +++
  ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml |  46 +++
  .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args|  16 +++
  .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml |  40 ++
  tests/qemuxml2argvtest.c   |  16 +++
  tests/qemuxml2xmltest.c|   5 +
  13 files changed, 524 insertions(+), 69 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml
 

 +  dtusb/dt
 +  ddFor USB devices, the user is responsible to call
 +codevirNodeDeviceDettach/code (or
 +codevirsh nodedev-dettach/code) before starting the guest

The (preferred) virsh command is nodedev-detach (we can't correct the
API typo, but did correct the virsh typo).


 +  dtscsi/dt
 +  ddSCSI devices are described by both the codeadapter/code
 +and codeaddress/code elements.
 +p
 +span class=sinceSince 1.2.7/span, the codesource/code

1.2.8 now.


 +++ b/docs/schemas/domaincommon.rng
 @@ -3563,13 +3563,47 @@
  /choice
/attribute
  /optional
 +optional
 +  ref name='diskAuth'/
 +/optional

Does diskAuth work for all configurations, or only for the new iscsi
configuration?  By putting it here, you are allowing it for all users,...

  element name=source
 -  interleave
 -ref name=sourceinfoadapter/
 -element name=address
 -  ref name=scsiaddress/
 -/element
 -  /interleave
 +  optional
 +attribute name=protocol
 +  choice
 +valueiscsi/value
 +  /choice

...whereas using some group magic could make it valid only when
protocol is iscsi.

 +/attribute
 +attribute name=name
 +  text/
 +/attribute
 +  /optional
 +  choice
 +group
 +  interleave
 +ref name=sourceinfoadapter/
 +element name=address
 +  ref name=scsiaddress/
 +/element
 +  /interleave
 +/group
 +group
 +  interleave
 +oneOrMore
 +  element name='host'
 +attribute name='name'
 +  text/
 +/attribute
 +optional
 +  attribute name='port'
 +ref name=PortNumber/
 +  /attribute
 +/optional
 +empty/
 +  /element
 +/oneOrMore
 +  /interleave

This interleave adds nothing.  It doesn't hurt to leave it in, but
when there is only one possible (repetition of a) child host
element, there is nothing to be interleaved.


 @@ -4205,10 +4210,93 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr 
 sourcenode,
  }
  
  static int
 +virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode,
 +   xmlNodePtr authnode,
 +

Re: [libvirt] [PATCH v2 01/25] src/xenxs:Refactor code parsing memory config

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 Introduce function
  xenParseXMMem(virConfPtr conf,..);
 which parses memory config instead
   

Since you mentioned off-list that a V3 is in the works, I'll point out a
nit below you can address.

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 28 ++--
  1 file changed, 18 insertions(+), 10 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index f6492b5..5e26e5e 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -244,6 +244,23 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, 
 unsigned char *uuid)
  return 0;
  }
  
 +
   

Since most functions in this file have 2 blank lines of whitespace
between them, this addition is fine.

 +static
 +int xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
 +{
 +if (xenXMConfigGetULongLong(conf, memory, def-mem.cur_balloon,
 +MIN_XEN_GUEST_SIZE * 2)  0)
 +return -1;
 +
 +if (xenXMConfigGetULongLong(conf, maxmem, def-mem.max_balloon,
 +def-mem.cur_balloon)  0)
 +return -1;
 +
 +def-mem.cur_balloon *= 1024;
 +def-mem.max_balloon *= 1024;
 +
 +return 0;
 +}
   

But you should do the same here.

Regards,
Jim

  #define MAX_VFB 1024
  /*
   * Turn a config record into a lump of XML describing the
 @@ -360,17 +377,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  }
  }
  
 -if (xenXMConfigGetULongLong(conf, memory, def-mem.cur_balloon,
 -MIN_XEN_GUEST_SIZE * 2)  0)
 +if (xenParseXMMem(conf, def)  0)
  goto cleanup;
 -
 -if (xenXMConfigGetULongLong(conf, maxmem, def-mem.max_balloon,
 -def-mem.cur_balloon)  0)
 -goto cleanup;
 -
 -def-mem.cur_balloon *= 1024;
 -def-mem.max_balloon *= 1024;
 -
  if (xenXMConfigGetULong(conf, vcpus, count, 1)  0 ||
  MAX_VIRT_CPUS  count)
  goto cleanup;
   

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

Re: [libvirt] [PATCH v2 02/25] src/xenxs:Refactor code parsing event actions config

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 Introduce function
  xenParseXMEventActions(virConfPtr conf,..);
 which parses events config instead

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 65 
 +++---
  1 file changed, 38 insertions(+), 27 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 5e26e5e..d0a166d 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -266,6 +266,42 @@ int xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
   * Turn a config record into a lump of XML describing the
   * domain, suitable for later feeding for virDomainCreateXML
   */
   

This existing comment refers to xenParseXM() and should be retained with
its definition.  Add this new function before the comment.

Regards,
Jim

 +static
 +int xenParseXMEventsActions(virConfPtr conf, virDomainDefPtr def)
 +{
 +const char *str;
 +
 +if (xenXMConfigGetString(conf, on_poweroff, str, destroy)  0)
 +return -1;
 +
 +if ((def-onPoweroff = virDomainLifecycleTypeFromString(str))  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unexpected value %s for on_poweroff), str);
 +return -1;
 +}
 +
 +if (xenXMConfigGetString(conf, on_reboot, str, restart)  0)
 +return -1;
 +
 +if ((def-onReboot = virDomainLifecycleTypeFromString(str))  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unexpected value %s for on_reboot), str);
 +return -1;
 +}
 +
 +if (xenXMConfigGetString(conf, on_crash, str, restart)  0)
 +return -1;
 +
 +if ((def-onCrash = virDomainLifecycleCrashTypeFromString(str))  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unexpected value %s for on_crash), str);
 +return -1;
 +}
 +
 +return 0;
 +}
 +
 +
  virDomainDefPtr
  xenParseXM(virConfPtr conf, int xendConfigVersion,
 virCapsPtr caps)
 @@ -379,6 +415,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  
  if (xenParseXMMem(conf, def)  0)
  goto cleanup;
 +if (xenParseXMEventsActions(conf, def)  0)
 +goto cleanup;
  if (xenXMConfigGetULong(conf, vcpus, count, 1)  0 ||
  MAX_VIRT_CPUS  count)
  goto cleanup;
 @@ -391,33 +429,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  goto cleanup;
  if (str  (virBitmapParse(str, 0, def-cpumask, 4096)  0))
  goto cleanup;
 -
 -if (xenXMConfigGetString(conf, on_poweroff, str, destroy)  0)
 -goto cleanup;
 -if ((def-onPoweroff = virDomainLifecycleTypeFromString(str))  0) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(unexpected value %s for on_poweroff), str);
 -goto cleanup;
 -}
 -
 -if (xenXMConfigGetString(conf, on_reboot, str, restart)  0)
 -goto cleanup;
 -if ((def-onReboot = virDomainLifecycleTypeFromString(str))  0) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(unexpected value %s for on_reboot), str);
 -goto cleanup;
 -}
 -
 -if (xenXMConfigGetString(conf, on_crash, str, restart)  0)
 -goto cleanup;
 -if ((def-onCrash = virDomainLifecycleCrashTypeFromString(str))  0) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(unexpected value %s for on_crash), str);
 -goto cleanup;
 -}
 -
 -
 -
  if (hvm) {
  if (xenXMConfigGetBool(conf, pae, val, 0)  0)
  goto cleanup;
   

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

Re: [libvirt] [PATCH v2 03/25] src/xenxs:Refactor code parsing virtual time controls config

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 Introduce function
  xenParseXMTimeOffset(virConfPtr conf,..);
 which parses virtual config instead

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 73 
 --
  1 file changed, 44 insertions(+), 29 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index d0a166d..c816198 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -266,6 +266,48 @@ int xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
   * Turn a config record into a lump of XML describing the
   * domain, suitable for later feeding for virDomainCreateXML
   */
   

As mentioned in 2/25, this comment should stay with xenParseXM().

Regards,
Jim

 +
 +
 +static
 +int xenParseXMTimeOffset(virConfPtr conf, virDomainDefPtr def,
 + int xendConfigVersion)
 +{
 +int vmlocaltime;
 +
 +if (xenXMConfigGetBool(conf, localtime, vmlocaltime, 0)  0)
 +return -1;
 +
 +if (STREQ(def-os.type, hvm)) {
 +/* only managed HVM domains since 3.1.0 have persistent 
 rtc_timeoffset */
 +if (xendConfigVersion  XEND_CONFIG_VERSION_3_1_0) {
 +if (vmlocaltime)
 +def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
 +else
 +def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
 +def-clock.data.utc_reset = true;
 +} else {
 +unsigned long rtc_timeoffset;
 +def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
 +if (xenXMConfigGetULong(conf, rtc_timeoffset, rtc_timeoffset, 
 0)  0)
 +return -1;
 +
 +def-clock.data.variable.adjustment = (int)rtc_timeoffset;
 +def-clock.data.variable.basis = vmlocaltime ?
 +VIR_DOMAIN_CLOCK_BASIS_LOCALTIME :
 +VIR_DOMAIN_CLOCK_BASIS_UTC;
 +}
 +} else {
 +/* PV domains do not have an emulated RTC and the offset is fixed. */
 +def-clock.offset = vmlocaltime ?
 +VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME :
 +VIR_DOMAIN_CLOCK_OFFSET_UTC;
 +def-clock.data.utc_reset = true;
 +} /* !hvm */
 +
 +return 0;
 +}
 +
 +
  static
  int xenParseXMEventsActions(virConfPtr conf, virDomainDefPtr def)
  {
 @@ -317,7 +359,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  virDomainHostdevDefPtr hostdev = NULL;
  size_t i;
  const char *defaultMachine;
 -int vmlocaltime = 0;
  unsigned long count;
  char *script = NULL;
  char *listenAddr = NULL;
 @@ -468,35 +509,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  def-clock.timers[0] = timer;
  }
  }
 -if (xenXMConfigGetBool(conf, localtime, vmlocaltime, 0)  0)
 -goto cleanup;
 -
 -if (hvm) {
 -/* only managed HVM domains since 3.1.0 have persistent 
 rtc_timeoffset */
 -if (xendConfigVersion  XEND_CONFIG_VERSION_3_1_0) {
 -if (vmlocaltime)
 -def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
 -else
 -def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
 -def-clock.data.utc_reset = true;
 -} else {
 -unsigned long rtc_timeoffset;
 -def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
 -if (xenXMConfigGetULong(conf, rtc_timeoffset, rtc_timeoffset, 
 0)  0)
 -goto cleanup;
 -def-clock.data.variable.adjustment = (int)rtc_timeoffset;
 -def-clock.data.variable.basis = vmlocaltime ?
 -VIR_DOMAIN_CLOCK_BASIS_LOCALTIME :
 -VIR_DOMAIN_CLOCK_BASIS_UTC;
 -}
 -} else {
 -/* PV domains do not have an emulated RTC and the offset is fixed. */
 -def-clock.offset = vmlocaltime ?
 -VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME :
 -VIR_DOMAIN_CLOCK_OFFSET_UTC;
 -def-clock.data.utc_reset = true;
 -} /* !hvm */
  
 +if (xenParseXMTimeOffset(conf, def, xendConfigVersion)  0)
 +goto cleanup;
  if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
  goto cleanup;
  
   

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

[libvirt] link error on cygwin

Even though I pushed a last-minute patch for 1.2.7 to fix the compiler
phase of the build on cygwin, I'm stumped on how to fix this error from
the link phase:

  CCLD libvirtd.exe
../src/.libs/libvirt_driver_remote.a(libvirt_net_rpc_server_la-virnetserver.oo):
In function `virNetServerNew':
/home/eblake/libvirt/src/rpc/virnetserver.c:357: multiple definition of
`virNetServerNew'
/home/eblake/libvirt/src/.libs/libvirt.dll.a(d001949.o):(.text+0x0):
first defined here

and several others.  Looks like we are including a link library through
two paths, and cygwin is not as tolerant of that as Linux; but I'm not
sure where in the Makefile.am to tweak to try and fix it.

-- 
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 06/25] src/xenxs:Refactor code parsing disk config

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 Introduce function
  xenParseXMDisk(virConfPtr conf,..);
 which parses XM disk config instead

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 302 
 -
  1 file changed, 157 insertions(+), 145 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 66d7b44..2c36c1b 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -507,126 +507,15 @@ int xenParseXMCPUFeatures(virConfPtr conf, 
 virDomainDefPtr def)
  }
  
  
 -virDomainDefPtr
 -xenParseXM(virConfPtr conf, int xendConfigVersion,
 -   virCapsPtr caps)
 +static
 +int xenParseXMDisk(virConfPtr conf, virDomainDefPtr def,
 +   int xendConfigVersion)
   

libvirt style is to have function return type and name on separate
lines, e.g.

static int
xenParseXMDisk(...)

  {
 -const char *str;
 -int hvm = 0;
 -int val;
 -virConfValuePtr list;
 -virDomainDefPtr def = NULL;
 +const char *str = NULL;
  virDomainDiskDefPtr disk = NULL;
 -virDomainNetDefPtr net = NULL;
 -virDomainGraphicsDefPtr graphics = NULL;
 -size_t i;
 -const char *defaultMachine;
 -char *script = NULL;
 -char *listenAddr = NULL;
 -
 -if (VIR_ALLOC(def)  0)
 -return NULL;
 -
 -def-virtType = VIR_DOMAIN_VIRT_XEN;
 -def-id = -1;
 -
 -if (xenXMConfigCopyString(conf, name, def-name)  0)
 -goto cleanup;
 -if (xenXMConfigGetUUID(conf, uuid, def-uuid)  0)
 -goto cleanup;
 -
 -
 -if ((xenXMConfigGetString(conf, builder, str, linux) == 0) 
 -STREQ(str, hvm))
 -hvm = 1;
 -
 -if (VIR_STRDUP(def-os.type, hvm ? hvm : xen)  0)
 -goto cleanup;
 -
 -def-os.arch =
 -virCapabilitiesDefaultGuestArch(caps,
 -def-os.type,
 -
 virDomainVirtTypeToString(def-virtType));
 -if (!def-os.arch) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(no supported architecture for os type '%s'),
 -   def-os.type);
 -goto cleanup;
 -}
 -
 -defaultMachine = virCapabilitiesDefaultGuestMachine(caps,
 -def-os.type,
 -def-os.arch,
 -
 virDomainVirtTypeToString(def-virtType));
 -if (defaultMachine != NULL) {
 -if (VIR_STRDUP(def-os.machine, defaultMachine)  0)
 -goto cleanup;
 -}
 -
 -if (hvm) {
 -const char *boot;
 -if (xenXMConfigCopyString(conf, kernel, def-os.loader)  0)
 -goto cleanup;
 -
 -if (xenXMConfigGetString(conf, boot, boot, c)  0)
 -goto cleanup;
 -
 -for (i = 0; i  VIR_DOMAIN_BOOT_LAST  boot[i]; i++) {
 -switch (*boot) {
 -case 'a':
 -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
 -break;
 -case 'd':
 -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
 -break;
 -case 'n':
 -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
 -break;
 -case 'c':
 -default:
 -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
 -break;
 -}
 -def-os.nBootDevs++;
 -}
 -} else {
 -const char *extra, *root;
 -
 -if (xenXMConfigCopyStringOpt(conf, bootloader, 
 def-os.bootloader)  0)
 -goto cleanup;
 -if (xenXMConfigCopyStringOpt(conf, bootargs, 
 def-os.bootloaderArgs)  0)
 -goto cleanup;
 -
 -if (xenXMConfigCopyStringOpt(conf, kernel, def-os.kernel)  0)
 -goto cleanup;
 -if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd)  0)
 -goto cleanup;
 -if (xenXMConfigGetString(conf, extra, extra, NULL)  0)
 -goto cleanup;
 -if (xenXMConfigGetString(conf, root, root, NULL)  0)
 -goto cleanup;
 -
 -if (root) {
 -if (virAsprintf(def-os.cmdline, root=%s %s, root, extra)  0)
 -goto cleanup;
 -} else {
 -if (VIR_STRDUP(def-os.cmdline, extra)  0)
 -goto cleanup;
 -}
 -}
 -
 -if (xenParseXMMem(conf, def)  0)
 -goto cleanup;
 -if (xenParseXMEventsActions(conf, def)  0)
 -goto cleanup;
 -if (xenParseXMTimeOffset(conf, def, xendConfigVersion)  0)
 -goto cleanup;
 -if (xenParseXMCPUFeatures(conf, def)  0)
 -goto cleanup;
 -if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
 -goto cleanup;
 +int hvm = STREQ(def-os.type, hvm);
 +virConfValuePtr list = virConfGetValue(conf, disk);
  
 -list = virConfGetValue(conf,

Re: [libvirt] [PATCH v2 07/25] src/xenxs:Refactor code parsing Vfb config

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 introduce function
   xenParseXMVfb(virConfPtr conf,...)
 which parses Vfb config

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 296 
 -
  1 file changed, 154 insertions(+), 142 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 2c36c1b..38434be 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -695,6 +695,158 @@ int xenParseXMDisk(virConfPtr conf, virDomainDefPtr def,
  return 0;
  }
  
 +static
 +int xenParseXMVfb(virConfPtr conf, virDomainDefPtr def,
 +  int xendConfigVersion)
   

Ah, just noticed all the patches thus far have the function return type
and name on the same line.  Remember that the libvirt pattern is to have
them on separate lines, e.g.

static int
xenParsexmVfb()


 +{
 +int val;
 +char *listenAddr = NULL;
 +//const char *str = NULL;
 +int hvm = STREQ(def-os.type, hvm);
 +virConfValuePtr list;
 +virDomainGraphicsDefPtr graphics = NULL;
 +
 +if (hvm || xendConfigVersion  XEND_CONFIG_VERSION_3_0_4) {
 +if (xenXMConfigGetBool(conf, vnc, val, 0)  0)
 +goto cleanup;
 +if (val) {
 +if (VIR_ALLOC(graphics)  0)
 +goto cleanup;
 +graphics-type = VIR_DOMAIN_GRAPHICS_TYPE_VNC;
 +if (xenXMConfigGetBool(conf, vncunused, val, 1)  0)
 +goto cleanup;
 +graphics-data.vnc.autoport = val ? 1 : 0;
 +if (!graphics-data.vnc.autoport) {
 +unsigned long vncdisplay;
 +if (xenXMConfigGetULong(conf, vncdisplay, vncdisplay, 0) 
  0)
 +goto cleanup;
 +graphics-data.vnc.port = (int)vncdisplay + 5900;
 +}
 +
 +if (xenXMConfigCopyStringOpt(conf, vnclisten, listenAddr)  0)
 +goto cleanup;
 +if (listenAddr 
 +virDomainGraphicsListenSetAddress(graphics, 0, listenAddr,
 +  -1, true)  0) {
 +   goto cleanup;
 +}
 +
 +VIR_FREE(listenAddr);
 +if (xenXMConfigCopyStringOpt(conf, vncpasswd, 
 graphics-data.vnc.auth.passwd)  0)
 +goto cleanup;
 +if (xenXMConfigCopyStringOpt(conf, keymap, 
 graphics-data.vnc.keymap)  0)
 +goto cleanup;
 +if (VIR_ALLOC_N(def-graphics, 1)  0)
 +goto cleanup;
 +def-graphics[0] = graphics;
 +def-ngraphics = 1;
 +graphics = NULL;
 +} else {
 +if (xenXMConfigGetBool(conf, sdl, val, 0)  0)
 +goto cleanup;
 +if (val) {
 +if (VIR_ALLOC(graphics)  0)
 +goto cleanup;
 +graphics-type = VIR_DOMAIN_GRAPHICS_TYPE_SDL;
 +if (xenXMConfigCopyStringOpt(conf, display, 
 graphics-data.sdl.display)  0)
 +goto cleanup;
 +if (xenXMConfigCopyStringOpt(conf, xauthority, 
 graphics-data.sdl.xauth)  0)
 +goto cleanup;
 +if (VIR_ALLOC_N(def-graphics, 1)  0)
 +goto cleanup;
 +def-graphics[0] = graphics;
 +def-ngraphics = 1;
 +graphics = NULL;
 +}
 +}
 +}
 +
 +if (!hvm  def-graphics == NULL) { /* New PV guests use this format */
 +list = virConfGetValue(conf, vfb);
 +if (list  list-type == VIR_CONF_LIST 
 +list-list  list-list-type == VIR_CONF_STRING 
 +list-list-str) {
 +char vfb[MAX_VFB];
 +char *key = vfb;
 +
 +if (virStrcpyStatic(vfb, list-list-str) == NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(VFB %s too big for destination),
 +   list-list-str);
 +goto cleanup;
 +}
 +
 +if (VIR_ALLOC(graphics)  0)
 +goto cleanup;
 +if (strstr(key, type=sdl))
 +graphics-type = VIR_DOMAIN_GRAPHICS_TYPE_SDL;
 +else
 +graphics-type = VIR_DOMAIN_GRAPHICS_TYPE_VNC;
 +while (key) {
 +char *nextkey = strchr(key, ',');
 +char *end = nextkey;
 +if (nextkey) {
 +*end = '\0';
 +nextkey++;
 +}
 +
 +if (!strchr(key, '='))
 +break;
 +if (graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
 +if (STRPREFIX(key, vncunused=)) {
 +if (STREQ(key + 10, 1))
 +graphics-data.vnc.autoport = true;
 +} else if (STRPREFIX(key, vnclisten=)) {
 +if

Re: [libvirt] [PATCH v2 10/25] src/xenxs:Refactor code parsing Vif

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 introduce function
   xenParseXMVif(virConfPtr conf, ...)
 which parses char Vif config instead

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 161 
 -
  1 file changed, 86 insertions(+), 75 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 48b68fb..e6a1b7d 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -1011,73 +1011,13 @@ int xenParseXMCharDev(virConfPtr conf, 
 virDomainDefPtr def)
  }
  
  
 -virDomainDefPtr
 -xenParseXM(virConfPtr conf, int xendConfigVersion,
 -   virCapsPtr caps)
 +static
 +int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
   

I'll stop mentioning return type and function name of separate lines :-).

  {
 -const char *str;
 -int hvm = 0;
 -virConfValuePtr list;
 -virDomainDefPtr def = NULL;
 -virDomainDiskDefPtr disk = NULL;
 -virDomainNetDefPtr net = NULL;
 -const char *defaultMachine;
  char *script = NULL;
 +virDomainNetDefPtr net = NULL;
 +virConfValuePtr list = virConfGetValue(conf, vif);
  
 -if (VIR_ALLOC(def)  0)
 -return NULL;
 -
 -def-virtType = VIR_DOMAIN_VIRT_XEN;
 -def-id = -1;
 -
 -if (xenXMConfigCopyString(conf, name, def-name)  0)
 -goto cleanup;
 -if (xenXMConfigGetUUID(conf, uuid, def-uuid)  0)
 -goto cleanup;
 -
 -
 -if ((xenXMConfigGetString(conf, builder, str, linux) == 0) 
 -STREQ(str, hvm))
 -hvm = 1;
 -
 -if (VIR_STRDUP(def-os.type, hvm ? hvm : xen)  0)
 -goto cleanup;
 -
 -def-os.arch =
 -virCapabilitiesDefaultGuestArch(caps,
 -def-os.type,
 -
 virDomainVirtTypeToString(def-virtType));
 -if (!def-os.arch) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(no supported architecture for os type '%s'),
 -   def-os.type);
 -goto cleanup;
 -}
 -
 -defaultMachine = virCapabilitiesDefaultGuestMachine(caps,
 -def-os.type,
 -def-os.arch,
 -
 virDomainVirtTypeToString(def-virtType));
 -if (defaultMachine != NULL) {
 -if (VIR_STRDUP(def-os.machine, defaultMachine)  0)
 -goto cleanup;
 -}
 -
 -if (xenParseXMOS(conf, def)  0)
 -goto cleanup;
 -if (xenParseXMMem(conf, def)  0)
 -goto cleanup;
 -if (xenParseXMEventsActions(conf, def)  0)
 -goto cleanup;
 -if (xenParseXMTimeOffset(conf, def, xendConfigVersion)  0)
 -goto cleanup;
 -if (xenParseXMCPUFeatures(conf, def)  0)
 -goto cleanup;
 -if (xenParseXMDisk(conf, def, xendConfigVersion)  0)
 -goto cleanup;
 -if (xenParseXMVfb(conf, def, xendConfigVersion)  0)
 -goto cleanup;
 -list = virConfGetValue(conf, vif);
  if (list  list-type == VIR_CONF_LIST) {
  list = list-list;
  while (list) {
 @@ -1098,7 +1038,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  
  if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
  goto skipnic;
 -
   

More spurious whitespace changes.

  key = list-str;
  while (key) {
  char *data;
 @@ -1107,7 +1046,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  if (!(data = strchr(key, '=')))
  goto skipnic;
  data++;
 -
   

Here too.

  if (STRPREFIX(key, mac=)) {
  int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
  if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
 @@ -1133,14 +1071,16 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  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);
 +   _(Model %s too big for destination),
 +   data);
  goto skipnic;
  }
  } 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);
 +   _(Type %s too big for destination),
 +

Re: [libvirt] [PATCH v2 14/25] src/xenxs:Refactor code formating general config

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 introduce function
  xenFormatXMGeneralMeta(virConfPtr conf, ...);
 which formats UUID and name instead

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 24 +++-
  1 file changed, 15 insertions(+), 9 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 289f0ad..ae474a4 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -1655,6 +1655,20 @@ xenFormatXMPCI(virConfPtr conf,
  /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
 either 32, or 64 on a platform where long is big enough.  */
  verify(MAX_VIRT_CPUS = sizeof(1UL) * CHAR_BIT);
 +static
 +int xenFormatXMGeneralMeta(virConfPtr conf, virDomainDefPtr def)
 +{
 +char uuid[VIR_UUID_STRING_BUFLEN];
 +
 +if (xenXMConfigSetString(conf, name, def-name)  0)
 +return -1;
 +
 +virUUIDFormat(def-uuid, uuid);
 +if (xenXMConfigSetString(conf, uuid, uuid)  0)
 +return -1;
 +
 +return 0;
 +}
  
  virConfPtr xenFormatXM(virConnectPtr conn,
 virDomainDefPtr def,
 @@ -1665,21 +1679,13 @@ virConfPtr xenFormatXM(virConnectPtr conn,
  size_t i;
  char *cpus = NULL;
  const char *lifecycle;
 -char uuid[VIR_UUID_STRING_BUFLEN];
  virConfValuePtr diskVal = NULL;
  virConfValuePtr netVal = NULL;
  
  if (!(conf = virConfNew()))
  goto cleanup;
 -
 -
   

For readability and consistency, I'd leave one of these blank lines.

 -if (xenXMConfigSetString(conf, name, def-name)  0)
 +if (xenFormatXMGeneralMeta(conf, def)  0)
  goto cleanup;
 -
   

And this one.

 -virUUIDFormat(def-uuid, uuid);
 -if (xenXMConfigSetString(conf, uuid, uuid)  0)
 -goto cleanup;
 -
   

And this one too.

Regards,
Jim

  if (xenXMConfigSetInt(conf, maxmem,
VIR_DIV_UP(def-mem.max_balloon, 1024))  0)
  goto cleanup;
   

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

Re: [libvirt] [PATCH v2 15/25] src/xenxs:Refactor code formating memory config

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 introduce function
  xenFormatXMMem(virConfPtr conf, ...);
 which formats mem and max-mem instead.This could be joined into
 XenFormatXMGeneralMeta but I separated them for consistency

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 24 +---
  1 file changed, 17 insertions(+), 7 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index ae474a4..78dc949 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -1670,6 +1670,22 @@ int xenFormatXMGeneralMeta(virConfPtr conf, 
 virDomainDefPtr def)
  return 0;
  }
  
 +
 +static
 +int xenFormatXMMem(virConfPtr conf, virDomainDefPtr def)
 +{
 +if (xenXMConfigSetInt(conf, maxmem,
 +  VIR_DIV_UP(def-mem.max_balloon, 1024))  0)
 +return -1;
 +
 +if (xenXMConfigSetInt(conf, memory,
 +  VIR_DIV_UP(def-mem.cur_balloon, 1024))  0)
 +return -1;
 +
 +return 0;
 +}
 +
 +
  virConfPtr xenFormatXM(virConnectPtr conn,
 virDomainDefPtr def,
 int xendConfigVersion)
 @@ -1686,14 +1702,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
  goto cleanup;
  if (xenFormatXMGeneralMeta(conf, def)  0)
  goto cleanup;
 -if (xenXMConfigSetInt(conf, maxmem,
 -  VIR_DIV_UP(def-mem.max_balloon, 1024))  0)
 +if (xenFormatXMMem(conf, def)  0)
  goto cleanup;
 -
 -if (xenXMConfigSetInt(conf, memory,
 -  VIR_DIV_UP(def-mem.cur_balloon, 1024))  0)
 -goto cleanup;
 -
   

Same comment here about preserving the whitespace.

Regards,
Jim

  if (xenXMConfigSetInt(conf, vcpus, def-maxvcpus)  0)
  goto cleanup;
  /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
   

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

Re: [libvirt] [PATCH v2 17/25] src/xenxs:Refactor code formating event actions config

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 introduce function
   xenFormatXMEventActions(virConfPtr conf, ...);
 which formats event actions config instead

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 64 
 ++
  1 file changed, 36 insertions(+), 28 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 9d868d7..bb7979d 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -1768,6 +1768,41 @@ int xenFormatXMTimeOffset(virConfPtr conf, 
 virDomainDefPtr def,
  }
  
  
 +static
 +int xenFormatXMEventActions(virConfPtr conf, virDomainDefPtr def)
 +{
 +const char *lifecycle = NULL;
 +
 +if (!(lifecycle = virDomainLifecycleTypeToString(def-onPoweroff))) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unexpected lifecycle action %d), def-onPoweroff);
 +return -1;
 +}
 +if (xenXMConfigSetString(conf, on_poweroff, lifecycle)  0)
 +return -1;
 +
 +
   

One blank line too many.

 +if (!(lifecycle = virDomainLifecycleTypeToString(def-onReboot))) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unexpected lifecycle action %d), def-onReboot);
 +return -1;
 +}
 +if (xenXMConfigSetString(conf, on_reboot, lifecycle)  0)
 +return -1;
 +
 +
   

Same here.

 +if (!(lifecycle = virDomainLifecycleCrashTypeToString(def-onCrash))) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unexpected lifecycle action %d), def-onCrash);
 +return -1;
 +}
 +if (xenXMConfigSetString(conf, on_crash, lifecycle)  0)
 +return -1;
 +
 +return 0;
 +}
 +
 +
  virConfPtr xenFormatXM(virConnectPtr conn,
 virDomainDefPtr def,
 int xendConfigVersion)
 @@ -1776,7 +1811,6 @@ virConfPtr xenFormatXM(virConnectPtr conn,
  int hvm = 0;
  size_t i;
  char *cpus = NULL;
 -const char *lifecycle;
  virConfValuePtr diskVal = NULL;
  virConfValuePtr netVal = NULL;
  
 @@ -1911,34 +1945,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
  
  if (xenFormatXMTimeOffset(conf, def, xendConfigVersion)  0)
  goto cleanup;
 -if (!(lifecycle = virDomainLifecycleTypeToString(def-onPoweroff))) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(unexpected lifecycle action %d), def-onPoweroff);
 -goto cleanup;
 -}
 -if (xenXMConfigSetString(conf, on_poweroff, lifecycle)  0)
 -goto cleanup;
 -
 -
 -if (!(lifecycle = virDomainLifecycleTypeToString(def-onReboot))) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(unexpected lifecycle action %d), def-onReboot);
 -goto cleanup;
 -}
 -if (xenXMConfigSetString(conf, on_reboot, lifecycle)  0)
 +if (xenFormatXMEventActions(conf, def)  0)
  goto cleanup;
 -
 -
 -if (!(lifecycle = virDomainLifecycleCrashTypeToString(def-onCrash))) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(unexpected lifecycle action %d), def-onCrash);
 -goto cleanup;
 -}
 -if (xenXMConfigSetString(conf, on_crash, lifecycle)  0)
 -goto cleanup;
 -
 -
 -
   

And look like too much whitespace stripped here.

Regards,
Jim

  if (hvm) {
  if (def-emulator 
  xenXMConfigSetString(conf, device_model, def-emulator)  0)
   

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

Re: [libvirt] [PATCH v2 16/25] src/xenxs:Reafactor code formating virtual time config

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 introduce function
   xenFormatXMTimeOffset(virConfPtr conf, ...);
 which formats time offset config instead

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 156 
 -
  1 file changed, 84 insertions(+), 72 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 78dc949..9d868d7 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -1686,12 +1686,94 @@ int xenFormatXMMem(virConfPtr conf, virDomainDefPtr 
 def)
  }
  
  
 +static
 +int xenFormatXMTimeOffset(virConfPtr conf, virDomainDefPtr def,
 +  int xendConfigVersion)
 +{
 +int vmlocaltime;
 +if (xendConfigVersion  XEND_CONFIG_VERSION_3_1_0) {
 +/* 3.1: UTC and LOCALTIME */
 +switch (def-clock.offset) {
 +case VIR_DOMAIN_CLOCK_OFFSET_UTC:
 +vmlocaltime = 0;
 +break;
 +case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
 +vmlocaltime = 1;
 +break;
 +default:
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   _(unsupported clock offset='%s'),
 +   
 virDomainClockOffsetTypeToString(def-clock.offset));
 +return -1;
 +}
 +
 +} else {
 +if (STREQ(def-os.type, hvm)) {
 +/* =3.1 HV: VARIABLE */
 +int rtc_timeoffset;
 +switch (def-clock.offset) {
 +case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
 +vmlocaltime = (int)def-clock.data.variable.basis;
 +rtc_timeoffset = def-clock.data.variable.adjustment;
 +break;
 +case VIR_DOMAIN_CLOCK_OFFSET_UTC:
 +if (def-clock.data.utc_reset) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(unsupported clock 
 adjustment='reset'));
 +return -1;
 +}
 +vmlocaltime = 0;
 +rtc_timeoffset = 0;
 +break;
 +case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
 +if (def-clock.data.utc_reset) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(unsupported clock 
 adjustment='reset'));
 +return -1;
 +}
 +vmlocaltime = 1;
 +rtc_timeoffset = 0;
 +break;
 +default:
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   _(unsupported clock offset='%s'),
 +   
 virDomainClockOffsetTypeToString(def-clock.offset));
 +return -1;
 +}
 +if (xenXMConfigSetInt(conf, rtc_timeoffset, rtc_timeoffset)  
 0)
 +return -1;
 +
 +} else {
 +/* =3.1 PV: UTC and LOCALTIME */
 +switch (def-clock.offset) {
 +case VIR_DOMAIN_CLOCK_OFFSET_UTC:
 +vmlocaltime = 0;
 +break;
 +case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
 +vmlocaltime = 1;
 +break;
 +default:
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   _(unsupported clock offset='%s'),
 +   
 virDomainClockOffsetTypeToString(def-clock.offset));
 +return -1;
 +}
 +} /* !hvm */
 +}
 +
 +if (xenXMConfigSetInt(conf, localtime, vmlocaltime)  0)
 +return -1;
 +
 +return 0;
 +}
 +
 +
  virConfPtr xenFormatXM(virConnectPtr conn,
 virDomainDefPtr def,
 int xendConfigVersion)
  {
  virConfPtr conf = NULL;
 -int hvm = 0, vmlocaltime = 0;
 +int hvm = 0;
  size_t i;
  char *cpus = NULL;
  const char *lifecycle;
 @@ -1827,78 +1909,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
  goto cleanup;
  } /* !hvm */
  
 -
 -if (xendConfigVersion  XEND_CONFIG_VERSION_3_1_0) {
 -/* 3.1: UTC and LOCALTIME */
 -switch (def-clock.offset) {
 -case VIR_DOMAIN_CLOCK_OFFSET_UTC:
 -vmlocaltime = 0;
 -break;
 -case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
 -vmlocaltime = 1;
 -break;
 -default:
 -virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 -   _(unsupported clock offset='%s'),
 -   
 virDomainClockOffsetTypeToString(def-clock.offset));
 -goto cleanup;
 -}
 -} else {
 -if (hvm) {
 -/* =3.1 HV: VARIABLE */
 -int rtc_timeoffset;
 -switch (def-clock.offset) {
 -case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
 -vmlocaltime =

Re: [libvirt] [PATCH v2 18/25] src/xenxs:Refactor code formating Vif config

David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 introduce function
   xenFormatXMVif(virConfPtr conf, );
 which formats Vif config instead

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 60 
 ++
  1 file changed, 38 insertions(+), 22 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index bb7979d..9a6a827 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -1803,16 +1803,49 @@ int xenFormatXMEventActions(virConfPtr conf, 
 virDomainDefPtr def)
  }
  
  
 -virConfPtr xenFormatXM(virConnectPtr conn,
 -   virDomainDefPtr def,
 -   int xendConfigVersion)
 +static
 +int xenFormatXMVif(virConfPtr conf, virConnectPtr conn,
 +   virDomainDefPtr def, int xendConfigVersion)
   

With the exception of my usual whitespace nits, the rest of this series
looks good.  Please address all of the whitespace comments in V3.  Thanks!

Regards,
Jim

 +{
 +   virConfValuePtr netVal = NULL;
 +   size_t i;
 +   int hvm = STREQ(def-os.type, hvm);
 +
 +   if (VIR_ALLOC(netVal)  0)
 +goto cleanup;
 +netVal-type = VIR_CONF_LIST;
 +netVal-list = NULL;
 +
 +for (i = 0; i  def-nnets; i++) {
 +if (xenFormatXMNet(conn, netVal, def-nets[i],
 +   hvm, xendConfigVersion)  0)
 +   goto cleanup;
 +}
 +
 +if (netVal-list != NULL) {
 +int ret = virConfSetValue(conf, vif, netVal);
 +netVal = NULL;
 +if (ret  0)
 +goto cleanup;
 +}
 +
 +VIR_FREE(netVal);
 +return 0;
 +
 + cleanup:
 +virConfFreeValue(netVal);
 +return -1;
 +}
 +
 +
 +virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr def,
 +   int xendConfigVersion)
  {
  virConfPtr conf = NULL;
  int hvm = 0;
  size_t i;
  char *cpus = NULL;
  virConfValuePtr diskVal = NULL;
 -virConfValuePtr netVal = NULL;
  
  if (!(conf = virConfNew()))
  goto cleanup;
 @@ -2102,24 +2135,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
  }
  VIR_FREE(diskVal);
  
 -if (VIR_ALLOC(netVal)  0)
 +if (xenFormatXMVif(conf, conn, def, xendConfigVersion)  0)
  goto cleanup;
 -netVal-type = VIR_CONF_LIST;
 -netVal-list = NULL;
 -
 -for (i = 0; i  def-nnets; i++) {
 -if (xenFormatXMNet(conn, netVal, def-nets[i],
 -   hvm, xendConfigVersion)  0)
 -goto cleanup;
 -}
 -if (netVal-list != NULL) {
 -int ret = virConfSetValue(conf, vif, netVal);
 -netVal = NULL;
 -if (ret  0)
 -goto cleanup;
 -}
 -VIR_FREE(netVal);
 -
  if (xenFormatXMPCI(conf, def)  0)
  goto cleanup;
  
 @@ -2214,7 +2231,6 @@ virConfPtr xenFormatXM(virConnectPtr conn,
  
   cleanup:
  virConfFreeValue(diskVal);
 -virConfFreeValue(netVal);
  VIR_FREE(cpus);
  if (conf)
  virConfFree(conf);
   

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

Re: [libvirt] [PATCH v2 8/8] hostdev: Add iSCSI hostdev XML

2014-08-04 Thread John Ferlan



On 08/04/2014 02:21 PM, Eric Blake wrote:
 On 07/21/2014 02:47 PM, John Ferlan wrote:
 Introduce a new structure to handle an iSCSI host device based on the
 existing virDomainHostdevSubsysSCSI by adding a protocol='iscsi' to
 the source/ element.  The hostdev structure mimics the existing
 disk/ element for an iSCSI device (network) device. New XML is:

   hostdev mode='subsystem' type='scsi' managed='yes'
 auth username='myname'
   secret type='iscsi' usage='mycluster_myname'/
 /auth
 source protocol='iscsi' name='iqn.1992-01.com.example'
   host name='example.org' port='3260'/
 /source
 address type='drive' controller='0' bus='0' target='2' unit='5'/
   /hostdev

 The controller element will mimic the existing scsi_host code insomuch
 as when 'lsi' and 'virtio-scsi' are used.

 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  docs/formatdomain.html.in  | 142 ---
  docs/schemas/domaincommon.rng  |  46 ++-
  src/conf/domain_conf.c | 152 
 ++---
  .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args  |  14 ++
  .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml   |  46 +++
  .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args   |  14 ++
  .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml|  40 ++
  ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args |  16 +++
  ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml |  46 +++
  .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args|  16 +++
  .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml |  40 ++
  tests/qemuxml2argvtest.c   |  16 +++
  tests/qemuxml2xmltest.c|   5 +
  13 files changed, 524 insertions(+), 69 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml

 
 +  dtusb/dt
 +  ddFor USB devices, the user is responsible to call
 +codevirNodeDeviceDettach/code (or
 +codevirsh nodedev-dettach/code) before starting the guest
 
 The (preferred) virsh command is nodedev-detach (we can't correct the
 API typo, but did correct the virsh typo).
 
 
Fixed (plus another such typo)

 +  dtscsi/dt
 +  ddSCSI devices are described by both the codeadapter/code
 +and codeaddress/code elements.
 +p
 +span class=sinceSince 1.2.7/span, the codesource/code
 
 1.2.8 now.
 
 

Updated...

 +++ b/docs/schemas/domaincommon.rng
 @@ -3563,13 +3563,47 @@
  /choice
/attribute
  /optional
 +optional
 +  ref name='diskAuth'/
 +/optional
 
 Does diskAuth work for all configurations, or only for the new iscsi
 configuration?  By putting it here, you are allowing it for all users,...
 
  element name=source
 -  interleave
 -ref name=sourceinfoadapter/
 -element name=address
 -  ref name=scsiaddress/
 -/element
 -  /interleave
 +  optional
 +attribute name=protocol
 +  choice
 +valueiscsi/value
 +  /choice
 
 ...whereas using some group magic could make it valid only when
 protocol is iscsi.
 


So clearly this RNG syntax is BFM and confusing...

The goal is/was to copy the same element logic from the disk element,
but unlike disk it will only be valid for iscsi. It should have the
following format:


  devices
hostdev mode='subsystem' type='scsi'
  source protocol='iscsi'
name='iqn.2014-08.com.example:iscsi-nopool/1'
host name='example.com' port='3260'/
  /source
  auth username='myuser'
secret type='iscsi' usage='libvirtiscsi'/
  /auth
  address type='drive' controller='0' bus='0' target='0' unit='0'/
/hostdev
  /devices
  ...

where source would be reqd, auth would be option

It's not very clear to me where/how to place it in the rng file...


 +/attribute
 +attribute name=name
 +  text/
 +/attribute
 +  /optional
 +  choice
 +group
 +  interleave
 +ref name=sourceinfoadapter/
 +element name=address
 +  ref name=scsiaddress/
 +/element
 +  /interleave
 +/group
 +group
 +  interleave
 +oneOrMore
 +  element

[libvirt] qemu driver fails to load on Xen after be0782e1

Until recently, the qemu driver has always succeeded in loading even on
a system booted to Xen.  Commit be0782e1 broke this, hence libvirtd no
longer loads

xendom0 # /usr/sbin/libvirtd -l
2014-08-04 22:27:31.586+: 35859: info : libvirt version: 1.2.8
2014-08-04 22:27:31.586+: 35859: error :
virFileGetDefaultHugepageSize:2958 : internal error: Unable to parse
/proc/meminfo
2014-08-04 22:27:31.586+: 35859: error : virStateInitialize:749 :
Initialization of QEMU state driver failed: internal error: Unable to
parse /proc/meminfo
2014-08-04 22:27:31.586+: 35859: error : daemonRunStateInit:922 :
Driver state initialization failed

Problem is the Xen kernel does not provide any huge page info in
/proc/meminfo

xendom0 # cat /proc/meminfo | grep -i huge
xendom0 #

Michal, short of always building with '--without-qemu' on a Xen system,
is it possible to make detection of huge pages less fatal wrt loading
the qemu driver?

Regards,
Jim

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

[libvirt] [PATCH] libxl: support domainReset