Re: [libvirt] [libvirt RFC PATCH 04/10] util: storage: Add support for URI based backing volumes in qemu's JSON pseudo-protocol

[Eric Blake]

On 07/15/2016 07:46 AM, Peter Krempa wrote:
> http(s), ftp(s) and tftp use URIs for volume definitions in the JSON
> pseudo protocol so it's pretty straightforward to add support for them.
> ---
>  src/util/virstoragefile.c | 34 ++
>  tests/virstoragetest.c|  8 
>  2 files changed, 42 insertions(+)

Still might be nice to include an example in the commit message, not
just the testsuite addition.

I know that qemu is hoping to move away from URI towards more structured
layouts as part of adding blockdev-add support for these drivers (back
to your observation that once qemu supports two ways, it will have to
continue to support both types of users).  That means we may need more
code to parse non-URI (but better-structured) uses, but we'd still need
this uri parsing.


> +++ b/tests/virstoragetest.c
> @@ -1367,6 +1367,14 @@ mymain(void)
>  TEST_BACKING_PARSE(12, "json:{\"file.driver\":\"host_cdrom\", "
>   "\"file.filename\":\"/path/to/cdrom\"}",
> "\n");
> +TEST_BACKING_PARSE(13, "json:{\"file.driver\":\"http\", "
> + "\"file.uri\":\"http://example.com/file\"};,
> +   "\n"
> +   "  \n"
> +   "\n");
> +TEST_BACKING_PARSE(14, "json:{\"file.driver\":\"ftp\", "
> + "\"file.uri\":\"http://example.com/file\"};,
> +   NULL);
> 

On the bright side, this patch made it look fairly simple, which means
we have good reuse of code.

-- 
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] [libvirt RFC PATCH 03/10] util: storage: Add support for host device backing specified via JSON

[Eric Blake]

On 07/15/2016 07:46 AM, Peter Krempa wrote:
> JSON pseudo protocol for qemu allows to explicitly specify devices.
> Add convertor to the internal type.

s/convertor/converter/

> ---
>  src/util/virstoragefile.c | 2 ++
>  tests/virstoragetest.c| 6 ++
>  2 files changed, 8 insertions(+)
> 

ACK for matching the style in 2/10. Any design changes that affect that
one will have obvious ripple effects into this one.

-- 
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 2/2] libvirt.spec.in: Adapt to newest wireshark plugindir

[Jean-Marc Liger]

Le 27/07/2016 à 17:03, Michal Privoznik a écrit :


In the old days, when wireshark plugin was introduced it was
installed under /usr/lib64/wireshark/plugins/$VERSION/ while with
wireshark-2.1.0 this path has changed just to
/usr/lib64/wireshark/plugins. We should teach our spec file about
this change.

Signed-off-by: Michal Privoznik 
---
  libvirt.spec.in | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index cc5d722..35c7449 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -406,7 +406,7 @@ BuildRequires: numad
  %endif
  
  %if %{with_wireshark}

-BuildRequires: wireshark-devel
+BuildRequires: wireshark-devel >= 2.1.0
  %endif
  
  Provides: bundled(gnulib)

@@ -1212,9 +1212,7 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/lock-driver/*.a
  rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.la
  rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a
  %if %{with_wireshark}
-rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.la
-mv $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.so \
-   $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.so
+rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.la
  %endif
  
  install -d -m 0755 $RPM_BUILD_ROOT%{_datadir}/lib/libvirt/dnsmasq/


%package wireshark
Requires: wireshark >= 1.12.6-4

Does this version number need to be bump ?

Regards,
Jean-Marc Liger

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

Re: [libvirt] [PATCH 2/2] storage: remove "luks" storage volume type

[Daniel P. Berrange]

On Wed, Jul 27, 2016 at 08:44:53AM +0200, Martin Kletzander wrote:
> On Tue, Jul 26, 2016 at 07:12:30PM +0100, Daniel P. Berrange wrote:
> > The current LUKS support has a "luks" volume type which has
> > a "luks" encryption format.
> > 
> > This partially makes sense if you consider the QEMU shorthand
> > syntax only requires you to specify a format=luks, and it'll
> > automagically uses "raw" as the next level driver. QEMU will
> > however let you override the "raw" with any other driver it
> > supports (vmdk, qcow, rbd, iscsi, etc, etc)
> > 
> > IOW the intention though is that the "luks" encryption format
> > is applied to all disk formats (whether raw, qcow2, rbd, gluster
> > or whatever). As such it doesn't make much sense for libvirt
> > to say the volume type is "luks" - we should be saying that it
> > is a "raw" file, but with "luks" encryption applied.
> > 
> > IOW, when creating a storage volume we should use this XML
> > 
> >  
> >demo.raw
> >5368709120
> >
> >  
> >  
> > > uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/>
> >  
> >
> >  
> > 
> > and when configuring a guest disk we should use
> > 
> >  
> >
> >
> >
> >
> >  
> >
> >  
> > 
> > This commit thus removes the "luks" storage volume type added
> > in
> > 
> >  commit 318ebb36f1027b3357a32d6f781bd77d7a9043fd
> >  Author: John Ferlan 
> >  Date:   Tue Jun 21 12:59:54 2016 -0400
> > 
> >util: Add 'luks' to the FileTypeInfo
> > 
> > The storage file probing code is modified so that it can probe
> > the actual encryption formats explicitly, rather than merely
> > probing existance of encryption and letting the storage driver
> > guess the format.
> > 
> > The rest of the code is then adapted to deal with
> > VIR_STORAGE_FILE_RAW w/ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS
> > instead of just VIR_STORAGE_FILE_LUKS.
> > 
> > The commit mentioned above was included in libvirt v2.0.0.
> > So when querying volume XML this will be a change in behaviour
> > vs the 2.0.0 release - it'll report 'raw' instead of 'luks'
> > for the volume format, but still report 'luks' for encryption
> > format.  I think this change is OK because the storage driver
> > did not include any support for creating volumes, nor starting
> > guets with luks volumes in v2.0.0 - that only since then.
> > Clearly if we change this we must do it before v2.1.0 though.
> > 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> > src/qemu/qemu_command.c|  10 +-
> > src/qemu/qemu_domain.c |   2 +-
> > src/qemu/qemu_hotplug.c|   2 +-
> > src/storage/storage_backend.c  |  41 +++--
> > src/storage/storage_backend_fs.c   |  17 +-
> > src/storage/storage_backend_gluster.c  |   5 -
> > src/util/virstoragefile.c  | 202 
> > -
> > src/util/virstoragefile.h  |   1 -
> > tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml |   4 +-
> > tests/storagevolxml2xmlin/vol-luks-cipher.xml  |   2 +-
> > tests/storagevolxml2xmlin/vol-luks.xml |   2 +-
> > tests/storagevolxml2xmlout/vol-luks-cipher.xml |   2 +-
> > tests/storagevolxml2xmlout/vol-luks.xml|   2 +-
> > 13 files changed, 198 insertions(+), 94 deletions(-)
> > 
> > diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> > index 862fb29..5ef536a 100644
> > --- a/src/storage/storage_backend.c
> > +++ b/src/storage/storage_backend.c
> > @@ -1116,9 +,9 @@ 
> > virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
> > int accessRetCode = -1;
> > char *absolutePath = NULL;
> > 
> > -if (info->format == VIR_STORAGE_FILE_LUKS) {
> > +if (info->format == VIR_STORAGE_FILE_RAW) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > -   _("cannot set backing store for luks volume"));
> > +   _("cannot set backing store for raw volume"));
> > return -1;
> > }
> > 
> 
> I think this whole condition can be removed as it wasn't there before
> luks volumes.

Previously it wasn't possible to reach this method when format == raw.
With LUKS now being treated as raw, we can reach this method in theory
and so we do still need to have this error check.

> 
> > @@ -1283,7 +1278,8 @@ 
> > virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
> >_("format features only available with qcow2"));
> > return NULL;
> > }
> > -if (info.format == VIR_STORAGE_FILE_LUKS) {
> > +if (info.format == VIR_STORAGE_FILE_RAW &&
> > +vol->target.encryption != NULL) {
> > if (inputvol) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> >_("cannot use inputvol with luks volume"));
> 
> You're still reporting the error for "luks volume" 

Re: [libvirt] [PATCH libvirt-glib] docs: Document gvir_connection_get_{storage_pools, networks, domains}

[Guido Günther]

On Wed, Jul 27, 2016 at 10:27:17AM +0200, Christophe Fergeau wrote:
> 
> Acked-by: Christophe Fergeau 
> 
> (I'm assuming you can push to libvirt-glib, let me know if that's not
> the case).

Pushed. Thanks!
 -- Guido

> 
> Christophe
> 
> On Wed, Jul 27, 2016 at 08:59:54AM +0200, Guido Günther wrote:
> > In contrast to libvirt itself all get_* methods need to prefetch the
> > corresponding information first so document this.
> > ---
> >  libvirt-gobject/libvirt-gobject-connection.c | 15 +++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
> > b/libvirt-gobject/libvirt-gobject-connection.c
> > index 00d5eda..3f17265 100644
> > --- a/libvirt-gobject/libvirt-gobject-connection.c
> > +++ b/libvirt-gobject/libvirt-gobject-connection.c
> > @@ -700,6 +700,11 @@ void gvir_connection_close(GVirConnection *conn)
> >   * gvir_connection_fetch_domains:
> >   * @conn: a #GVirConnection
> >   * @cancellable: (allow-none)(transfer none): cancellation object
> > + *
> > + * Use this method to fetch all domains managed by connection
> > + * @conn. Use e.g. #gvir_connection_find_domain_by_id or
> > + * #gvir_connection_get_domain afterwards to query the fetched
> > + * domains.
> >   */
> >  gboolean gvir_connection_fetch_domains(GVirConnection *conn,
> > GCancellable *cancellable,
> > @@ -783,6 +788,11 @@ cleanup:
> >   * gvir_connection_fetch_storage_pools:
> >   * @conn: a #GVirConnection
> >   * @cancellable: (allow-none)(transfer none): cancellation object
> > + *
> > + * Use this method to fetch all storage pools managed by connection
> > + * @conn. Use e.g. #gvir_connection_find_storage_pool_by_name or
> > + * #gvir_connection_get_storage_pools afterwards to query the fetched
> > + * pools.
> >   */
> >  gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn,
> >   GCancellable *cancellable,
> > @@ -1722,6 +1732,11 @@ GVirInterface 
> > *gvir_connection_find_interface_by_mac(GVirConnection *conn,
> >   * gvir_connection_fetch_networks:
> >   * @conn: a #GVirConnection
> >   * @cancellable: (allow-none)(transfer none): cancellation object
> > + *
> > + * Use this method to fetch all networks managed by connection
> > + * @conn. Use e.g. #gvir_connection_find_network_by_name or
> > + * #gvir_connection_get_networks afterwards to query the fetched
> > + * domains.
> >   */
> >  gboolean gvir_connection_fetch_networks(GVirConnection *conn,
> >  GCancellable *cancellable,
> > -- 
> > 2.8.1
> > 
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list


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

Re: [libvirt] [PATCH v1 0/4] libxl: host cpu element in capabilities

[Joao Martins]

On 07/27/2016 02:28 PM, Cedric Bosdonnat wrote:
> Hey Joao,
> 
> The series looks good to me, but I'm surely not expert enough to give a
> full ACK.
> 
Thank you!

Joao
> On Wed, 2016-07-20 at 20:08 +0100, Joao Martins wrote:
>> Hey,
>>
>> This small series implements host cpu description in caps, by getting
>> topology and xen hwcaps parsing done, followed by having
>> cpu-{compare,baseline} APIs implemented. Last thing missing I think it
>> would be to libxl_cpuid_set the features to enable/disable whichever
>> format we choose plus the appropriate XML convertion to/from XM and XL
>> config formats.
>>
>> Note that since RFC[0] I removed the Get CPU model names API since it
>> would make more sense having it exported once we do support guest 
>> cpu model/features setting. Changelog included in individual patches
>> (only second patch got changed)
>>
>> Cheers,
>> Joao
>>
>> [0] http://www.redhat.com/archives/libvir-list/2016-July/msg00245.html
>>
>> Joao Martins (4):
>>   libxl: describe host topology in capabilities
>>   libxl: describe host cpu features based on hwcaps
>>   libxl: implement virConnectCompareCPU
>>   libxl: implement virConnectBaselineCPU
>>
>>  src/libxl/libxl_capabilities.c | 168 
>> ++---
>>  src/libxl/libxl_driver.c   |  60 +++
>>  2 files changed, 218 insertions(+), 10 deletions(-)
>>
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

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

Re: [libvirt] [PATCH 2/3] libxl: fix segfault in libxlReconnectDomain

[Joao Martins]

On 07/27/2016 02:42 PM, Cédric Bosdonnat wrote:
> In case of error, libxlReconnectDomain may call
> virDomainObjListRemoveLocked. However it has no local reference on
> the domain object, leading to segfault. Get a reference to the domain
> object at the start of the function and release it at the end to avoid
> problems.
> 
> This commit also factorizes code between the error and normal ends.
Also noticed in the rest of the patches, but I think you forgot to include
the SoB tag (Signed-off-by).

> ---
>  src/libxl/libxl_driver.c | 28 
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index cb501cf..5008c00 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -373,11 +373,13 @@ libxlReconnectDomain(virDomainObjPtr vm,
>  uint8_t *data = NULL;
>  virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
>  unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;
> +int ret = -1;
>  
>  #ifdef LIBXL_HAVE_PVUSB
>  hostdev_flags |= VIR_HOSTDEV_SP_USB;
>  #endif
>  
> +virObjectRef(vm);
>  virObjectLock(vm);
>  
>  libxl_dominfo_init(_info);
> @@ -385,18 +387,18 @@ libxlReconnectDomain(virDomainObjPtr vm,
>  /* Does domain still exist? */
>  rc = libxl_domain_info(cfg->ctx, _info, vm->def->id);
>  if (rc == ERROR_INVAL) {
> -goto out;
> +goto error;
>  } else if (rc != 0) {
>  VIR_DEBUG("libxl_domain_info failed (code %d), ignoring domain %d",
>rc, vm->def->id);
> -goto out;
> +goto error;
>  }
>  
>  /* Is this a domain that was under libvirt control? */
>  if (libxl_userdata_retrieve(cfg->ctx, vm->def->id,
>  "libvirt-xml", , )) {
>  VIR_DEBUG("libxl_userdata_retrieve failed, ignoring domain %d", 
> vm->def->id);
> -goto out;
> +goto error;
>  }
>  
>  /* Update domid in case it changed (e.g. reboot) while we were gone? */
> @@ -405,7 +407,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
>  /* Update hostdev state */
>  if (virHostdevUpdateActiveDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
>  vm->def, hostdev_flags) < 0)
> -goto out;
> +goto error;
>  
>  if (virAtomicIntInc(>nactive) == 1 && driver->inhibitCallback)
>  driver->inhibitCallback(true, driver->inhibitOpaque);
> @@ -413,21 +415,23 @@ libxlReconnectDomain(virDomainObjPtr vm,
>  /* Enable domain death events */
>  libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, >deathW);
After this patch it will always returns an error. Although
patch 3 of this series does add the "ret = 0" here. I think it should a part of 
this
patch.

>  
> + cleanup:
>  libxl_dominfo_dispose(_info);
>  virObjectUnlock(vm);
> +virObjectUnref(vm);
>  virObjectUnref(cfg);
> -return 0;
> +return ret;
>  
> - out:
> -libxl_dominfo_dispose(_info);
> + error:
>  libxlDomainCleanup(driver, vm);
> -if (!vm->persistent)
> +if (!vm->persistent) {
>  virDomainObjListRemoveLocked(driver->domains, vm);
> -else
> -virObjectUnlock(vm);
> -virObjectUnref(cfg);
>  
> -return -1;
> +/* virDomainObjListRemoveLocked leaves the object unlocked,
> + * lock it again to factorize more code. */
> +virObjectLock(vm);
> +}
> +goto cleanup;
>  }
>  
>  static void
> 

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

Re: [libvirt] [PATCH 3/3] libxl: add hooks support

[Joao Martins]

On 07/27/2016 02:42 PM, Cédric Bosdonnat wrote:
> Introduce libxl hook and use it for start, prepare, started,
> stop, stopped, migrate events.
Looks good to me with one comment and few nits. Looking at lxc and qemu drivers
virHook support seems to be similar so I am assuming this is correct. But this
initial review has a grain of salt as I am not fully familiar (yet) with 
virHook support.

> ---
>  docs/hooks.html.in  | 53 ++--
>  src/libxl/libxl_domain.c| 75 
> +
>  src/libxl/libxl_driver.c| 24 +++
>  src/libxl/libxl_migration.c | 58 +++
>  src/util/virhook.c  | 16 +-
>  src/util/virhook.h  | 13 
>  6 files changed, 236 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/hooks.html.in b/docs/hooks.html.in
> index d4f4ac3..11073cb 100644
> --- a/docs/hooks.html.in
> +++ b/docs/hooks.html.in
> @@ -17,8 +17,10 @@
>(since 0.8.0)
>A QEMU guest is started or stopped
>   (since 0.8.0)
> - An LXC guest is started or stopped
> +  An LXC guest is started or stopped
>   (since 0.8.0)
> +  A libxl-handled Xen guest is started or stopped
> + (since 2.1.0)
>A network is started or stopped or an interface is
>plugged/unplugged to/from the network
>(since 1.2.2)
> @@ -41,7 +43,7 @@
>  
>  
>  Script names
> -At present, there are three hook scripts that can be called:
> +At present, there are five hook scripts that can be called:
>  
>/etc/libvirt/hooks/daemon
>Executed when the libvirt daemon is started, stopped, or reloads
> @@ -50,6 +52,9 @@
>Executed when a QEMU guest is started, stopped, or 
> migrated
>/etc/libvirt/hooks/lxc
>Executed when an LXC guest is started or stopped
> +  /etc/libvirt/hooks/libxl
> +  Executed when a libxl-handled Xen guest is started, stopped, or
> +  migrated
>/etc/libvirt/hooks/network
>Executed when a network is started or stopped or an
>interface is plugged/unplugged to/from the network
> @@ -235,6 +240,50 @@
>
>  
>  
> +/etc/libvirt/hooks/libxl
> +
> +  Before a Xen guest is started using libxl driver, the libxl hook
> +script is called in three locations; if any location fails, the guest
> +is not started.  The first location, since
> +2.1.0, is before libvirt performs any resource
> +labeling, and the hook can allocate resources not managed by
> +libvirt.  This is called as:
> +/etc/libvirt/hooks/libxl guest_name prepare begin -
> +The second location, available Since
> +2.1.0, occurs after libvirt has finished labeling
> +all resources, but has not yet started the guest, called as:
> +/etc/libvirt/hooks/libxl guest_name start begin -
> +The third location, 2.1.0,
> +occurs after the domain has successfully started up:
> +/etc/libvirt/hooks/libxl guest_name started begin -
> +  
> +  When a libxl-handled Xen guest is stopped, the libxl hook script
> +is called in two locations, to match the startup.
> +First, since 2.1.0, the hook is
> +called before libvirt restores any labels:
> +/etc/libvirt/hooks/libxl guest_name stopped end -
> +Then, after libvirt has released all resources, the hook is
> +called again, since 2.1.0, to allow
> +any additional resource cleanup:
> +/etc/libvirt/hooks/libxl guest_name release end -
> +  Since 2.1.0, the libxl hook script
> +is also called at the beginning of incoming migration. It is called
> +as: /etc/libvirt/hooks/libxl guest_name migrate begin -
> +with domain XML sent to standard input of the script. In this case,
> +the script acts as a filter and is supposed to modify the domain
> +XML and print it out on its standard output. Empty output is
> +identical to copying the input XML without changing it. In case the
> +script returns failure or the output XML is not valid, incoming
> +migration will be canceled. This hook may be used, e.g., to change
> +location of disk images for incoming domains.
> +  Since 2.1.0, the libxl hook script
> +is also called when the libvirtd daemon restarts and reconnects
> +to previously running Xen domains. If the script fails, the
> +existing Xen domains will be killed off. It is called as:
> +/etc/libvirt/hooks/libxl guest_name reconnect begin -
> +  
> +
> +
Not sure what's the norm for documentation patches on libvirt, but I still 
leave the
suggestion of making docs part on a separate patch.

>  /etc/libvirt/hooks/network
>  
>Since 1.2.2, before a network is 
> started,
> diff --git 

[libvirt] [PATCH 0/2] Couple of build fixes

[Michal Privoznik]

Contradictory to my previous patch set [1], these are not pushed yet
and thus require proper review.

1: https://www.redhat.com/archives/libvir-list/2016-July/msg01147.html

Michal Privoznik (2):
  vshCmddefGetOption: Change type of opt_index
  vshReadlineParse: Drop some unused variables

 tools/vsh.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

-- 
2.8.4

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

[libvirt] [PATCH 1/2] vshCmddefGetOption: Change type of opt_index

[Michal Privoznik]

This function tries to look up desired option for a given parsed
command. Upon successful return it also stores option position
into passed *opt_index. Now, this variable is type of int, even
though it is never ever used to store negative value. Moreover,
the variable is set from a local variable which is type of
size_t.

Signed-off-by: Michal Privoznik 
---
 tools/vsh.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 9ac4f21..a7ce141 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -434,7 +434,7 @@ static vshCmdOptDef helpopt = {
 };
 static const vshCmdOptDef *
 vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name,
-   uint64_t *opts_seen, int *opt_index, char **optstr,
+   uint64_t *opts_seen, size_t *opt_index, char **optstr,
bool report)
 {
 size_t i;
@@ -1418,7 +1418,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser)
 } else if (tkdata[0] == '-' && tkdata[1] == '-' &&
c_isalnum(tkdata[2])) {
 char *optstr = strchr(tkdata + 2, '=');
-int opt_index = 0;
+size_t opt_index = 0;
 
 if (optstr) {
 *optstr = '\0'; /* convert the '=' to '\0' */
@@ -2641,7 +2641,7 @@ vshReadlineParse(const char *text, int state)
 static char *ctext, *sanitized_text;
 static uint64_t const_opts_need_arg, const_opts_required, const_opts_seen;
 uint64_t opts_need_arg, opts_required, opts_seen;
-unsigned int opt_index;
+size_t opt_index;
 static bool cmd_exists, opts_filled, opt_exists;
 static bool non_bool_opt_exists, data_acomplete;
 
-- 
2.8.4

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

[libvirt] [PATCH 2/2] vshReadlineParse: Drop some unused variables

[Michal Privoznik]

My compiler identified some variables that were set, but never
actually used. For instance, opts_required, and data_acomplete.

Signed-off-by: Michal Privoznik 
---
 tools/vsh.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index a7ce141..540c78c 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2640,10 +2640,10 @@ vshReadlineParse(const char *text, int state)
 char *res = NULL;
 static char *ctext, *sanitized_text;
 static uint64_t const_opts_need_arg, const_opts_required, const_opts_seen;
-uint64_t opts_need_arg, opts_required, opts_seen;
+uint64_t opts_need_arg, opts_seen;
 size_t opt_index;
 static bool cmd_exists, opts_filled, opt_exists;
-static bool non_bool_opt_exists, data_acomplete;
+static bool non_bool_opt_exists;
 
 if (!state) {
 parser.pos = rl_line_buffer;
@@ -2687,7 +2687,6 @@ vshReadlineParse(const char *text, int state)
 cmd_exists = false;
 opts_filled = false;
 non_bool_opt_exists = false;
-data_acomplete = false;
 
 const_opts_need_arg = 0;
 const_opts_required = 0;
@@ -2713,7 +2712,6 @@ vshReadlineParse(const char *text, int state)
c_isalnum(tkdata[2])) {
 /* Command retrieved successfully, move to options */
 opts_need_arg = const_opts_need_arg;
-opts_required = const_opts_required;
 opts_seen = const_opts_seen;
 optstr = strchr(tkdata + 2, '=');
 opt_index = 0;
@@ -2749,7 +2747,6 @@ vshReadlineParse(const char *text, int state)
 tkdata = const_tkdata;
 if (STREQ(tkdata, sanitized_text)) {
 /* auto-complete non-bool option */
-data_acomplete = true;
 break;
 }
 }
-- 
2.8.4

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

[libvirt] Entering freeze for libvirt-2.1.0

[Daniel Veillard]

  So as planned I tagged the Release Candidate 1 version in git and
pushed signed tarball and rpms to the usual place:

   ftp://libvirt.org/libvirt/

  Please give it a try, I plan to push the rc2 on Friday and if everything
goes fine have the final release on Monday or Tuesday.

   Please give it a good test :)

thanks !

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] [PATCH 5/5] qemu: Advertise OVMF_CODE.secboot.fd

[Laszlo Ersek]

On 07/27/16 10:43, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_conf.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index fa9d65e..b51f36f 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -126,6 +126,8 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)
>  
>  #define VIR_QEMU_OVMF_LOADER_PATH "/usr/share/OVMF/OVMF_CODE.fd"
>  #define VIR_QEMU_OVMF_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"
> +#define VIR_QEMU_OVMF_SEC_LOADER_PATH "/usr/share/OVMF/OVMF_CODE.secboot.fd"
> +#define VIR_QEMU_OVMF_SEC_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"
>  #define VIR_QEMU_AAVMF_LOADER_PATH "/usr/share/AAVMF/AAVMF_CODE.fd"
>  #define VIR_QEMU_AAVMF_NVRAM_PATH "/usr/share/AAVMF/AAVMF_VARS.fd"
>  
> @@ -292,16 +294,19 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
> privileged)
>  goto error;
>  
>  #else
> -if (VIR_ALLOC_N(cfg->firmwares, 2) < 0)
> +if (VIR_ALLOC_N(cfg->firmwares, 3) < 0)
>  goto error;
> -cfg->nfirmwares = 2;
> -if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0)
> +cfg->nfirmwares = 3;
> +if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0 
> ||
> +VIR_ALLOC(cfg->firmwares[2]) < 0)
>  goto error;
>  
>  if (VIR_STRDUP(cfg->firmwares[0]->name, VIR_QEMU_AAVMF_LOADER_PATH) < 0 
> ||
>  VIR_STRDUP(cfg->firmwares[0]->nvram, VIR_QEMU_AAVMF_NVRAM_PATH) < 0  
> ||
>  VIR_STRDUP(cfg->firmwares[1]->name, VIR_QEMU_OVMF_LOADER_PATH) < 0 ||
> -VIR_STRDUP(cfg->firmwares[1]->nvram, VIR_QEMU_OVMF_NVRAM_PATH) < 0)
> +VIR_STRDUP(cfg->firmwares[1]->nvram, VIR_QEMU_OVMF_NVRAM_PATH) < 0 ||
> +VIR_STRDUP(cfg->firmwares[2]->name, VIR_QEMU_OVMF_SEC_LOADER_PATH) < 
> 0 ||
> +VIR_STRDUP(cfg->firmwares[2]->nvram, VIR_QEMU_OVMF_SEC_NVRAM_PATH) < 
> 0)
>  goto error;
>  #endif
>  
> 

Acked-by: Laszlo Ersek 

You might want to add the same to the default qemu.conf file, in the
"nvram" stanza.

Thanks
Laszlo

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

Re: [libvirt] [PATCH 4/5] qemu: Enable secure boot

[Laszlo Ersek]

On 07/27/16 10:43, Michal Privoznik wrote:
> In qemu, enabling this feature boils down to adding the following
> onto the command line:
> 
>   -global driver=cfi.pflash01,property=secure,value=on
> 
> However, there are some constraints resulting from the
> implementation. For instance, System Management Mode (SMM) is
> required to be enabled, the machine type must be q35-2.5 or
> later, and the guest should be x86_64. While technically it is
> possible to have 32 bit guests with secure boot, some non-trivial
> CPU flags tuning is required (for instance lm and nx flags must
> be prohibited). Given complexity of our CPU driver, this is not
> trivial. Therefore I've chosen to forbid 32 bit guests for now.
> If there's ever need, we can refine the check later.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_command.c|  7 ++
>  src/qemu/qemu_domain.c | 27 
>  .../qemuxml2argv-bios-nvram-secure.args| 29 
> ++
>  tests/qemuxml2argvtest.c   |  7 ++
>  4 files changed, 70 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args

This patch looks almost complete to me (it causes all necessary QEMU
options to appear, directly or indirectly (= via requiring SMM)).
However, can you also enforce that the Q35 machtype has version 2.5 or
later? Technically, "pc-q35-2.4" exists too, and it's not good enough
(according to the instructions I wrote up in OvmfPkg/README earlier). I
certainly never tested it.

Thanks,
Laszlo

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 831aba1..c6e629e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8731,6 +8731,13 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
>  goto cleanup;
>  }
>  
> +if (loader->secure == VIR_TRISTATE_BOOL_YES) {
> +virCommandAddArgList(cmd,
> + "-global",
> + 
> "driver=cfi.pflash01,property=secure,value=on",
> + NULL);
> +}
> +
>  virBufferAsprintf(,
>"file=%s,if=pflash,format=raw,unit=%d",
>loader->path, unit);
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 9045f37..ed51481 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2321,6 +2321,33 @@ qemuDomainDefValidate(const virDomainDef *def,
>  return -1;
>  }
>  
> +if (def->os.loader &&
> +def->os.loader->secure == VIR_TRISTATE_BOOL_YES) {
> +/* These are the QEMU implementation limitations. But we
> + * have to live with them for now. */
> +
> +if (!qemuDomainMachineIsQ35(def)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("Secure boot is supported with q35 machine 
> types only"));
> +return -1;
> +}
> +
> +/* Now, technically it is possible to have secure boot on
> + * 32bits too, but that would require some -cpu xxx magic
> + * too. Not worth it unless we are explicitly asked. */
> +if (def->os.arch != VIR_ARCH_X86_64) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("Secure boot is supported for x86_64 
> architecture only"));
> +return -1;
> +}
> +
> +if (def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) 
> {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("Secure boot requires SMM feature enabled"));
> +return -1;
> +}
> +}
> +
>  return 0;
>  }
>  
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args 
> b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args
> new file mode 100644
> index 000..c014254
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args
> @@ -0,0 +1,29 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu \
> +-name test-bios \
> +-S \
> +-machine pc-q35-2.5,accel=tcg,smm=on \
> +-global driver=cfi.pflash01,property=secure,value=on \
> +-drive 
> file=/usr/share/OVMF/OVMF_CODE.secboot.fd,if=pflash,format=raw,unit=0,\
> +readonly=on \
> +-drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \
> +-m 1024 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \
> +-nographic \
> +-nodefaults \
> +-monitor unix:/tmp/lib/domain--1-test-bios/monitor.sock,server,nowait \
> +-boot c \
> +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
> +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \
> +-device virtio-scsi-pci,id=scsi0,bus=pci.2,addr=0x1 \
> +-drive 

[libvirt] [PATCH 2/2] libvirt.spec.in: Adapt to newest wireshark plugindir

[Michal Privoznik]

In the old days, when wireshark plugin was introduced it was
installed under /usr/lib64/wireshark/plugins/$VERSION/ while with
wireshark-2.1.0 this path has changed just to
/usr/lib64/wireshark/plugins. We should teach our spec file about
this change.

Signed-off-by: Michal Privoznik 
---
 libvirt.spec.in | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index cc5d722..35c7449 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -406,7 +406,7 @@ BuildRequires: numad
 %endif
 
 %if %{with_wireshark}
-BuildRequires: wireshark-devel
+BuildRequires: wireshark-devel >= 2.1.0
 %endif
 
 Provides: bundled(gnulib)
@@ -1212,9 +1212,7 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/lock-driver/*.a
 rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.la
 rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a
 %if %{with_wireshark}
-rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.la
-mv $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.so \
-   $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.so
+rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.la
 %endif
 
 install -d -m 0755 $RPM_BUILD_ROOT%{_datadir}/lib/libvirt/dnsmasq/
-- 
2.8.4

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

[libvirt] [PATCH 0/2] Two wireshark fixes

[Michal Privoznik]

While trying to do an RPM, I've ran into two problems. I've
already pushed the fixes under 'build breaker rule'. I'm sending
them for completeness.

Michal Privoznik (2):
  virt-wireshark: Properly substract wireshark prefix
  libvirt.spec.in: Adapt to newest wireshark plugindir

 libvirt.spec.in  | 6 ++
 m4/virt-wireshark.m4 | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

-- 
2.8.4

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

[libvirt] [PATCH 1/2] virt-wireshark: Properly substract wireshark prefix

[Michal Privoznik]

So, when building wireshark plugin, we get the plugindir variable
from the wireshark.pc as well as prefix. Then we replace the
prefix in the plugindir with our own prefix where libvirt is
building to:

  plugindir="${prefix}${plugindir#ws_prefix}"

However, as you can see, there's '$' missing in front of the
ws_prefix variable. This results in the mangled plugindir, for
instance like this:

  plugindir='/usr/usr/lib64/wireshark/plugins'

Signed-off-by: Michal Privoznik 
---
 m4/virt-wireshark.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index d8cb7c8..f383e2b 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -38,7 +38,7 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
 if test "x$ws_prefix" = "x" ; then
   ws_prefix="/usr";
 fi
-plugindir="${prefix}${plugindir#ws_prefix}"
+plugindir="${prefix}${plugindir#$ws_prefix}"
   fi
 elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" = 
"xyes"; then
   AC_MSG_ERROR([ws-plugindir must be used only with valid path])
-- 
2.8.4

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

Re: [libvirt] [PATCH 2/5] Introduce SMM feature

[Laszlo Ersek]

Hi Michal,

thanks a lot for posting this. One question:

On 07/27/16 10:43, Michal Privoznik wrote:
> Since its release of 2.4.0 qemu is able to enable System
> Management Module in the firmware, or disable it. We should
> expose this capability in the XML. Unfortunately, there's no good
> way to determine whether the binary we are talking to supports
> it. I mean, if qemu's run with real machine type, the smm
> attribute can be seen in 'qom-list /machine' output. But it's not
> there when qemu's run with -M none. Therefore we're stuck with
> version based check.

Where does your information come from that says QEMU 2.4 is good enough
for this? To my knowledge, the pc-q35-2.5 machine type is minimally
required, i.e., no i440fx at all, and for q35, 2.5 or later, which can
only be provided by QEMU v2.5.0 or later.

Hmmm... I found QEMU commit 355023f2010c4 ("pc: add SMM property"), and
it's indeed part of v2.4.0. I wonder where *I* got the information from
that only pc-q35-2.5+ machtypes are suitable for SMM (as OVMF needs it).
Perhaps there were other requirements too (SMRAM emulation etc). Ah,
wait, *large* SMRAM (T-SEG) is Q35-only. The SMM property is available
on i440fx as well.

Hm, even Paolo's presentation ("Securing secure boot with
System Management Mode") mentions "QEMU: released in 2.4".

... Ah, I think I might now why. In this RHBZ:

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

Gerd set

> Gerd Hoffmann 2016-03-21 08:23:06 CET
> Fixed In Version: qemu-2.5

I checked

  git log --no-merges --oneline --reverse v2.4.0..v2.5.0

but nothing says "smm", "smram", or "tseg" (case-insensitively).

I recall commit bafc90bdc594 ("q35: implement TSEG") as one of Gerd's
QEMU patches, but that's also part of v2.4.0. Confirmed by:
.

... Gerd's above metadata change is dated 2016-03-21, at which point
both QEMU v2.4.0 and v2.5.0 had been tagged (on 2015-08-11 and on
2015-12-16, respectively). I guess Gerd may have mis-remembered the
exact QEMU release which included SMM support?

I don't remember ever testing SMM (using OVMF) with released 2.4 machine
types, so I feel a bit uncertain about mentioning and looking for 2.4 in
this patch.

... Anyway, looking over your patch quickly, I think it really only
concerns itself with "-machine smm=on", and for that, the 2.4 version
check should suffice. It's only OVMF that needs a lot more than that.

Acked-by: Laszlo Ersek 

Thanks!
Laszlo

> 
> Signed-off-by: Michal Privoznik 
> ---
>  docs/formatdomain.html.in  |  6 +
>  docs/schemas/domaincommon.rng  |  9 +++
>  src/conf/domain_conf.c |  5 +++-
>  src/conf/domain_conf.h |  1 +
>  src/qemu/qemu_capabilities.c   | 16 +
>  src/qemu/qemu_capabilities.h   |  4 
>  src/qemu/qemu_command.c| 12 ++
>  tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml   |  1 +
>  tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml   |  1 +
>  .../caps_2.6.0-gicv2.aarch64.xml   |  1 +
>  .../caps_2.6.0-gicv3.aarch64.xml   |  1 +
>  tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml  |  1 +
>  tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |  1 +
>  .../qemuxml2argv-machine-smm-opt.args  | 25 +++
>  .../qemuxml2argv-machine-smm-opt.xml   | 28 
> ++
>  tests/qemuxml2argvtest.c   |  7 ++
>  16 files changed, 118 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 59a8bb9..3f67182 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1655,6 +1655,12 @@
>values are 2, 3 and host.
>Since 1.2.16
>
> +  smm
> +  Enable System Management Mode. Possible values are
> +  on and off. The default is left
> +  for hypervisor to decide.
> +  Since 2.1.0
> +  
>  
>  
>  Time keeping
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 741f268..39fcb7e 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4291,6 +4291,15 @@
>
>  
>
> +  
> +
> +  
> +
> +  
> +
> +  
> +
> +  
>  
>
>  
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 13e5dc9..3b6493e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -137,7 +137,8 @@ VIR_ENUM_IMPL(virDomainFeature, 

Re: [libvirt] [PATCH] virconf: Fix config file path construction

[Martin Kletzander]

On Wed, Jul 27, 2016 at 08:50:42AM +0200, Erik Skultety wrote:

Since commit c4bdff19, the path to the configuration file has been constructed
in the following manner:
- if no config filename was passed to virConfLoadConfigPath, libvirt.conf was
used as default
- otherwise the filename was concatenated with
"/libvirt/libvirt%s%s.conf" which in admin case resulted in
"libvirt-libvirt-admin.conf.conf". Obviously, this non-existent config led to
ignoring  all user settings in libvirt-admin.conf. This patch requires the
config filename to be always provided as an argument with the concatenation
being simplified.

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

Signed-off-by: Erik Skultety 
---
src/libvirt.c  |  2 +-
src/util/virconf.c | 12 
2 files changed, 5 insertions(+), 9 deletions(-)



More versatile and nicer.  ACK.  I guess it should've been called with "admin"
instead of the full name.


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

Re: [libvirt] [PATCH] admin: Fix default uri config option name s/admin_uri_default/uri_default

[Martin Kletzander]

On Wed, Jul 27, 2016 at 02:22:52PM +0200, Erik Skultety wrote:

The original name 'admin_uri_default' was introduced to our code by commit
dbecb87f. However, at that time we already had a separate config file for
admin library but the commit mentioned above didn't properly adjust the
config's option name. The result is that when we're loading the config, we
check a non-existent config option (there's not much to do with the URIs
anyway, since we only allow local connection). Additionally, virt-admin's man
page documents, that the default URI can be altered by setting
admin_uri_default option. So the fix proposed by this patch leaves the
libvirt-admin.conf as is and adjusts the naming in the code as well as in the
virt-admin's man page.

Signed-off-by: Erik Skultety 
---
src/libvirt-admin.c  | 2 +-
tools/virt-admin.pod | 7 ---
2 files changed, 5 insertions(+), 4 deletions(-)



ACK


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

[libvirt] [PATCH v2] qemu: fix domain id after domainCreateWithFlags()

[Sascha Silbe]

Ever since virDomainCreateWithFlags() was introduced by de3aadaa
[drivers: add virDomainCreateWithFlags if virDomainCreate exists], the
domain ID retrieved with virDomainGetID() was incorrect for several
drivers after virDomainCreateWithFlags() was called. The API consumer
had to look up the domain anew to retrieve the correct ID.

For the ESX driver, this was fixed in 6139b274 [esx: Update ID after
starting a domain]. For the openvz driver, it was fixed in fd81a097
[openvzDomainCreateWithFlags: set domain id to the correct value]. The
test driver, the OpenNebula driver (removed in the meantime) and the
vbox driver were already updating the domain ID correctly in
domainCreate().

Copy over the ID in qemuDomainCreateWithFlags() to fix this for the qemu
driver, too.

Fixes: de3aadaa ("drivers: add virDomainCreateWithFlags if virDomainCreate 
exists")
Reported-by: Marc Hartmayer 
Signed-off-by: Sascha Silbe 
Tested-by: Marc Hartmayer 
Reviewed-by: Marc Hartmayer 
---
v1→v2:
  - Adjusted commit message. remoteDomainCreate() is actually fine since
it explicitly queries the ID in a second RPC, only
remoteDomainCreateWithFlags() is broken. Found and mentioned several
other commits that fixed this for several of the other hypervisor
drivers.

 src/qemu/qemu_driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1820f85..43242e0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7100,6 +7100,7 @@ qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int 
flags)
QEMU_ASYNC_JOB_START) < 0)
 goto endjob;
 
+dom->id = vm->def->id;
 ret = 0;
 
  endjob:
-- 
1.9.1

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

Re: [libvirt] [PATCH v7 0/7] qemu: expand domain memory statistics

[Pavel Hrdina]

On Wed, Jul 27, 2016 at 03:05:43PM +0300, Dmitry Derbyshev wrote:
> 7/27/2016 2:10 PM, Pavel Hrdina пишет:
> > On Wed, Jul 13, 2016 at 01:42:11PM +0300, Derbyshev Dmitriy wrote:
> >> From: Derbyshev Dmitry 
> > There are some things that needs to be updated, but if you're OK with me
> > updating those issues I can push the first 5 patches for you with the issues
> > fixed.
> >
> > Pavel
> I would be very glad if did so, thank you.
> Last 2 patches are auxiliary, while the first 5 are important to us with 
> or without them.

I've pushed 1,4,5 patches and for patches 2 and 3 only the diff to already
pushed patches.

> 
> If I would like to resend new version of these last 2 patches, should I 
> send them in the same patch series or other, if first patches are 
> already applied?

I'm still not convinced that we need to do the caching.  However if something is
pushed from patch series you usually send a new patch series without the patches
that are already pushed, so in this case only the last two patches.

Pavel

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

[libvirt] [PATCH 3/3] libxl: add hooks support

[Cédric Bosdonnat]

Introduce libxl hook and use it for start, prepare, started,
stop, stopped, migrate events.
---
 docs/hooks.html.in  | 53 ++--
 src/libxl/libxl_domain.c| 75 +
 src/libxl/libxl_driver.c| 24 +++
 src/libxl/libxl_migration.c | 58 +++
 src/util/virhook.c  | 16 +-
 src/util/virhook.h  | 13 
 6 files changed, 236 insertions(+), 3 deletions(-)

diff --git a/docs/hooks.html.in b/docs/hooks.html.in
index d4f4ac3..11073cb 100644
--- a/docs/hooks.html.in
+++ b/docs/hooks.html.in
@@ -17,8 +17,10 @@
   (since 0.8.0)
   A QEMU guest is started or stopped
  (since 0.8.0)
- An LXC guest is started or stopped
+  An LXC guest is started or stopped
  (since 0.8.0)
+  A libxl-handled Xen guest is started or stopped
+ (since 2.1.0)
   A network is started or stopped or an interface is
   plugged/unplugged to/from the network
   (since 1.2.2)
@@ -41,7 +43,7 @@
 
 
 Script names
-At present, there are three hook scripts that can be called:
+At present, there are five hook scripts that can be called:
 
   /etc/libvirt/hooks/daemon
   Executed when the libvirt daemon is started, stopped, or reloads
@@ -50,6 +52,9 @@
   Executed when a QEMU guest is started, stopped, or 
migrated
   /etc/libvirt/hooks/lxc
   Executed when an LXC guest is started or stopped
+  /etc/libvirt/hooks/libxl
+  Executed when a libxl-handled Xen guest is started, stopped, or
+  migrated
   /etc/libvirt/hooks/network
   Executed when a network is started or stopped or an
   interface is plugged/unplugged to/from the network
@@ -235,6 +240,50 @@
   
 
 
+/etc/libvirt/hooks/libxl
+
+  Before a Xen guest is started using libxl driver, the libxl hook
+script is called in three locations; if any location fails, the guest
+is not started.  The first location, since
+2.1.0, is before libvirt performs any resource
+labeling, and the hook can allocate resources not managed by
+libvirt.  This is called as:
+/etc/libvirt/hooks/libxl guest_name prepare begin -
+The second location, available Since
+2.1.0, occurs after libvirt has finished labeling
+all resources, but has not yet started the guest, called as:
+/etc/libvirt/hooks/libxl guest_name start begin -
+The third location, 2.1.0,
+occurs after the domain has successfully started up:
+/etc/libvirt/hooks/libxl guest_name started begin -
+  
+  When a libxl-handled Xen guest is stopped, the libxl hook script
+is called in two locations, to match the startup.
+First, since 2.1.0, the hook is
+called before libvirt restores any labels:
+/etc/libvirt/hooks/libxl guest_name stopped end -
+Then, after libvirt has released all resources, the hook is
+called again, since 2.1.0, to allow
+any additional resource cleanup:
+/etc/libvirt/hooks/libxl guest_name release end -
+  Since 2.1.0, the libxl hook script
+is also called at the beginning of incoming migration. It is called
+as: /etc/libvirt/hooks/libxl guest_name migrate begin -
+with domain XML sent to standard input of the script. In this case,
+the script acts as a filter and is supposed to modify the domain
+XML and print it out on its standard output. Empty output is
+identical to copying the input XML without changing it. In case the
+script returns failure or the output XML is not valid, incoming
+migration will be canceled. This hook may be used, e.g., to change
+location of disk images for incoming domains.
+  Since 2.1.0, the libxl hook script
+is also called when the libvirtd daemon restarts and reconnects
+to previously running Xen domains. If the script fails, the
+existing Xen domains will be killed off. It is called as:
+/etc/libvirt/hooks/libxl guest_name reconnect begin -
+  
+
+
 /etc/libvirt/hooks/network
 
   Since 1.2.2, before a network is started,
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 886b40f..d04dc5e 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -32,6 +32,7 @@
 #include "viratomic.h"
 #include "virfile.h"
 #include "virerror.h"
+#include "virhook.h"
 #include "virlog.h"
 #include "virstring.h"
 #include "virtime.h"
@@ -737,6 +738,17 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
 hostdev_flags |= VIR_HOSTDEV_SP_USB;
 #endif
 
+/* now that we know it's stopped call the hook if present */
+if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
+char *xml = virDomainDefFormat(vm->def, cfg->caps, 0);
+
+/* we can't stop the 

[libvirt] [PATCH 0/3] libxl hooks

[Cédric Bosdonnat]

Hi there!

Here is small patchset adding hooks support to the libxl driver.

Cédric Bosdonnat (3):
  libxl: add a flag to mark guests as tainted by a hook
  libxl: fix segfault in libxlReconnectDomain
  libxl: add hooks support

 docs/hooks.html.in  | 53 ++--
 src/libxl/libxl_domain.c| 85 +
 src/libxl/libxl_domain.h|  2 ++
 src/libxl/libxl_driver.c| 52 ---
 src/libxl/libxl_migration.c | 58 +++
 src/util/virhook.c  | 16 -
 src/util/virhook.h  | 13 +++
 7 files changed, 264 insertions(+), 15 deletions(-)

-- 
2.6.6

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

[libvirt] [PATCH 1/3] libxl: add a flag to mark guests as tainted by a hook

[Cédric Bosdonnat]

The migrate hook will affect the migrated guest definition. Allow
these domains be marked as tainted in the libxl driver.
---
 src/libxl/libxl_domain.c | 10 ++
 src/libxl/libxl_domain.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 0e26b91..886b40f 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1135,6 +1135,16 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
vm->def, hostdev_flags) < 0)
 goto cleanup_dom;
 
+if (priv->hookRun) {
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virUUIDFormat(vm->def->uuid, uuidstr);
+
+VIR_WARN("Domain id='%d' name='%s' uuid='%s' is tainted: hook",
+ vm->def->id,
+ vm->def->name,
+ uuidstr);
+}
+
 /* Unlock virDomainObj while creating the domain */
 virObjectUnlock(vm);
 
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index af11a2c..3a3890b 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -69,6 +69,8 @@ struct _libxlDomainObjPrivate {
 char *lockState;
 
 struct libxlDomainJobObj job;
+
+bool hookRun;  /* true if there was a hook run over this domain */
 };
 
 
-- 
2.6.6

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

[libvirt] [PATCH 2/3] libxl: fix segfault in libxlReconnectDomain

[Cédric Bosdonnat]

In case of error, libxlReconnectDomain may call
virDomainObjListRemoveLocked. However it has no local reference on
the domain object, leading to segfault. Get a reference to the domain
object at the start of the function and release it at the end to avoid
problems.

This commit also factorizes code between the error and normal ends.
---
 src/libxl/libxl_driver.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index cb501cf..5008c00 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -373,11 +373,13 @@ libxlReconnectDomain(virDomainObjPtr vm,
 uint8_t *data = NULL;
 virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
 unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;
+int ret = -1;
 
 #ifdef LIBXL_HAVE_PVUSB
 hostdev_flags |= VIR_HOSTDEV_SP_USB;
 #endif
 
+virObjectRef(vm);
 virObjectLock(vm);
 
 libxl_dominfo_init(_info);
@@ -385,18 +387,18 @@ libxlReconnectDomain(virDomainObjPtr vm,
 /* Does domain still exist? */
 rc = libxl_domain_info(cfg->ctx, _info, vm->def->id);
 if (rc == ERROR_INVAL) {
-goto out;
+goto error;
 } else if (rc != 0) {
 VIR_DEBUG("libxl_domain_info failed (code %d), ignoring domain %d",
   rc, vm->def->id);
-goto out;
+goto error;
 }
 
 /* Is this a domain that was under libvirt control? */
 if (libxl_userdata_retrieve(cfg->ctx, vm->def->id,
 "libvirt-xml", , )) {
 VIR_DEBUG("libxl_userdata_retrieve failed, ignoring domain %d", 
vm->def->id);
-goto out;
+goto error;
 }
 
 /* Update domid in case it changed (e.g. reboot) while we were gone? */
@@ -405,7 +407,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
 /* Update hostdev state */
 if (virHostdevUpdateActiveDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
 vm->def, hostdev_flags) < 0)
-goto out;
+goto error;
 
 if (virAtomicIntInc(>nactive) == 1 && driver->inhibitCallback)
 driver->inhibitCallback(true, driver->inhibitOpaque);
@@ -413,21 +415,23 @@ libxlReconnectDomain(virDomainObjPtr vm,
 /* Enable domain death events */
 libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, >deathW);
 
+ cleanup:
 libxl_dominfo_dispose(_info);
 virObjectUnlock(vm);
+virObjectUnref(vm);
 virObjectUnref(cfg);
-return 0;
+return ret;
 
- out:
-libxl_dominfo_dispose(_info);
+ error:
 libxlDomainCleanup(driver, vm);
-if (!vm->persistent)
+if (!vm->persistent) {
 virDomainObjListRemoveLocked(driver->domains, vm);
-else
-virObjectUnlock(vm);
-virObjectUnref(cfg);
 
-return -1;
+/* virDomainObjListRemoveLocked leaves the object unlocked,
+ * lock it again to factorize more code. */
+virObjectLock(vm);
+}
+goto cleanup;
 }
 
 static void
-- 
2.6.6

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

Re: [libvirt] [PATCH] qemu: remove panic device with model s390 when migrating

[Andrea Bolognani]

On Tue, 2016-07-26 at 18:41 +0200, Ján Tomko wrote:
> [cc-ing Andrea who auto-added panic for PPC64]
> 
> Should we do the same for PANIC_MODEL_PSERIES?

Thanks for the heads-up.

If we care about migrating guests to <= 1.2.16, then we
should indeed to the same for pSeries guests.

I don't see how it could hurt, either way: >= 1.2.17 will
automatically add the  element, and >= 1.3.0 will
include the model attribute as well, so I think we can
safely drop it from the migratable xml.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v1 0/4] libxl: host cpu element in capabilities

[Cedric Bosdonnat]

Hey Joao,

The series looks good to me, but I'm surely not expert enough to give a
full ACK.

--
Cedric

On Wed, 2016-07-20 at 20:08 +0100, Joao Martins wrote:
> Hey,
> 
> This small series implements host cpu description in caps, by getting
> topology and xen hwcaps parsing done, followed by having
> cpu-{compare,baseline} APIs implemented. Last thing missing I think it
> would be to libxl_cpuid_set the features to enable/disable whichever
> format we choose plus the appropriate XML convertion to/from XM and XL
> config formats.
> 
> Note that since RFC[0] I removed the Get CPU model names API since it
> would make more sense having it exported once we do support guest 
> cpu model/features setting. Changelog included in individual patches
> (only second patch got changed)
> 
> Cheers,
> Joao
> 
> [0] http://www.redhat.com/archives/libvir-list/2016-July/msg00245.html
> 
> Joao Martins (4):
>   libxl: describe host topology in capabilities
>   libxl: describe host cpu features based on hwcaps
>   libxl: implement virConnectCompareCPU
>   libxl: implement virConnectBaselineCPU
> 
>  src/libxl/libxl_capabilities.c | 168 
> ++---
>  src/libxl/libxl_driver.c   |  60 +++
>  2 files changed, 218 insertions(+), 10 deletions(-)
> 

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

Re: [libvirt] [PATCH] virsh: use virConnectGetDomainCapabilities with maxvcpus

[Peter Krempa]

On Wed, Jul 27, 2016 at 18:08:29 +0530, Shivaprasad G Bhat wrote:
> virsh maxvcpus --type kvm output is useless on PPC. Also, in
> commit e6806d79 we documented not rely on virConnectGetMaxVcpus
> output. Fix the  maxvcpus to use virConnectGetDomainCapabilities
> now to make it useful. The call is made to use the default qemu
> binary and to check for the host machine and arch which is what the
> command intends to do anyway.
> 
> Signed-off-by: Shivaprasad G Bhat 
> ---
>  tools/virsh-host.c |   33 +
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index 57f0c0e..cf001c6 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -606,18 +606,43 @@ static bool
>  cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
>  {
>  const char *type = NULL;
> -int vcpus;
> +unsigned int vcpus;
> +char *caps = NULL;
> +const unsigned int flags = 0; /* No flags so far */
> +xmlDocPtr xml = NULL;
> +xmlXPathContextPtr ctxt = NULL;
> +bool ret = false;
>  virshControlPtr priv = ctl->privData;
>  
>  if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
>  return false;
>  
> -if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
> -return false;
> +caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, 
> type, flags);
> +if (!caps) {
> +vshError(ctl, "%s", _("failed to get domain capabilities"));
> +goto cleanup;
> +}

This will break compatibility when connecting to older versions of the
daemon which don't support the virConnectGetDomainCapabilities.

Peter

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

[libvirt] [PATCH] virsh: use virConnectGetDomainCapabilities with maxvcpus

[Shivaprasad G Bhat]

The following patch fixes the maxvcpus output on PPC64.
Earlier I tried fixing this in kvmGetMaxVcpus() which was
later decided to be document not use the virConnectGetMaxVcpus() instead
use the virConnectGetDomainCapabilities api.

I have not implemented the suggestedcpus as mentioned in my previous
patches in this yet.

Previous series fixing the virConnectGetDomainCapabilities can be found here.
https://www.redhat.com/archives/libvir-list/2016-June/msg01873.html

---

Shivaprasad G Bhat (1):
  virsh: use virConnectGetDomainCapabilities with maxvcpus


 tools/virsh-host.c |   33 +
 1 file changed, 29 insertions(+), 4 deletions(-)

--
Signature

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

[libvirt] [PATCH] virsh: use virConnectGetDomainCapabilities with maxvcpus

[Shivaprasad G Bhat]

virsh maxvcpus --type kvm output is useless on PPC. Also, in
commit e6806d79 we documented not rely on virConnectGetMaxVcpus
output. Fix the  maxvcpus to use virConnectGetDomainCapabilities
now to make it useful. The call is made to use the default qemu
binary and to check for the host machine and arch which is what the
command intends to do anyway.

Signed-off-by: Shivaprasad G Bhat 
---
 tools/virsh-host.c |   33 +
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 57f0c0e..cf001c6 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -606,18 +606,43 @@ static bool
 cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
 {
 const char *type = NULL;
-int vcpus;
+unsigned int vcpus;
+char *caps = NULL;
+const unsigned int flags = 0; /* No flags so far */
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
+bool ret = false;
 virshControlPtr priv = ctl->privData;
 
 if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
 return false;
 
-if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
-return false;
+caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, type, 
flags);
+if (!caps) {
+vshError(ctl, "%s", _("failed to get domain capabilities"));
+goto cleanup;
+}
+
+xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), );
+if (!xml) {
+vshError(ctl, "%s", _("unable to parse domain capabilities"));
+goto cleanup;
+}
+
+if ((virXPathUInt("string(./vcpu[1]/@max)", ctxt, )) < 0) {
+vshError(ctl, "%s", _("unable to get maxvcpus"));
+goto cleanup;
+}
 
 vshPrint(ctl, "%d\n", vcpus);
 
-return true;
+ret = true;
+ cleanup:
+VIR_FREE(caps);
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml);
+
+return ret;
 }
 
 /*

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

[libvirt] [PATCH] admin: Fix default uri config option name s/admin_uri_default/uri_default

[Erik Skultety]

The original name 'admin_uri_default' was introduced to our code by commit
dbecb87f. However, at that time we already had a separate config file for
admin library but the commit mentioned above didn't properly adjust the
config's option name. The result is that when we're loading the config, we
check a non-existent config option (there's not much to do with the URIs
anyway, since we only allow local connection). Additionally, virt-admin's man
page documents, that the default URI can be altered by setting
admin_uri_default option. So the fix proposed by this patch leaves the
libvirt-admin.conf as is and adjusts the naming in the code as well as in the
virt-admin's man page.

Signed-off-by: Erik Skultety 
---
 src/libvirt-admin.c  | 2 +-
 tools/virt-admin.pod | 7 ---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
index 859f937..4bf29b1 100644
--- a/src/libvirt-admin.c
+++ b/src/libvirt-admin.c
@@ -167,7 +167,7 @@ virAdmGetDefaultURI(virConfPtr conf, char **uristr)
 return -1;
 VIR_DEBUG("Using LIBVIRT_ADMIN_DEFAULT_URI '%s'", *uristr);
 } else {
-if (virConfGetValueString(conf, "admin_uri_default", uristr) < 0)
+if (virConfGetValueString(conf, "uri_default", uristr) < 0)
 return -1;
 
 if (*uristr) {
diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod
index 2765bc5..2199038 100644
--- a/tools/virt-admin.pod
+++ b/tools/virt-admin.pod
@@ -127,7 +127,7 @@ Will print the current directory.
 
 (Re)-Connect to a daemon's administrating server. The I parameter
 specifies how to connect to the administrating server.
-If I or I (see below) were set,
+If I or I (see below) were set,
 I is automatically issued every time a command that requires an
 active connection is executed. Note that this only applies if there is no
 connection at all or there is an inactive one.
@@ -137,8 +137,9 @@ To find the currently used URI, check the I command 
documented below.
 =item B
 
 Prints the administrating server canonical URI, can be useful in shell mode. If
-no I was specified, neither I or
-I were set, libvirtd:///system is used.
+no I was specified, neither I environment
+variable nor I option (libvirt-admin.conf) were set,
+libvirtd:///system is used.
 
 =back
 
-- 
2.5.5

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

Re: [libvirt] [PATCH sandbox] image: Rename 'create' and 'delete' to 'prepare' and 'purge'

[Cedric Bosdonnat]

On Wed, 2016-07-27 at 13:01 +0100, Daniel P. Berrange wrote:
> Currently we have three virt-sandbox-image commands
> 
>  - 'create' - downloads a template and creates qcow2
>  - 'delete' - deletes template qcow2 files
>  - 'run' - runs an instance of a template
> 
> The 'run' command is generating a transient guest which
> disappears when it stops. We want to have the ability to
> create / delete persistent guests later, for which the
> command names "create" and "delete" are a natural fit.
> 
> So to avoid clash, rename the existing "create" command
> to "prepare" and the "delete" command to "purge"
> ---
>  libvirt-sandbox/image/cli.py | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
> index 66854e4..3a5ccfa 100644
> --- a/libvirt-sandbox/image/cli.py
> +++ b/libvirt-sandbox/image/cli.py
> @@ -68,13 +68,13 @@ def get_template_dir(args):
>  tmpl = template.Template.from_uri(args.template)
>  return "%s/%s" % (args.template_dir, tmpl.source)
>  
> -def delete(args):
> +def purge(args):
>  tmpl = template.Template.from_uri(args.template)
>  source = tmpl.get_source_impl()
>  source.delete_template(template=tmpl,
> templatedir=get_template_dir(args))
>  
> -def create(args):
> +def prepare(args):
>  tmpl = template.Template.from_uri(args.template)
>  source = tmpl.get_source_impl()
>  source.create_template(template=tmpl,
> @@ -91,7 +91,7 @@ def run(args):
>  
>  # Create the template image if needed
>  if not source.has_template(tmpl, template_dir):
> -create(args)
> +prepare(args)
>  
>  name = args.name
>  if name is None:
> @@ -197,26 +197,26 @@ Example supported URI formats:
>  """)
>  return parser
>  
> -def gen_delete_args(subparser):
> -parser = gen_command_parser(subparser, "delete",
> -_("Delete template data"))
> +def gen_purge_args(subparser):
> +parser = gen_command_parser(subparser, "purge",
> +_("Purge cached template"))
>  requires_debug(parser)
>  requires_template(parser)
>  requires_template_dir(parser)
> -parser.set_defaults(func=delete)
> +parser.set_defaults(func=purge)
>  
> -def gen_create_args(subparser):
> -parser = gen_command_parser(subparser, "create",
> -_("Create image from template data"))
> +def gen_prepare_args(subparser):
> +parser = gen_command_parser(subparser, "prepare",
> +_("Prepare local template"))
>  requires_debug(parser)
>  requires_template(parser)
>  requires_connect(parser)
>  requires_template_dir(parser)
> -parser.set_defaults(func=create)
> +parser.set_defaults(func=prepare)
>  
>  def gen_run_args(subparser):
>  parser = gen_command_parser(subparser, "run",
> -_("Run an already built image"))
> +_("Run an instance of a template"))
>  requires_debug(parser)
>  requires_name(parser)
>  requires_template(parser)
> @@ -248,8 +248,8 @@ def main():
>  parser = argparse.ArgumentParser(description="Sandbox Container Image 
> Tool")
>  
>  subparser = parser.add_subparsers(help=_("commands"))
> -gen_delete_args(subparser)
> -gen_create_args(subparser)
> +gen_purge_args(subparser)
> +gen_prepare_args(subparser)
>  gen_run_args(subparser)
>  gen_list_args(subparser)
>  

ACK

--
Cedric

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

Re: [libvirt] [PATCH v7 0/7] qemu: expand domain memory statistics

[Dmitry Derbyshev]

7/27/2016 2:10 PM, Pavel Hrdina пишет:

On Wed, Jul 13, 2016 at 01:42:11PM +0300, Derbyshev Dmitriy wrote:

From: Derbyshev Dmitry 

There are some things that needs to be updated, but if you're OK with me
updating those issues I can push the first 5 patches for you with the issues
fixed.

Pavel

I would be very glad if did so, thank you.
Last 2 patches are auxiliary, while the first 5 are important to us with 
or without them.


If I would like to resend new version of these last 2 patches, should I 
send them in the same patch series or other, if first patches are 
already applied?


Dmitry

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

[libvirt] [PATCH] virt-admin.pod: Remove a statement about remote access to the daemon

[Erik Skultety]

There's been a forgotten fragment (copy-paste error probably) in the
virt-admin's man page referring the reader to our web page on how to construct
URIs in case of remote access, which sort of implies that we support it which
we don't at the moment, so better remove that.

Signed-off-by: Erik Skultety 
---
Pushed under trivial rule.
Erik


 tools/virt-admin.pod | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod
index afc4689..2765bc5 100644
--- a/tools/virt-admin.pod
+++ b/tools/virt-admin.pod
@@ -134,9 +134,6 @@ connection at all or there is an inactive one.
 
 To find the currently used URI, check the I command documented below.
 
-For remote access see the documentation page at
-L on how to make URIs.
-
 =item B
 
 Prints the administrating server canonical URI, can be useful in shell mode. If
-- 
2.5.5

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

[libvirt] [PATCH sandbox] image: Rename 'create' and 'delete' to 'prepare' and 'purge'

[Daniel P. Berrange]

Currently we have three virt-sandbox-image commands

 - 'create' - downloads a template and creates qcow2
 - 'delete' - deletes template qcow2 files
 - 'run' - runs an instance of a template

The 'run' command is generating a transient guest which
disappears when it stops. We want to have the ability to
create / delete persistent guests later, for which the
command names "create" and "delete" are a natural fit.

So to avoid clash, rename the existing "create" command
to "prepare" and the "delete" command to "purge"
---
 libvirt-sandbox/image/cli.py | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 66854e4..3a5ccfa 100644
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -68,13 +68,13 @@ def get_template_dir(args):
 tmpl = template.Template.from_uri(args.template)
 return "%s/%s" % (args.template_dir, tmpl.source)
 
-def delete(args):
+def purge(args):
 tmpl = template.Template.from_uri(args.template)
 source = tmpl.get_source_impl()
 source.delete_template(template=tmpl,
templatedir=get_template_dir(args))
 
-def create(args):
+def prepare(args):
 tmpl = template.Template.from_uri(args.template)
 source = tmpl.get_source_impl()
 source.create_template(template=tmpl,
@@ -91,7 +91,7 @@ def run(args):
 
 # Create the template image if needed
 if not source.has_template(tmpl, template_dir):
-create(args)
+prepare(args)
 
 name = args.name
 if name is None:
@@ -197,26 +197,26 @@ Example supported URI formats:
 """)
 return parser
 
-def gen_delete_args(subparser):
-parser = gen_command_parser(subparser, "delete",
-_("Delete template data"))
+def gen_purge_args(subparser):
+parser = gen_command_parser(subparser, "purge",
+_("Purge cached template"))
 requires_debug(parser)
 requires_template(parser)
 requires_template_dir(parser)
-parser.set_defaults(func=delete)
+parser.set_defaults(func=purge)
 
-def gen_create_args(subparser):
-parser = gen_command_parser(subparser, "create",
-_("Create image from template data"))
+def gen_prepare_args(subparser):
+parser = gen_command_parser(subparser, "prepare",
+_("Prepare local template"))
 requires_debug(parser)
 requires_template(parser)
 requires_connect(parser)
 requires_template_dir(parser)
-parser.set_defaults(func=create)
+parser.set_defaults(func=prepare)
 
 def gen_run_args(subparser):
 parser = gen_command_parser(subparser, "run",
-_("Run an already built image"))
+_("Run an instance of a template"))
 requires_debug(parser)
 requires_name(parser)
 requires_template(parser)
@@ -248,8 +248,8 @@ def main():
 parser = argparse.ArgumentParser(description="Sandbox Container Image 
Tool")
 
 subparser = parser.add_subparsers(help=_("commands"))
-gen_delete_args(subparser)
-gen_create_args(subparser)
+gen_purge_args(subparser)
+gen_prepare_args(subparser)
 gen_run_args(subparser)
 gen_list_args(subparser)
 
-- 
2.7.4

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

Re: [libvirt] [PATCH v7 6/7] qemu: add cache for DomainMemoryStats

[Dmitry Derbyshev]

7/27/2016 2:14 PM, Daniel P. Berrange пишет:

On Wed, Jul 27, 2016 at 01:09:38PM +0200, Pavel Hrdina wrote:

On Wed, Jul 13, 2016 at 01:42:17PM +0300, Derbyshev Dmitriy wrote:

From: Igor Redko 

Communication with qemu monitor is time-consuming and there is
additional overhead for converting qemu_binary->json->libvirt_binary.

This patch tries to avoid unnecessary qmp queries.
Knowing period of balloon statistics update cycle and timestamps of last update
provided by qemu, it is possible to make cache of last response and reuse it
for next requests.

I don't think that this is necessary.  It doesn't take that much time to get the
memory stats and if someone want's to spam libvirt about those stats, well it's
pointless.  NACK from me to this patch and the last one, but let's see if
someone else has a different opinion.

Yeah, I don't really see a need for this. The stats refresh period is
exposed in the XML, so an application using libvirt can simply avoid
calling this libvirt API more frequently than the configured stats
period.


Regards,
Daniel
Our use-case may be somewhat different from the one you have in mind: 
several almost independent applications sending requests for new stats 
every now and then.
Even though each one of these applications may reduce the frequency of 
its own send request, overall load on the system is still high.
We could get around that by making one application devoted to caching 
these requests, but it is more natural to do this work in libvirt instead.


Thanks for your reply,
Dmitry

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

Re: [libvirt] [PATCH 00/25] Add support for multi-host gluster drives

[Peter Krempa]

On Tue, Jul 26, 2016 at 17:57:32 +0200, Ján Tomko wrote:
> On Mon, Jul 25, 2016 at 08:11:45PM +0200, Peter Krempa wrote:
> >This is a updated take based on stuff I had laying around and parts from
> >https://www.redhat.com/archives/libvir-list/2016-July/msg00872.html
> >
> >This addresses the backing store parser, adds and improves bits to the
> >JSON->commandline generator prior to plugging in the gluster support.
> >
> >This series does not yet address block jobs (snapshot/block copy) using
> >multi-host gluster volumes.
> >
> >Peter Krempa (24):
> >  tests: qemuxml2xml: Avoid crash when processing an XML that fails to
> >parse
> >  tests: Add testing of backing store string parser
> >  util: storage: Add parser for qemu's "json" backing pseudo-protocol
> >  util: storage: Add support for host device backing specified via JSON
> >  util: storage: Add support for URI based backing volumes in qemu's
> >JSON pseudo-protocol
> >  util: storage: Add json pseudo protocol support for gluster volumes
> >  util: storage: Add json pseudo protocol support for iSCSI volumes
> >  util: storage: Add JSON backing volume parser for 'nbd' protocol
> >  util: storage: Add JSON backing store parser for 'sheepdog' protocol
> >  util: storage: Add 'ssh' network storage protocol
> >  util: storage: Add JSON backing volume parser for 'ssh' protocol
> >  util: json: Make first argument of virJSONValueObjectForeachKeyValue
> >const
> >  util: qemu: Add wrapper for JSON -> commandline conversion
> >  util: qemu: Add support for user-passed strings in JSON->commandline
> >  util: qemu: Allow nested objects in JSON -> commandline generator
> >  util: qemu: Allow for different approaches to format JSON arrays
> >  util: qemu: Don't generate any extra commas in
> >virQEMUBuildCommandLineJSON
> >  qemu: command: Rename qemuBuildNetworkDriveURI to
> >qemuBuildNetworkDriveStr
> >  qemu: command: Split out network disk URI building
> >  qemu: command: Extract drive source command line formatter
> >  qemu: command: Refactor code extracted to qemuBuildDriveSourceStr
> >  storage: gluster: Support multiple hosts in backend functions
> >  util: qemu: Add support for numbered array members
> >  qemu: command: Add infrastructure for object specified disk sources
> >
> >Prasanna Kumar Kalever (1):
> >  qemu: command: Add support for multi-host gluster disksa

[...]

> ACK series

Thanks for the review. I've pushed this now since the freeze didn't
happen yet along with the updated JSON part as per review.

Peter

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

Re: [libvirt] [PATCHv2 00/10] JSON pseudo-protocol support for backing chains

[Ján Tomko]

On Wed, Jul 27, 2016 at 12:50:07PM +0200, Peter Krempa wrote:

This part is extracted from the gluster series since Eric's comment on the
original RFC triggered a rework of certain parts.

This version allows to handle both flattened and hierarchical syntaxes (see
patch 1/10) for details.

Peter Krempa (10):
 util: storage: Add parser for qemu's json backing pseudo-protocol
 util: json: Make first argument of virJSONValueCopy const
 util: storage: Add support for host device backing specified via JSON
 util: storage: Add support for URI based backing volumes in qemu's
   JSON pseudo-protocol
 util: storage: Add json pseudo protocol support for gluster volumes
 util: storage: Add json pseudo protocol support for iSCSI volumes
 util: storage: Add JSON backing volume parser for 'nbd' protocol
 util: storage: Add JSON backing store parser for 'sheepdog' protocol
 util: storage: Add 'ssh' network storage protocol
 util: storage: Add JSON backing volume parser for 'ssh' protocol

src/libxl/libxl_conf.c|   1 +
src/qemu/qemu_command.c   |   7 +
src/qemu/qemu_driver.c|   3 +
src/qemu/qemu_parse_command.c |   1 +
src/util/virjson.c|   2 +-
src/util/virjson.h|   2 +-
src/util/virstoragefile.c | 447 +-
src/util/virstoragefile.h |   1 +
src/xenconfig/xen_xl.c|   1 +
tests/virstoragetest.c| 131 +
10 files changed, 586 insertions(+), 10 deletions(-)


ACK series with the first two patches swapped.

Jan

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

Re: [libvirt] [PATCH] qemu: Adjust the cur_ballon on coldplug/unplug of dimms

[Peter Krempa]

On Thu, Jul 21, 2016 at 15:39:30 +0530, Shivaprasad G Bhat wrote:
> The cur_balloon also increases/decreases with dimm hotplug/unplug.
> To be consistent, adjust the value for coldplug too. This was inconsistently
> taken care when cur_ballon != memory to begin with. The patch fixes it
> irrespective of that.
> 
> Signed-off-by: Shivaprasad G Bhat 
> ---
>  src/conf/domain_conf.c |3 +--
>  src/qemu/qemu_driver.c |3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6dfcf81..8b0b790 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -14674,8 +14674,7 @@ virDomainMemoryRemove(virDomainDefPtr def,
>  VIR_DELETE_ELEMENT(def->mems, idx, def->nmems);
>  
>  /* fix up balloon size */
> -if (def->mem.cur_balloon > virDomainDefGetMemoryTotal(def))
> -def->mem.cur_balloon = virDomainDefGetMemoryTotal(def);

It will be better to move the balloon adjustment to the qemu driver
completely since the modification is done just there.

> +def->mem.cur_balloon -= ret->size;
>  
>  /* fix total memory size of the domain */
>  virDomainDefSetMemoryTotal(def, memory - ret->size);

For the hotplug case we are fixing this by querying the balloon so no
adjustments are necessary there.

ACK with the change (I've already made it locally) and I'll push it in a
while.

Peter

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

Re: [libvirt] [PATCH v7 6/7] qemu: add cache for DomainMemoryStats

[Daniel P. Berrange]

On Wed, Jul 27, 2016 at 01:09:38PM +0200, Pavel Hrdina wrote:
> On Wed, Jul 13, 2016 at 01:42:17PM +0300, Derbyshev Dmitriy wrote:
> > From: Igor Redko 
> > 
> > Communication with qemu monitor is time-consuming and there is
> > additional overhead for converting qemu_binary->json->libvirt_binary.
> > 
> > This patch tries to avoid unnecessary qmp queries.
> > Knowing period of balloon statistics update cycle and timestamps of last 
> > update
> > provided by qemu, it is possible to make cache of last response and reuse it
> > for next requests.
> 
> I don't think that this is necessary.  It doesn't take that much time to get 
> the
> memory stats and if someone want's to spam libvirt about those stats, well 
> it's
> pointless.  NACK from me to this patch and the last one, but let's see if
> someone else has a different opinion.

Yeah, I don't really see a need for this. The stats refresh period is
exposed in the XML, so an application using libvirt can simply avoid
calling this libvirt API more frequently than the configured stats
period.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH v7 0/7] qemu: expand domain memory statistics

[Pavel Hrdina]

On Wed, Jul 13, 2016 at 01:42:11PM +0300, Derbyshev Dmitriy wrote:
> From: Derbyshev Dmitry 

There are some things that needs to be updated, but if you're OK with me
updating those issues I can push the first 5 patches for you with the issues
fixed.

Pavel

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

Re: [libvirt] [PATCH v7 6/7] qemu: add cache for DomainMemoryStats

[Pavel Hrdina]

On Wed, Jul 13, 2016 at 01:42:17PM +0300, Derbyshev Dmitriy wrote:
> From: Igor Redko 
> 
> Communication with qemu monitor is time-consuming and there is
> additional overhead for converting qemu_binary->json->libvirt_binary.
> 
> This patch tries to avoid unnecessary qmp queries.
> Knowing period of balloon statistics update cycle and timestamps of last 
> update
> provided by qemu, it is possible to make cache of last response and reuse it
> for next requests.

I don't think that this is necessary.  It doesn't take that much time to get the
memory stats and if someone want's to spam libvirt about those stats, well it's
pointless.  NACK from me to this patch and the last one, but let's see if
someone else has a different opinion.

Pavel

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

Re: [libvirt] [PATCH v7 5/7] qemu: return balloon statistics when all domain statistics reported

[Pavel Hrdina]

On Wed, Jul 13, 2016 at 01:42:16PM +0300, Derbyshev Dmitriy wrote:
> From: Derbyshev Dmitry 
> 
> To collect all balloon statistics for all guests it was necessary to make
> several libvirt requests. Now it's possible to get all balloon statiscs via
> single connectGetAllDomainStats call.
> 
> Signed-off-by: Derbyshev Dmitry 
> ---
>  src/qemu/qemu_driver.c | 39 +--
>  tools/virsh.pod| 12 +++-
>  2 files changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6fa8d01..70f3faa 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18511,15 +18511,29 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver 
> ATTRIBUTE_UNUSED,
>  return 0;
>  }
>  
> +
> +#define STORE_MEM_RECORD(TAG, NAME) {  \
> +if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ ##TAG) \
> +if (virTypedParamsAddULLong(>params,   \
> +>nparams,  \
> +maxparams, \
> +"balloon." NAME,   \
> +stats[i].val) < 0) \
> +return -1; \
> +}

This isn't a correct way how to write a MACRO.  For starters, the syntax isn't
the same as for functions.  If you need to ensure that it is a separate block
the correct way is to do something like this:

#define MACRO() \
do {\
#body of the macro  \
} while (0)

But in this case it's not required to use the do-while construct.

Next point to this MACRO is that it uses other variables not passed as
parameters.  It's OK to do that but only inside a function and right before its
usage and also you should #undef it right away.  For example:

#define MACRO()

MACRO()
MACRO()
MACRO()

#undef MACRO

> +
>  static int
> -qemuDomainGetStatsBalloon(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> +qemuDomainGetStatsBalloon(virQEMUDriverPtr driver,
>virDomainObjPtr dom,
>virDomainStatsRecordPtr record,
>int *maxparams,
> -  unsigned int privflags ATTRIBUTE_UNUSED)
> +  unsigned int privflags)
>  {
>  qemuDomainObjPrivatePtr priv = dom->privateData;
> +virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR];
> +int nr_stats;
>  unsigned long long cur_balloon = 0;
> +size_t i;
>  int err = 0;
>  
>  if (!virDomainDefHasMemballoon(dom->def)) {
> @@ -18544,8 +18558,29 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver 
> ATTRIBUTE_UNUSED,
>  virDomainDefGetMemoryTotal(dom->def)) < 0)
>  return -1;
>  
> +if (err || !HAVE_JOB(privflags))
> +return 0;

There is no need to check the *err* variable, it's used only to indicate whether
the "balloon.current" should be reported or not.  I would change the condition
to this:

if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom))
return 0;

> +
> +nr_stats = qemuDomainMemoryStatsInternal(driver, dom, stats,
> + VIR_DOMAIN_MEMORY_STAT_NR);
> +if (nr_stats < 0)
> +return 0;
> +
> +for (i = 0; i < nr_stats; i++) {
> +STORE_MEM_RECORD(SWAP_IN, "swap_in")
> +STORE_MEM_RECORD(SWAP_OUT, "swap_out")
> +STORE_MEM_RECORD(MAJOR_FAULT, "major_fault")
> +STORE_MEM_RECORD(MINOR_FAULT, "minor_fault")
> +STORE_MEM_RECORD(UNUSED, "unused")
> +STORE_MEM_RECORD(AVAILABLE, "available")
> +STORE_MEM_RECORD(RSS, "rss")
> +STORE_MEM_RECORD(LAST_UPDATE, "last-update")
> +STORE_MEM_RECORD(USABLE, "usable")
> +}
> +
>  return 0;
>  }
> +#undef STORE_MEM_RECORD
>  
>  
>  static int
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 6af94d1..cda1874 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -899,7 +899,17 @@ I<--cpu-total> returns:
>  
>  I<--balloon> returns:
>  "balloon.current" - the memory in kiB currently used,
> -"balloon.maximum" - the maximum memory in kiB allowed
> +"balloon.maximum" - the maximum memory in kiB allowed,
> +"balloon.swap_in" - the amount of data read from swap space (in kB),
> +"balloon.swap_out" - the amount of memory written out to swap space (in kB),
> +"balloon.major_fault" - the number of page faults then disk IO was required,
> +"balloon.minor_fault" - the number of other page faults,
> +"balloon.unused" - the amount of memory left unused by the system (in kB),
> +"balloon.available" - the amount of usable memory as seen by the domain (in 
> kB),
> +"balloon.rss" - Resident Set Size of running domain's process (in kB),
> +"balloon.usable" - the amount of memory which can be reclaimed by balloon
> +without causing host swapping (in KB),
> +"balloon.last-update" - 

Re: [libvirt] [PATCH 1/2] util: Introduce virISCSINodeNew

[John Ferlan]

On 07/27/2016 06:48 AM, Ján Tomko wrote:
> On Tue, Jul 26, 2016 at 10:25:41PM -0400, John Ferlan wrote:
>>
>>
>> On 07/26/2016 12:24 PM, Ján Tomko wrote:
>>> On Mon, Jul 18, 2016 at 07:37:20AM -0400, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1356436

 According to RFC 3721 (https://www.ietf.org/rfc/rfc3721.txt), there are
 two ways to "discover" targets in/for the iSCSI environment. Discovery
 is the process which allows the initiator to find the targets to which
 it has access and at least one address at which each target may be
 accessed.

 The method currently implemented in libvirt using the
 virISCSIScanTargets
 API is known as "SendTargets" discovery. This method is more useful
 when
 the target IP Address and TCP port information are available, e.g. in
 libvirt terms the "portal". It returns a list of targets for the
 portal.
> From that list, the target can be found. This operation can also
> fill an
 iSCSI node table into which iSCSI logins may occur. Commit id
 '56057900'
 altered that filling by adding the "--op nonpersistent" since it was
 not necessarily desired to perform that for non libvirt related
 targets.

 The second method is "Static Configuration". This method not only needs
 the IP Address and TCP port (e.g. portal), but also the iSCSI target
 name.
 In libvirt terms this would be the device path field from the iSCSI
 pool
  XML. This patch implements the second methodology using that
 required device path as the targetname.

 Signed-off-by: John Ferlan 
 ---
 src/libvirt_private.syms |  1 +
 src/util/viriscsi.c  | 45
 +
 src/util/viriscsi.h  |  6 ++
 3 files changed, 52 insertions(+)

 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 3580a72..edef70b 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1714,6 +1714,7 @@ iptablesRemoveUdpOutput;
 virISCSIConnectionLogin;
 virISCSIConnectionLogout;
 virISCSIGetSession;
 +virISCSINodeNew;
 virISCSINodeUpdate;
 virISCSIRescanLUNs;
 virISCSIScanTargets;
 diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
 index e705517..8115d09 100644
 --- a/src/util/viriscsi.c
 +++ b/src/util/viriscsi.c
 @@ -444,6 +444,51 @@ virISCSIScanTargets(const char *portal,
 return ret;
 }

 +/*
 + * virISCSINodeNew:
 + * @portal: address for iSCSI target
 + * @target: IQN and specific LUN target
 + *
 + * Usage of nonpersistent discovery in virISCSIScanTargets is useful
 primarily
 + * only when the target IQN is not known; however, since we have the
 target IQN
 + * usage of the "--op new" can be done. This avoids problems if "--op
 delete"
 + * had been used wiping out the static nodes determined by the
 scanning of
 + * all targets.
 + *
 + * NB: If already created, subsequent "--op new" commands do not
 return
 + * an error.
>>>
>>> If it does not return an error, do we need to ignore non-zero status?
>>
>> I thought the point of the comment is that subsequent --op new commands
>> won't cause an error "if" the node record was generated. IOW: It's ok to
>> have multiple NodeNew commands and those won't cause an error.  If there
>> was some other failure, then I'd expect to get and report some error.
>> But for this particular command the code isn't ignoring any error, so
>> the code isn't ignoring errors...
>>
> 
> Perhaps I should have written it below the code, not the above comment:
> 
> +/* Ignore non-zero status.  */
> +if (virCommandRun(cmd, ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed new node mode for target '%s'"),
> +   target);
> +goto cleanup;
> +}
> 
> With a non-NULL second argument, virCommandRun ignores non-zero exit
> status and expects the caller to deal with it.
> 
> If we don't need to be compatible with old broken iscsiadm and multiple
> --op new commands do not return an error, we should error out on
> non-zero status.
> 

A  OK - I see your point now.  Of course none of this code deals
with the exitstatus anyway yet, but I can add something before pushing -

John

> Jan
> 
>> John
>>>
>>> AFAIK the rest of the code ignoring exit codes was written before
>>> iscsiadm
>>> returned them properly:
>>> https://github.com/open-iscsi/open-iscsi/commit/2c839a2
>>>
>>> Even this libvirt commit from Jan 2010 speaks of 'older versions of
>>> iscsiadm':
>>> http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=6aabcb5b
>>>
>>> Jan

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

[libvirt] [PATCHv2 09/10] util: storage: Add 'ssh' network storage protocol

[Peter Krempa]

Allow using 'ssh' protocol in backing chains and later for disks
themselves.
---
 src/libxl/libxl_conf.c| 1 +
 src/qemu/qemu_command.c   | 7 +++
 src/qemu/qemu_driver.c| 3 +++
 src/qemu/qemu_parse_command.c | 1 +
 src/util/virstoragefile.c | 4 +++-
 src/util/virstoragefile.h | 1 +
 src/xenconfig/xen_xl.c| 1 +
 7 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 146e08a..2d6d5da 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -581,6 +581,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src,
 case VIR_STORAGE_NET_PROTOCOL_ISCSI:
 case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
 case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
+case VIR_STORAGE_NET_PROTOCOL_SSH:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 virReportError(VIR_ERR_NO_SUPPORT,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3dc131b..5ac0725 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -480,6 +480,9 @@ qemuNetworkDriveGetPort(int protocol,
 case VIR_STORAGE_NET_PROTOCOL_NBD:
 return 10809;

+case VIR_STORAGE_NET_PROTOCOL_SSH:
+return 22;
+
 case VIR_STORAGE_NET_PROTOCOL_ISCSI:
 case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
 /* no default port specified */
@@ -878,6 +881,10 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
 ret = virBufferContentAndReset();
 break;

+case VIR_STORAGE_NET_PROTOCOL_SSH:
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("'ssh' protocol is not yet supported"));
+goto cleanup;

 case VIR_STORAGE_NET_PROTOCOL_LAST:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0501003..dd5a9ff 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13372,6 +13372,7 @@ 
qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk)
 case VIR_STORAGE_NET_PROTOCOL_FTP:
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
+case VIR_STORAGE_NET_PROTOCOL_SSH:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("external inactive snapshots are not supported on 
"
@@ -13434,6 +13435,7 @@ 
qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
 case VIR_STORAGE_NET_PROTOCOL_FTP:
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
+case VIR_STORAGE_NET_PROTOCOL_SSH:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("external active snapshots are not supported on "
@@ -13578,6 +13580,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr 
conn,
 case VIR_STORAGE_NET_PROTOCOL_FTP:
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
+case VIR_STORAGE_NET_PROTOCOL_SSH:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("internal inactive snapshots are not supported on 
"
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 3f7e445..82d1621 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -2014,6 +2014,7 @@ qemuParseCommandLine(virCapsPtr caps,
 case VIR_STORAGE_NET_PROTOCOL_FTP:
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
+case VIR_STORAGE_NET_PROTOCOL_SSH:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 /* ignored for now */
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 169d70e..3e22b0c 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -84,7 +84,8 @@ VIR_ENUM_IMPL(virStorageNetProtocol, 
VIR_STORAGE_NET_PROTOCOL_LAST,
   "https",
   "ftp",
   "ftps",
-  "tftp")
+  "tftp",
+  "ssh")

 VIR_ENUM_IMPL(virStorageNetHostTransport, VIR_STORAGE_NET_HOST_TRANS_LAST,
   "tcp",
@@ -2501,6 +2502,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src,
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
 case VIR_STORAGE_NET_PROTOCOL_ISCSI:
 case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
+case VIR_STORAGE_NET_PROTOCOL_SSH:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("malformed backing store path for protocol %s"),
protocol);
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 1a76fad..3ea3a60 100644
--- a/src/util/virstoragefile.h
+++ 

[libvirt] [PATCHv2 05/10] util: storage: Add json pseudo protocol support for gluster volumes

[Peter Krempa]

Along with the legacy URI based syntax add support for the brand-new
fully object based syntax.
---
 src/util/virstoragefile.c | 108 ++
 tests/virstoragetest.c|  46 
 2 files changed, 154 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index af79f79..c9fd475 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2575,6 +2575,113 @@ virStorageSourceParseBackingJSONUri(virStorageSourcePtr 
src,
 }


+static int
+virStorageSourceParseBackingJSONGlusterHost(virStorageNetHostDefPtr host,
+virJSONValuePtr json)
+{
+const char *type = virJSONValueObjectGetString(json, "type");
+const char *hostname = virJSONValueObjectGetString(json, "host");
+const char *port = virJSONValueObjectGetString(json, "port");
+const char *socket = virJSONValueObjectGetString(json, "socket");
+int transport;
+
+if ((transport = virStorageNetHostTransportTypeFromString(type)) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("unknown backing store transport protocol '%s'"), 
type);
+return -1;
+}
+
+host->transport = transport;
+
+switch ((virStorageNetHostTransport) transport) {
+case VIR_STORAGE_NET_HOST_TRANS_TCP:
+if (!hostname) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing hostname for tcp backing server in "
+ "JSON backing definition for gluster volume"));
+return -1;
+}
+
+if (VIR_STRDUP(host->name, hostname) < 0 ||
+VIR_STRDUP(host->port, port) < 0)
+return -1;
+break;
+
+case VIR_STORAGE_NET_HOST_TRANS_UNIX:
+if (!socket) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing socket path for udp backing server in "
+ "JSON backing definition for gluster volume"));
+return -1;
+}
+
+
+if (VIR_STRDUP(host->socket, socket) < 0)
+return -1;
+break;
+
+case VIR_STORAGE_NET_HOST_TRANS_RDMA:
+case VIR_STORAGE_NET_HOST_TRANS_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("backing store protocol '%s' is not yet supported"),
+   type);
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src,
+virJSONValuePtr json,
+int opaque ATTRIBUTE_UNUSED)
+{
+const char *uri = virJSONValueObjectGetString(json, "filename");
+const char *volume = virJSONValueObjectGetString(json, "volume");
+const char *path = virJSONValueObjectGetString(json, "path");
+virJSONValuePtr server = virJSONValueObjectGetArray(json, "server");
+size_t nservers;
+size_t i;
+
+/* legacy URI based syntax passed via 'filename' option */
+if (uri)
+return virStorageSourceParseBackingJSONUriStr(src, uri,
+  
VIR_STORAGE_NET_PROTOCOL_GLUSTER);
+
+if (!volume || !path || !server) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing 'volume', 'path' or 'server' attribute in "
+ "JSON backing definition for gluster volume"));
+return -1;
+}
+
+if (VIR_STRDUP(src->volume, volume) < 0 ||
+virAsprintf(>path, "/%s", path) < 0)
+return -1;
+
+nservers = virJSONValueArraySize(server);
+
+if (nservers < 1) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("at least 1 server is necessary in "
+ "JSON backing definition for gluster volume"));
+}
+
+if (VIR_ALLOC_N(src->hosts, nservers) < 0)
+return -1;
+src->nhosts = nservers;
+
+for (i = 0; i < nservers; i++) {
+if (virStorageSourceParseBackingJSONGlusterHost(src->hosts + i,
+
virJSONValueArrayGet(server, i)) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
 struct virStorageSourceJSONDriverParser {
 const char *drvname;
 int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
@@ -2590,6 +2697,7 @@ static const struct virStorageSourceJSONDriverParser 
jsonParsers[] = {
 {"ftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTP},
 {"ftps", virStorageSourceParseBackingJSONUri, 
VIR_STORAGE_NET_PROTOCOL_FTPS},
 {"tftp", virStorageSourceParseBackingJSONUri, 
VIR_STORAGE_NET_PROTOCOL_TFTP},
+{"gluster", virStorageSourceParseBackingJSONGluster, 0},
 };


diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index f8e6e9a..22aae47 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1391,6 +1391,52 @@ 

[libvirt] [PATCHv2 00/10] JSON pseudo-protocol support for backing chains

[Peter Krempa]

This part is extracted from the gluster series since Eric's comment on the
original RFC triggered a rework of certain parts.

This version allows to handle both flattened and hierarchical syntaxes (see
patch 1/10) for details.

Peter Krempa (10):
  util: storage: Add parser for qemu's json backing pseudo-protocol
  util: json: Make first argument of virJSONValueCopy const
  util: storage: Add support for host device backing specified via JSON
  util: storage: Add support for URI based backing volumes in qemu's
JSON pseudo-protocol
  util: storage: Add json pseudo protocol support for gluster volumes
  util: storage: Add json pseudo protocol support for iSCSI volumes
  util: storage: Add JSON backing volume parser for 'nbd' protocol
  util: storage: Add JSON backing store parser for 'sheepdog' protocol
  util: storage: Add 'ssh' network storage protocol
  util: storage: Add JSON backing volume parser for 'ssh' protocol

 src/libxl/libxl_conf.c|   1 +
 src/qemu/qemu_command.c   |   7 +
 src/qemu/qemu_driver.c|   3 +
 src/qemu/qemu_parse_command.c |   1 +
 src/util/virjson.c|   2 +-
 src/util/virjson.h|   2 +-
 src/util/virstoragefile.c | 447 +-
 src/util/virstoragefile.h |   1 +
 src/xenconfig/xen_xl.c|   1 +
 tests/virstoragetest.c| 131 +
 10 files changed, 586 insertions(+), 10 deletions(-)

-- 
2.9.0

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

[libvirt] [PATCHv2 10/10] util: storage: Add JSON backing volume parser for 'ssh' protocol

[Peter Krempa]

---
 src/util/virstoragefile.c | 38 ++
 tests/virstoragetest.c| 19 +++
 2 files changed, 57 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 3e22b0c..fa12b28 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2772,6 +2772,43 @@ 
virStorageSourceParseBackingJSONSheepdog(virStorageSourcePtr src,
 }


+static int
+virStorageSourceParseBackingJSONSSH(virStorageSourcePtr src,
+virJSONValuePtr json,
+int opaque ATTRIBUTE_UNUSED)
+{
+const char *path = virJSONValueObjectGetString(json, "path");
+const char *host = virJSONValueObjectGetString(json, "host");
+const char *port = virJSONValueObjectGetString(json, "port");
+
+if (!host || !path) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing host or path of SSH JSON backing "
+ "volume definition"));
+return -1;
+}
+
+src->type = VIR_STORAGE_TYPE_NETWORK;
+src->protocol = VIR_STORAGE_NET_PROTOCOL_SSH;
+
+if (VIR_STRDUP(src->path, path) < 0)
+return -1;
+
+if (VIR_ALLOC_N(src->hosts, 1) < 0)
+return -1;
+src->nhosts = 1;
+
+src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
+if (VIR_STRDUP(src->hosts[0].name, host) < 0)
+return -1;
+
+if (VIR_STRDUP(src->hosts[0].port, port) < 0)
+return -1;
+
+return 0;
+}
+
+
 struct virStorageSourceJSONDriverParser {
 const char *drvname;
 int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
@@ -2791,6 +2828,7 @@ static const struct virStorageSourceJSONDriverParser 
jsonParsers[] = {
 {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0},
 {"nbd", virStorageSourceParseBackingJSONNbd, 0},
 {"sheepdog", virStorageSourceParseBackingJSONSheepdog, 0},
+{"ssh", virStorageSourceParseBackingJSONSSH, 0},
 };


diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 4ddcac0..3b19f59 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1467,6 +1467,25 @@ mymain(void)
"\n"
"  \n"
"\n");
+TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"ssh\","
+   "\"host\":\"example.org\","
+   "\"port\":\"6000\","
+   "\"path\":\"blah\","
+   "\"user\":\"user\""
+  "}"
+"}",
+   "\n"
+   "  \n"
+   "\n");
+TEST_BACKING_PARSE("json:{\"file.driver\":\"ssh\","
+ "\"file.host\":\"example.org\","
+ "\"file.port\":\"6000\","
+ "\"file.path\":\"blah\","
+ "\"file.user\":\"user\""
+"}",
+   "\n"
+   "  \n"
+   "\n");

  cleanup:
 /* Final cleanup */
-- 
2.9.0

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

[libvirt] [PATCHv2 01/10] util: storage: Add parser for qemu's json backing pseudo-protocol

[Peter Krempa]

Add a modular parser that will allow to parse 'json' backing definitions
that are supported by qemu. The initial implementation adds support for
the 'file' driver.

Due to the approach qemu took to implement the JSON backing strings it's
possible to specify them in two approaches.

The object approach:
json:{ "file" : { "driver":"file",
  "filename":"/path/to/file"
}
 }

And a partially flattened approach:
json:{"file.driver":"file"
  "file.filename":"/path/to/file"
 }

Both of the above are supported by qemu and by the code added in this
commit. The current implementation de-flattens the first level ('file.')
if possible and required. Other handling may be added later but
currently only one level was possible anyways.
---
 src/util/virstoragefile.c | 161 --
 tests/virstoragetest.c|  15 +
 2 files changed, 169 insertions(+), 7 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 0fa9681..763bec6 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -43,6 +43,7 @@
 #include "viruri.h"
 #include "dirname.h"
 #include "virbuffer.h"
+#include "virjson.h"

 #define VIR_FROM_THIS VIR_FROM_STORAGE

@@ -2514,10 +2515,154 @@ virStorageSourceParseBackingColon(virStorageSourcePtr 
src,
 }


+static int
+virStorageSourceParseBackingJSONPath(virStorageSourcePtr src,
+ virJSONValuePtr json,
+ int type)
+{
+const char *path;
+
+if (!(path = virJSONValueObjectGetString(json, "filename"))) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing 'filename' field in JSON backing volume "
+ "definition"));
+return -1;
+}
+
+if (VIR_STRDUP(src->path, path) < 0)
+return -1;
+
+src->type = type;
+return 0;
+}
+
+
+struct virStorageSourceJSONDriverParser {
+const char *drvname;
+int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
+int opaque;
+};
+
+static const struct virStorageSourceJSONDriverParser jsonParsers[] = {
+{"file", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_FILE},
+};
+
+
+static int
+virStorageSourceParseBackingJSONDeflattenWorker(const char *key,
+const virJSONValue *value,
+void *opaque)
+{
+virJSONValuePtr retobj = opaque;
+virJSONValuePtr newval = NULL;
+const char *newkey;
+
+if (!(newkey = STRSKIP(key, "file."))) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("JSON backing file syntax is neither nested nor "
+ "flattened"));
+return -1;
+}
+
+if (!(newval = virJSONValueCopy(value)))
+return -1;
+
+if (virJSONValueObjectAppend(retobj, newkey, newval) < 0) {
+virJSONValueFree(newval);
+return -1;
+}
+
+return 0;
+}
+
+
+/**
+ * virStorageSourceParseBackingJSONDeflatten:
+ *
+ * The json: pseudo-protocol syntax in qemu allows multiple approaches to
+ * describe nesting of the values. This is due to the lax handling of the 
string
+ * in qemu and the fact that internally qemu is flattening the values using 
'.'.
+ *
+ * This allows to specify nested json strings either using nested json objects
+ * or prefixing object members with the parent object name followed by the dot.
+ *
+ * This function will attempt to reverse the process and provide a nested json
+ * hierarchy so that the parsers can be kept simple and we still can use the
+ * weird syntax some users might use.
+ *
+ * Currently this function will flatten out just the 'file.' prefix into a new
+ * tree. Any other syntax will be rejected.
+ */
+static virJSONValuePtr
+virStorageSourceParseBackingJSONDeflatten(virJSONValuePtr json)
+{
+virJSONValuePtr ret;
+
+if (!(ret = virJSONValueNewObject()))
+return NULL;
+
+if (virJSONValueObjectForeachKeyValue(json,
+  
virStorageSourceParseBackingJSONDeflattenWorker,
+  ret) < 0) {
+virJSONValueFree(ret);
+return NULL;
+}
+
+return ret;
+}
+
+
+static int
+virStorageSourceParseBackingJSON(virStorageSourcePtr src,
+ const char *json)
+{
+virJSONValuePtr root = NULL;
+virJSONValuePtr fixedroot = NULL;
+virJSONValuePtr file;
+const char *drvname;
+size_t i;
+int ret = -1;
+
+if (!(root = virJSONValueFromString(json)))
+return -1;
+
+if (!(file = virJSONValueObjectGetObject(root, "file"))) {
+if (!(fixedroot = virStorageSourceParseBackingJSONDeflatten(root)))
+goto cleanup;
+
+file = fixedroot;
+}
+
+if (!(drvname = virJSONValueObjectGetString(file, "driver"))) {
+

[libvirt] [PATCHv2 08/10] util: storage: Add JSON backing store parser for 'sheepdog' protocol

[Peter Krempa]

---
 src/util/virstoragefile.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 53ff710..169d70e 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2746,6 +2746,30 @@ virStorageSourceParseBackingJSONNbd(virStorageSourcePtr 
src,
 }


+static int
+virStorageSourceParseBackingJSONSheepdog(virStorageSourcePtr src,
+ virJSONValuePtr json,
+ int opaque ATTRIBUTE_UNUSED)
+{
+const char *filename;
+
+/* legacy URI based syntax passed via 'filename' option */
+if ((filename = virJSONValueObjectGetString(json, "filename"))) {
+if (strstr(filename, "://"))
+return virStorageSourceParseBackingJSONUriStr(src, filename,
+  
VIR_STORAGE_NET_PROTOCOL_SHEEPDOG);
+
+/* libvirt doesn't implement a parser for the legacy non-URI syntax */
+}
+
+/* Sheepdog currently supports only URI and legacy syntax passed in as 
filename */
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing sheepdog URI in JSON backing volume 
definition"));
+
+return -1;
+}
+
+
 struct virStorageSourceJSONDriverParser {
 const char *drvname;
 int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
@@ -2764,6 +2788,7 @@ static const struct virStorageSourceJSONDriverParser 
jsonParsers[] = {
 {"gluster", virStorageSourceParseBackingJSONGluster, 0},
 {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0},
 {"nbd", virStorageSourceParseBackingJSONNbd, 0},
+{"sheepdog", virStorageSourceParseBackingJSONSheepdog, 0},
 };


-- 
2.9.0

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

[libvirt] [PATCHv2 06/10] util: storage: Add json pseudo protocol support for iSCSI volumes

[Peter Krempa]

iSCSI is a bit odd in this aspect since it only supports URIs but using
the 'filename' property and does not have any alternative syntax.
---
 src/util/virstoragefile.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index c9fd475..66dbbef 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2682,6 +2682,26 @@ 
virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src,
 }


+static int
+virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
+  virJSONValuePtr json,
+  int opaque ATTRIBUTE_UNUSED)
+{
+const char *uri;
+
+/* legacy URI based syntax passed via 'filename' option */
+if ((uri = virJSONValueObjectGetString(json, "filename")))
+return virStorageSourceParseBackingJSONUriStr(src, uri,
+  
VIR_STORAGE_NET_PROTOCOL_ISCSI);
+
+/* iSCSI currently supports only URI syntax passed in as filename */
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing iSCSI URI in JSON backing volume definition"));
+
+return -1;
+}
+
+
 struct virStorageSourceJSONDriverParser {
 const char *drvname;
 int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
@@ -2698,6 +2718,7 @@ static const struct virStorageSourceJSONDriverParser 
jsonParsers[] = {
 {"ftps", virStorageSourceParseBackingJSONUri, 
VIR_STORAGE_NET_PROTOCOL_FTPS},
 {"tftp", virStorageSourceParseBackingJSONUri, 
VIR_STORAGE_NET_PROTOCOL_TFTP},
 {"gluster", virStorageSourceParseBackingJSONGluster, 0},
+{"iscsi", virStorageSourceParseBackingJSONiSCSI, 0},
 };


-- 
2.9.0

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

[libvirt] [PATCHv2 07/10] util: storage: Add JSON backing volume parser for 'nbd' protocol

[Peter Krempa]

---
 src/util/virstoragefile.c | 45 +
 tests/virstoragetest.c| 30 ++
 2 files changed, 75 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 66dbbef..53ff710 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2702,6 +2702,50 @@ 
virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
 }


+static int
+virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src,
+virJSONValuePtr json,
+int opaque ATTRIBUTE_UNUSED)
+{
+const char *path = virJSONValueObjectGetString(json, "path");
+const char *host = virJSONValueObjectGetString(json, "host");
+const char *port = virJSONValueObjectGetString(json, "port");
+const char *export = virJSONValueObjectGetString(json, "export");
+
+if (!path && !host) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing path or host of NBD server in JSON backing "
+ "volume definition"));
+return -1;
+}
+
+src->type = VIR_STORAGE_TYPE_NETWORK;
+src->protocol = VIR_STORAGE_NET_PROTOCOL_NBD;
+
+if (VIR_STRDUP(src->path, export) < 0)
+return -1;
+
+if (VIR_ALLOC_N(src->hosts, 1) < 0)
+return -1;
+src->nhosts = 1;
+
+if (path) {
+src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_UNIX;
+if (VIR_STRDUP(src->hosts[0].socket, path) < 0)
+return -1;
+} else {
+src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
+if (VIR_STRDUP(src->hosts[0].name, host) < 0)
+return -1;
+
+if (VIR_STRDUP(src->hosts[0].port, port) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
 struct virStorageSourceJSONDriverParser {
 const char *drvname;
 int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
@@ -2719,6 +2763,7 @@ static const struct virStorageSourceJSONDriverParser 
jsonParsers[] = {
 {"tftp", virStorageSourceParseBackingJSONUri, 
VIR_STORAGE_NET_PROTOCOL_TFTP},
 {"gluster", virStorageSourceParseBackingJSONGluster, 0},
 {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0},
+{"nbd", virStorageSourceParseBackingJSONNbd, 0},
 };


diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 22aae47..4ddcac0 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1437,6 +1437,36 @@ mymain(void)
 "  \n"
 "  \n"
 "\n");
+TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"nbd\","
+   "\"path\":\"/path/to/socket\""
+  "}"
+"}",
+   "\n"
+   "  \n"
+   "\n");
+TEST_BACKING_PARSE("json:{\"file.driver\":\"nbd\","
+ "\"file.path\":\"/path/to/socket\""
+"}",
+   "\n"
+   "  \n"
+   "\n");
+TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"nbd\","
+   "\"export\":\"blah\","
+   "\"host\":\"example.org\","
+   "\"port\":\"6000\""
+  "}"
+"}",
+   "\n"
+   "  \n"
+   "\n");
+TEST_BACKING_PARSE("json:{\"file.driver\":\"nbd\","
+ "\"file.export\":\"blah\","
+ "\"file.host\":\"example.org\","
+ "\"file.port\":\"6000\""
+"}",
+   "\n"
+   "  \n"
+   "\n");

  cleanup:
 /* Final cleanup */
-- 
2.9.0

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

[libvirt] [PATCHv2 03/10] util: storage: Add support for host device backing specified via JSON

[Peter Krempa]

JSON pseudo protocol for qemu allows to explicitly specify devices.
Add convertor to the internal type.
---
 src/util/virstoragefile.c | 2 ++
 tests/virstoragetest.c| 6 ++
 2 files changed, 8 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 763bec6..766ae28 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2545,6 +2545,8 @@ struct virStorageSourceJSONDriverParser {

 static const struct virStorageSourceJSONDriverParser jsonParsers[] = {
 {"file", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_FILE},
+{"host_device", virStorageSourceParseBackingJSONPath, 
VIR_STORAGE_TYPE_BLOCK},
+{"host_cdrom", virStorageSourceParseBackingJSONPath, 
VIR_STORAGE_TYPE_BLOCK},
 };


diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 04575f2..6873180 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1370,6 +1370,12 @@ mymain(void)
 "}"
 "}",
"\n");
+TEST_BACKING_PARSE("json:{\"file.driver\":\"host_device\", "
+ "\"file.filename\":\"/path/to/dev\"}",
+   "\n");
+TEST_BACKING_PARSE("json:{\"file.driver\":\"host_cdrom\", "
+ "\"file.filename\":\"/path/to/cdrom\"}",
+   "\n");

  cleanup:
 /* Final cleanup */
-- 
2.9.0

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

[libvirt] [PATCHv2 02/10] util: json: Make first argument of virJSONValueCopy const

[Peter Krempa]

It's just read.
---
 src/util/virjson.c | 2 +-
 src/util/virjson.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/virjson.c b/src/util/virjson.c
index afc98e3..b6d9a34 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1241,7 +1241,7 @@ virJSONValueObjectForeachKeyValue(const virJSONValue 
*object,


 virJSONValuePtr
-virJSONValueCopy(virJSONValuePtr in)
+virJSONValueCopy(const virJSONValue *in)
 {
 size_t i;
 virJSONValuePtr out = NULL;
diff --git a/src/util/virjson.h b/src/util/virjson.h
index a5aef39..64cae88 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -171,6 +171,6 @@ int virJSONValueObjectForeachKeyValue(const virJSONValue 
*object,
   virJSONValueObjectIteratorFunc cb,
   void *opaque);

-virJSONValuePtr virJSONValueCopy(virJSONValuePtr in);
+virJSONValuePtr virJSONValueCopy(const virJSONValue *in);

 #endif /* __VIR_JSON_H_ */
-- 
2.9.0

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

[libvirt] [PATCHv2 04/10] util: storage: Add support for URI based backing volumes in qemu's JSON pseudo-protocol

[Peter Krempa]

http(s), ftp(s) and tftp use URIs for volume definitions in the JSON
pseudo protocol so it's pretty straightforward to add support for them.
---
 src/util/virstoragefile.c | 43 +++
 tests/virstoragetest.c| 15 +++
 2 files changed, 58 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 766ae28..af79f79 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2537,6 +2537,44 @@ virStorageSourceParseBackingJSONPath(virStorageSourcePtr 
src,
 }


+static int
+virStorageSourceParseBackingJSONUriStr(virStorageSourcePtr src,
+   const char *uri,
+   int protocol)
+{
+if (virStorageSourceParseBackingURI(src, uri) < 0)
+return -1;
+
+if (src->protocol != protocol) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("expected protocol '%s' but got '%s' in URI JSON 
volume "
+ "definition"),
+   virStorageNetProtocolTypeToString(protocol),
+   virStorageNetProtocolTypeToString(src->protocol));
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+virStorageSourceParseBackingJSONUri(virStorageSourcePtr src,
+virJSONValuePtr json,
+int protocol)
+{
+const char *uri;
+
+if (!(uri = virJSONValueObjectGetString(json, "uri"))) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing URI in JSON backing volume definition"));
+return -1;
+}
+
+return virStorageSourceParseBackingJSONUriStr(src, uri, protocol);
+}
+
+
 struct virStorageSourceJSONDriverParser {
 const char *drvname;
 int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
@@ -2547,6 +2585,11 @@ static const struct virStorageSourceJSONDriverParser 
jsonParsers[] = {
 {"file", virStorageSourceParseBackingJSONPath, VIR_STORAGE_TYPE_FILE},
 {"host_device", virStorageSourceParseBackingJSONPath, 
VIR_STORAGE_TYPE_BLOCK},
 {"host_cdrom", virStorageSourceParseBackingJSONPath, 
VIR_STORAGE_TYPE_BLOCK},
+{"http", virStorageSourceParseBackingJSONUri, 
VIR_STORAGE_NET_PROTOCOL_HTTP},
+{"https", virStorageSourceParseBackingJSONUri, 
VIR_STORAGE_NET_PROTOCOL_HTTPS},
+{"ftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_FTP},
+{"ftps", virStorageSourceParseBackingJSONUri, 
VIR_STORAGE_NET_PROTOCOL_FTPS},
+{"tftp", virStorageSourceParseBackingJSONUri, 
VIR_STORAGE_NET_PROTOCOL_TFTP},
 };


diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 6873180..f8e6e9a 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1376,6 +1376,21 @@ mymain(void)
 TEST_BACKING_PARSE("json:{\"file.driver\":\"host_cdrom\", "
  "\"file.filename\":\"/path/to/cdrom\"}",
"\n");
+TEST_BACKING_PARSE("json:{\"file.driver\":\"http\", "
+ "\"file.uri\":\"http://example.com/file\"};,
+   "\n"
+   "  \n"
+   "\n");
+TEST_BACKING_PARSE("json:{\"file\":{ \"driver\":\"http\","
+"\"uri\":\"http://example.com/file\";
+  "}"
+"}",
+   "\n"
+   "  \n"
+   "\n");
+TEST_BACKING_PARSE("json:{\"file.driver\":\"ftp\", "
+ "\"file.uri\":\"http://example.com/file\"};,
+   NULL);

  cleanup:
 /* Final cleanup */
-- 
2.9.0

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

Re: [libvirt] [PATCH 1/2] util: Introduce virISCSINodeNew

[Ján Tomko]

On Tue, Jul 26, 2016 at 10:25:41PM -0400, John Ferlan wrote:



On 07/26/2016 12:24 PM, Ján Tomko wrote:

On Mon, Jul 18, 2016 at 07:37:20AM -0400, John Ferlan wrote:

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

According to RFC 3721 (https://www.ietf.org/rfc/rfc3721.txt), there are
two ways to "discover" targets in/for the iSCSI environment. Discovery
is the process which allows the initiator to find the targets to which
it has access and at least one address at which each target may be
accessed.

The method currently implemented in libvirt using the virISCSIScanTargets
API is known as "SendTargets" discovery. This method is more useful when
the target IP Address and TCP port information are available, e.g. in
libvirt terms the "portal". It returns a list of targets for the portal.

From that list, the target can be found. This operation can also fill an

iSCSI node table into which iSCSI logins may occur. Commit id '56057900'
altered that filling by adding the "--op nonpersistent" since it was
not necessarily desired to perform that for non libvirt related targets.

The second method is "Static Configuration". This method not only needs
the IP Address and TCP port (e.g. portal), but also the iSCSI target
name.
In libvirt terms this would be the device path field from the iSCSI pool
 XML. This patch implements the second methodology using that
required device path as the targetname.

Signed-off-by: John Ferlan 
---
src/libvirt_private.syms |  1 +
src/util/viriscsi.c  | 45
+
src/util/viriscsi.h  |  6 ++
3 files changed, 52 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3580a72..edef70b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1714,6 +1714,7 @@ iptablesRemoveUdpOutput;
virISCSIConnectionLogin;
virISCSIConnectionLogout;
virISCSIGetSession;
+virISCSINodeNew;
virISCSINodeUpdate;
virISCSIRescanLUNs;
virISCSIScanTargets;
diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
index e705517..8115d09 100644
--- a/src/util/viriscsi.c
+++ b/src/util/viriscsi.c
@@ -444,6 +444,51 @@ virISCSIScanTargets(const char *portal,
return ret;
}

+/*
+ * virISCSINodeNew:
+ * @portal: address for iSCSI target
+ * @target: IQN and specific LUN target
+ *
+ * Usage of nonpersistent discovery in virISCSIScanTargets is useful
primarily
+ * only when the target IQN is not known; however, since we have the
target IQN
+ * usage of the "--op new" can be done. This avoids problems if "--op
delete"
+ * had been used wiping out the static nodes determined by the
scanning of
+ * all targets.
+ *
+ * NB: If already created, subsequent "--op new" commands do not return
+ * an error.


If it does not return an error, do we need to ignore non-zero status?


I thought the point of the comment is that subsequent --op new commands
won't cause an error "if" the node record was generated. IOW: It's ok to
have multiple NodeNew commands and those won't cause an error.  If there
was some other failure, then I'd expect to get and report some error.
But for this particular command the code isn't ignoring any error, so
the code isn't ignoring errors...



Perhaps I should have written it below the code, not the above comment:

+/* Ignore non-zero status.  */
+if (virCommandRun(cmd, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed new node mode for target '%s'"),
+   target);
+goto cleanup;
+}

With a non-NULL second argument, virCommandRun ignores non-zero exit
status and expects the caller to deal with it.

If we don't need to be compatible with old broken iscsiadm and multiple
--op new commands do not return an error, we should error out on non-zero 
status.

Jan


John


AFAIK the rest of the code ignoring exit codes was written before iscsiadm
returned them properly:
https://github.com/open-iscsi/open-iscsi/commit/2c839a2

Even this libvirt commit from Jan 2010 speaks of 'older versions of
iscsiadm':
http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=6aabcb5b

Jan


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

Re: [libvirt] [PATCH v7 4/7] qemu: split qemuDomainMemoryStats into internal and external functions

[Pavel Hrdina]

On Wed, Jul 13, 2016 at 01:42:15PM +0300, Derbyshev Dmitriy wrote:
> From: Derbyshev Dmitry 
> 
> Is necessary to call it from other contexts, such as 
> qemuDomainGetStatsBalloon.
> 
> Signed-off-by: Derbyshev Dmitry 
> ---
>  src/qemu/qemu_driver.c | 55 
> --
>  1 file changed, 35 insertions(+), 20 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f8d9afe..6fa8d01 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10959,32 +10959,22 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,
>  return ret;
>  }
>  
> +/* This functions assumes that job QEMU_JOB_QUERY is started by a caller */
> +

This was already pointed out, we don't put a new line between function and a
comment for that function.

>  static int
> -qemuDomainMemoryStats(virDomainPtr dom,
> -  virDomainMemoryStatPtr stats,
> -  unsigned int nr_stats,
> -  unsigned int flags)
> +qemuDomainMemoryStatsInternal(virQEMUDriverPtr driver,
> +  virDomainObjPtr vm,
> +  virDomainMemoryStatPtr stats,
> +  unsigned int nr_stats)
> +
>  {
> -virQEMUDriverPtr driver = dom->conn->privateData;
> -virDomainObjPtr vm;
>  int ret = -1;
>  long rss;
>  

ACK

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

Re: [libvirt] [PATCH] virconf: Fix config file path construction

[Erik Skultety]

On 27/07/16 11:21, Andrea Bolognani wrote:
> On Wed, 2016-07-27 at 08:50 +0200, Erik Skultety wrote:
>> Since commit c4bdff19, the path to the configuration file has been 
>> constructed
>> in the following manner:
>>   - if no config filename was passed to virConfLoadConfigPath, libvirt.conf 
>> was
>>   used as default
>>   - otherwise the filename was concatenated with
>>   "/libvirt/libvirt%s%s.conf" which in admin case resulted in
>>   "libvirt-libvirt-admin.conf.conf". Obviously, this non-existent config led 
>> to
>>   ignoring  all user settings in libvirt-admin.conf. This patch requires the
>>   config filename to be always provided as an argument with the concatenation
>>   being simplified.
>>  
>>   Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357364
>>  
>> Signed-off-by: Erik Skultety 
>> ---
>>   src/libvirt.c  |  2 +-
>>   src/util/virconf.c | 12 
>>   2 files changed, 5 insertions(+), 9 deletions(-)
>>  
>> diff --git a/src/libvirt.c b/src/libvirt.c
>> index 68c8317..52462e3 100644
>> --- a/src/libvirt.c
>> +++ b/src/libvirt.c
>> @@ -969,7 +969,7 @@ virConnectOpenInternal(const char *name,
>>   if (ret == NULL)
>>   return NULL;
>>   
>> -if (virConfLoadConfig(, NULL) < 0)
>> +if (virConfLoadConfig(, "libvirt.conf") < 0)
>>   goto failed;
>>   
>>   if (name && name[0] == '\0')
>> diff --git a/src/util/virconf.c b/src/util/virconf.c
>> index ee54072..3e49f41 100644
>> --- a/src/util/virconf.c
>> +++ b/src/util/virconf.c
>> @@ -1566,20 +1566,16 @@ virConfLoadConfigPath(const char *name)
>>   {
>>   char *path;
>>   if (geteuid() == 0) {
>> -if (virAsprintf(, "%s/libvirt/libvirt%s%s.conf",
>> -SYSCONFDIR,
>> -name ? "-" : "",
>> -name ? name : "") < 0)
>> +if (virAsprintf(, "%s/libvirt/%s",
>> +SYSCONFDIR, name) < 0)
>>   return NULL;
>>   } else {
>>   char *userdir = virGetUserConfigDirectory();
>>   if (!userdir)
>>   return NULL;
>>   
>> -if (virAsprintf(, "%s/libvirt%s%s.conf",
>> -userdir,
>> -name ? "-" : "",
>> -name ? name : "") < 0) {
>> +if (virAsprintf(, "%s/%s",
>> +userdir, name) < 0) {
>>   VIR_FREE(userdir);
>>   return NULL;
>>   }
> 
> You could also have changed src/libvirt-admin.c to call
> 
> virConfLoadConfig(, "admin")
> 
> instead, but your solution is clearer.
> 

Yeah, I know I could and I also thought about doing that, but I didn't
like it in the end, because imho (and you mentioned that too) being
explicit about the config file in this case is somewhat clearer. Also,
I'm not sure and I might be talking rubbish but I think I tried to use
the approach you mentioned above in the earlier versions of the patch
that modified this logic and was discouraged to do it that way in
reviews (but I would have to check the history :P to really confirm
that...). Anyway, thanks for review, I'll push that in a moment.

Erik

> ACK
> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 

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

Re: [libvirt] [PATCH 2/2] storage: remove "luks" storage volume type

[Martin Kletzander]

On Wed, Jul 27, 2016 at 10:23:18AM +0100, Daniel P. Berrange wrote:

On Wed, Jul 27, 2016 at 11:16:57AM +0200, Martin Kletzander wrote:

On Wed, Jul 27, 2016 at 09:29:29AM +0100, Daniel P. Berrange wrote:
> On Wed, Jul 27, 2016 at 08:44:53AM +0200, Martin Kletzander wrote:
> > On Tue, Jul 26, 2016 at 07:12:30PM +0100, Daniel P. Berrange wrote:
> > > The current LUKS support has a "luks" volume type which has
> > > a "luks" encryption format.
> > >
> > > This partially makes sense if you consider the QEMU shorthand
> > > syntax only requires you to specify a format=luks, and it'll
> > > automagically uses "raw" as the next level driver. QEMU will
> > > however let you override the "raw" with any other driver it
> > > supports (vmdk, qcow, rbd, iscsi, etc, etc)
> > >
> > > IOW the intention though is that the "luks" encryption format
> > > is applied to all disk formats (whether raw, qcow2, rbd, gluster
> > > or whatever). As such it doesn't make much sense for libvirt
> > > to say the volume type is "luks" - we should be saying that it
> > > is a "raw" file, but with "luks" encryption applied.
> > >
> > > IOW, when creating a storage volume we should use this XML
> > >
> > >  
> > >demo.raw
> > >5368709120
> > >
> > >  
> > >  
> > >
> > >  
> > >
> > >  
> > >
> > > and when configuring a guest disk we should use
> > >
> > >  
> > >
> > >
> > >
> > >
> > >  
> > >
> > >  
> > >
> > > This commit thus removes the "luks" storage volume type added
> > > in
> > >
> > >  commit 318ebb36f1027b3357a32d6f781bd77d7a9043fd
> > >  Author: John Ferlan 
> > >  Date:   Tue Jun 21 12:59:54 2016 -0400
> > >
> > >util: Add 'luks' to the FileTypeInfo
> > >
> > > The storage file probing code is modified so that it can probe
> > > the actual encryption formats explicitly, rather than merely
> > > probing existance of encryption and letting the storage driver
> > > guess the format.
> > >
> > > The rest of the code is then adapted to deal with
> > > VIR_STORAGE_FILE_RAW w/ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS
> > > instead of just VIR_STORAGE_FILE_LUKS.
> > >
> > > The commit mentioned above was included in libvirt v2.0.0.
> > > So when querying volume XML this will be a change in behaviour
> > > vs the 2.0.0 release - it'll report 'raw' instead of 'luks'
> > > for the volume format, but still report 'luks' for encryption
> > > format.  I think this change is OK because the storage driver
> > > did not include any support for creating volumes, nor starting
> > > guets with luks volumes in v2.0.0 - that only since then.
> > > Clearly if we change this we must do it before v2.1.0 though.
> > >
> > > Signed-off-by: Daniel P. Berrange 
> > > ---
> > > src/qemu/qemu_command.c|  10 +-
> > > src/qemu/qemu_domain.c |   2 +-
> > > src/qemu/qemu_hotplug.c|   2 +-
> > > src/storage/storage_backend.c  |  41 +++--
> > > src/storage/storage_backend_fs.c   |  17 +-
> > > src/storage/storage_backend_gluster.c  |   5 -
> > > src/util/virstoragefile.c  | 202 
-
> > > src/util/virstoragefile.h  |   1 -
> > > tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml |   4 +-
> > > tests/storagevolxml2xmlin/vol-luks-cipher.xml  |   2 +-
> > > tests/storagevolxml2xmlin/vol-luks.xml |   2 +-
> > > tests/storagevolxml2xmlout/vol-luks-cipher.xml |   2 +-
> > > tests/storagevolxml2xmlout/vol-luks.xml|   2 +-
> > > 13 files changed, 198 insertions(+), 94 deletions(-)
> > >
> > > diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> > > index 862fb29..5ef536a 100644
> > > --- a/src/storage/storage_backend.c
> > > +++ b/src/storage/storage_backend.c
> > > @@ -1116,9 +,9 @@ 
virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
> > > int accessRetCode = -1;
> > > char *absolutePath = NULL;
> > >
> > > -if (info->format == VIR_STORAGE_FILE_LUKS) {
> > > +if (info->format == VIR_STORAGE_FILE_RAW) {
> > > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > > -   _("cannot set backing store for luks volume"));
> > > +   _("cannot set backing store for raw volume"));
> > > return -1;
> > > }
> > >
> >
> > I think this whole condition can be removed as it wasn't there before
> > luks volumes.
> >
> > > @@ -1283,7 +1278,8 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
> > >_("format features only available with qcow2"));
> > > return NULL;
> > > }
> > > -if (info.format == VIR_STORAGE_FILE_LUKS) {
> > > +if (info.format == VIR_STORAGE_FILE_RAW &&
> > > +vol->target.encryption != NULL) {
> > > if (inputvol) {
> > > 

Re: [libvirt] [PATCH 2/2] storage: remove "luks" storage volume type

[Daniel P. Berrange]

On Wed, Jul 27, 2016 at 11:16:57AM +0200, Martin Kletzander wrote:
> On Wed, Jul 27, 2016 at 09:29:29AM +0100, Daniel P. Berrange wrote:
> > On Wed, Jul 27, 2016 at 08:44:53AM +0200, Martin Kletzander wrote:
> > > On Tue, Jul 26, 2016 at 07:12:30PM +0100, Daniel P. Berrange wrote:
> > > > The current LUKS support has a "luks" volume type which has
> > > > a "luks" encryption format.
> > > >
> > > > This partially makes sense if you consider the QEMU shorthand
> > > > syntax only requires you to specify a format=luks, and it'll
> > > > automagically uses "raw" as the next level driver. QEMU will
> > > > however let you override the "raw" with any other driver it
> > > > supports (vmdk, qcow, rbd, iscsi, etc, etc)
> > > >
> > > > IOW the intention though is that the "luks" encryption format
> > > > is applied to all disk formats (whether raw, qcow2, rbd, gluster
> > > > or whatever). As such it doesn't make much sense for libvirt
> > > > to say the volume type is "luks" - we should be saying that it
> > > > is a "raw" file, but with "luks" encryption applied.
> > > >
> > > > IOW, when creating a storage volume we should use this XML
> > > >
> > > >  
> > > >demo.raw
> > > >5368709120
> > > >
> > > >  
> > > >  
> > > > > > > uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/>
> > > >  
> > > >
> > > >  
> > > >
> > > > and when configuring a guest disk we should use
> > > >
> > > >  
> > > >
> > > >
> > > >
> > > >
> > > >   > > > uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/>
> > > >
> > > >  
> > > >
> > > > This commit thus removes the "luks" storage volume type added
> > > > in
> > > >
> > > >  commit 318ebb36f1027b3357a32d6f781bd77d7a9043fd
> > > >  Author: John Ferlan 
> > > >  Date:   Tue Jun 21 12:59:54 2016 -0400
> > > >
> > > >util: Add 'luks' to the FileTypeInfo
> > > >
> > > > The storage file probing code is modified so that it can probe
> > > > the actual encryption formats explicitly, rather than merely
> > > > probing existance of encryption and letting the storage driver
> > > > guess the format.
> > > >
> > > > The rest of the code is then adapted to deal with
> > > > VIR_STORAGE_FILE_RAW w/ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS
> > > > instead of just VIR_STORAGE_FILE_LUKS.
> > > >
> > > > The commit mentioned above was included in libvirt v2.0.0.
> > > > So when querying volume XML this will be a change in behaviour
> > > > vs the 2.0.0 release - it'll report 'raw' instead of 'luks'
> > > > for the volume format, but still report 'luks' for encryption
> > > > format.  I think this change is OK because the storage driver
> > > > did not include any support for creating volumes, nor starting
> > > > guets with luks volumes in v2.0.0 - that only since then.
> > > > Clearly if we change this we must do it before v2.1.0 though.
> > > >
> > > > Signed-off-by: Daniel P. Berrange 
> > > > ---
> > > > src/qemu/qemu_command.c|  10 +-
> > > > src/qemu/qemu_domain.c |   2 +-
> > > > src/qemu/qemu_hotplug.c|   2 +-
> > > > src/storage/storage_backend.c  |  41 +++--
> > > > src/storage/storage_backend_fs.c   |  17 +-
> > > > src/storage/storage_backend_gluster.c  |   5 -
> > > > src/util/virstoragefile.c  | 202 
> > > > -
> > > > src/util/virstoragefile.h  |   1 -
> > > > tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml |   4 +-
> > > > tests/storagevolxml2xmlin/vol-luks-cipher.xml  |   2 +-
> > > > tests/storagevolxml2xmlin/vol-luks.xml |   2 +-
> > > > tests/storagevolxml2xmlout/vol-luks-cipher.xml |   2 +-
> > > > tests/storagevolxml2xmlout/vol-luks.xml|   2 +-
> > > > 13 files changed, 198 insertions(+), 94 deletions(-)
> > > >
> > > > diff --git a/src/storage/storage_backend.c 
> > > > b/src/storage/storage_backend.c
> > > > index 862fb29..5ef536a 100644
> > > > --- a/src/storage/storage_backend.c
> > > > +++ b/src/storage/storage_backend.c
> > > > @@ -1116,9 +,9 @@ 
> > > > virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
> > > > int accessRetCode = -1;
> > > > char *absolutePath = NULL;
> > > >
> > > > -if (info->format == VIR_STORAGE_FILE_LUKS) {
> > > > +if (info->format == VIR_STORAGE_FILE_RAW) {
> > > > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > > > -   _("cannot set backing store for luks volume"));
> > > > +   _("cannot set backing store for raw volume"));
> > > > return -1;
> > > > }
> > > >
> > > 
> > > I think this whole condition can be removed as it wasn't there before
> > > luks volumes.
> > > 
> > > > @@ -1283,7 +1278,8 @@ 
> > > > virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
> > > >_("format features only 

Re: [libvirt] [PATCH] virconf: Fix config file path construction

[Andrea Bolognani]

On Wed, 2016-07-27 at 08:50 +0200, Erik Skultety wrote:
> Since commit c4bdff19, the path to the configuration file has been constructed
> in the following manner:
>  - if no config filename was passed to virConfLoadConfigPath, libvirt.conf was
>  used as default
>  - otherwise the filename was concatenated with
>  "/libvirt/libvirt%s%s.conf" which in admin case resulted in
>  "libvirt-libvirt-admin.conf.conf". Obviously, this non-existent config led to
>  ignoring  all user settings in libvirt-admin.conf. This patch requires the
>  config filename to be always provided as an argument with the concatenation
>  being simplified.
> 
>  Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357364
> 
> Signed-off-by: Erik Skultety 
> ---
>  src/libvirt.c  |  2 +-
>  src/util/virconf.c | 12 
>  2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 68c8317..52462e3 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -969,7 +969,7 @@ virConnectOpenInternal(const char *name,
>  if (ret == NULL)
>  return NULL;
>  
> -if (virConfLoadConfig(, NULL) < 0)
> +if (virConfLoadConfig(, "libvirt.conf") < 0)
>  goto failed;
>  
>  if (name && name[0] == '\0')
> diff --git a/src/util/virconf.c b/src/util/virconf.c
> index ee54072..3e49f41 100644
> --- a/src/util/virconf.c
> +++ b/src/util/virconf.c
> @@ -1566,20 +1566,16 @@ virConfLoadConfigPath(const char *name)
>  {
>  char *path;
>  if (geteuid() == 0) {
> -if (virAsprintf(, "%s/libvirt/libvirt%s%s.conf",
> -SYSCONFDIR,
> -name ? "-" : "",
> -name ? name : "") < 0)
> +if (virAsprintf(, "%s/libvirt/%s",
> +SYSCONFDIR, name) < 0)
>  return NULL;
>  } else {
>  char *userdir = virGetUserConfigDirectory();
>  if (!userdir)
>  return NULL;
>  
> -if (virAsprintf(, "%s/libvirt%s%s.conf",
> -userdir,
> -name ? "-" : "",
> -name ? name : "") < 0) {
> +if (virAsprintf(, "%s/%s",
> +userdir, name) < 0) {
>  VIR_FREE(userdir);
>  return NULL;
>  }

You could also have changed src/libvirt-admin.c to call

virConfLoadConfig(, "admin")

instead, but your solution is clearer.

ACK

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 2/2] storage: remove "luks" storage volume type

[Martin Kletzander]

On Wed, Jul 27, 2016 at 09:29:29AM +0100, Daniel P. Berrange wrote:

On Wed, Jul 27, 2016 at 08:44:53AM +0200, Martin Kletzander wrote:

On Tue, Jul 26, 2016 at 07:12:30PM +0100, Daniel P. Berrange wrote:
> The current LUKS support has a "luks" volume type which has
> a "luks" encryption format.
>
> This partially makes sense if you consider the QEMU shorthand
> syntax only requires you to specify a format=luks, and it'll
> automagically uses "raw" as the next level driver. QEMU will
> however let you override the "raw" with any other driver it
> supports (vmdk, qcow, rbd, iscsi, etc, etc)
>
> IOW the intention though is that the "luks" encryption format
> is applied to all disk formats (whether raw, qcow2, rbd, gluster
> or whatever). As such it doesn't make much sense for libvirt
> to say the volume type is "luks" - we should be saying that it
> is a "raw" file, but with "luks" encryption applied.
>
> IOW, when creating a storage volume we should use this XML
>
>  
>demo.raw
>5368709120
>
>  
>  
>
>  
>
>  
>
> and when configuring a guest disk we should use
>
>  
>
>
>
>
>  
>
>  
>
> This commit thus removes the "luks" storage volume type added
> in
>
>  commit 318ebb36f1027b3357a32d6f781bd77d7a9043fd
>  Author: John Ferlan 
>  Date:   Tue Jun 21 12:59:54 2016 -0400
>
>util: Add 'luks' to the FileTypeInfo
>
> The storage file probing code is modified so that it can probe
> the actual encryption formats explicitly, rather than merely
> probing existance of encryption and letting the storage driver
> guess the format.
>
> The rest of the code is then adapted to deal with
> VIR_STORAGE_FILE_RAW w/ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS
> instead of just VIR_STORAGE_FILE_LUKS.
>
> The commit mentioned above was included in libvirt v2.0.0.
> So when querying volume XML this will be a change in behaviour
> vs the 2.0.0 release - it'll report 'raw' instead of 'luks'
> for the volume format, but still report 'luks' for encryption
> format.  I think this change is OK because the storage driver
> did not include any support for creating volumes, nor starting
> guets with luks volumes in v2.0.0 - that only since then.
> Clearly if we change this we must do it before v2.1.0 though.
>
> Signed-off-by: Daniel P. Berrange 
> ---
> src/qemu/qemu_command.c|  10 +-
> src/qemu/qemu_domain.c |   2 +-
> src/qemu/qemu_hotplug.c|   2 +-
> src/storage/storage_backend.c  |  41 +++--
> src/storage/storage_backend_fs.c   |  17 +-
> src/storage/storage_backend_gluster.c  |   5 -
> src/util/virstoragefile.c  | 202 -
> src/util/virstoragefile.h  |   1 -
> tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml |   4 +-
> tests/storagevolxml2xmlin/vol-luks-cipher.xml  |   2 +-
> tests/storagevolxml2xmlin/vol-luks.xml |   2 +-
> tests/storagevolxml2xmlout/vol-luks-cipher.xml |   2 +-
> tests/storagevolxml2xmlout/vol-luks.xml|   2 +-
> 13 files changed, 198 insertions(+), 94 deletions(-)
>
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 862fb29..5ef536a 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -1116,9 +,9 @@ 
virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
> int accessRetCode = -1;
> char *absolutePath = NULL;
>
> -if (info->format == VIR_STORAGE_FILE_LUKS) {
> +if (info->format == VIR_STORAGE_FILE_RAW) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("cannot set backing store for luks volume"));
> +   _("cannot set backing store for raw volume"));
> return -1;
> }
>

I think this whole condition can be removed as it wasn't there before
luks volumes.

> @@ -1283,7 +1278,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr 
conn,
>_("format features only available with qcow2"));
> return NULL;
> }
> -if (info.format == VIR_STORAGE_FILE_LUKS) {
> +if (info.format == VIR_STORAGE_FILE_RAW &&
> +vol->target.encryption != NULL) {
> if (inputvol) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>_("cannot use inputvol with luks volume"));

You're still reporting the error for "luks volume" here.

> @@ -1484,13 +1490,16 @@ 
virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol,
> if (!inputvol)
> return NULL;
>
> -/* If either volume is a non-raw file vol, we need to use an external
> - * tool for converting
> +VIR_WARN("BUild vol from func");

Leftover from debugging?

> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 2834baa..c264041 100644
> --- 

Re: [libvirt] [PATCH] docs: Add Kimchi as Web Application

[Andrea Bolognani]

On Thu, 2016-07-14 at 15:54 -0300, Ramon Medeiros wrote:
> Kimchi is a open-source interface to kvm. It runs with HTML5, simple and
> easy to manage kvm guests.
> 
> Signed-off-by: Ramon Medeiros 
> ---
>  docs/apps.html.in | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/docs/apps.html.in b/docs/apps.html.in
> index b8337b9..1a6fd3f 100644
> --- a/docs/apps.html.in
> +++ b/docs/apps.html.in
> @@ -404,6 +404,14 @@
>  infrastructure. You can deploy a new service just dragging and
>  dropping a VM.
>
> +   href="https://kimchi-project.github.io/kimchi/;>Kimchi;;
> +  
> +Kimchi is an HTML5 based management tool for KVM. It is designed to
> +make it as easy as possible to get started with KVM and create your 
> first guest.
> +
> +Kimchi manages KVM guests through libvirt. The management interface 
> is accessed
> +over the web using a browser that supports HTML5.
> +  
>http://ovirt.org/;>oVirt;;
>
>  oVirt provides the ability to manage large numbers of virtual

ACK and pushed. Thanks!

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH 4/5] qemu: Enable secure boot

[Michal Privoznik]

In qemu, enabling this feature boils down to adding the following
onto the command line:

  -global driver=cfi.pflash01,property=secure,value=on

However, there are some constraints resulting from the
implementation. For instance, System Management Mode (SMM) is
required to be enabled, the machine type must be q35-2.5 or
later, and the guest should be x86_64. While technically it is
possible to have 32 bit guests with secure boot, some non-trivial
CPU flags tuning is required (for instance lm and nx flags must
be prohibited). Given complexity of our CPU driver, this is not
trivial. Therefore I've chosen to forbid 32 bit guests for now.
If there's ever need, we can refine the check later.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_command.c|  7 ++
 src/qemu/qemu_domain.c | 27 
 .../qemuxml2argv-bios-nvram-secure.args| 29 ++
 tests/qemuxml2argvtest.c   |  7 ++
 4 files changed, 70 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 831aba1..c6e629e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8731,6 +8731,13 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
 goto cleanup;
 }
 
+if (loader->secure == VIR_TRISTATE_BOOL_YES) {
+virCommandAddArgList(cmd,
+ "-global",
+ 
"driver=cfi.pflash01,property=secure,value=on",
+ NULL);
+}
+
 virBufferAsprintf(,
   "file=%s,if=pflash,format=raw,unit=%d",
   loader->path, unit);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9045f37..ed51481 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2321,6 +2321,33 @@ qemuDomainDefValidate(const virDomainDef *def,
 return -1;
 }
 
+if (def->os.loader &&
+def->os.loader->secure == VIR_TRISTATE_BOOL_YES) {
+/* These are the QEMU implementation limitations. But we
+ * have to live with them for now. */
+
+if (!qemuDomainMachineIsQ35(def)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Secure boot is supported with q35 machine types 
only"));
+return -1;
+}
+
+/* Now, technically it is possible to have secure boot on
+ * 32bits too, but that would require some -cpu xxx magic
+ * too. Not worth it unless we are explicitly asked. */
+if (def->os.arch != VIR_ARCH_X86_64) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Secure boot is supported for x86_64 architecture 
only"));
+return -1;
+}
+
+if (def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Secure boot requires SMM feature enabled"));
+return -1;
+}
+}
+
 return 0;
 }
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args 
b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args
new file mode 100644
index 000..c014254
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args
@@ -0,0 +1,29 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name test-bios \
+-S \
+-machine pc-q35-2.5,accel=tcg,smm=on \
+-global driver=cfi.pflash01,property=secure,value=on \
+-drive file=/usr/share/OVMF/OVMF_CODE.secboot.fd,if=pflash,format=raw,unit=0,\
+readonly=on \
+-drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-test-bios/monitor.sock,server,nowait \
+-boot c \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \
+-device virtio-scsi-pci,id=scsi0,bus=pci.2,addr=0x1 \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-scsi0-0-0-0 \
+-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\
+drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \
+-serial pty \
+-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x2
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f9588d5..7b05150 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -685,6 +685,13 @@ mymain(void)
 
 DO_TEST("bios", QEMU_CAPS_SGA);
 DO_TEST("bios-nvram", NONE);
+DO_TEST("bios-nvram-secure",
+QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+QEMU_CAPS_DEVICE_PCI_BRIDGE,
+QEMU_CAPS_ICH9_AHCI,
+

[libvirt] [PATCH 0/5] Enable secure boot

[Michal Privoznik]

We have UEFI enabled guests for a while now. But only recently
qemu introduced secure boot. We should reflect that in our code
too.

Michal Privoznik (5):
  qemuBuildMachineCommandLine: Follow our pattern
  Introduce SMM feature
  Introduce @secure attribute to os loader element
  qemu: Enable secure boot
  qemu: Advertise OVMF_CODE.secboot.fd

 docs/formatdomain.html.in  | 13 -
 docs/schemas/domaincommon.rng  | 17 +++
 src/conf/domain_conf.c | 19 ++-
 src/conf/domain_conf.h |  2 +
 src/qemu/qemu_capabilities.c   | 16 ++
 src/qemu/qemu_capabilities.h   |  4 ++
 src/qemu/qemu_command.c| 58 ++
 src/qemu/qemu_conf.c   | 13 +++--
 src/qemu/qemu_domain.c | 27 ++
 tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml   |  1 +
 tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml   |  1 +
 .../caps_2.6.0-gicv2.aarch64.xml   |  1 +
 .../caps_2.6.0-gicv3.aarch64.xml   |  1 +
 tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml  |  1 +
 tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |  1 +
 .../qemuxml2argv-bios-nvram-secure.args| 29 +++
 .../qemuxml2argv-bios-nvram-secure.xml | 41 +++
 .../qemuxml2argv-machine-smm-opt.args  | 25 ++
 .../qemuxml2argv-machine-smm-opt.xml   | 28 +++
 tests/qemuxml2argvtest.c   | 14 ++
 20 files changed, 284 insertions(+), 28 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.xml

-- 
2.8.4

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

[libvirt] [PATCH 3/5] Introduce @secure attribute to os loader element

[Michal Privoznik]

This element will control secure boot implemented by some
firmwares. If the firmware used in  does support the
feature we must tell it to the underlying hypervisor. However, we
can't know whether loader does support it or not just by looking
at the file. Therefore we have to have an attribute to the
element where users can tell us whether the firmware is secure
boot enabled or not.

Signed-off-by: Michal Privoznik 
---
 docs/formatdomain.html.in  |  7 ++--
 docs/schemas/domaincommon.rng  |  8 +
 src/conf/domain_conf.c | 14 
 src/conf/domain_conf.h |  1 +
 .../qemuxml2argv-bios-nvram-secure.xml | 41 ++
 5 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3f67182..102ae55 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -102,7 +102,7 @@
   ...
   os
 typehvm/type
-loader readonly='yes' 
type='rom'/usr/lib/xen/boot/hvmloader/loader
+loader readonly='yes' secure='no' 
type='rom'/usr/lib/xen/boot/hvmloader/loader
 nvram 
template='/usr/share/OVMF/OVMF_VARS.fd'/var/lib/libvirt/nvram/guest_VARS.fd/nvram
 boot dev='hd'/
 boot dev='cdrom'/
@@ -140,7 +140,10 @@
 pflash. It tells the hypervisor where in the guest
 memory the file should be mapped.  For instance, if the loader
 path points to an UEFI image, type should be
-pflash.
+pflash. Moreover, some firmwares may
+implement the Secure boot feature. Attribute
+secure can be used then to control it.
+Since 2.1.0
   nvram
   Some UEFI firmwares may want to use a non-volatile memory to store
 some variables. In the host, this is represented as a file and the
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 39fcb7e..c5a263b 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -260,6 +260,14 @@
   
 
 
+  
+
+  yes
+  no
+
+  
+
+
   
 
   rom
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3b6493e..e0ecca3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15327,9 +15327,11 @@ virDomainLoaderDefParseXML(xmlNodePtr node,
 {
 int ret = -1;
 char *readonly_str = NULL;
+char *secure_str = NULL;
 char *type_str = NULL;
 
 readonly_str = virXMLPropString(node, "readonly");
+secure_str = virXMLPropString(node, "secure");
 type_str = virXMLPropString(node, "type");
 loader->path = (char *) xmlNodeGetContent(node);
 
@@ -15340,6 +15342,13 @@ virDomainLoaderDefParseXML(xmlNodePtr node,
 goto cleanup;
 }
 
+if (secure_str &&
+(loader->secure = virTristateBoolTypeFromString(secure_str)) <= 0) {
+virReportError(VIR_ERR_XML_DETAIL,
+   _("unknown secure value: %s"), secure_str);
+goto cleanup;
+}
+
 if (type_str) {
 int type;
 if ((type = virDomainLoaderTypeFromString(type_str)) < 0) {
@@ -15353,6 +15362,7 @@ virDomainLoaderDefParseXML(xmlNodePtr node,
 ret = 0;
  cleanup:
 VIR_FREE(readonly_str);
+VIR_FREE(secure_str);
 VIR_FREE(type_str);
 return ret;
 }
@@ -22521,6 +22531,7 @@ virDomainLoaderDefFormat(virBufferPtr buf,
  virDomainLoaderDefPtr loader)
 {
 const char *readonly = virTristateBoolTypeToString(loader->readonly);
+const char *secure = virTristateBoolTypeToString(loader->secure);
 const char *type = virDomainLoaderTypeToString(loader->type);
 
 virBufferAddLit(buf, "readonly)
 virBufferAsprintf(buf, " readonly='%s'", readonly);
 
+if (loader->secure)
+virBufferAsprintf(buf, " secure='%s'", secure);
+
 virBufferAsprintf(buf, " type='%s'>", type);
 
 virBufferEscapeString(buf, "%s\n", loader->path);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 60b4be5..9c63257 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1733,6 +1733,7 @@ struct _virDomainLoaderDef {
 char *path;
 int readonly;   /* enum virTristateBool */
 virDomainLoader type;
+int secure; /* enum virTristateBool */
 char *nvram;/* path to non-volatile RAM */
 char *templt;   /* user override of path to master nvram */
 };
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram-secure.xml
new file mode 100644
index 000..0fe3
--- /dev/null
+++ 

[libvirt] [PATCH 2/5] Introduce SMM feature

[Michal Privoznik]

Since its release of 2.4.0 qemu is able to enable System
Management Module in the firmware, or disable it. We should
expose this capability in the XML. Unfortunately, there's no good
way to determine whether the binary we are talking to supports
it. I mean, if qemu's run with real machine type, the smm
attribute can be seen in 'qom-list /machine' output. But it's not
there when qemu's run with -M none. Therefore we're stuck with
version based check.

Signed-off-by: Michal Privoznik 
---
 docs/formatdomain.html.in  |  6 +
 docs/schemas/domaincommon.rng  |  9 +++
 src/conf/domain_conf.c |  5 +++-
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_capabilities.c   | 16 +
 src/qemu/qemu_capabilities.h   |  4 
 src/qemu/qemu_command.c| 12 ++
 tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml   |  1 +
 tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml   |  1 +
 .../caps_2.6.0-gicv2.aarch64.xml   |  1 +
 .../caps_2.6.0-gicv3.aarch64.xml   |  1 +
 tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml  |  1 +
 tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |  1 +
 .../qemuxml2argv-machine-smm-opt.args  | 25 +++
 .../qemuxml2argv-machine-smm-opt.xml   | 28 ++
 tests/qemuxml2argvtest.c   |  7 ++
 16 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-smm-opt.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 59a8bb9..3f67182 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1655,6 +1655,12 @@
   values are 2, 3 and host.
   Since 1.2.16
   
+  smm
+  Enable System Management Mode. Possible values are
+  on and off. The default is left
+  for hypervisor to decide.
+  Since 2.1.0
+  
 
 
 Time keeping
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 741f268..39fcb7e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4291,6 +4291,15 @@
   
 
   
+  
+
+  
+
+  
+
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 13e5dc9..3b6493e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -137,7 +137,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
   "capabilities",
   "pmu",
   "vmport",
-  "gic")
+  "gic",
+  "smm")
 
 VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
   "default",
@@ -16318,6 +16319,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 case VIR_DOMAIN_FEATURE_PMU:
 case VIR_DOMAIN_FEATURE_PVSPINLOCK:
 case VIR_DOMAIN_FEATURE_VMPORT:
+case VIR_DOMAIN_FEATURE_SMM:
 node = ctxt->node;
 ctxt->node = nodes[i];
 if ((tmp = virXPathString("string(./@state)", ctxt))) {
@@ -23291,6 +23293,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 case VIR_DOMAIN_FEATURE_PMU:
 case VIR_DOMAIN_FEATURE_PVSPINLOCK:
 case VIR_DOMAIN_FEATURE_VMPORT:
+case VIR_DOMAIN_FEATURE_SMM:
 switch ((virTristateSwitch) def->features[i]) {
 case VIR_TRISTATE_SWITCH_LAST:
 case VIR_TRISTATE_SWITCH_ABSENT:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3c2f182..60b4be5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1600,6 +1600,7 @@ typedef enum {
 VIR_DOMAIN_FEATURE_PMU,
 VIR_DOMAIN_FEATURE_VMPORT,
 VIR_DOMAIN_FEATURE_GIC,
+VIR_DOMAIN_FEATURE_SMM,
 
 VIR_DOMAIN_FEATURE_LAST
 } virDomainFeature;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d5b73e6..0b36819 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -339,6 +339,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "tls-creds-x509", /* 230 */
   "display",
   "intel-iommu",
+  "smm",
 );
 
 
@@ -3533,6 +3534,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 if (qemuCaps->version >= 2004000)
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE);
 
+/* smm option is supported from v2.4.0 */
+if (qemuCaps->version >= 2004000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT);
+
 /* Since 2.4.50 ARM virt machine supports gic-version option */
 if 

[libvirt] [PATCH 5/5] qemu: Advertise OVMF_CODE.secboot.fd

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_conf.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index fa9d65e..b51f36f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -126,6 +126,8 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)
 
 #define VIR_QEMU_OVMF_LOADER_PATH "/usr/share/OVMF/OVMF_CODE.fd"
 #define VIR_QEMU_OVMF_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"
+#define VIR_QEMU_OVMF_SEC_LOADER_PATH "/usr/share/OVMF/OVMF_CODE.secboot.fd"
+#define VIR_QEMU_OVMF_SEC_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"
 #define VIR_QEMU_AAVMF_LOADER_PATH "/usr/share/AAVMF/AAVMF_CODE.fd"
 #define VIR_QEMU_AAVMF_NVRAM_PATH "/usr/share/AAVMF/AAVMF_VARS.fd"
 
@@ -292,16 +294,19 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 goto error;
 
 #else
-if (VIR_ALLOC_N(cfg->firmwares, 2) < 0)
+if (VIR_ALLOC_N(cfg->firmwares, 3) < 0)
 goto error;
-cfg->nfirmwares = 2;
-if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0)
+cfg->nfirmwares = 3;
+if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0 ||
+VIR_ALLOC(cfg->firmwares[2]) < 0)
 goto error;
 
 if (VIR_STRDUP(cfg->firmwares[0]->name, VIR_QEMU_AAVMF_LOADER_PATH) < 0 ||
 VIR_STRDUP(cfg->firmwares[0]->nvram, VIR_QEMU_AAVMF_NVRAM_PATH) < 0  ||
 VIR_STRDUP(cfg->firmwares[1]->name, VIR_QEMU_OVMF_LOADER_PATH) < 0 ||
-VIR_STRDUP(cfg->firmwares[1]->nvram, VIR_QEMU_OVMF_NVRAM_PATH) < 0)
+VIR_STRDUP(cfg->firmwares[1]->nvram, VIR_QEMU_OVMF_NVRAM_PATH) < 0 ||
+VIR_STRDUP(cfg->firmwares[2]->name, VIR_QEMU_OVMF_SEC_LOADER_PATH) < 0 
||
+VIR_STRDUP(cfg->firmwares[2]->nvram, VIR_QEMU_OVMF_SEC_NVRAM_PATH) < 0)
 goto error;
 #endif
 
-- 
2.8.4

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

[libvirt] [PATCH 1/5] qemuBuildMachineCommandLine: Follow our pattern

[Michal Privoznik]

We use 'goto cleanup' for a reason. If a function can exit at
many places but doesn't follow the pattern, it has to copy the
free code in multiple places.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_command.c | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3dc131b..6295eeb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6751,7 +6751,9 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 const virDomainDef *def,
 virQEMUCapsPtr qemuCaps)
 {
+virBuffer buf = VIR_BUFFER_INITIALIZER;
 bool obsoleteAccel = false;
+int ret = -1;
 
 /* This should *never* be NULL, since we always provide
  * a machine in the capabilities data for QEMU. So this
@@ -6776,7 +6778,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("disable shared memory is not available "
  "with this QEMU binary"));
- return -1;
+return -1;
 }
 
 obsoleteAccel = true;
@@ -6788,7 +6790,6 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 return -1;
 }
 } else {
-virBuffer buf = VIR_BUFFER_INITIALIZER;
 virTristateSwitch vmport = def->features[VIR_DOMAIN_FEATURE_VMPORT];
 
 virCommandAddArg(cmd, "-machine");
@@ -6812,8 +6813,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("vmport is not available "
  "with this QEMU binary"));
-virBufferFreeAndReset();
-return -1;
+goto cleanup;
 }
 
 virBufferAsprintf(, ",vmport=%s",
@@ -6825,8 +6825,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("dump-guest-core is not available "
  "with this QEMU binary"));
-virBufferFreeAndReset();
-return -1;
+goto cleanup;
 }
 
 virBufferAsprintf(, ",dump-guest-core=%s",
@@ -6834,22 +6833,19 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 }
 
 if (def->mem.nosharepages) {
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_MERGE)) {
-virBufferAddLit(, ",mem-merge=off");
-} else {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_MERGE)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("disable shared memory is not available "
  "with this QEMU binary"));
-virBufferFreeAndReset();
-return -1;
+goto cleanup;
 }
+
+virBufferAddLit(, ",mem-merge=off");
 }
 
 if (def->keywrap &&
-!qemuAppendKeyWrapMachineParms(, qemuCaps, def->keywrap)) {
-virBufferFreeAndReset();
-return -1;
-}
+!qemuAppendKeyWrapMachineParms(, qemuCaps, def->keywrap))
+goto cleanup;
 
 if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) {
 if (def->gic_version != VIR_GIC_VERSION_NONE) {
@@ -6857,8 +6853,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("gic-version option is available "
  "only for ARM virt machine"));
-virBufferFreeAndReset();
-return -1;
+goto cleanup;
 }
 
 /* The default GIC version should not be specified on the
@@ -6869,8 +6864,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("gic-version option is not available "
  "with this QEMU binary"));
-virBufferFreeAndReset();
-return -1;
+goto cleanup;
 }
 
 virBufferAsprintf(, ",gic-version=%s",
@@ -6884,9 +6878,12 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 
 if (obsoleteAccel &&
 qemuBuildObsoleteAccelArg(cmd, def, qemuCaps) < 0)
-return -1;
+goto cleanup;
 
-return 0;
+ret = 0;
+ cleanup:
+virBufferFreeAndReset();
+return ret;
 }
 
 static int
-- 
2.8.4

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

Re: [libvirt] [PATCH 2/2] storage: remove "luks" storage volume type

[Daniel P. Berrange]

On Wed, Jul 27, 2016 at 08:44:53AM +0200, Martin Kletzander wrote:
> On Tue, Jul 26, 2016 at 07:12:30PM +0100, Daniel P. Berrange wrote:
> > The current LUKS support has a "luks" volume type which has
> > a "luks" encryption format.
> > 
> > This partially makes sense if you consider the QEMU shorthand
> > syntax only requires you to specify a format=luks, and it'll
> > automagically uses "raw" as the next level driver. QEMU will
> > however let you override the "raw" with any other driver it
> > supports (vmdk, qcow, rbd, iscsi, etc, etc)
> > 
> > IOW the intention though is that the "luks" encryption format
> > is applied to all disk formats (whether raw, qcow2, rbd, gluster
> > or whatever). As such it doesn't make much sense for libvirt
> > to say the volume type is "luks" - we should be saying that it
> > is a "raw" file, but with "luks" encryption applied.
> > 
> > IOW, when creating a storage volume we should use this XML
> > 
> >  
> >demo.raw
> >5368709120
> >
> >  
> >  
> > > uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/>
> >  
> >
> >  
> > 
> > and when configuring a guest disk we should use
> > 
> >  
> >
> >
> >
> >
> >  
> >
> >  
> > 
> > This commit thus removes the "luks" storage volume type added
> > in
> > 
> >  commit 318ebb36f1027b3357a32d6f781bd77d7a9043fd
> >  Author: John Ferlan 
> >  Date:   Tue Jun 21 12:59:54 2016 -0400
> > 
> >util: Add 'luks' to the FileTypeInfo
> > 
> > The storage file probing code is modified so that it can probe
> > the actual encryption formats explicitly, rather than merely
> > probing existance of encryption and letting the storage driver
> > guess the format.
> > 
> > The rest of the code is then adapted to deal with
> > VIR_STORAGE_FILE_RAW w/ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS
> > instead of just VIR_STORAGE_FILE_LUKS.
> > 
> > The commit mentioned above was included in libvirt v2.0.0.
> > So when querying volume XML this will be a change in behaviour
> > vs the 2.0.0 release - it'll report 'raw' instead of 'luks'
> > for the volume format, but still report 'luks' for encryption
> > format.  I think this change is OK because the storage driver
> > did not include any support for creating volumes, nor starting
> > guets with luks volumes in v2.0.0 - that only since then.
> > Clearly if we change this we must do it before v2.1.0 though.
> > 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> > src/qemu/qemu_command.c|  10 +-
> > src/qemu/qemu_domain.c |   2 +-
> > src/qemu/qemu_hotplug.c|   2 +-
> > src/storage/storage_backend.c  |  41 +++--
> > src/storage/storage_backend_fs.c   |  17 +-
> > src/storage/storage_backend_gluster.c  |   5 -
> > src/util/virstoragefile.c  | 202 
> > -
> > src/util/virstoragefile.h  |   1 -
> > tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml |   4 +-
> > tests/storagevolxml2xmlin/vol-luks-cipher.xml  |   2 +-
> > tests/storagevolxml2xmlin/vol-luks.xml |   2 +-
> > tests/storagevolxml2xmlout/vol-luks-cipher.xml |   2 +-
> > tests/storagevolxml2xmlout/vol-luks.xml|   2 +-
> > 13 files changed, 198 insertions(+), 94 deletions(-)
> > 
> > diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> > index 862fb29..5ef536a 100644
> > --- a/src/storage/storage_backend.c
> > +++ b/src/storage/storage_backend.c
> > @@ -1116,9 +,9 @@ 
> > virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
> > int accessRetCode = -1;
> > char *absolutePath = NULL;
> > 
> > -if (info->format == VIR_STORAGE_FILE_LUKS) {
> > +if (info->format == VIR_STORAGE_FILE_RAW) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > -   _("cannot set backing store for luks volume"));
> > +   _("cannot set backing store for raw volume"));
> > return -1;
> > }
> > 
> 
> I think this whole condition can be removed as it wasn't there before
> luks volumes.
> 
> > @@ -1283,7 +1278,8 @@ 
> > virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
> >_("format features only available with qcow2"));
> > return NULL;
> > }
> > -if (info.format == VIR_STORAGE_FILE_LUKS) {
> > +if (info.format == VIR_STORAGE_FILE_RAW &&
> > +vol->target.encryption != NULL) {
> > if (inputvol) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> >_("cannot use inputvol with luks volume"));
> 
> You're still reporting the error for "luks volume" here.
> 
> > @@ -1484,13 +1490,16 @@ 
> > virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol,
> > if (!inputvol)
> > return NULL;
> > 
> > -/* If either volume is a 

Re: [libvirt] [PATCH libvirt-glib] docs: Document gvir_connection_get_{storage_pools, networks, domains}

[Christophe Fergeau]

Acked-by: Christophe Fergeau 

(I'm assuming you can push to libvirt-glib, let me know if that's not
the case).

Christophe

On Wed, Jul 27, 2016 at 08:59:54AM +0200, Guido Günther wrote:
> In contrast to libvirt itself all get_* methods need to prefetch the
> corresponding information first so document this.
> ---
>  libvirt-gobject/libvirt-gobject-connection.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
> b/libvirt-gobject/libvirt-gobject-connection.c
> index 00d5eda..3f17265 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.c
> +++ b/libvirt-gobject/libvirt-gobject-connection.c
> @@ -700,6 +700,11 @@ void gvir_connection_close(GVirConnection *conn)
>   * gvir_connection_fetch_domains:
>   * @conn: a #GVirConnection
>   * @cancellable: (allow-none)(transfer none): cancellation object
> + *
> + * Use this method to fetch all domains managed by connection
> + * @conn. Use e.g. #gvir_connection_find_domain_by_id or
> + * #gvir_connection_get_domain afterwards to query the fetched
> + * domains.
>   */
>  gboolean gvir_connection_fetch_domains(GVirConnection *conn,
> GCancellable *cancellable,
> @@ -783,6 +788,11 @@ cleanup:
>   * gvir_connection_fetch_storage_pools:
>   * @conn: a #GVirConnection
>   * @cancellable: (allow-none)(transfer none): cancellation object
> + *
> + * Use this method to fetch all storage pools managed by connection
> + * @conn. Use e.g. #gvir_connection_find_storage_pool_by_name or
> + * #gvir_connection_get_storage_pools afterwards to query the fetched
> + * pools.
>   */
>  gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn,
>   GCancellable *cancellable,
> @@ -1722,6 +1732,11 @@ GVirInterface 
> *gvir_connection_find_interface_by_mac(GVirConnection *conn,
>   * gvir_connection_fetch_networks:
>   * @conn: a #GVirConnection
>   * @cancellable: (allow-none)(transfer none): cancellation object
> + *
> + * Use this method to fetch all networks managed by connection
> + * @conn. Use e.g. #gvir_connection_find_network_by_name or
> + * #gvir_connection_get_networks afterwards to query the fetched
> + * domains.
>   */
>  gboolean gvir_connection_fetch_networks(GVirConnection *conn,
>  GCancellable *cancellable,
> -- 
> 2.8.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


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

Re: [libvirt] [PATCH v7 3/7] qemu: expand domain memory statistics with 'last-update' timestamp

[Pavel Hrdina]

On Wed, Jul 13, 2016 at 01:42:14PM +0300, Derbyshev Dmitriy wrote:
> From: Derbyshev Dmitry 
> 
> QEMU reports timestamp along with other memory statistics, but this
> information is not reported by libvirt statistics API.
> It could be useful to determine if the data reported is fresh or not.
> 
> Signed-off-by: Derbyshev Dmitry 
> ---

ACK

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

Re: [libvirt] [PATCH v7 2/7] qemu: expand domain memory statistics with 'usable'

[Pavel Hrdina]

On Wed, Jul 13, 2016 at 01:42:13PM +0300, Derbyshev Dmitriy wrote:
> From: Derbyshev Dmitry 
> 
> Currently 'memtotal' in virtio drivers and qemu corresponds
> to 'available' in libvirt. Because of that we introduce libvirt 'usable'
> parameter, which maps to 'stat-available-memory' balloon statistics.
> As balloon statistics isn't reported in hmp, so no modification is made
> in qemu_monitor_text.c.
> 
> Signed-off-by: Derbyshev Dmitry 
> ---
>  include/libvirt/libvirt-domain.h | 8 +++-
>  src/libvirt-domain.c | 3 +++
>  src/qemu/qemu_monitor_json.c | 4 
>  tools/virsh-domain-monitor.c | 2 ++
>  tools/virsh.pod  | 2 ++
>  5 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index 7ea93aa..3fb482b 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -604,10 +604,16 @@ typedef enum {
>  VIR_DOMAIN_MEMORY_STAT_RSS = 7,
>  
>  /*
> + * How big the balloon can be inflated without pushing the guest system

s/big/much/

> + * to swap, corresponds to 'Available' in /proc/meminfo
> + */
> +VIR_DOMAIN_MEMORY_STAT_USABLE  = 8,
> +
> +/*
>   * The number of statistics supported by this version of the interface.
>   * To add new statistics, add them to the enum and increase this value.
>   */
> -VIR_DOMAIN_MEMORY_STAT_NR  = 8,
> +VIR_DOMAIN_MEMORY_STAT_NR  = 9,
>  
>  # ifdef VIR_ENUM_SENTINELS
>  VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 4e71a94..1467030 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -5986,6 +5986,9 @@ virDomainGetInterfaceParameters(virDomainPtr domain,
>   * The amount of memory which is not being used for any purpose (in kb).
>   * VIR_DOMAIN_MEMORY_STAT_AVAILABLE:
>   * The total amount of memory available to the domain's OS (in kb).
> + * VIR_DOMAIN_MEMORY_STAT_USABLE:
> + * How big the balloon can be inflated without pushing the guest system

s/big/much/

> + * to swap, corresponds to 'Available' in /proc/meminfo
>   * VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON:
>   * Current balloon value (in kb).
>   *
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index bb426dc..c83e0cc 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1719,7 +1719,11 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
>VIR_DOMAIN_MEMORY_STAT_UNUSED, 1024);
>  GET_BALLOON_STATS("stat-total-memory",
>VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 1024);
> +GET_BALLOON_STATS("stat-available-memory",
> +  VIR_DOMAIN_MEMORY_STAT_USABLE, 1024);
> +
>  ret = got;
> +
>   cleanup:
>  virJSONValueFree(cmd);
>  virJSONValueFree(reply);
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index c712fa5..7f30da2 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -369,6 +369,8 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
>  vshPrint(ctl, "unused %llu\n", stats[i].val);
>  if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_AVAILABLE)
>  vshPrint(ctl, "available %llu\n", stats[i].val);
> +if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_USABLE)
> +vshPrint(ctl, "usable %llu\n", stats[i].val);
>  if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON)
>  vshPrint(ctl, "actual %llu\n", stats[i].val);
>  if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_RSS)
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 4ebf7ad..5444f91 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -819,6 +819,8 @@ B:
>available - The amount of usable memory as seen by the domain (in 
> kB)
>actual- Current balloon value (in KB)
>rss   - Resident Set Size of the running domain's process (in 
> kB)
> +  usable- The amount of memory which can be reclaimed by balloon 

trailing white space

> +without causing host swapping (in KB)
>  
>  For QEMU/KVM with a memory balloon, setting the optional I<--period> to a
>  value larger than 0 in seconds will allow the balloon driver to return

ACK

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

Re: [libvirt] [PATCH v7 1/7] virsh: Add balloon stats description to .pod

[Pavel Hrdina]

On Wed, Jul 13, 2016 at 01:42:12PM +0300, Derbyshev Dmitriy wrote:
> From: Derbyshev Dmitry 
> 
> Description for existing balloon stats was missing for dommemstat.
> 
> Signed-off-by: Derbyshev Dmitry 
> ---
>  tools/virsh.pod | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index d7cd10e..4ebf7ad 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -806,7 +806,19 @@ on hypervisor.
>  
>  Get memory stats for a running domain.
>  
> -Depending on the hypervisor a variety of statistics can be returned
> +Availability of these fields depends on hypervisor. Unsupported fields are
> +missing from the output. Other fields may appear if communicating with a 
> newer
> +version of libvirtd.
> +
> +B:
> +  swap_in   - The amount of data read from swap space (in kB)
> +  swap_out  - The amount of memory written out to swap space (in kB)
> +  major_fault   - The number of page faults then disk IO was required

s/then/where/

> +  minor_fault   - The number of other page faults
> +  unused- The amount of memory left unused by the system (in kB)
> +  available - The amount of usable memory as seen by the domain (in 
> kB)
> +  actual- Current balloon value (in KB)
> +  rss   - Resident Set Size of the running domain's process (in 
> kB)
>  
>  For QEMU/KVM with a memory balloon, setting the optional I<--period> to a
>  value larger than 0 in seconds will allow the balloon driver to return

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

Re: [libvirt] [libvirt RFC PATCH 02/10] util: storage: Add parser for qemu's "json" backing pseudo-protocol

[Peter Krempa]

On Tue, Jul 26, 2016 at 14:28:23 -0600, Eric Blake wrote:
> On 07/15/2016 07:46 AM, Peter Krempa wrote:
> > Add a modular parser that will allow to parse 'json' backing definitions
> > that are supported by qemu. The initial implementation adds support for
> > the 'file' driver.
> 
> Might be nice for this commit message to show an actual json: string
> that it now accepts.
> 
> > ---
> >  src/util/virstoragefile.c | 87 
> > +++
> >  tests/virstoragetest.c|  8 +
> >  2 files changed, 88 insertions(+), 7 deletions(-)
> > 
> 
> At some point, I wonder if we can use qemu's QMP introspection to write
> a meta-modular parser (basically, once QMP blockdev-add is
> fully-functional, introspection will give a self-describing layout of
> all possible combinations that QMP will accept, and therefore parsing
> the introspection output would give a reasonable approach to all
> possible json: strings that a given qemu will understand).  But that's
> definitely more work, and blockdev-add is not fully working in qemu yet,
> so hand-duplicating common parses for now at least gets us further than
> libvirt's current stance of outright choking on json:.
> 
> 
> > @@ -2514,10 +2515,80 @@ 
> > virStorageSourceParseBackingColon(virStorageSourcePtr src,
> >  }
> > 
> > 
> > +static int
> > +virStorageSourceParseBackingJSONPath(virStorageSourcePtr src,
> > + virJSONValuePtr json,
> > + int type)
> > +{
> > +const char *path;
> > +
> > +if (!(path = virJSONValueObjectGetString(json, "file.filename"))) {
> > +virReportError(VIR_ERR_INVALID_ARG, "%s",
> > +   _("missing 'filename' field in JSON backing volume "
> > + "definition"));
> 
> Do we really want to require libvirt to support
> "file.filename":"/path/to/foo" compressed naming, or do we want a more

The problem is that once you allow something in qemu there are users
which actually will start using it. And that happened.

> hierarchical "file":{"filename":"/path/to/foo"} layout?  Or should both

Yes I'll switch to this for the main parser and all the weird layouts
will be mapped first to this.

> be supported?  I also wonder if the virJSON code should be enhanced to

So I think we will need to support both.

> make it easier to parse deep information, via something similar to xpath
> notation (other shortcuts that might be nice is "key[0]" as shorthand
> for accessing element0 out of "key":[element0, element1]).
> 
> Part of the problem is that qemu is doing so much gross under-the-hood

And the other part is lack of documentation for that. This also makes
users try what works and use that rather than the preferred syntax.

> conversion from pure QAPI into QemuOpts (which is completely flat, so we
> have hacks like "file.filename"), rather than converting from QemuOpts
> into QAPI, and so now libvirt has to match that grossness.

And since both approaches seem to have attacted their users qemu will
need to support it forever.

One other weird part is that the parser in qemu(-img) parses the string
but then puts it in the user supplied format into the qcow image. Any
extra spaces and other stuff are included. By using the formatter for
that all the weird syntax could be avoided.

> 
> > +++ b/tests/virstoragetest.c
> > @@ -1353,6 +1353,14 @@ mymain(void)
> > "\n"
> > "  \n"
> > "\n");
> > +TEST_BACKING_PARSE(5, "json:", NULL);
> > +TEST_BACKING_PARSE(6, "json:asdgsdfg", NULL);
> > +TEST_BACKING_PARSE(7, "json:{}", NULL);
> > +TEST_BACKING_PARSE(8, "json: { \"file.driver\":\"blah\"}", NULL);
> > +TEST_BACKING_PARSE(9, "json:{\"file.driver\":\"file\"}", NULL);
> > +TEST_BACKING_PARSE(10, "json:{\"file.driver\":\"file\", "
> > + "\"file.filename\":\"/path/to/file\"}",
> > +   "\n");
> > 
> 
> Okay, the testsuite shows what you now parse.  Again, I'm wondering if
> qemu should first be improved, and/or whether it already allows:
> 
> "json:{\"file\":{\"driver\":\"file\", \"filename\":\"/path/to/file\"}}"
> 
> as equivalent to the "file.driver" and "file.filename" at the flat
> level.  Or put another way, if qemu accepts more than one JSON
> representation because of the way it crumples and flattens JSON into
> QDict, maybe libvirt should first have a way to canonicalize into
> preferred form.

Yep, the nested objects will be preferred in this case as others are
weird syntax can be adapted.

Since this RFC was already reused in a different series I'll fix the
parser first and then re-post it.

Peter

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

Re: [libvirt] [PATCH v3 0/2] qemu: expand domain memory statistics

[Pavel Hrdina]

On Tue, Jul 26, 2016 at 09:32:30PM +0300, Maxim Nestratov wrote:
> 26-Jul-16 18:32, Pavel Hrdina пишет:
> 
> > On Sat, Jun 04, 2016 at 06:41:35PM +0300, Maxim Nestratov wrote:
> >> 01.06.2016 20:07, Derbyshev Dmitriy пишет:
> >> Makes sense, looks good to me.
> >> ACK the series.
> > Pushed now.  I've just found out that this wasn't pushed so far.
> >
> > Pavel
> 
> Yeah, and in fact the work has been continued in another patchset 
> version though no one replied to this one. It even reached v7 
> https://www.redhat.com/archives/libvir-list/2016-July/msg00428.html and 
> should be adapted now because these two are pushed.
> Anyway, thanks for pushing.
> 
> Maxim

Good to know, I've somehow missed that series.  I'll take a look at it.

Pavel

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

[libvirt] [PATCH libvirt-glib] docs: Document gvir_connection_get_{storage_pools, networks, domains}

[Guido Günther]

In contrast to libvirt itself all get_* methods need to prefetch the
corresponding information first so document this.
---
 libvirt-gobject/libvirt-gobject-connection.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
b/libvirt-gobject/libvirt-gobject-connection.c
index 00d5eda..3f17265 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -700,6 +700,11 @@ void gvir_connection_close(GVirConnection *conn)
  * gvir_connection_fetch_domains:
  * @conn: a #GVirConnection
  * @cancellable: (allow-none)(transfer none): cancellation object
+ *
+ * Use this method to fetch all domains managed by connection
+ * @conn. Use e.g. #gvir_connection_find_domain_by_id or
+ * #gvir_connection_get_domain afterwards to query the fetched
+ * domains.
  */
 gboolean gvir_connection_fetch_domains(GVirConnection *conn,
GCancellable *cancellable,
@@ -783,6 +788,11 @@ cleanup:
  * gvir_connection_fetch_storage_pools:
  * @conn: a #GVirConnection
  * @cancellable: (allow-none)(transfer none): cancellation object
+ *
+ * Use this method to fetch all storage pools managed by connection
+ * @conn. Use e.g. #gvir_connection_find_storage_pool_by_name or
+ * #gvir_connection_get_storage_pools afterwards to query the fetched
+ * pools.
  */
 gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn,
  GCancellable *cancellable,
@@ -1722,6 +1732,11 @@ GVirInterface 
*gvir_connection_find_interface_by_mac(GVirConnection *conn,
  * gvir_connection_fetch_networks:
  * @conn: a #GVirConnection
  * @cancellable: (allow-none)(transfer none): cancellation object
+ *
+ * Use this method to fetch all networks managed by connection
+ * @conn. Use e.g. #gvir_connection_find_network_by_name or
+ * #gvir_connection_get_networks afterwards to query the fetched
+ * domains.
  */
 gboolean gvir_connection_fetch_networks(GVirConnection *conn,
 GCancellable *cancellable,
-- 
2.8.1

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

[libvirt] [PATCH] virconf: Fix config file path construction

[Erik Skultety]

Since commit c4bdff19, the path to the configuration file has been constructed
in the following manner:
 - if no config filename was passed to virConfLoadConfigPath, libvirt.conf was
 used as default
 - otherwise the filename was concatenated with
 "/libvirt/libvirt%s%s.conf" which in admin case resulted in
 "libvirt-libvirt-admin.conf.conf". Obviously, this non-existent config led to
 ignoring  all user settings in libvirt-admin.conf. This patch requires the
 config filename to be always provided as an argument with the concatenation
 being simplified.

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

Signed-off-by: Erik Skultety 
---
 src/libvirt.c  |  2 +-
 src/util/virconf.c | 12 
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 68c8317..52462e3 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -969,7 +969,7 @@ virConnectOpenInternal(const char *name,
 if (ret == NULL)
 return NULL;
 
-if (virConfLoadConfig(, NULL) < 0)
+if (virConfLoadConfig(, "libvirt.conf") < 0)
 goto failed;
 
 if (name && name[0] == '\0')
diff --git a/src/util/virconf.c b/src/util/virconf.c
index ee54072..3e49f41 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1566,20 +1566,16 @@ virConfLoadConfigPath(const char *name)
 {
 char *path;
 if (geteuid() == 0) {
-if (virAsprintf(, "%s/libvirt/libvirt%s%s.conf",
-SYSCONFDIR,
-name ? "-" : "",
-name ? name : "") < 0)
+if (virAsprintf(, "%s/libvirt/%s",
+SYSCONFDIR, name) < 0)
 return NULL;
 } else {
 char *userdir = virGetUserConfigDirectory();
 if (!userdir)
 return NULL;
 
-if (virAsprintf(, "%s/libvirt%s%s.conf",
-userdir,
-name ? "-" : "",
-name ? name : "") < 0) {
+if (virAsprintf(, "%s/%s",
+userdir, name) < 0) {
 VIR_FREE(userdir);
 return NULL;
 }
-- 
2.5.5

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

Re: [libvirt] [PATCH 2/2] storage: remove "luks" storage volume type

[Martin Kletzander]

On Tue, Jul 26, 2016 at 07:12:30PM +0100, Daniel P. Berrange wrote:

The current LUKS support has a "luks" volume type which has
a "luks" encryption format.

This partially makes sense if you consider the QEMU shorthand
syntax only requires you to specify a format=luks, and it'll
automagically uses "raw" as the next level driver. QEMU will
however let you override the "raw" with any other driver it
supports (vmdk, qcow, rbd, iscsi, etc, etc)

IOW the intention though is that the "luks" encryption format
is applied to all disk formats (whether raw, qcow2, rbd, gluster
or whatever). As such it doesn't make much sense for libvirt
to say the volume type is "luks" - we should be saying that it
is a "raw" file, but with "luks" encryption applied.

IOW, when creating a storage volume we should use this XML

 
   demo.raw
   5368709120
   
 
 
   
 
   
 

and when configuring a guest disk we should use

 
   
   
   
   
 
   
 

This commit thus removes the "luks" storage volume type added
in

 commit 318ebb36f1027b3357a32d6f781bd77d7a9043fd
 Author: John Ferlan 
 Date:   Tue Jun 21 12:59:54 2016 -0400

   util: Add 'luks' to the FileTypeInfo

The storage file probing code is modified so that it can probe
the actual encryption formats explicitly, rather than merely
probing existance of encryption and letting the storage driver
guess the format.

The rest of the code is then adapted to deal with
VIR_STORAGE_FILE_RAW w/ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS
instead of just VIR_STORAGE_FILE_LUKS.

The commit mentioned above was included in libvirt v2.0.0.
So when querying volume XML this will be a change in behaviour
vs the 2.0.0 release - it'll report 'raw' instead of 'luks'
for the volume format, but still report 'luks' for encryption
format.  I think this change is OK because the storage driver
did not include any support for creating volumes, nor starting
guets with luks volumes in v2.0.0 - that only since then.
Clearly if we change this we must do it before v2.1.0 though.

Signed-off-by: Daniel P. Berrange 
---
src/qemu/qemu_command.c|  10 +-
src/qemu/qemu_domain.c |   2 +-
src/qemu/qemu_hotplug.c|   2 +-
src/storage/storage_backend.c  |  41 +++--
src/storage/storage_backend_fs.c   |  17 +-
src/storage/storage_backend_gluster.c  |   5 -
src/util/virstoragefile.c  | 202 -
src/util/virstoragefile.h  |   1 -
tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml |   4 +-
tests/storagevolxml2xmlin/vol-luks-cipher.xml  |   2 +-
tests/storagevolxml2xmlin/vol-luks.xml |   2 +-
tests/storagevolxml2xmlout/vol-luks-cipher.xml |   2 +-
tests/storagevolxml2xmlout/vol-luks.xml|   2 +-
13 files changed, 198 insertions(+), 94 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 862fb29..5ef536a 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1116,9 +,9 @@ 
virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
int accessRetCode = -1;
char *absolutePath = NULL;

-if (info->format == VIR_STORAGE_FILE_LUKS) {
+if (info->format == VIR_STORAGE_FILE_RAW) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("cannot set backing store for luks volume"));
+   _("cannot set backing store for raw volume"));
return -1;
}



I think this whole condition can be removed as it wasn't there before
luks volumes.


@@ -1283,7 +1278,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr 
conn,
   _("format features only available with qcow2"));
return NULL;
}
-if (info.format == VIR_STORAGE_FILE_LUKS) {
+if (info.format == VIR_STORAGE_FILE_RAW &&
+vol->target.encryption != NULL) {
if (inputvol) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
   _("cannot use inputvol with luks volume"));


You're still reporting the error for "luks volume" here.


@@ -1484,13 +1490,16 @@ 
virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol,
if (!inputvol)
return NULL;

-/* If either volume is a non-raw file vol, we need to use an external
- * tool for converting
+VIR_WARN("BUild vol from func");


Leftover from debugging?


diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 2834baa..c264041 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -808,6 +882,45 @@ qcow2GetFeatures(virBitmapPtr *features,
}


+static bool
+virStorageFileHasEncryptionFormat(const struct FileEncryptionInfo *info,
+  char *buf,
+  size_t len)
+{
+if (!info->magic && info->modeOffset == -1)

Re: [libvirt] [PATCH v2 00/10] Remove caching of address sets

[Martin Kletzander]

On Sat, Jul 23, 2016 at 03:47:05AM +0200, Tomasz Flendrich wrote:

From: Tomasz Flendrich 

These patches delete the caching of pci, virtioSerial and ccw address sets.
I am deleting them, because they can be recalculated from the domain definition,
and there's no point in keeping redundant data, especially because handling
a persistent cache of addresses required using functions that released 
addresses.
These functions aren't useful anymore, so they are dropped too.

I know that USB addresses were added and that they are cached now. I am sure
that they can be calculated on demand too though.

If this gets accepted, I will rebase the other patches that depend on this one,
making functions less dependant on qemu, and removing the code duplication.



I went ahead and pushed patches 1-6 (that is ccw and virtio-serial
related).  Looking forward to v2 of the rest.

I forgot to send this yesterday and it got stuck in my drafts O:-)

Martin


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

Re: [libvirt] [PATCH 1/2] virstoragefile: refactor virStorageFileMatchesNNN methods

[Martin Kletzander]

On Tue, Jul 26, 2016 at 07:12:29PM +0100, Daniel P. Berrange wrote:

Refactor the virStorageFileMatchesNNN methods so that
they don't take a struct FileFormatInfo parameter, but
instead get the actual raw dat items they needs. This
will facilitate reuse in other contexts.

Signed-off-by: Daniel P. Berrange 
---
src/util/virstoragefile.c | 63 ---
1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 16de603..2834baa 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -644,44 +646,44 @@ virStorageFileMatchesVersion(int format,
size_t i;

/* Validate version number info */
-if (fileTypeInfo[format].versionOffset == -1)
+if (versionOffset == -1)
return false;

/* -2 == non-versioned file format, so trivially match */
-if (fileTypeInfo[format].versionOffset == -2)
+if (versionOffset == -2)
return true;

/* A positive versionOffset, requires using a valid versionSize */
-if (fileTypeInfo[format].versionSize != 2 &&
-fileTypeInfo[format].versionSize != 4)
+if (versionSize != 2 &&
+versionSize != 4)
return false;

-if ((fileTypeInfo[format].versionOffset +
- fileTypeInfo[format].versionSize) > buflen)
+if ((versionOffset +
+ versionSize) > buflen)


Unwrap shortened lines like this ^^ so it's readable, please.

ACK with that changed.


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