[libvirt] [PATCH] qemu: Align memory module sizes to 2MiB

[Peter Krempa]

My original implementation was based on a qemu version that still did
not have all the checks in place. Using sizes that would align to odd
megabyte increments will produce the following error:

qemu-kvm: -device pc-dimm,node=0,memdev=memdimm0,id=dimm0: backend memory size 
must be multiple of 0x20
qemu-kvm: -device pc-dimm,node=0,memdev=memdimm0,id=dimm0: Device 'pc-dimm' 
could not be initialized

Introduce an alignment retrieval function for memory devices and use it
to align the devices separately and modify a test case to verify it.
---
 src/qemu/qemu_domain.c| 19 ++-
 .../qemuxml2argv-memory-hotplug-dimm.xml  |  2 +-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7d92f3a..7680c87 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3376,6 +3376,21 @@ qemuDomainGetMemorySizeAlignment(virDomainDefPtr def)
 }


+static unsigned long long
+qemuDomainGetMemoryModuleSizeAlignment(const virDomainDef *def,
+   const virDomainMemoryDef *mem 
ATTRIBUTE_UNUSED)
+{
+/* PPC requires the memory sizes to be rounded to 256MiB increments, so
+ * round them to the size always. */
+if (ARCH_IS_PPC64(def->os.arch))
+return 256 * 1024;
+
+/* dimm memory modules require 2MiB alignment rather than the 1MiB we are
+ * using elsewhere. */
+return 2048;
+}
+
+
 int
 qemuDomainAlignMemorySizes(virDomainDefPtr def)
 {
@@ -3402,8 +3417,10 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
 def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, align);

 /* Align memory module sizes */
-for (i = 0; i < def->nmems; i++)
+for (i = 0; i < def->nmems; i++) {
+align = qemuDomainGetMemoryModuleSizeAlignment(def, def->mems[i]);
 def->mems[i]->size = VIR_ROUND_UP(def->mems[i]->size, align);
+}

 return 0;
 }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml
index 3f468ec..fbcac84 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.xml
@@ -36,7 +36,7 @@
 
 
   
-524287
+523264
 0
   
 
-- 
2.4.5

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

Re: [libvirt] [PATCH] add compress stream support

[Daniel P. Berrange]

On Tue, Sep 22, 2015 at 10:26:19PM +, Vasiliy Tolstov wrote:
> Some libvirt functions use streams, this patch add
> compress stream support.
> So VolumeDownload/VolumeUpload can greatly speedup by using
> compressed streams to save network bandtwidth and don't transfer
> zero bytes (in case of raw disk format)

How have you actually tested this in practice - your patch does
not change any code to make use of this new feate. You're changing
the public API which suggests you expect the client apps to use
this when passing a virStreamPtr to the virStorageVolDownload
method, but the stream client apps pass is not backed by a
virFDStream object. Only libvirtd uses the virFDSteam objects
and you've not changed anything to make use ot that. So this is
all rather strange still. Can you more clearly state what you
are expecting to do.

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] add compress stream support

[Vasiliy Tolstov]

2015-09-23 14:54 GMT+03:00 Daniel P. Berrange :
> How have you actually tested this in practice - your patch does
> not change any code to make use of this new feate. You're changing
> the public API which suggests you expect the client apps to use
> this when passing a virStreamPtr to the virStorageVolDownload
> method, but the stream client apps pass is not backed by a
> virFDStream object. Only libvirtd uses the virFDSteam objects
> and you've not changed anything to make use ot that. So this is
> all rather strange still. Can you more clearly state what you
> are expecting to do.


As i'm understand client create stream via virStreamNew , when user
invoke virStoreVolDownload this function use created stream.
If i'm correct, libvirt when read/write in
virStoreVolDownload/virStoreVolUpload uses fdstream functions
internally. So data compressed or decompressed.
Or when client create virStreamNew and pass it libvirt not always use
fdstream functions ?

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

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

Re: [libvirt] [libvirt-guests] New feature ALWAYS_START

[Martin Kletzander]

On Wed, Sep 23, 2015 at 12:50:57PM +0100, Daniel P. Berrange wrote:

On Wed, Sep 23, 2015 at 10:50:58AM +0200, Martin Kletzander wrote:

On Tue, Sep 22, 2015 at 07:33:30AM +0200, Marek Lukács wrote:
>Hi,
>
>It will be nice feature to have configuration option
>ALWAYS_START="$uri:$name $uri:name ..." in libvirt-guests
>configuration file.
>
>If ON_BOOT is "start" and if ALWAYS_START is not empty, it iterates
>over the ALWAYS_START and starts guests with same conditions (delays
>etc.) before it starts guests from LISTFILE.
>

To be honest, I don't think that's _exactly_ what you want _just_ from
libvirt itself; let me explain.

>Benefits:
>- guests are started with delays

Delays that are done due to the guests are not something we should
handle.  Guests and mainly the applications inside them should handle
this gracefully.  Just delaying the starts is still error-prone.

>- guests are started after host failure

That's what libvirt-guests does already.  And if you want some domains
to be started on every start, there's the 'autostart' parameter for
domains.


Indeed, and I have a generall desire for libvirt-guests to go away
entirely and have all its functionality just built into libvirtd. So
I don't think taking functionality that already exist in libvirtd and
adding it to libvirt-guest is the right direction for us to take.



As Jirka pointed out, it's harder to do this in libvirtd because you
want all the domains to be saved when shutting down, but not when
stopping the service.  Another option would be that libvirt-guests
would just tell libvirt over some other API, which is not
hypervisor-specific (let's say admin api for example), that would call
some function in all drivers that would do what libvirt-guests does
now.



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 :|


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

Re: [libvirt] [libvirt-glib] gobject: Correct docs for gvir_storage_pool_get_info()

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 01:44:28PM +0100, Zeeshan Ali (Khattak) wrote:
> The returned GVirStoragePoolInfo pointer is not a GObject so it must not
> be unrefed using g_object_unref(). Since gvir_storage_pool_info_free()
> is private function, callers must either use g_slice_free() or
> g_boxed_free().
> ---
> 
> Perhaps we should just make gvir_storage_pool_info_free() public instead?
> 
>  libvirt-gobject/libvirt-gobject-storage-pool.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
> b/libvirt-gobject/libvirt-gobject-storage-pool.c
> index 7f26b1b..f015efa 100644
> --- a/libvirt-gobject/libvirt-gobject-storage-pool.c
> +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
> @@ -277,8 +277,8 @@ GVirConfigStoragePool 
> *gvir_storage_pool_get_config(GVirStoragePool *pool,
>   * @pool: the storage_pool
>   * @err: Place-holder for possible errors
>   *
> - * Returns: (transfer full): the info. The returned object should be
> - * unreffed with g_object_unref() when no longer needed.
> + * Returns: (transfer full): the info. The returned pointer should be
> + * freed using either #g_slice_free() or #g_boxed_free() when no longer 
> needed.
>   */
>  GVirStoragePoolInfo *gvir_storage_pool_get_info(GVirStoragePool *pool,
>  GError **err)

ACK


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] add compress stream support

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 03:20:51PM +0300, Vasiliy Tolstov wrote:
> 2015-09-23 14:54 GMT+03:00 Daniel P. Berrange :
> > How have you actually tested this in practice - your patch does
> > not change any code to make use of this new feate. You're changing
> > the public API which suggests you expect the client apps to use
> > this when passing a virStreamPtr to the virStorageVolDownload
> > method, but the stream client apps pass is not backed by a
> > virFDStream object. Only libvirtd uses the virFDSteam objects
> > and you've not changed anything to make use ot that. So this is
> > all rather strange still. Can you more clearly state what you
> > are expecting to do.
> 
> 
> As i'm understand client create stream via virStreamNew , when user
> invoke virStoreVolDownload this function use created stream.
> If i'm correct, libvirt when read/write in
> virStoreVolDownload/virStoreVolUpload uses fdstream functions
> internally. So data compressed or decompressed.
> Or when client create virStreamNew and pass it libvirt not always use
> fdstream functions ?

It is not quite that simple because we have a client/server architecture.

So the client app creates a virStreamPtr. This is used by the remote
driver, via a virNetClientStream object to tunnel data over the
libvirtd connection.

The libvirtd daemon then creates another virStreamPtr object. This is
used by the straoge driver, via the virFDStream object to fetch the
data the client is requesting.

So having the client app request compression on its virStreamPtr
does not work, because that stream object is not the one used
by the virFDStream code.

An alternative approach would be to not try to change the stream API
at all.

Instead pass flags to the virStorageVolDownload/Upload functions,
eg VIR_STORAGE_VOL_STREAM_COMPRESS_ZLIB. This flag would get passed
across to the storage driver, which can then turn on compression in
the virFDStream impl. This would require

 - Add the enum flags to include/libvirt/libvirt-storage.h
 - Add libarchive support to src/fdstream.c
 - Handle the new flags in src/storage/storage_driver.c to
   turn on the libarchive compression in fdsream.c

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 RFC 2/7] libxl: implement virDomainMemorystats

[Joao Martins]

On 09/17/2015 11:16 PM, Jim Fehlig wrote:
> On 09/08/2015 02:27 AM, Joao Martins wrote:
>> Introduce support for domainMemoryStats API call, which
>> consequently enables the use of `virsh dommemstat` command to
>> query for memory statistics of a domain. We support
>> the following statistics: balloon info, available and currently
>> in use. swap-in, swap-out, major-faults, minor-faults require
>> cooperation of the guest and thus currently not supported.
>>
>> We build on the data returned from libxl_domain_info and deliver
>> it in the virDomainMemoryStat format.
>>
>> Signed-off-by: Joao Martins 
>> ---
>>   src/libxl/libxl_driver.c | 68 
>> 
>>   1 file changed, 68 insertions(+)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 101e4c7..43e9e47 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -4747,6 +4747,73 @@ libxlDomainGetCPUStats(virDomainPtr dom,
>>   return ret;
>>   }
>>   
>> +#define LIBXL_SET_MEMSTAT(TAG, VAL) \
>> +if (i < nr_stats) { \
>> +stats[i].tag = TAG; \
>> +stats[i].val = VAL; \
>> +i++; \
>> +}
>> +
>> +static int
>> +libxlDomainMemoryStats(virDomainPtr dom,
>> +   virDomainMemoryStatPtr stats,
>> +   unsigned int nr_stats,
>> +   unsigned int flags)
>> +{
>> +libxlDriverPrivatePtr driver = dom->conn->privateData;
>> +libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
>> +virDomainObjPtr vm;
>> +libxl_dominfo d_info;
>> +unsigned mem, maxmem;
>> +size_t i = 0;
>> +int ret = -1;
>> +
>> +virCheckFlags(0, -1);
>> +
>> +if (!(vm = libxlDomObjFromDomain(dom)))
>> +goto cleanup;
>> +
>> +if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0)
>> +goto cleanup;
>> +
>> +if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_QUERY) < 0)
>> +goto cleanup;
>> +
>> +if (!virDomainObjIsActive(vm)) {
>> +virReportError(VIR_ERR_OPERATION_INVALID,
>> +   "%s", _("domain is not running"));
>> +goto endjob;
>> +}
>> +
>> +if (libxl_domain_info(cfg->ctx, _info, vm->def->id) != 0) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("libxl_domain_info failed for domain '%d'"),
>> +   vm->def->id);
>> +return -1;
> 
> goto endjob; ?
> 
Yeah. Will fix that for v2.

> Also, as you've already noted, another case of missing 
> libxl_dominfo_dispose(). 
> But I'll stop mentioning those now :-).
> 
OK.

Thanks,
Joao

> Regards,
> Jim
> 
>> +}
>> +mem = d_info.current_memkb;
>> +maxmem = d_info.max_memkb;
>> +
>> +LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, maxmem - mem);
>> +LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, maxmem);
>> +LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_RSS, mem);
>> +
>> +ret = i;
>> +
>> + endjob:
>> +if (!libxlDomainObjEndJob(driver, vm)) {
>> +virObjectUnlock(vm);
>> +vm = NULL;
>> +}
>> +
>> + cleanup:
>> +if (vm)
>> +virObjectUnlock(vm);
>> +return ret;
>> +}
>> +
>> +#undef LIBXL_SET_MEMSTAT
>> +
>>   static int
>>   libxlConnectDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, 
>> int eventID,
>>  virConnectDomainEventGenericCallback 
>> callback,
>> @@ -5340,6 +5407,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
>>   #endif
>>   .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
>>   .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
>> +.domainMemoryStats = libxlDomainMemoryStats, /* 1.2.20 */
>>   .domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.20 */
>>   .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 
>> 0.9.0 */
>>   .connectDomainEventDeregister = libxlConnectDomainEventDeregister, /* 
>> 0.9.0 */
> 

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

Re: [libvirt] [PATCH v2] qemu: Refresh memory size only on fresh starts

[Peter Krempa]

On Wed, Sep 23, 2015 at 14:56:05 +0200, Ján Tomko wrote:
> On Wed, Sep 23, 2015 at 02:23:09PM +0200, Peter Krempa wrote:
> > Qemu unfortunately doesn't update internal state right after migration
> > and so the actual balloon size as returned by 'query-balloon' are
> > invalid for a while after the CPUs are started after migration. If we'd
> > refresh our internal state at this point we would report invalid current
> > memory size until the next balloon event would arrive.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1242940
> > ---
> > Version 2:
> > - completely different from original posting due to unexpected qemu behavior
> > 
> >  src/qemu/qemu_process.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> 
> ACK

Pushed; Thanks.

Peter


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

[libvirt] [PATCH v2 2/4] virDomainCreateXML: Make domain definition transient

[Michal Privoznik]

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

So, you want to create a domain from XML. The domain already
exists in libvirt's database of domains. It's okay, because name
and UUID matches. However, on domain startup, internal
representation of the domain is overwritten with your XML even
though we claim that the XML you've provided is a transient one.
The bug is to be found across nearly all the drivers.
Le sigh.

Signed-off-by: Michal Privoznik 
---
 src/bhyve/bhyve_driver.c   | 1 +
 src/libxl/libxl_driver.c   | 1 +
 src/lxc/lxc_driver.c   | 1 +
 src/openvz/openvz_driver.c | 1 +
 src/qemu/qemu_driver.c | 1 +
 src/test/test_driver.c | 1 +
 src/uml/uml_driver.c   | 1 +
 src/vmware/vmware_driver.c | 1 +
 8 files changed, 8 insertions(+)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 7f365b1..d44cf2c 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -918,6 +918,7 @@ bhyveDomainCreateXML(virConnectPtr conn,
 
 if (!(vm = virDomainObjListAdd(privconn->domains, def,
privconn->xmlopt,
+   VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL)))
 goto cleanup;
 def = NULL;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 5048957..fc6dcec 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -974,6 +974,7 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
 
 if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
+   VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
NULL)))
 goto cleanup;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index e5e6c5a..b408be0 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1229,6 +1229,7 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn,
 
 if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
+   VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
NULL)))
 goto cleanup;
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index fc8db7e..d78e2f5 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1093,6 +1093,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml,
 if (!(vm = virDomainObjListAdd(driver->domains,
vmdef,
driver->xmlopt,
+   VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
NULL)))
 goto cleanup;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 30d2d98..2a4b026 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1745,6 +1745,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
 
 if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
+   VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
NULL)))
 goto cleanup;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index b40b079..01ab1e3 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1616,6 +1616,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml,
 if (!(dom = virDomainObjListAdd(privconn->domains,
 def,
 privconn->xmlopt,
+VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
 VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
 NULL)))
 goto cleanup;
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index d4b03b3..14598fc 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1615,6 +1615,7 @@ static virDomainPtr umlDomainCreateXML(virConnectPtr 
conn, const char *xml,
 
 if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
+   VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
NULL)))
 goto cleanup;
diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index 152af39..a12b03a 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -704,6 +704,7 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml,
 if (!(vm = virDomainObjListAdd(driver->domains,
   

[libvirt] [PATCH v2] qemu: Refresh memory size only on fresh starts

[Peter Krempa]

Qemu unfortunately doesn't update internal state right after migration
and so the actual balloon size as returned by 'query-balloon' are
invalid for a while after the CPUs are started after migration. If we'd
refresh our internal state at this point we would report invalid current
memory size until the next balloon event would arrive.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1242940
---
Version 2:
- completely different from original posting due to unexpected qemu behavior

 src/qemu/qemu_process.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7187dc1..b961f40 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5023,7 +5023,8 @@ int qemuProcessStart(virConnectPtr conn,
 /* Since CPUs were not started yet, the balloon could not return the memory
  * to the host and thus cur_balloon needs to be updated so that GetXMLdesc
  * and friends return the correct size in case they can't grab the job */
-if (qemuProcessRefreshBalloonState(driver, vm, asyncJob) < 0)
+if (!migrateFrom && !snapshot &&
+qemuProcessRefreshBalloonState(driver, vm, asyncJob) < 0)
 goto cleanup;

 VIR_DEBUG("Detecting actual memory size for video device");
-- 
2.4.5

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

[libvirt] [PATCH v2 1/4] virDomainCreateXML: Don't remove persistent domains on error

[Michal Privoznik]

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

Okay, so we allow users to 'virsh create' an already existing
domain, providing completely different XML than the one stored in
Libvirt. Well, as long as name and UUID matches. However, in some
drivers the code that handles errors unconditionally removes the
domain that failed to start even though the domain might have
been persistent. Fortunately, the domain is removed just from the
internal list of domains and the config file is kept around.

Steps to reproduce:

1) virsh dumpxml $dom > /tmp/dom.xml
2) change XML so that it is still parse-able but won't boot, e.g.
change guest agent path to /foo/bar
3) virsh create /tmp/dom.xml
4) virsh dumpxml $dom
5) Observe "No such domain" error

Signed-off-by: Michal Privoznik 
---
 src/lxc/lxc_driver.c   | 6 --
 src/qemu/qemu_driver.c | 6 --
 src/test/test_driver.c | 7 ++-
 src/uml/uml_driver.c   | 7 ---
 src/vmware/vmware_driver.c | 6 --
 5 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a9f0005..e5e6c5a 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1239,8 +1239,10 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn,
(flags & VIR_DOMAIN_START_AUTODESTROY),
VIR_DOMAIN_RUNNING_BOOTED) < 0) {
 virDomainAuditStart(vm, "booted", false);
-virDomainObjListRemove(driver->domains, vm);
-vm = NULL;
+if (!vm->persistent) {
+virDomainObjListRemove(driver->domains, vm);
+vm = NULL;
+}
 goto cleanup;
 }
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2387cf3..30d2d98 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1752,7 +1752,8 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
 def = NULL;
 
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) {
-qemuDomainRemoveInactive(driver, vm);
+if (!vm->persistent)
+qemuDomainRemoveInactive(driver, vm);
 goto cleanup;
 }
 
@@ -1762,7 +1763,8 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
  start_flags) < 0) {
 virDomainAuditStart(vm, "booted", false);
 qemuDomainObjEndJob(driver, vm);
-qemuDomainRemoveInactive(driver, vm);
+if (!vm->persistent)
+qemuDomainRemoveInactive(driver, vm);
 goto cleanup;
 }
 
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d11cda1..b40b079 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1621,8 +1621,13 @@ testDomainCreateXML(virConnectPtr conn, const char *xml,
 goto cleanup;
 def = NULL;
 
-if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_BOOTED) < 0)
+if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_BOOTED) < 0) {
+if (!dom->persistent) {
+virDomainObjListRemove(privconn->domains, dom);
+dom = NULL;
+}
 goto cleanup;
+}
 
 event = virDomainEventLifecycleNewFromObj(dom,
  VIR_DOMAIN_EVENT_STARTED,
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 2b61f73..d4b03b3 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1623,9 +1623,10 @@ static virDomainPtr umlDomainCreateXML(virConnectPtr 
conn, const char *xml,
 if (umlStartVMDaemon(conn, driver, vm,
  (flags & VIR_DOMAIN_START_AUTODESTROY)) < 0) {
 virDomainAuditStart(vm, "booted", false);
-virDomainObjListRemove(driver->domains,
-   vm);
-vm = NULL;
+if (!vm->persistent) {
+virDomainObjListRemove(driver->domains, vm);
+vm = NULL;
+}
 goto cleanup;
 }
 virDomainAuditStart(vm, "booted", true);
diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index e228aaa..152af39 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -716,8 +716,10 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml,
 vmdef = NULL;
 
 if (vmwareStartVM(driver, vm) < 0) {
-virDomainObjListRemove(driver->domains, vm);
-vm = NULL;
+if (!vm->persistent) {
+virDomainObjListRemove(driver->domains, vm);
+vm = NULL;
+}
 goto cleanup;
 }
 
-- 
2.4.9

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

[libvirt] [PATCH v2 3/4] qemu: Move vm->persistent check into qemuDomainRemoveInactive

[Michal Privoznik]

So far we have the following pattern occurring over and over
again:

  if (!vm->persistent)
  qemuDomainRemoveInactive(driver, vm);

It's safe to put the check into the function and save some LoC.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_domain.c|  9 -
 src/qemu/qemu_driver.c| 42 --
 src/qemu/qemu_migration.c | 14 +-
 src/qemu/qemu_process.c   | 12 
 4 files changed, 33 insertions(+), 44 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7d92f3a..367d662 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2632,7 +2632,14 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver,
 {
 bool haveJob = true;
 char *snapDir;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+virQEMUDriverConfigPtr cfg;
+
+if (vm->persistent) {
+/* Short-circuit, we don't want to remove a persistent domain */
+return;
+}
+
+cfg = virQEMUDriverGetConfig(driver);
 
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
 haveJob = false;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2a4b026..3532973 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1753,8 +1753,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
 def = NULL;
 
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) {
-if (!vm->persistent)
-qemuDomainRemoveInactive(driver, vm);
+qemuDomainRemoveInactive(driver, vm);
 goto cleanup;
 }
 
@@ -1764,8 +1763,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
  start_flags) < 0) {
 virDomainAuditStart(vm, "booted", false);
 qemuDomainObjEndJob(driver, vm);
-if (!vm->persistent)
-qemuDomainRemoveInactive(driver, vm);
+qemuDomainRemoveInactive(driver, vm);
 goto cleanup;
 }
 
@@ -2250,7 +2248,7 @@ qemuDomainDestroyFlags(virDomainPtr dom,
 ret = 0;
  endjob:
 qemuDomainObjEndJob(driver, vm);
-if (ret == 0 && !vm->persistent)
+if (ret == 0)
 qemuDomainRemoveInactive(driver, vm);
 
  cleanup:
@@ -3273,7 +3271,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
 }
 }
 qemuDomainObjEndAsyncJob(driver, vm);
-if (ret == 0 && !vm->persistent)
+if (ret == 0)
 qemuDomainRemoveInactive(driver, vm);
 
  cleanup:
@@ -3774,7 +3772,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom,
 }
 
 qemuDomainObjEndAsyncJob(driver, vm);
-if (ret == 0 && flags & VIR_DUMP_CRASH && !vm->persistent)
+if (ret == 0 && flags & VIR_DUMP_CRASH)
 qemuDomainRemoveInactive(driver, vm);
 
  cleanup:
@@ -4054,8 +4052,7 @@ processGuestPanicEvent(virQEMUDriverPtr driver,
 
 virDomainAuditStop(vm, "destroyed");
 
-if (!vm->persistent)
-qemuDomainRemoveInactive(driver, vm);
+qemuDomainRemoveInactive(driver, vm);
 break;
 
 case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_RESTART:
@@ -6831,7 +6828,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 VIR_WARN("Failed to close %s", path);
 
 qemuDomainObjEndJob(driver, vm);
-if (ret < 0 && !vm->persistent)
+if (ret < 0)
 qemuDomainRemoveInactive(driver, vm);
 
  cleanup:
@@ -7526,6 +7523,7 @@ static virDomainPtr 
qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml
 } else {
 /* Brand new domain. Remove it */
 VIR_INFO("Deleting domain '%s'", vm->def->name);
+vm->persistent = 0;
 qemuDomainRemoveInactive(driver, vm);
 }
 goto cleanup;
@@ -7651,11 +7649,9 @@ qemuDomainUndefineFlags(virDomainPtr dom,
  * domainDestroy and domainShutdown will take care of removing the
  * domain obj from the hash table.
  */
-if (virDomainObjIsActive(vm)) {
-vm->persistent = 0;
-} else {
+vm->persistent = 0;
+if (!virDomainObjIsActive(vm))
 qemuDomainRemoveInactive(driver, vm);
-}
 
 ret = 0;
 
@@ -15550,12 +15546,9 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 }
 
 if (qemuDomainSnapshotRevertInactive(driver, vm, snap) < 0) {
-if (!vm->persistent) {
-qemuDomainObjEndJob(driver, vm);
-qemuDomainRemoveInactive(driver, vm);
-goto cleanup;
-}
-goto endjob;
+qemuDomainObjEndJob(driver, vm);
+qemuDomainRemoveInactive(driver, vm);
+goto cleanup;
 }
 if (config)
 virDomainObjAssignDef(vm, config, false, NULL);
@@ -15575,12 +15568,9 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
   start_flags);
 virDomainAuditStart(vm, "from-snapshot", rc >= 0);
 if (rc < 0) {
-if 

[libvirt] [PATCH v2 0/4] Couple of 'virsh create' fixes

[Michal Privoznik]

And this time 'virsh restore' too.
The diff to v1 is that more drivers is fixed.

Michal Privoznik (4):
  virDomainCreateXML: Don't remove persistent domains on error
  virDomainCreateXML: Make domain definition transient
  qemu: Move vm->persistent check into qemuDomainRemoveInactive
  virDomainRestore: Don't keep transient domains around

 src/bhyve/bhyve_driver.c   |  1 +
 src/libxl/libxl_driver.c   |  1 +
 src/lxc/lxc_driver.c   |  7 +--
 src/openvz/openvz_driver.c |  1 +
 src/qemu/qemu_domain.c |  9 -
 src/qemu/qemu_driver.c | 39 ---
 src/qemu/qemu_migration.c  | 14 +-
 src/qemu/qemu_process.c| 12 
 src/test/test_driver.c | 15 +--
 src/uml/uml_driver.c   |  8 +---
 src/vmware/vmware_driver.c |  7 +--
 11 files changed, 64 insertions(+), 50 deletions(-)

-- 
2.4.9

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

[libvirt] [PATCH v2 4/4] virDomainRestore: Don't keep transient domains around

[Michal Privoznik]

So while working on my previous patches, I've noticed that
virDomainRestore implementation in qemu and test drivers has the
same problem as I am fixing.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 4 ++--
 src/test/test_driver.c | 7 ++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3532973..562a0b6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6828,8 +6828,6 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 VIR_WARN("Failed to close %s", path);
 
 qemuDomainObjEndJob(driver, vm);
-if (ret < 0)
-qemuDomainRemoveInactive(driver, vm);
 
  cleanup:
 virDomainDefFree(def);
@@ -6837,6 +6835,8 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 VIR_FREE(xml);
 VIR_FREE(xmlout);
 virFileWrapperFdFree(wrapperFd);
+if (vm && ret < 0)
+qemuDomainRemoveInactive(driver, vm);
 virDomainObjEndAPI();
 virNWFilterUnlockFilterUpdates();
 return ret;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 01ab1e3..9ccd567 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2132,8 +2132,13 @@ testDomainRestoreFlags(virConnectPtr conn,
 goto cleanup;
 def = NULL;
 
-if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_RESTORED) < 0)
+if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_RESTORED) < 0) {
+if (!dom->persistent) {
+virDomainObjListRemove(privconn->domains, dom);
+dom = NULL;
+}
 goto cleanup;
+}
 
 event = virDomainEventLifecycleNewFromObj(dom,
  VIR_DOMAIN_EVENT_STARTED,
-- 
2.4.9

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

Re: [libvirt] [libvirt-guests] New feature ALWAYS_START

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 03:16:04PM +0200, Martin Kletzander wrote:
> On Wed, Sep 23, 2015 at 12:50:57PM +0100, Daniel P. Berrange wrote:
> >On Wed, Sep 23, 2015 at 10:50:58AM +0200, Martin Kletzander wrote:
> >>On Tue, Sep 22, 2015 at 07:33:30AM +0200, Marek Lukács wrote:
> >>>Hi,
> >>>
> >>>It will be nice feature to have configuration option
> >>>ALWAYS_START="$uri:$name $uri:name ..." in libvirt-guests
> >>>configuration file.
> >>>
> >>>If ON_BOOT is "start" and if ALWAYS_START is not empty, it iterates
> >>>over the ALWAYS_START and starts guests with same conditions (delays
> >>>etc.) before it starts guests from LISTFILE.
> >>>
> >>
> >>To be honest, I don't think that's _exactly_ what you want _just_ from
> >>libvirt itself; let me explain.
> >>
> >>>Benefits:
> >>>- guests are started with delays
> >>
> >>Delays that are done due to the guests are not something we should
> >>handle.  Guests and mainly the applications inside them should handle
> >>this gracefully.  Just delaying the starts is still error-prone.
> >>
> >>>- guests are started after host failure
> >>
> >>That's what libvirt-guests does already.  And if you want some domains
> >>to be started on every start, there's the 'autostart' parameter for
> >>domains.
> >
> >Indeed, and I have a generall desire for libvirt-guests to go away
> >entirely and have all its functionality just built into libvirtd. So
> >I don't think taking functionality that already exist in libvirtd and
> >adding it to libvirt-guest is the right direction for us to take.
> >
> 
> As Jirka pointed out, it's harder to do this in libvirtd because you
> want all the domains to be saved when shutting down, but not when
> stopping the service.  Another option would be that libvirt-guests
> would just tell libvirt over some other API, which is not
> hypervisor-specific (let's say admin api for example), that would call
> some function in all drivers that would do what libvirt-guests does
> now.

Yeah, I would anticipate adding a function in the admin API to trigger
"complete shutdown" of resources. It could be a useful admin task even
when a host is not shutting down.

When running in session mode, libvirtd listens for a dbus signal to
indicate that the desktop session is exiting, and does the save. I
wonder if system emits any kind of signal to indicate that a system
shutdown is taking place. If so we might be able to hook into that
instead of needing the admin AP


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] conf: escape string for disk driver name attribute

[lhuang]

On 09/23/2015 05:54 PM, Martin Kletzander wrote:

On Tue, Sep 22, 2015 at 04:13:53PM +0800, Luyao Huang wrote:

Just like e92e5ba1, this attribute was missed.

Signed-off-by: Luyao Huang 
---
src/conf/domain_conf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)



ACK && Pushed.


Thanks your review, Martin :)

Luyao

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

Re: [libvirt] [PATCH V2 0/9] support multi-thread compress migration.

[Feng, Shaohe]

Hi, Eric and Jiri

I build a qemu with multi-thread compress migration.

And I have do some test on live migration with multi-thread compress by libvirt 
after apply this patch set.

Can you give come comments on this patch set? 





> -Original Message-
> From: Feng, Shaohe
> Sent: Tuesday, September 22, 2015 5:05 PM
> To: 'libvir-list@redhat.com'
> Cc: 'Jiri Denemark'; 'Eric Blake'; Li, Liang Z; Dugger, Donald D; Ding, 
> Jian-feng
> Subject: RE: [PATCH V2 0/9] support multi-thread compress migration.
> 
> Ping.
> 
> > -Original Message-
> > From: Feng, Shaohe
> > Sent: Monday, August 10, 2015 5:32 PM
> > To: 'libvir-list@redhat.com'
> > Cc: 'Jiri Denemark'; 'Eric Blake'; Li, Liang Z; Dugger, Donald D
> > Subject: RE: [PATCH V2 0/9] support multi-thread compress migration.
> >
> > Hi, Eric and Jiri
> >
> > Can you have a look at this patch and give some comments at your leisure?
> >
> >
> >
> > > -Original Message-
> > > From: Feng, Shaohe
> > > Sent: Thursday, July 9, 2015 9:02 PM
> > > To: libvir-list@redhat.com
> > > Cc: Jiri Denemark; Eric Blake; Li, Liang Z; Feng, Shaohe
> > > Subject: [PATCH V2 0/9] support multi-thread compress migration.
> > >
> > > These series patches support multi-thread compress during live migration.
> > >
> > > Eli Qiao (4):
> > >   Add test cases for qemuMonitorJSONGetMigrationParameter
> > >   remote: Add support for set and get multil thread migration parameters
> > >   qemu_driver: Add support to set/get migration parameters.
> > >   virsh: Add set and get multi-thread migration parameters commands
> > >
> > > ShaoHe Feng (5):
> > >   qemu_migration: Add support for mutil-thread compressed migration
> > > enable
> > >   qemu: Add monitor API for get/set migration parameters
> > >   set multi-thread compress params for Migrate3 during live migration
> > >   virsh: add multi-thread migration option for live migrate command
> > >   Implement the public APIs for multi-thread compress parameters.
> > >
> > >  .gnulib  |   2 +-
> > >  daemon/remote.c  |  62 +++
> > >  include/libvirt/libvirt-domain.h |  31 ++
> > >  src/driver-hypervisor.h  |  14 +++
> > >  src/libvirt-domain.c | 110 +++
> > >  src/libvirt_public.syms  |   5 +
> > >  src/qemu/qemu_domain.h   |   3 +
> > >  src/qemu/qemu_driver.c   | 186 
> > >  src/qemu/qemu_migration.c| 105 ++
> > >  src/qemu/qemu_migration.h|  32 --
> > >  src/qemu/qemu_monitor.c  |  40 ++-
> > >  src/qemu/qemu_monitor.h  |  11 ++
> > >  src/qemu/qemu_monitor_json.c |  93 
> > >  src/qemu/qemu_monitor_json.h |   9 ++
> > >  src/qemu/qemu_monitor_text.c |  95 +
> > >  src/qemu/qemu_monitor_text.h |  10 ++
> > >  src/remote/remote_driver.c   |  54 ++
> > >  src/remote/remote_protocol.x |  30 +-
> > >  src/remote_protocol-structs  |  26 +
> > >  tests/qemumonitorjsontest.c  |  53 ++
> > >  tools/virsh-domain.c | 223 
> > > ++-
> > >  tools/virsh.pod  |  37 +--
> > >  22 files changed, 1212 insertions(+), 19 deletions(-)
> > >
> > > --
> > > 2.1.4


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

Re: [libvirt] [PATCH v2] qemu: Refresh memory size only on fresh starts

[Ján Tomko]

On Wed, Sep 23, 2015 at 02:23:09PM +0200, Peter Krempa wrote:
> Qemu unfortunately doesn't update internal state right after migration
> and so the actual balloon size as returned by 'query-balloon' are
> invalid for a while after the CPUs are started after migration. If we'd
> refresh our internal state at this point we would report invalid current
> memory size until the next balloon event would arrive.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1242940
> ---
> Version 2:
> - completely different from original posting due to unexpected qemu behavior
> 
>  src/qemu/qemu_process.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

ACK

Jan


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

Re: [libvirt] [PATCH] qemu: Align memory module sizes to 2MiB

[Pavel Hrdina]

On Wed, Sep 23, 2015 at 01:18:02PM +0200, Peter Krempa wrote:
> My original implementation was based on a qemu version that still did
> not have all the checks in place. Using sizes that would align to odd
> megabyte increments will produce the following error:
> 
> qemu-kvm: -device pc-dimm,node=0,memdev=memdimm0,id=dimm0: backend memory 
> size must be multiple of 0x20
> qemu-kvm: -device pc-dimm,node=0,memdev=memdimm0,id=dimm0: Device 'pc-dimm' 
> could not be initialized
> 
> Introduce an alignment retrieval function for memory devices and use it
> to align the devices separately and modify a test case to verify it.
> ---
>  src/qemu/qemu_domain.c| 19 
> ++-
>  .../qemuxml2argv-memory-hotplug-dimm.xml  |  2 +-
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 

ACK

Pavel

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

Re: [libvirt] [libvirt-guests] New feature ALWAYS_START

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 10:50:58AM +0200, Martin Kletzander wrote:
> On Tue, Sep 22, 2015 at 07:33:30AM +0200, Marek Lukács wrote:
> >Hi,
> >
> >It will be nice feature to have configuration option
> >ALWAYS_START="$uri:$name $uri:name ..." in libvirt-guests
> >configuration file.
> >
> >If ON_BOOT is "start" and if ALWAYS_START is not empty, it iterates
> >over the ALWAYS_START and starts guests with same conditions (delays
> >etc.) before it starts guests from LISTFILE.
> >
> 
> To be honest, I don't think that's _exactly_ what you want _just_ from
> libvirt itself; let me explain.
> 
> >Benefits:
> >- guests are started with delays
> 
> Delays that are done due to the guests are not something we should
> handle.  Guests and mainly the applications inside them should handle
> this gracefully.  Just delaying the starts is still error-prone.
> 
> >- guests are started after host failure
> 
> That's what libvirt-guests does already.  And if you want some domains
> to be started on every start, there's the 'autostart' parameter for
> domains.

Indeed, and I have a generall desire for libvirt-guests to go away
entirely and have all its functionality just built into libvirtd. So
I don't think taking functionality that already exist in libvirtd and
adding it to libvirt-guest is the right direction for us to take.


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] [sandbox 04/11] virt-sandbox-image: remove undefined default_disk_format

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:34AM +0200, Cédric Bosdonnat wrote:
> ---
>  libvirt-sandbox/image/sources/DockerSource.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
> b/libvirt-sandbox/image/sources/DockerSource.py
> index ab0b765..6c02cc3 100644
> --- a/libvirt-sandbox/image/sources/DockerSource.py
> +++ b/libvirt-sandbox/image/sources/DockerSource.py
> @@ -243,7 +243,7 @@ class DockerSource(Source):
>  
>  def create_template(self, template, templatedir, connect=None, 
> format=None):
>  if format is None:
> -format = self.default_disk_format
> +format = "qcow2"
>  self._check_disk_format(format)
>  imagelist = self._get_image_list(template, templatedir)
>  imagelist.reverse()

ACK


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 3/3] virsh: Notify users about disconnects

[Jiri Denemark]

On Tue, Sep 22, 2015 at 13:21:18 +0200, Jiri Denemark wrote:
> On Mon, Sep 21, 2015 at 17:32:41 -0400, John Ferlan wrote:
> > 
> > 
> > On 09/17/2015 08:23 AM, Jiri Denemark wrote:
> > > After my "client rpc: Report proper error for keepalive disconnections"
> > > patch, virsh would no long print a warning when it closes a connection
> > > to a daemon after a keepalive timeout. Although the warning
> > > 
> > > virsh # 2015-09-15 10:59:26.729+: 642080: info :
> > > libvirt version: 1.2.19
> > > 2015-09-15 10:59:26.729+: 642080: warning :
> > > virKeepAliveTimerInternal:143 : No response from client
> > > 0x7efdc0a46730 after 1 keepalive messages in 2 seconds
> > > 
> > > was pretty ugly, it was still useful. This patch brings the useful part
> > > back while making it much nicer:
> > > 
> > > virsh # error: Disconnected from qemu:///system due to keepalive timeout
> > > 
> > > Signed-off-by: Jiri Denemark 
> > > ---
> > >  tools/virsh.c | 35 ---
> > >  1 file changed, 32 insertions(+), 3 deletions(-)
> > > 
> > 
> > Patch series seems reasonable to me, except for one minor thing
> > Found by my coverity checker...  Naturally
> > 
> > > diff --git a/tools/virsh.c b/tools/virsh.c
> > > index bb12dec..23436ea 100644
> > > --- a/tools/virsh.c
> > > +++ b/tools/virsh.c
> > > @@ -95,12 +95,41 @@ static int disconnected; /* we may have been 
> > > disconnected */
> > >   * handler, just save the fact it was raised.
> > >   */
> > >  static void
> > > -virshCatchDisconnect(virConnectPtr conn ATTRIBUTE_UNUSED,
> > > +virshCatchDisconnect(virConnectPtr conn,
> > >   int reason,
> > > - void *opaque ATTRIBUTE_UNUSED)
> > > + void *opaque)
> > >  {
> > > -if (reason != VIR_CONNECT_CLOSE_REASON_CLIENT)
> > > +if (reason != VIR_CONNECT_CLOSE_REASON_CLIENT) {
> > 
> > [1]
> > 
> > > +vshControl *ctl = opaque;
> > > +const char *str = "unknown reason";
> > > +virErrorPtr error;
> > > +char *uri;
> > > +
> > > +error = virSaveLastError();
> > > +uri = virConnectGetURI(conn);
> > > +
> > > +switch ((virConnectCloseReason) reason) {
> > > +case VIR_CONNECT_CLOSE_REASON_ERROR:
> > > +str = N_("Disconnected from %s due to I/O error");
> > > +break;
> > > +case VIR_CONNECT_CLOSE_REASON_EOF:
> > > +str = N_("Disconnected from %s due to end of file");
> > > +break;
> > > +case VIR_CONNECT_CLOSE_REASON_KEEPALIVE:
> > > +str = N_("Disconnected from %s due to keepalive timeout");
> > > +break;
> > > +case VIR_CONNECT_CLOSE_REASON_CLIENT:
> > 
> > [1]^^^ Coverity complains about DEADCODE
> > 
> > Other than setting str = NULL and testing for it later, I'm not exactly
> > sure of the 'best' course of action.  e.g., if (str) { vshError();
> > disconnected++; }
> > 
> > I think if this is handled, then the series is ACK-able.
> 
> I think /* coverity[dead_error_begin] */ is the right solution in this
> case.

I pushed the series with this comment added above case
VIR_CONNECT_CLOSE_REASON_CLIENT.

Thanks,

Jirka

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

[libvirt] [PATCH v2] qemuDomainDeviceDefPostParse: Drop useless checks

[Michal Privoznik]

Now that virQEMUDriverCreateXMLConf is never called with NULL
(after 086f37e97aab) we can safely drop useless check in
qemuDomainDeviceDefPostParse as we are guaranteed to be always
called with the driver initialized. Therefore checking if driver
is NULL makes no sense. Moreover, if we mix it with direct driver
dereference. And after that, we are sure that nor @cfg will be
NULL, therefore we can drop checks for that too.

Signed-off-by: Michal Privoznik 
---

diff to v1:
- drop checks for !cfg too

 src/qemu/qemu_domain.c | 64 --
 1 file changed, 26 insertions(+), 38 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7d92f3a..78f305f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1233,12 +1233,9 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 {
 virQEMUDriverPtr driver = opaque;
 virQEMUCapsPtr qemuCaps = NULL;
-virQEMUDriverConfigPtr cfg = NULL;
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 int ret = -1;
 
-if (driver)
-cfg = virQEMUDriverGetConfig(driver);
-
 qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
 
 if (dev->type == VIR_DOMAIN_DEVICE_NET &&
@@ -1253,37 +1250,34 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
 virDomainDiskDefPtr disk = dev->data.disk;
 
-/* both of these require data from the driver config */
-if (cfg) {
-/* assign default storage format and driver according to config */
-if (cfg->allowDiskFormatProbing) {
-/* default disk format for drives */
-if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE &&
-(virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_FILE ||
- virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK))
-virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_AUTO);
+/* assign default storage format and driver according to config */
+if (cfg->allowDiskFormatProbing) {
+/* default disk format for drives */
+if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE &&
+(virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_FILE ||
+ virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK))
+virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_AUTO);
 
-/* default disk format for mirrored drive */
-if (disk->mirror &&
-disk->mirror->format == VIR_STORAGE_FILE_NONE)
-disk->mirror->format = VIR_STORAGE_FILE_AUTO;
-} else {
-/* default driver if probing is forbidden */
-if (!virDomainDiskGetDriver(disk) &&
-virDomainDiskSetDriver(disk, "qemu") < 0)
-goto cleanup;
+/* default disk format for mirrored drive */
+if (disk->mirror &&
+disk->mirror->format == VIR_STORAGE_FILE_NONE)
+disk->mirror->format = VIR_STORAGE_FILE_AUTO;
+} else {
+/* default driver if probing is forbidden */
+if (!virDomainDiskGetDriver(disk) &&
+virDomainDiskSetDriver(disk, "qemu") < 0)
+goto cleanup;
 
-/* default disk format for drives */
-if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE &&
-(virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_FILE ||
- virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK))
-virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
+/* default disk format for drives */
+if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE &&
+(virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_FILE ||
+ virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK))
+virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
 
-/* default disk format for mirrored drive */
-if (disk->mirror &&
-disk->mirror->format == VIR_STORAGE_FILE_NONE)
-disk->mirror->format = VIR_STORAGE_FILE_RAW;
-}
+/* default disk format for mirrored drive */
+if (disk->mirror &&
+disk->mirror->format == VIR_STORAGE_FILE_NONE)
+disk->mirror->format = VIR_STORAGE_FILE_RAW;
 }
 }
 
@@ -1315,12 +1309,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 dev->data.chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO 
&&
 dev->data.chr->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
 !dev->data.chr->source.data.nix.path) {
-if (!cfg) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("cannot 

[libvirt] [libvirt-glib] gobject: Correct docs for gvir_storage_pool_get_info()

[Zeeshan Ali (Khattak)]

The returned GVirStoragePoolInfo pointer is not a GObject so it must not
be unrefed using g_object_unref(). Since gvir_storage_pool_info_free()
is private function, callers must either use g_slice_free() or
g_boxed_free().
---

Perhaps we should just make gvir_storage_pool_info_free() public instead?

 libvirt-gobject/libvirt-gobject-storage-pool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
b/libvirt-gobject/libvirt-gobject-storage-pool.c
index 7f26b1b..f015efa 100644
--- a/libvirt-gobject/libvirt-gobject-storage-pool.c
+++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
@@ -277,8 +277,8 @@ GVirConfigStoragePool 
*gvir_storage_pool_get_config(GVirStoragePool *pool,
  * @pool: the storage_pool
  * @err: Place-holder for possible errors
  *
- * Returns: (transfer full): the info. The returned object should be
- * unreffed with g_object_unref() when no longer needed.
+ * Returns: (transfer full): the info. The returned pointer should be
+ * freed using either #g_slice_free() or #g_boxed_free() when no longer needed.
  */
 GVirStoragePoolInfo *gvir_storage_pool_get_info(GVirStoragePool *pool,
 GError **err)
-- 
2.4.3

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

Re: [libvirt] [libvirt-guests] New feature ALWAYS_START

[Martin Kletzander]

On Wed, Sep 23, 2015 at 11:39:29AM +0200, Marek Lukács wrote:

Hi Martin,

thank you for your reply. Please check my comments.

On Wed, Sep 23, 2015 at 10:50 AM, Martin Kletzander 
wrote:


On Tue, Sep 22, 2015 at 07:33:30AM +0200, Marek Lukács wrote:


Hi,

It will be nice feature to have configuration option
ALWAYS_START="$uri:$name $uri:name ..." in libvirt-guests
configuration file.

If ON_BOOT is "start" and if ALWAYS_START is not empty, it iterates
over the ALWAYS_START and starts guests with same conditions (delays
etc.) before it starts guests from LISTFILE.



To be honest, I don't think that's _exactly_ what you want _just_ from
libvirt itself; let me explain.

Benefits:

- guests are started with delays



Delays that are done due to the guests are not something we should
handle.  Guests and mainly the applications inside them should handle
this gracefully.  Just delaying the starts is still error-prone.



I fully understand this and fully agree with you. Production application
has to handle this gracefully. But still I see a space for doing it in
environments where it is not so necessary to configure application so
gracefully, like development or testing environments. Delays are fine for
many not production environments, where applications are not in production
state.



- guests are started after host failure




That's what libvirt-guests does already.  And if you want some domains
to be started on every start, there's the 'autostart' parameter for
domains.



No, libvirt-guests only starts those domains which has been running before
"service" libvirt-guests has been stopped. In case of host failure, there
is no LISTFILE in filesystem, as it has not been generated by
libvirt-guests stop mechanism.



Oh, so here is what I have misunderstood.  Well, just misread, of
course libvirt-guests doesn't handle host failures.


But probably I do not understand "script" libvirt-guests correctly and why
it is in libvirt. I will be happy, if you will give me more details, why
there is this script, even if libvirt has 'autostart' parameter for domains.

- For what usage is libvirt-guests designed?


My interpretation is that it is there in order for you to be able to
shutdown and boot later without losing the machines you were running.

Autostart says that particular domain should be started every time the
daemon is started.  Basically says that particular guest should be
running on the host all the time.


- Why it supports delays?


Because most of the time you'll want to resume from managed save and
you might cause a big load in case you're starting bunch of machines
because all of those will start loading everything from disk and so
on.


- Why to have libvirt-guests if there is 'autostart' domain parameter?


It does two different things.  Domain with autostart will be started
every time daemon (actually not even the computer) is started, but
libvirt-guests will stop/save domains that are running when the
computer is being shut down and start/resume them when it is starting
back.


- For who is libvirt-guests and who should use 'autostart' domain parameter?



Well, libvirt-guests was added by a guy who was too lazy to clean up
his machine before rebooting (sorry Jirka, I had to).  But you get the
picture.  Use it for whatever you like and whatever suits you.  I'm
not even against adding what you suggested, I just wanted to make sure
you're not relying on the script for something critical as my
understanding of it that it is not very error prone.




- guests are started in specific order (for example complex

environment, when DB should be started before other guest, etc.)



Again, same as the first point.  This should be handled gracefully in
the application itself or at least worked around in the guest (not
starting DB-related app before DB is accessible).

Anyway, if you *really* want this, then the easiest thing to do is
creating a service that starts before libvirt-guests, but after
libvirt, which is just something similar to "local", so it just runs a
script that does:

 for i in domain_one some_other_domain database_dom
 do
   virsh start "$i"
   sleep 60 # or you can try connecting to make sure it started
 done

or something similar.  However, you might still propose a simple patch
for the feature you described.



For me it is no problem to design my own script to handle my needs. I have
spent some time googling, if there is already a tool for it. I found only
similar questions, so I got feeling, that I am not the only one with
similar requirements.

Script libvirt-guests in my eyes handles very similar task. It starts
domains with delays, it starts domains what has been running at previous
stop, but do not handle situation in case of host failure and do not starts
domain in specific order. I prefer and I think, that it is better to not
create new script no one knows about, but to modify existing one everybody
knows about. But again, maybe I do not understand why and 

Re: [libvirt] [PATCH v2] virsh: Fix job status indicator for 0 length block jobs

[Peter Krempa]

On Tue, Sep 22, 2015 at 11:12:28 +0200, Erik Skultety wrote:
> 
> 
> On 21/09/15 19:46, Peter Krempa wrote:
> > Although 0 length block jobs aren't entirely useful, the output of virsh
> > blockjob is empty due to the condition that suppresses the output for
> > migration jobs that did not start. Since the only place that actually
> > uses the condition that suppresses the output is in migration, let's
> > move the check there and thus add support for 0 of 0 equaling to 100%.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1196711
> > ---
> >  tools/virsh-domain.c | 6 +-
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 73c476d..fb138d5 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -1700,10 +1700,6 @@ virshPrintJobProgress(const char *label, unsigned 
> > long long remaining,
> >  {
> >  int progress;
> > 
> > -if (total == 0)
> > -/* migration has not been started */
> > -return;
> > -
> >  if (remaining == 0) {
> >  /* migration has completed */
> >  progress = 100;
> > @@ -4401,7 +4397,7 @@ virshWatchJob(vshControl *ctl,
> >  ret = virDomainGetJobInfo(dom, );
> >  pthread_sigmask(SIG_SETMASK, , NULL);
> >  if (ret == 0) {
> > -if (verbose)
> > +if (verbose && jobinfo.dataTotal > 0)
> >  virshPrintJobProgress(label, jobinfo.dataRemaining,
> >jobinfo.dataTotal);
> > 
> Seems reasonable to me, ACK.

Pushed; Thanks.

Peter


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

Re: [libvirt] [libvirt-guests] New feature ALWAYS_START

[Marek Lukács]

Hi Martin,

thank you. Definitely I want to propose patch to libvirt-guests, but this
weekend I am taking family holiday, so be patient and wait little bit more.

Regards,

Marek

On Wed, Sep 23, 2015 at 3:12 PM, Martin Kletzander 
wrote:

> On Wed, Sep 23, 2015 at 11:39:29AM +0200, Marek Lukács wrote:
>
>> Hi Martin,
>>
>> thank you for your reply. Please check my comments.
>>
>> On Wed, Sep 23, 2015 at 10:50 AM, Martin Kletzander 
>> wrote:
>>
>> On Tue, Sep 22, 2015 at 07:33:30AM +0200, Marek Lukács wrote:
>>>
>>> Hi,

 It will be nice feature to have configuration option
 ALWAYS_START="$uri:$name $uri:name ..." in libvirt-guests
 configuration file.

 If ON_BOOT is "start" and if ALWAYS_START is not empty, it iterates
 over the ALWAYS_START and starts guests with same conditions (delays
 etc.) before it starts guests from LISTFILE.


 To be honest, I don't think that's _exactly_ what you want _just_ from
>>> libvirt itself; let me explain.
>>>
>>> Benefits:
>>>
 - guests are started with delays


>>> Delays that are done due to the guests are not something we should
>>> handle.  Guests and mainly the applications inside them should handle
>>> this gracefully.  Just delaying the starts is still error-prone.
>>>
>>>
>> I fully understand this and fully agree with you. Production application
>> has to handle this gracefully. But still I see a space for doing it in
>> environments where it is not so necessary to configure application so
>> gracefully, like development or testing environments. Delays are fine for
>> many not production environments, where applications are not in production
>> state.
>>
>>
>> - guests are started after host failure
>>>


>>> That's what libvirt-guests does already.  And if you want some domains
>>> to be started on every start, there's the 'autostart' parameter for
>>> domains.
>>>
>>>
>> No, libvirt-guests only starts those domains which has been running before
>> "service" libvirt-guests has been stopped. In case of host failure, there
>> is no LISTFILE in filesystem, as it has not been generated by
>> libvirt-guests stop mechanism.
>>
>>
> Oh, so here is what I have misunderstood.  Well, just misread, of
> course libvirt-guests doesn't handle host failures.
>
> But probably I do not understand "script" libvirt-guests correctly and why
>> it is in libvirt. I will be happy, if you will give me more details, why
>> there is this script, even if libvirt has 'autostart' parameter for
>> domains.
>>
>> - For what usage is libvirt-guests designed?
>>
>
> My interpretation is that it is there in order for you to be able to
> shutdown and boot later without losing the machines you were running.
>
> Autostart says that particular domain should be started every time the
> daemon is started.  Basically says that particular guest should be
> running on the host all the time.
>
> - Why it supports delays?
>>
>
> Because most of the time you'll want to resume from managed save and
> you might cause a big load in case you're starting bunch of machines
> because all of those will start loading everything from disk and so
> on.
>
> - Why to have libvirt-guests if there is 'autostart' domain parameter?
>>
>
> It does two different things.  Domain with autostart will be started
> every time daemon (actually not even the computer) is started, but
> libvirt-guests will stop/save domains that are running when the
> computer is being shut down and start/resume them when it is starting
> back.
>
> - For who is libvirt-guests and who should use 'autostart' domain
>> parameter?
>>
>>
> Well, libvirt-guests was added by a guy who was too lazy to clean up
> his machine before rebooting (sorry Jirka, I had to).  But you get the
> picture.  Use it for whatever you like and whatever suits you.  I'm
> not even against adding what you suggested, I just wanted to make sure
> you're not relying on the script for something critical as my
> understanding of it that it is not very error prone.
>
>
>
>> - guests are started in specific order (for example complex
>>>
 environment, when DB should be started before other guest, etc.)


 Again, same as the first point.  This should be handled gracefully in
>>> the application itself or at least worked around in the guest (not
>>> starting DB-related app before DB is accessible).
>>>
>>> Anyway, if you *really* want this, then the easiest thing to do is
>>> creating a service that starts before libvirt-guests, but after
>>> libvirt, which is just something similar to "local", so it just runs a
>>> script that does:
>>>
>>>  for i in domain_one some_other_domain database_dom
>>>  do
>>>virsh start "$i"
>>>sleep 60 # or you can try connecting to make sure it started
>>>  done
>>>
>>> or something similar.  However, you might still propose a simple patch
>>> for the feature you described.
>>>
>>>
>> For me it is no problem to design my own 

Re: [libvirt] [PATCH] add compress stream support

[Vasiliy Tolstov]

23 сент. 2015 г. 16:18 пользователь "Daniel P. Berrange" <
berra...@redhat.com> написал:
>
> On Wed, Sep 23, 2015 at 03:20:51PM +0300, Vasiliy Tolstov wrote:
> > 2015-09-23 14:54 GMT+03:00 Daniel P. Berrange :
> > > How have you actually tested this in practice - your patch does
> > > not change any code to make use of this new feate. You're changing
> > > the public API which suggests you expect the client apps to use
> > > this when passing a virStreamPtr to the virStorageVolDownload
> > > method, but the stream client apps pass is not backed by a
> > > virFDStream object. Only libvirtd uses the virFDSteam objects
> > > and you've not changed anything to make use ot that. So this is
> > > all rather strange still. Can you more clearly state what you
> > > are expecting to do.
> >
> >
> > As i'm understand client create stream via virStreamNew , when user
> > invoke virStoreVolDownload this function use created stream.
> > If i'm correct, libvirt when read/write in
> > virStoreVolDownload/virStoreVolUpload uses fdstream functions
> > internally. So data compressed or decompressed.
> > Or when client create virStreamNew and pass it libvirt not always use
> > fdstream functions ?
>
> It is not quite that simple because we have a client/server architecture.
>
> So the client app creates a virStreamPtr. This is used by the remote
> driver, via a virNetClientStream object to tunnel data over the
> libvirtd connection.
>

So client can't say what stream libvirtd need to create for storage driver?

> The libvirtd daemon then creates another virStreamPtr object. This is
> used by the straoge driver, via the virFDStream object to fetch the
> data the client is requesting.
>
> So having the client app request compression on its virStreamPtr
> does not work, because that stream object is not the one used
> by the virFDStream code.
>
> An alternative approach would be to not try to change the stream API
> at all.
>
> Instead pass flags to the virStorageVolDownload/Upload functions,
> eg VIR_STORAGE_VOL_STREAM_COMPRESS_ZLIB. This flag would get passed
> across to the storage driver, which can then turn on compression in
> the virFDStream impl. This would require
>
>  - Add the enum flags to include/libvirt/libvirt-storage.h
>  - Add libarchive support to src/fdstream.c
>  - Handle the new flags in src/storage/storage_driver.c to
>turn on the libarchive compression in fdsream.c
>
> 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] [libvirt-glib] gobject: Correct docs for gvir_storage_pool_get_info()

[Zeeshan Ali (Khattak)]

On Wed, Sep 23, 2015 at 2:18 PM, Daniel P. Berrange  wrote:
> On Wed, Sep 23, 2015 at 01:44:28PM +0100, Zeeshan Ali (Khattak) wrote:
>> The returned GVirStoragePoolInfo pointer is not a GObject so it must not
>> be unrefed using g_object_unref(). Since gvir_storage_pool_info_free()
>> is private function, callers must either use g_slice_free() or
>> g_boxed_free().
>> ---
>>
>> Perhaps we should just make gvir_storage_pool_info_free() public instead?
>>
>>  libvirt-gobject/libvirt-gobject-storage-pool.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
>> b/libvirt-gobject/libvirt-gobject-storage-pool.c
>> index 7f26b1b..f015efa 100644
>> --- a/libvirt-gobject/libvirt-gobject-storage-pool.c
>> +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
>> @@ -277,8 +277,8 @@ GVirConfigStoragePool 
>> *gvir_storage_pool_get_config(GVirStoragePool *pool,
>>   * @pool: the storage_pool
>>   * @err: Place-holder for possible errors
>>   *
>> - * Returns: (transfer full): the info. The returned object should be
>> - * unreffed with g_object_unref() when no longer needed.
>> + * Returns: (transfer full): the info. The returned pointer should be
>> + * freed using either #g_slice_free() or #g_boxed_free() when no longer 
>> needed.
>>   */
>>  GVirStoragePoolInfo *gvir_storage_pool_get_info(GVirStoragePool *pool,
>>  GError **err)
>
> ACK

Thanks but just to be sure, I hope you didn't miss the details and
this comment below it:

> Perhaps we should just make gvir_storage_pool_info_free() public instead?

-- 
Regards,

Zeeshan Ali (Khattak)

Befriend GNOME: http://www.gnome.org/friends/

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

Re: [libvirt] [libvirt-glib] gobject: Correct docs for gvir_storage_pool_get_info()

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 02:54:51PM +0100, Zeeshan Ali (Khattak) wrote:
> On Wed, Sep 23, 2015 at 2:18 PM, Daniel P. Berrange  
> wrote:
> > On Wed, Sep 23, 2015 at 01:44:28PM +0100, Zeeshan Ali (Khattak) wrote:
> >> The returned GVirStoragePoolInfo pointer is not a GObject so it must not
> >> be unrefed using g_object_unref(). Since gvir_storage_pool_info_free()
> >> is private function, callers must either use g_slice_free() or
> >> g_boxed_free().
> >> ---
> >>
> >> Perhaps we should just make gvir_storage_pool_info_free() public instead?
> >>
> >>  libvirt-gobject/libvirt-gobject-storage-pool.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
> >> b/libvirt-gobject/libvirt-gobject-storage-pool.c
> >> index 7f26b1b..f015efa 100644
> >> --- a/libvirt-gobject/libvirt-gobject-storage-pool.c
> >> +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
> >> @@ -277,8 +277,8 @@ GVirConfigStoragePool 
> >> *gvir_storage_pool_get_config(GVirStoragePool *pool,
> >>   * @pool: the storage_pool
> >>   * @err: Place-holder for possible errors
> >>   *
> >> - * Returns: (transfer full): the info. The returned object should be
> >> - * unreffed with g_object_unref() when no longer needed.
> >> + * Returns: (transfer full): the info. The returned pointer should be
> >> + * freed using either #g_slice_free() or #g_boxed_free() when no longer 
> >> needed.
> >>   */
> >>  GVirStoragePoolInfo *gvir_storage_pool_get_info(GVirStoragePool *pool,
> >>  GError **err)
> >
> > ACK
> 
> Thanks but just to be sure, I hope you didn't miss the details and
> this comment below it:
> 
> > Perhaps we should just make gvir_storage_pool_info_free() public instead?

Since we've defined it as a boxed type, I think recommending g_boxed_free
is the right solution. We should make sure our docs actually tell people
this is a boxed type if they don't already :-)

We should *not* in fact recommend  g_slice_free(), as the use of the slice
allocator is an internal implementation detail. We should be free to change
to use a different allocator without breaking clients.

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] [libvirt-glib] gobject: Correct docs for gvir_storage_pool_get_info()

[Zeeshan Ali (Khattak)]

On Wed, Sep 23, 2015 at 3:00 PM, Daniel P. Berrange  wrote:
> On Wed, Sep 23, 2015 at 02:54:51PM +0100, Zeeshan Ali (Khattak) wrote:
>> On Wed, Sep 23, 2015 at 2:18 PM, Daniel P. Berrange  
>> wrote:
>> > On Wed, Sep 23, 2015 at 01:44:28PM +0100, Zeeshan Ali (Khattak) wrote:
>> >> The returned GVirStoragePoolInfo pointer is not a GObject so it must not
>> >> be unrefed using g_object_unref(). Since gvir_storage_pool_info_free()
>> >> is private function, callers must either use g_slice_free() or
>> >> g_boxed_free().
>> >> ---
>> >>
>> >> Perhaps we should just make gvir_storage_pool_info_free() public instead?
>> >>
>> >>  libvirt-gobject/libvirt-gobject-storage-pool.c | 4 ++--
>> >>  1 file changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c 
>> >> b/libvirt-gobject/libvirt-gobject-storage-pool.c
>> >> index 7f26b1b..f015efa 100644
>> >> --- a/libvirt-gobject/libvirt-gobject-storage-pool.c
>> >> +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
>> >> @@ -277,8 +277,8 @@ GVirConfigStoragePool 
>> >> *gvir_storage_pool_get_config(GVirStoragePool *pool,
>> >>   * @pool: the storage_pool
>> >>   * @err: Place-holder for possible errors
>> >>   *
>> >> - * Returns: (transfer full): the info. The returned object should be
>> >> - * unreffed with g_object_unref() when no longer needed.
>> >> + * Returns: (transfer full): the info. The returned pointer should be
>> >> + * freed using either #g_slice_free() or #g_boxed_free() when no longer 
>> >> needed.
>> >>   */
>> >>  GVirStoragePoolInfo *gvir_storage_pool_get_info(GVirStoragePool *pool,
>> >>  GError **err)
>> >
>> > ACK
>>
>> Thanks but just to be sure, I hope you didn't miss the details and
>> this comment below it:
>>
>> > Perhaps we should just make gvir_storage_pool_info_free() public instead?
>
> Since we've defined it as a boxed type, I think recommending g_boxed_free
> is the right solution.

It's just that it's slightly awkward to use that, since you need to
pass it the gtype, not just the pointer.

> We should make sure our docs actually tell people
> this is a boxed type if they don't already :-)
>
> We should *not* in fact recommend  g_slice_free(), as the use of the slice
> allocator is an internal implementation detail. We should be free to change
> to use a different allocator without breaking clients.

Sure, I pushed with g_slice_free() mention removed.

-- 
Regards,

Zeeshan Ali (Khattak)

Befriend GNOME: http://www.gnome.org/friends/

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

Re: [libvirt] [PATCH RFC 3/7] libxl: implement virDomainInterfaceStats

[Jim Fehlig]

Joao Martins wrote:
> Introduce support for domainInterfaceStats API call for querying
> network interface statistics. Consequently it also enables the
> use of `virsh domifstat  ` command.
>
> For getting statistics we resort to virNetInterfaceStats and let
> libvirt handle the platform specific nits. Note that the latter
> is not yet supported in FreeBSD.
>
> Signed-off-by: Joao Martins 
> ---
>  src/libxl/libxl_driver.c | 53 
> 
>  1 file changed, 53 insertions(+)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 43e9e47..dc83083 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -58,6 +58,7 @@
>  #include "virhostdev.h"
>  #include "network/bridge_driver.h"
>  #include "locking/domain_lock.h"
> +#include "virstats.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_LIBXL
>  
> @@ -4640,6 +4641,57 @@ libxlDomainIsUpdated(virDomainPtr dom)
>  }
>  
>  static int
> +libxlDomainInterfaceStats(virDomainPtr dom,
> +  const char *path,
> +  virDomainInterfaceStatsPtr stats)
> +{
> +libxlDriverPrivatePtr driver = dom->conn->privateData;
> +virDomainObjPtr vm;
> +int ret = -1;
> +int domid, devid;
> +
> +if (!(vm = libxlDomObjFromDomain(dom)))
> +goto cleanup;
> +
> +if (virDomainInterfaceStatsEnsureACL(dom->conn, vm->def) < 0)
> +goto cleanup;
> +
> +if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_QUERY) < 0)
> +goto cleanup;
> +
> +if (!virDomainObjIsActive(vm)) {
> +virReportError(VIR_ERR_OPERATION_INVALID,
> +   "%s", _("domain is not running"));
> +goto endjob;
> +}
> +
> +if (sscanf(path, "vif%d.%d", , ) != 2) {
> +virReportError(VIR_ERR_INVALID_ARG, "%s",
> +   _("invalid path, unknown device"));
> +goto endjob;
> +}
> +
> +if (domid != vm->def->id) {
> +virReportError(VIR_ERR_INVALID_ARG, "%s",
> +   _("invalid path, domid doesn't match"));
> +goto endjob;
> +}
>   

Should we also ensure the domain has an interface matching devid before
calling virNetInterfaceStats()? I see the qemu driver has such a check,
but virNetInterfaceStats() also reports "Interface not found".

Regards,
Jim

> +
> +ret = virNetInterfaceStats(path, stats);
> +
> + endjob:
> +if (!libxlDomainObjEndJob(driver, vm)) {
> +virObjectUnlock(vm);
> +vm = NULL;
> +}
> +
> + cleanup:
> +if (vm)
> +virObjectUnlock(vm);
> +return ret;
> +}
> +
> +static int
>  libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
>  virDomainObjPtr vm,
>  virTypedParameterPtr params,
> @@ -5407,6 +5459,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
>  #endif
>  .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
>  .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
> +.domainInterfaceStats = libxlDomainInterfaceStats, /* 1.2.20 */
>  .domainMemoryStats = libxlDomainMemoryStats, /* 1.2.20 */
>  .domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.20 */
>  .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 
> */
>   

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

Re: [libvirt] [PATCH] Makefile: fix build fail when make rpm

[lhuang]

On 09/23/2015 02:44 PM, Ján Tomko wrote:

On Wed, Sep 23, 2015 at 10:09:47AM +0800, Luyao Huang wrote:

Build fail and error like this:

   CC   qemu/libvirt_driver_qemu_impl_la-qemu_command.lo
qemu/qemu_capabilities.c:46:27: fatal error: qemu_capspriv.h: No such file or 
directory
  #include "qemu_capspriv.h"

Add qemu_capspriv.h to source.

Signed-off-by: Luyao Huang 
---
  src/Makefile.am | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)


ACK and pushed.

Thanks for catching that.


You are welcome, thanks your quick review.


Jan


Luyao

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

Re: [libvirt] [libvirt-users] New software based on libvirt

[Ján Tomko]

On Tue, Sep 22, 2015 at 09:41:21AM -0400, Cole Robinson wrote:
> On 09/22/2015 05:12 AM, Michal Privoznik wrote:
> > On 21.09.2015 21:28, Gustav Fransson Nyvell wrote:
> >> Hello,
> >>
> >> I'm introducing to you the decentralized cloud Cherrypop.
> >>
> >> Combining libvirt and LizardFS (as of now) it becomes a cloud completely
> >> without masters. Thus, any node is sufficient for the cloud to be up
> >> and therefore no wasted resources and no single point of failure.
> >>
> >> It's still pretty crude software but will work with some tinkering. Hope
> >> you try it and like it!
> >>
> >> For more information, source and binary:
> >> https://github.com/gustavfranssonnyvell/cherrypop
> >>
> >> //Gustav
> >>
> > 
> > Hey,
> > 
> > what an awesome news! We keep list of software based on libvirt:
> > 
> > http://libvirt.org/apps.html
> > 
> > Mind posting a patch against docs/apps.html.in? Or I can do that too, if
> > you don't want to bother.
> > 
> 
> At this point we should probably just make apps.html a wiki page...
> 

Which would not make editing it any easier, because registrations are
broken.

Jan

> - Cole
> 
> ___
> libvirt-users mailing list
> libvirt-us...@redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-users


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

Re: [libvirt] [PATCH] Makefile: fix build fail when make rpm

[Ján Tomko]

On Wed, Sep 23, 2015 at 10:09:47AM +0800, Luyao Huang wrote:
> Build fail and error like this:
> 
>   CC   qemu/libvirt_driver_qemu_impl_la-qemu_command.lo
> qemu/qemu_capabilities.c:46:27: fatal error: qemu_capspriv.h: No such file or 
> directory
>  #include "qemu_capspriv.h"
> 
> Add qemu_capspriv.h to source.
> 
> Signed-off-by: Luyao Huang 
> ---
>  src/Makefile.am | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

ACK and pushed.

Thanks for catching that.

Jan


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

Re: [libvirt] [libvirt-guests] New feature ALWAYS_START

[Martin Kletzander]

On Tue, Sep 22, 2015 at 07:33:30AM +0200, Marek Lukács wrote:

Hi,

It will be nice feature to have configuration option
ALWAYS_START="$uri:$name $uri:name ..." in libvirt-guests
configuration file.

If ON_BOOT is "start" and if ALWAYS_START is not empty, it iterates
over the ALWAYS_START and starts guests with same conditions (delays
etc.) before it starts guests from LISTFILE.



To be honest, I don't think that's _exactly_ what you want _just_ from
libvirt itself; let me explain.


Benefits:
- guests are started with delays


Delays that are done due to the guests are not something we should
handle.  Guests and mainly the applications inside them should handle
this gracefully.  Just delaying the starts is still error-prone.


- guests are started after host failure


That's what libvirt-guests does already.  And if you want some domains
to be started on every start, there's the 'autostart' parameter for
domains.


- guests are started in specific order (for example complex
environment, when DB should be started before other guest, etc.)



Again, same as the first point.  This should be handled gracefully in
the application itself or at least worked around in the guest (not
starting DB-related app before DB is accessible).

Anyway, if you *really* want this, then the easiest thing to do is
creating a service that starts before libvirt-guests, but after
libvirt, which is just something similar to "local", so it just runs a
script that does:

 for i in domain_one some_other_domain database_dom
 do
   virsh start "$i"
   sleep 60 # or you can try connecting to make sure it started
 done

or something similar.  However, you might still propose a simple patch
for the feature you described.


Regards,

Marek Lukács

--
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] [sandbox 03/11] Make virt-sandbox-image executable

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:33AM +0200, Cédric Bosdonnat wrote:
> ---
>  bin/virt-sandbox-image | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  mode change 100644 => 100755 bin/virt-sandbox-image
> 
> diff --git a/bin/virt-sandbox-image b/bin/virt-sandbox-image
> old mode 100644
> new mode 100755

ACK

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

[libvirt] [sandbox 02/11] virt-sandbox-image: fix error string formatting.

[Cédric Bosdonnat]

---
 libvirt-sandbox/image/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 776b71f..75daf72 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -80,7 +80,7 @@ def delete(args):
 source.delete_template(template=tmpl,
templatedir=args.template_dir)
 except Exception,e:
-print "Delete Error %s", str(e)
+print "Delete Error %s" % str(e)
 
 def create(args):
 try:
-- 
2.1.4

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

[libvirt] [sandbox 10/11] virt-sandbox-image: fix exception catching

[Cédric Bosdonnat]

Catch the exception in the main to avoid problems when auto-calling
another function that fails. For example if run calls create and that
one fails, run would have ignored the error.
---
 libvirt-sandbox/image/cli.py | 156 ---
 1 file changed, 73 insertions(+), 83 deletions(-)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index f4472d9..eb6bbb4 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -66,98 +66,85 @@ def info(msg):
 sys.stdout.write(msg)
 
 def download(args):
-try:
-tmpl = template.Template.from_uri(args.template)
-source = tmpl.get_source_impl()
-source.download_template(template=tmpl,
- templatedir=args.template_dir)
-except Exception,e:
-print "Download Error %s" % str(e)
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+source.download_template(template=tmpl,
+ templatedir=args.template_dir)
 
 def delete(args):
-try:
-tmpl = template.Template.from_uri(args.template)
-source = tmpl.get_source_impl()
-source.delete_template(template=tmpl,
-   templatedir=args.template_dir)
-except Exception,e:
-print "Delete Error %s" % str(e)
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+source.delete_template(template=tmpl,
+   templatedir=args.template_dir)
 
 def create(args):
-try:
-tmpl = template.Template.from_uri(args.template)
-source = tmpl.get_source_impl()
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
 
-if not source.was_downloaded(tmpl, args.template_dir):
-download(args)
+if not source.was_downloaded(tmpl, args.template_dir):
+download(args)
 
-fmt = default_format
-if "format" in vars(args):
-fmt = args.format
+fmt = default_format
+if "format" in vars(args):
+fmt = args.format
 
-source.create_template(template=tmpl,
-   templatedir=args.template_dir,
-   connect=args.connect,
-   format=fmt)
-except Exception,e:
-print "Create Error %s" % str(e)
+source.create_template(template=tmpl,
+   templatedir=args.template_dir,
+   connect=args.connect,
+   format=fmt)
 
 def run(args):
-try:
-if args.connect is not None:
-check_connect(args.connect)
-tmpl = template.Template.from_uri(args.template)
-source = tmpl.get_source_impl()
-
-# Create the template image if needed
-if not source.has_template(tmpl, args.template_dir):
-create(args)
-
-name = args.name
-if name is None:
-randomid = ''.join(random.choice(string.lowercase) for i in 
range(10))
-name = tmpl.path[1:] + ":" + randomid
-
-diskfile = source.get_disk(template=tmpl,
-   templatedir=args.template_dir,
-   imagedir=args.image_dir,
-   sandboxname=name)
-
-format = "qcow2"
-commandToRun = source.get_command(tmpl, args.template_dir, args.args)
-if len(commandToRun) == 0:
-commandToRun = ["/bin/sh"]
-cmd = ['virt-sandbox', '--name', name]
-if args.connect is not None:
-cmd.append("-c")
-cmd.append(args.connect)
-params = ['-m','host-image:/=%s,format=%s' %(diskfile,format)]
-
-networkArgs = args.network
-if networkArgs is not None:
-params.append('-N')
-params.append(networkArgs)
-
-allEnvs = source.get_env(tmpl, args.template_dir)
-envArgs = args.env
-if envArgs is not None:
-allEnvs = allEnvs + envArgs
-for env in allEnvs:
-envsplit = env.split("=")
-envlen = len(envsplit)
-if envlen == 2:
-params.append("--env")
-params.append(env)
-else:
-pass
-
-cmd = cmd + params + ['--'] + commandToRun
-subprocess.call(cmd)
-os.unlink(diskfile)
-source.post_run(tmpl, args.template_dir, name)
-
-except Exception,e:
-print "Run Error %s" % str(e)
+if args.connect is not None:
+check_connect(args.connect)
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+
+# Create the template image if needed
+if not source.has_template(tmpl, args.template_dir):
+create(args)
+
+name = args.name
+if name is None:
+randomid = ''.join(random.choice(string.lowercase) for i in 

[libvirt] [sandbox 07/11] virt-sandbox-image: smarter source name computing

[Cédric Bosdonnat]

To compute the source class name from the URI scheme, we need to be a
bit smarter to have a nice scheme for sources like the virt-builder one.
In order to have a scheme like virt-builder:// and a class name like
VirtBuilderSource, strip the non-word characters from the scheme and
camel case the words.
---
 libvirt-sandbox/image/template.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libvirt-sandbox/image/template.py 
b/libvirt-sandbox/image/template.py
index 17da6c0..4713b0a 100644
--- a/libvirt-sandbox/image/template.py
+++ b/libvirt-sandbox/image/template.py
@@ -21,6 +21,7 @@
 
 import urlparse
 import importlib
+import re
 
 class Template(object):
 
@@ -56,10 +57,13 @@ class Template(object):
 self.params = {}
 
 def get_source_impl(self):
+p = re.compile("\W")
+sourcename = "".join([i.capitalize() for i in p.split(self.source)])
+
 mod = importlib.import_module(
 "libvirt_sandbox.image.sources." +
-self.source.capitalize() + "Source")
-classname = self.source.capitalize() + "Source"
+sourcename + "Source")
+classname = sourcename + "Source"
 classimpl = getattr(mod, classname)
 return classimpl()
 
-- 
2.1.4

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

[libvirt] [sandbox 05/11] virt-sandbox-image: add a source post_run hook

[Cédric Bosdonnat]

This hook in the source allows additional cleanup to take place once
the run command is terminated. This is not used by the docker source,
but will be used by the virt-builder one
---
 libvirt-sandbox/image/cli.py|  1 +
 libvirt-sandbox/image/sources/Source.py | 12 
 2 files changed, 13 insertions(+)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 75daf72..e991e2d 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -142,6 +142,7 @@ def run(args):
 cmd = cmd + params + ['--'] + commandToRun
 subprocess.call(cmd)
 os.unlink(diskfile)
+source.post_run(tmpl, args.template_dir, name)
 
 except Exception,e:
 print "Run Error %s" % str(e)
diff --git a/libvirt-sandbox/image/sources/Source.py 
b/libvirt-sandbox/image/sources/Source.py
index 33feb7c..20f4af0 100644
--- a/libvirt-sandbox/image/sources/Source.py
+++ b/libvirt-sandbox/image/sources/Source.py
@@ -102,3 +102,15 @@ class Source():
 Get the dict of environment variables to set
 """
 pass
+
+def post_run(self, template, templatedir, imagename):
+"""
+:param template: libvirt_sandbox.template.Template object
+:param templatedir: local directory path in which to find template
+:param imagename: name of the image that just stopped running
+
+Hook called after the image has been stopped. By default is doesn't
+do anything, subclasses can override this to do some additional
+cleanup.
+"""
+pass
-- 
2.1.4

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

[libvirt] [sandbox 04/11] virt-sandbox-image: remove undefined default_disk_format

[Cédric Bosdonnat]

---
 libvirt-sandbox/image/sources/DockerSource.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
b/libvirt-sandbox/image/sources/DockerSource.py
index ab0b765..6c02cc3 100644
--- a/libvirt-sandbox/image/sources/DockerSource.py
+++ b/libvirt-sandbox/image/sources/DockerSource.py
@@ -243,7 +243,7 @@ class DockerSource(Source):
 
 def create_template(self, template, templatedir, connect=None, 
format=None):
 if format is None:
-format = self.default_disk_format
+format = "qcow2"
 self._check_disk_format(format)
 imagelist = self._get_image_list(template, templatedir)
 imagelist.reverse()
-- 
2.1.4

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

[libvirt] [sandbox 06/11] virt-sandbox-image: move DockerSource _format_disk to Source

[Cédric Bosdonnat]

Formatting a disk is a generic operation that will be needed by other
sources, at least a virt-builder one.
---
 libvirt-sandbox/image/sources/DockerSource.py | 14 +-
 libvirt-sandbox/image/sources/Source.py   | 16 
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
b/libvirt-sandbox/image/sources/DockerSource.py
index 6c02cc3..41df7a7 100644
--- a/libvirt-sandbox/image/sources/DockerSource.py
+++ b/libvirt-sandbox/image/sources/DockerSource.py
@@ -261,7 +261,7 @@ class DockerSource(Source):
 subprocess.call(cmd)
 
 if parentImage is None:
-self._format_disk(templateImage,format,connect)
+self.format_disk(templateImage,format,connect)
 
 self._extract_tarballs(templatedir + "/" + imagetagid + 
"/template.",format,connect)
 parentImage = templateImage
@@ -299,18 +299,6 @@ class DockerSource(Source):
 imagetagid = parent
 return imagelist
 
-def _format_disk(self,disk,format,connect):
-cmd = ['virt-sandbox']
-if connect is not None:
-cmd.append("-c")
-cmd.append(connect)
-cmd.append("-p")
-params = ['--disk=file:disk_image=%s,format=%s' %(disk,format),
-  '/sbin/mkfs.ext3',
-  '/dev/disk/by-tag/disk_image']
-cmd = cmd + params
-subprocess.call(cmd)
-
 def _extract_tarballs(self,directory,format,connect):
 tarfile = directory + "tar.gz"
 diskfile = directory + "qcow2"
diff --git a/libvirt-sandbox/image/sources/Source.py 
b/libvirt-sandbox/image/sources/Source.py
index 20f4af0..444baa3 100644
--- a/libvirt-sandbox/image/sources/Source.py
+++ b/libvirt-sandbox/image/sources/Source.py
@@ -21,6 +21,7 @@
 # Author: Eren Yagdiran 
 
 from abc import ABCMeta, abstractmethod
+import subprocess
 
 class Source():
 '''The Source class defines the base interface for
@@ -114,3 +115,18 @@ class Source():
 cleanup.
 """
 pass
+
+
+# Utility functions to share between the sources.
+
+def format_disk(self,disk,format,connect):
+cmd = ['virt-sandbox']
+if connect is not None:
+cmd.append("-c")
+cmd.append(connect)
+cmd.append("-p")
+params = ['--disk=file:disk_image=%s,format=%s' %(disk,format),
+  '/sbin/mkfs.ext3',
+  '/dev/disk/by-tag/disk_image']
+cmd = cmd + params
+subprocess.call(cmd)
-- 
2.1.4

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

Re: [libvirt] [sandbox 07/11] virt-sandbox-image: smarter source name computing

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:37AM +0200, Cédric Bosdonnat wrote:
> To compute the source class name from the URI scheme, we need to be a
> bit smarter to have a nice scheme for sources like the virt-builder one.
> In order to have a scheme like virt-builder:// and a class name like
> VirtBuilderSource, strip the non-word characters from the scheme and
> camel case the words.
> ---
>  libvirt-sandbox/image/template.py | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

ACK


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] [sandbox 09/11] virt-sandbox-image: automatically call download and create if needed

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:39AM +0200, Cédric Bosdonnat wrote:
> To provide a smooth user experience, run automatically calls create if
> needed and create automatically calls download if needed.
> ---
>  libvirt-sandbox/image/cli.py   | 18 +++---
>  libvirt-sandbox/image/sources/DockerSource.py  | 16 
>  libvirt-sandbox/image/sources/Source.py| 22 
> ++
>  libvirt-sandbox/image/sources/VirtBuilderSource.py |  7 +++
>  4 files changed, 60 insertions(+), 3 deletions(-)

This makes sense, though see my previous suggestion about us merging
downkoad+create into a single prepare step, which would simplify this
even more.

> diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
> index fb1104a..f4472d9 100755
> --- a/libvirt-sandbox/image/cli.py
> +++ b/libvirt-sandbox/image/cli.py
> @@ -42,6 +42,7 @@ if os.geteuid() == 0:
>  else:
>  default_template_dir = os.environ['HOME'] + 
> "/.local/share/libvirt/templates"
>  default_image_dir = os.environ['HOME'] + "/.local/share/libvirt/images"
> +default_format = "qcow2"
>  
>  debug = False
>  verbose = False
> @@ -86,10 +87,18 @@ def create(args):
>  try:
>  tmpl = template.Template.from_uri(args.template)
>  source = tmpl.get_source_impl()
> +
> +if not source.was_downloaded(tmpl, args.template_dir):
> +download(args)
> +
> +fmt = default_format
> +if "format" in vars(args):
> +fmt = args.format

Lets just kill format from the arguments for now and hardcode
qcow2. We can re-think it later when we actually want to
consider using different storage backends.

> +
>  source.create_template(template=tmpl,
> templatedir=args.template_dir,
> connect=args.connect,
> -   format=args.format)
> +   format=fmt)
>  except Exception,e:
>  print "Create Error %s" % str(e)
>  
> @@ -97,10 +106,13 @@ def run(args):
>  try:
>  if args.connect is not None:
>  check_connect(args.connect)
> -
>  tmpl = template.Template.from_uri(args.template)
>  source = tmpl.get_source_impl()
>  
> +# Create the template image if needed
> +if not source.has_template(tmpl, args.template_dir):
> +create(args)
> +
>  name = args.name
>  if name is None:
>  randomid = ''.join(random.choice(string.lowercase) for i in 
> range(10))
> @@ -213,7 +225,7 @@ def gen_create_args(subparser):
>  requires_connect(parser)
>  requires_template_dir(parser)
>  parser.add_argument("-f","--format",
> -default="qcow2",
> +default=default_format,
>  help=_("format format for image"))
>  parser.set_defaults(func=create)
>  
> diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
> b/libvirt-sandbox/image/sources/DockerSource.py
> index 41df7a7..be9063d 100644
> --- a/libvirt-sandbox/image/sources/DockerSource.py
> +++ b/libvirt-sandbox/image/sources/DockerSource.py
> @@ -59,6 +59,22 @@ class DockerSource(Source):
>  if  (major == 2 and sys.hexversion < py2_7_9_hexversion) or (major 
> == 3 and sys.hexversion < py3_4_3_hexversion):
>  sys.stderr.write(SSL_WARNING)
>  
> +def was_downloaded(self, template, templatedir):
> +try:
> +self._get_image_list(template, templatedir)
> +return True
> +except Exception:
> +return False
> +
> +
> +def has_template(self, template, templatedir):
> +try:
> +configfile, diskfile = self._get_template_data(template, 
> templatedir)
> +return os.path.exists(diskfile)
> +except Exception:
> +return False
> +
> +
>  def download_template(self, template, templatedir):
>  self._check_cert_validate()
>  
> diff --git a/libvirt-sandbox/image/sources/Source.py 
> b/libvirt-sandbox/image/sources/Source.py
> index 444baa3..e647448 100644
> --- a/libvirt-sandbox/image/sources/Source.py
> +++ b/libvirt-sandbox/image/sources/Source.py
> @@ -35,6 +35,28 @@ class Source():
>  pass
>  
>  @abstractmethod
> +def was_downloaded(self, template, templatedir):
> +"""
> +:param template: libvirt_sandbox.template.Template object
> +:param templatedir: local directory path in which to store the 
> template
> +
> +Check if a template has already been downloaded.
> +"""
> +pass
> +
> +
> +@abstractmethod
> +def has_template(self, template, templatedir):
> +"""
> +:param template: libvirt_sandbox.template.Template object
> +:param templatedir: local directory path in which to store the 
> template
> +
> +Check if a template has already been created.
> +"""
> +   

Re: [libvirt] [sandbox 11/11] virt-sandbox-image: add error handling for sources import

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:41AM +0200, Cédric Bosdonnat wrote:
> Rather than letting the python exception hit the user, catch them and
> provide a more meaningful message if no or a bad scheme is provided in
> the URI.
> ---
>  libvirt-sandbox/image/template.py | 24 +++-
>  1 file changed, 15 insertions(+), 9 deletions(-)

ACK


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

[libvirt] [sandbox 09/11] virt-sandbox-image: automatically call download and create if needed

[Cédric Bosdonnat]

To provide a smooth user experience, run automatically calls create if
needed and create automatically calls download if needed.
---
 libvirt-sandbox/image/cli.py   | 18 +++---
 libvirt-sandbox/image/sources/DockerSource.py  | 16 
 libvirt-sandbox/image/sources/Source.py| 22 ++
 libvirt-sandbox/image/sources/VirtBuilderSource.py |  7 +++
 4 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index fb1104a..f4472d9 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -42,6 +42,7 @@ if os.geteuid() == 0:
 else:
 default_template_dir = os.environ['HOME'] + 
"/.local/share/libvirt/templates"
 default_image_dir = os.environ['HOME'] + "/.local/share/libvirt/images"
+default_format = "qcow2"
 
 debug = False
 verbose = False
@@ -86,10 +87,18 @@ def create(args):
 try:
 tmpl = template.Template.from_uri(args.template)
 source = tmpl.get_source_impl()
+
+if not source.was_downloaded(tmpl, args.template_dir):
+download(args)
+
+fmt = default_format
+if "format" in vars(args):
+fmt = args.format
+
 source.create_template(template=tmpl,
templatedir=args.template_dir,
connect=args.connect,
-   format=args.format)
+   format=fmt)
 except Exception,e:
 print "Create Error %s" % str(e)
 
@@ -97,10 +106,13 @@ def run(args):
 try:
 if args.connect is not None:
 check_connect(args.connect)
-
 tmpl = template.Template.from_uri(args.template)
 source = tmpl.get_source_impl()
 
+# Create the template image if needed
+if not source.has_template(tmpl, args.template_dir):
+create(args)
+
 name = args.name
 if name is None:
 randomid = ''.join(random.choice(string.lowercase) for i in 
range(10))
@@ -213,7 +225,7 @@ def gen_create_args(subparser):
 requires_connect(parser)
 requires_template_dir(parser)
 parser.add_argument("-f","--format",
-default="qcow2",
+default=default_format,
 help=_("format format for image"))
 parser.set_defaults(func=create)
 
diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
b/libvirt-sandbox/image/sources/DockerSource.py
index 41df7a7..be9063d 100644
--- a/libvirt-sandbox/image/sources/DockerSource.py
+++ b/libvirt-sandbox/image/sources/DockerSource.py
@@ -59,6 +59,22 @@ class DockerSource(Source):
 if  (major == 2 and sys.hexversion < py2_7_9_hexversion) or (major == 
3 and sys.hexversion < py3_4_3_hexversion):
 sys.stderr.write(SSL_WARNING)
 
+def was_downloaded(self, template, templatedir):
+try:
+self._get_image_list(template, templatedir)
+return True
+except Exception:
+return False
+
+
+def has_template(self, template, templatedir):
+try:
+configfile, diskfile = self._get_template_data(template, 
templatedir)
+return os.path.exists(diskfile)
+except Exception:
+return False
+
+
 def download_template(self, template, templatedir):
 self._check_cert_validate()
 
diff --git a/libvirt-sandbox/image/sources/Source.py 
b/libvirt-sandbox/image/sources/Source.py
index 444baa3..e647448 100644
--- a/libvirt-sandbox/image/sources/Source.py
+++ b/libvirt-sandbox/image/sources/Source.py
@@ -35,6 +35,28 @@ class Source():
 pass
 
 @abstractmethod
+def was_downloaded(self, template, templatedir):
+"""
+:param template: libvirt_sandbox.template.Template object
+:param templatedir: local directory path in which to store the template
+
+Check if a template has already been downloaded.
+"""
+pass
+
+
+@abstractmethod
+def has_template(self, template, templatedir):
+"""
+:param template: libvirt_sandbox.template.Template object
+:param templatedir: local directory path in which to store the template
+
+Check if a template has already been created.
+"""
+pass
+
+
+@abstractmethod
 def download_template(self, template, templatedir):
 """
 :param template: libvirt_sandbox.template.Template object
diff --git a/libvirt-sandbox/image/sources/VirtBuilderSource.py 
b/libvirt-sandbox/image/sources/VirtBuilderSource.py
index 4a7e383..2dde715 100644
--- a/libvirt-sandbox/image/sources/VirtBuilderSource.py
+++ b/libvirt-sandbox/image/sources/VirtBuilderSource.py
@@ -31,6 +31,13 @@ class VirtBuilderSource(Source):
 # nobody can try to alter the folders structure later.
 return template.path[1:].replace('/', '_')
 
+def was_downloaded(self, 

[libvirt] [sandbox 08/11] virt-sandbox-image: add a virt-builder source

[Cédric Bosdonnat]

Allow virt-sandbox-image to pull templates from virt-builder and run
sandboxes on top of them.
---
 libvirt-sandbox.spec.in|   1 +
 libvirt-sandbox/image/cli.py   |   1 +
 libvirt-sandbox/image/sources/Makefile.am  |   1 +
 libvirt-sandbox/image/sources/VirtBuilderSource.py | 136 +
 libvirt-sandbox/image/template.py  |   2 +
 5 files changed, 141 insertions(+)
 create mode 100644 libvirt-sandbox/image/sources/VirtBuilderSource.py

diff --git a/libvirt-sandbox.spec.in b/libvirt-sandbox.spec.in
index 54fde55..f84eabc 100644
--- a/libvirt-sandbox.spec.in
+++ b/libvirt-sandbox.spec.in
@@ -36,6 +36,7 @@ Requires: systemd >= 198
 Requires: pygobject3-base
 Requires: libselinux-python
 Requires: %{name}-libs = %{version}-%{release}
+Requires: %{_bindir}/virt-builder
 
 %package libs
 Group: Development/Libraries
diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index e991e2d..fb1104a 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -188,6 +188,7 @@ Example supported URI formats:
   docker:///ubuntu?tag=15.04
   docker://username:passw...@index.docker.io/private/image
   docker://registry.access.redhat.com/rhel6
+  virt-builder:///fedora-20
 """)
 return parser
 
diff --git a/libvirt-sandbox/image/sources/Makefile.am 
b/libvirt-sandbox/image/sources/Makefile.am
index 069557d..52e9a7e 100644
--- a/libvirt-sandbox/image/sources/Makefile.am
+++ b/libvirt-sandbox/image/sources/Makefile.am
@@ -4,6 +4,7 @@ pythonimage_DATA = \
__init__.py \
Source.py \
DockerSource.py \
+   VirtBuilderSource.py \
$(NULL)
 
 EXTRA_DIST = $(pythonimage_DATA)
diff --git a/libvirt-sandbox/image/sources/VirtBuilderSource.py 
b/libvirt-sandbox/image/sources/VirtBuilderSource.py
new file mode 100644
index 000..4a7e383
--- /dev/null
+++ b/libvirt-sandbox/image/sources/VirtBuilderSource.py
@@ -0,0 +1,136 @@
+#!/usr/bin/python
+#
+# Copyright (C) 2015 SUSE LLC
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  
USA
+#
+# Author: Cedric Bosdonnat 
+#
+
+from Source import Source
+import os
+import os.path
+import subprocess
+
+class VirtBuilderSource(Source):
+
+def _get_template_name(self, template):
+# We shouldn't have '/' in the names, but let's make sure
+# nobody can try to alter the folders structure later.
+return template.path[1:].replace('/', '_')
+
+def download_template(self, template, templatedir):
+# We don't do anything here: virt-builder will do it for us in
+# the create action
+pass
+
+def _check_disk_format(self,format):
+supportedFormats = ['qcow2', 'raw']
+if not format in supportedFormats:
+raise ValueError(["Unsupported image format %s" % format])
+
+def create_template(self, template, templatedir,
+connect=None, format=None):
+# FIXME Force qcow2 format: do we really want people to mess with that?
+if not os.path.exists(templatedir):
+os.makedirs(templatedir)
+
+# Get the image using virt-builder
+templatename = self._get_template_name(template)
+imagepath_original = "%s/%s-original.qcow2" % (templatedir, 
templatename)
+imagepath = "%s/%s.%s" % (templatedir, templatename, format)
+cmd = ["virt-builder", templatename,
+   "-o", imagepath_original, "--format", "qcow2"]
+subprocess.call(cmd)
+
+# We need to convert this image into a single partition one.
+tarfile = "%s/%s.tar" % (templatedir, templatename)
+cmd = ["virt-tar-out", "-a", imagepath_original, "/", tarfile]
+subprocess.call(cmd)
+
+os.unlink(imagepath_original)
+
+cmd = ["qemu-img", "create", "-q", "-f", format, imagepath, "10G"]
+subprocess.call(cmd)
+
+self.format_disk(imagepath, format, connect)
+self._extract_tarball(imagepath, format, tarfile, connect)
+os.unlink(tarfile)
+
+
+def delete_template(self, template, templatedir):
+# Check for running sandboxes using this template before killing it
+images = self._get_template_uses(template, templatedir)
+if len(images) != 

[libvirt] [sandbox 11/11] virt-sandbox-image: add error handling for sources import

[Cédric Bosdonnat]

Rather than letting the python exception hit the user, catch them and
provide a more meaningful message if no or a bad scheme is provided in
the URI.
---
 libvirt-sandbox/image/template.py | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/libvirt-sandbox/image/template.py 
b/libvirt-sandbox/image/template.py
index efdd845..58904a2 100644
--- a/libvirt-sandbox/image/template.py
+++ b/libvirt-sandbox/image/template.py
@@ -59,15 +59,21 @@ class Template(object):
 self.params = {}
 
 def get_source_impl(self):
-p = re.compile("\W")
-sourcename = "".join([i.capitalize() for i in p.split(self.source)])
-
-mod = importlib.import_module(
-"libvirt_sandbox.image.sources." +
-sourcename + "Source")
-classname = sourcename + "Source"
-classimpl = getattr(mod, classname)
-return classimpl()
+if self.source == "":
+raise Exception("Missing scheme in image URI")
+
+try:
+p = re.compile("\W")
+sourcename = "".join([i.capitalize() for i in 
p.split(self.source)])
+
+mod = importlib.import_module(
+"libvirt_sandbox.image.sources." +
+sourcename + "Source")
+classname = sourcename + "Source"
+classimpl = getattr(mod, classname)
+return classimpl()
+except Exception:
+raise Exception("Invalid source: '%s'" % self.source)
 
 def __repr__(self):
 if self.protocol is not None:
-- 
2.1.4

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

[libvirt] [sandbox 03/11] Make virt-sandbox-image executable

[Cédric Bosdonnat]

---
 bin/virt-sandbox-image | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 bin/virt-sandbox-image

diff --git a/bin/virt-sandbox-image b/bin/virt-sandbox-image
old mode 100644
new mode 100755
-- 
2.1.4

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

[libvirt] [sandbox 00/11] Adding a virt-builder source

[Cédric Bosdonnat]

Hi all,

Here are a few patches to add a virt-builder source to virt-sandbox-image. There
are also patches fixing a few issues here and there.

Cédric Bosdonnat (11):
  Fix memory leak
  virt-sandbox-image: fix error string formatting.
  Make virt-sandbox-image executable
  virt-sandbox-image: remove undefined default_disk_format
  virt-sandbox-image: add a source post_run hook
  virt-sandbox-image: move DockerSource _format_disk to Source
  virt-sandbox-image: smarter source name computing
  virt-sandbox-image: add a virt-builder source
  virt-sandbox-image: automatically call download and create if needed
  virt-sandbox-image: fix exception catching
  virt-sandbox-image: add error handling for sources import

 bin/virt-sandbox-image |   0
 libvirt-sandbox.spec.in|   1 +
 libvirt-sandbox/image/cli.py   | 154 +++--
 libvirt-sandbox/image/sources/DockerSource.py  |  32 +++--
 libvirt-sandbox/image/sources/Makefile.am  |   1 +
 libvirt-sandbox/image/sources/Source.py|  50 +++
 libvirt-sandbox/image/sources/VirtBuilderSource.py | 143 +++
 libvirt-sandbox/image/template.py  |  24 +++-
 libvirt-sandbox/libvirt-sandbox-builder.c  |   1 +
 9 files changed, 311 insertions(+), 95 deletions(-)
 mode change 100644 => 100755 bin/virt-sandbox-image
 create mode 100644 libvirt-sandbox/image/sources/VirtBuilderSource.py

-- 
2.1.4

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

[libvirt] [sandbox 01/11] Fix memory leak

[Cédric Bosdonnat]

---
 libvirt-sandbox/libvirt-sandbox-builder.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c 
b/libvirt-sandbox/libvirt-sandbox-builder.c
index ea7d064..cbfa780 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder.c
@@ -759,6 +759,7 @@ gboolean 
gvir_sandbox_builder_clean_post_stop(GVirSandboxBuilder *builder,
 g_object_unref(enumerator);
 g_object_unref(libsFile);
 g_free(libsdir);
+g_free(dskfile);
 return ret;
 }
 
-- 
2.1.4

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

[libvirt] [PATCH] qemuDomainDeviceDefPostParse: Drop useless check

[Michal Privoznik]

Now that virQEMUDriverCreateXMLConf is never called with NULL
(after 086f37e97aab) we can safely drop useless check in
qemuDomainDeviceDefPostParse as we are guaranteed to be always
called with the driver initialized. Therefore checking if driver
is NULL makes no sense. Moreover, if we mix it with direct driver
dereference.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_domain.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7d92f3a..8e3a7d3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1233,12 +1233,9 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 {
 virQEMUDriverPtr driver = opaque;
 virQEMUCapsPtr qemuCaps = NULL;
-virQEMUDriverConfigPtr cfg = NULL;
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 int ret = -1;
 
-if (driver)
-cfg = virQEMUDriverGetConfig(driver);
-
 qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
 
 if (dev->type == VIR_DOMAIN_DEVICE_NET &&
-- 
2.4.9

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

Re: [libvirt] [sandbox 01/11] Fix memory leak

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:31AM +0200, Cédric Bosdonnat wrote:
> ---
>  libvirt-sandbox/libvirt-sandbox-builder.c | 1 +
>  1 file changed, 1 insertion(+)

ACK


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] [sandbox 05/11] virt-sandbox-image: add a source post_run hook

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:35AM +0200, Cédric Bosdonnat wrote:
> This hook in the source allows additional cleanup to take place once
> the run command is terminated. This is not used by the docker source,
> but will be used by the virt-builder one
> ---
>  libvirt-sandbox/image/cli.py|  1 +
>  libvirt-sandbox/image/sources/Source.py | 12 
>  2 files changed, 13 insertions(+)

ACK


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] [sandbox 02/11] virt-sandbox-image: fix error string formatting.

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:32AM +0200, Cédric Bosdonnat wrote:
> ---
>  libvirt-sandbox/image/cli.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

ACK


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] [sandbox 06/11] virt-sandbox-image: move DockerSource _format_disk to Source

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:36AM +0200, Cédric Bosdonnat wrote:
> Formatting a disk is a generic operation that will be needed by other
> sources, at least a virt-builder one.
> ---
>  libvirt-sandbox/image/sources/DockerSource.py | 14 +-
>  libvirt-sandbox/image/sources/Source.py   | 16 
>  2 files changed, 17 insertions(+), 13 deletions(-)

ACK

> diff --git a/libvirt-sandbox/image/sources/Source.py 
> b/libvirt-sandbox/image/sources/Source.py
> index 20f4af0..444baa3 100644
> --- a/libvirt-sandbox/image/sources/Source.py
> +++ b/libvirt-sandbox/image/sources/Source.py
> @@ -21,6 +21,7 @@
>  # Author: Eren Yagdiran 
>  
>  from abc import ABCMeta, abstractmethod
> +import subprocess
>  
>  class Source():
>  '''The Source class defines the base interface for
> @@ -114,3 +115,18 @@ class Source():
>  cleanup.
>  """
>  pass
> +
> +
> +# Utility functions to share between the sources.
> +
> +def format_disk(self,disk,format,connect):
> +cmd = ['virt-sandbox']
> +if connect is not None:
> +cmd.append("-c")
> +cmd.append(connect)
> +cmd.append("-p")
> +params = ['--disk=file:disk_image=%s,format=%s' %(disk,format),
> +  '/sbin/mkfs.ext3',

This reminds me we should switch to mkfs.ext4 to be consistent
with our previous switch to ext4 mount by default.

> +  '/dev/disk/by-tag/disk_image']
> +cmd = cmd + params
> +subprocess.call(cmd)

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] [sandbox 08/11] virt-sandbox-image: add a virt-builder source

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:38AM +0200, Cédric Bosdonnat wrote:
> Allow virt-sandbox-image to pull templates from virt-builder and run
> sandboxes on top of them.
> ---
>  libvirt-sandbox.spec.in|   1 +
>  libvirt-sandbox/image/cli.py   |   1 +
>  libvirt-sandbox/image/sources/Makefile.am  |   1 +
>  libvirt-sandbox/image/sources/VirtBuilderSource.py | 136 
> +
>  libvirt-sandbox/image/template.py  |   2 +
>  5 files changed, 141 insertions(+)
>  create mode 100644 libvirt-sandbox/image/sources/VirtBuilderSource.py

> +class VirtBuilderSource(Source):
> +
> +def _get_template_name(self, template):
> +# We shouldn't have '/' in the names, but let's make sure
> +# nobody can try to alter the folders structure later.
> +return template.path[1:].replace('/', '_')
> +
> +def download_template(self, template, templatedir):
> +# We don't do anything here: virt-builder will do it for us in
> +# the create action
> +pass

I guess I could see us invoking virt-builder in the download step to
pull down the image and format it to qcow2.

> +
> +def _check_disk_format(self,format):
> +supportedFormats = ['qcow2', 'raw']
> +if not format in supportedFormats:
> +raise ValueError(["Unsupported image format %s" % format])
> +
> +def create_template(self, template, templatedir,
> +connect=None, format=None):
> +# FIXME Force qcow2 format: do we really want people to mess with 
> that?
> +if not os.path.exists(templatedir):
> +os.makedirs(templatedir)
> +
> +# Get the image using virt-builder
> +templatename = self._get_template_name(template)
> +imagepath_original = "%s/%s-original.qcow2" % (templatedir, 
> templatename)
> +imagepath = "%s/%s.%s" % (templatedir, templatename, format)
> +cmd = ["virt-builder", templatename,
> +   "-o", imagepath_original, "--format", "qcow2"]
> +subprocess.call(cmd)

eg, this bit could live in download, and then the following bit stay
in create. Honestly the distinction is kind of blury though. I wonder
if this is a sign that we should kill off the distinction between
download + create. The key idea behind having the separate commands
is that users may want to "prime the cache" for images they expect
to need to run in the future, so saving time later. This doesn't
require us to have download+create separate though - we could just
as easily have a single "prepare" command that combines them both.



> +
> +# We need to convert this image into a single partition one.
> +tarfile = "%s/%s.tar" % (templatedir, templatename)
> +cmd = ["virt-tar-out", "-a", imagepath_original, "/", tarfile]
> +subprocess.call(cmd)
> +
> +os.unlink(imagepath_original)
> +
> +cmd = ["qemu-img", "create", "-q", "-f", format, imagepath, "10G"]
> +subprocess.call(cmd)
> +
> +self.format_disk(imagepath, format, connect)
> +self._extract_tarball(imagepath, format, tarfile, connect)
> +os.unlink(tarfile)
> +
> +
> +def delete_template(self, template, templatedir):
> +# Check for running sandboxes using this template before killing it
> +images = self._get_template_uses(template, templatedir)
> +if len(images) != 0:
> +imagesStr = ", ".join(images)
> +raise ValueError(["Images still using the template: %s" % 
> imagesStr])
> +
> +os.unlink("%s/%s.qcow2" % (templatedir, 
> self._get_template_name(template)))
> +os.unlink("%s/%s.uses" % (templatedir, 
> self._get_template_name(template)))
> +
> +def _get_template_uses(self, template, templatedir):
> +uses = open("%s/%s.uses" % (templatedir, 
> self._get_template_name(template)), "r")
> +lines = uses.read()
> +uses.close()
> +return [l for l in lines.split("\n") if l != ""]
> +
> +def post_run(self, template, templatedir, imagename):
> +images = self._get_template_uses(template, templatedir)
> +images.remove(imagename)
> +
> +uses = open("%s/%s.uses" % (templatedir, 
> self._get_template_name(template)), "w")
> +uses.write("\n".join(images))
> +uses.close()
> +
> +def get_command(self, template, templatedir, userargs):
> +return userargs
> +
> +def get_disk(self,template, templatedir, imagedir, sandboxname):
> +diskfile = "%s/%s.qcow2" % (templatedir, 
> self._get_template_name(template))
> +tempfile = imagedir + "/" + sandboxname + ".qcow2"
> +if not os.path.exists(imagedir):
> +os.makedirs(imagedir)
> +cmd = ["qemu-img", "create", "-q",
> +   "-f", "qcow2",
> +   "-o", "backing_fmt=qcow2,backing_file=%s" % diskfile,
> +   tempfile]
> +subprocess.call(cmd)
> 

Re: [libvirt] [sandbox 10/11] virt-sandbox-image: fix exception catching

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 09:53:40AM +0200, Cédric Bosdonnat wrote:
> Catch the exception in the main to avoid problems when auto-calling
> another function that fails. For example if run calls create and that
> one fails, run would have ignored the error.
> ---
>  libvirt-sandbox/image/cli.py | 156 
> ---
>  1 file changed, 73 insertions(+), 83 deletions(-)

ACK, this is much simpler and avoids repetition.


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 v2] qemuDomainDeviceDefPostParse: Drop useless checks

[John Ferlan]

On 09/23/2015 07:19 AM, Michal Privoznik wrote:
> Now that virQEMUDriverCreateXMLConf is never called with NULL
> (after 086f37e97aab) we can safely drop useless check in
> qemuDomainDeviceDefPostParse as we are guaranteed to be always
> called with the driver initialized. Therefore checking if driver
> is NULL makes no sense. Moreover, if we mix it with direct driver
> dereference. And after that, we are sure that nor @cfg will be
> NULL, therefore we can drop checks for that too.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> diff to v1:
> - drop checks for !cfg too
> 
>  src/qemu/qemu_domain.c | 64 
> --
>  1 file changed, 26 insertions(+), 38 deletions(-)
> 

ACK

John

Coverity error goes away too.

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

Re: [libvirt] [PATCH RFC 4/7] libxl: implement virDomainBlockStats

[Jim Fehlig]

Joao Martins wrote:
> Introduce initial support for domainBlockStats API call that
> allow us to query block device statistics. openstack nova
> uses this API call to query block statistics, alongside
> virDomainMemoryStats and virDomainInterfaceStats.  Note that
> this patch only introduces it for VBD for starters. QDisk
> will come in a separate patch series.
>
> A new statistics data structure is introduced to fit common
> statistics among others specific to the underlying block
> backends. For the VBD statistics on linux these are exported
> via sysfs on the path:
>
> "/sys/bus/xen-backend/devices/vbd--/statistics"
>
> To calculate the block devid two libxl functions were ported
> (libxlDiskPathMatches and libxlDiskPathParse) to libvirt to
> make sure the devid is calculate in the same way as libxl.
> Each backend implements its own function to extract statistics
> and let there be handled the different platforms. An alternative
> would be to reuse libvirt xen driver function.
>
> VBD stats are exposed in reqs and number of sectors from
> blkback, and it's up to us to convert it to sector sizes.
> The sector size is gathered through xenstore in the device
> backend entry "physical-sector-size". This adds up an extra
> dependency namely of xenstore for doing the xs_read.
>
> BlockStatsFlags variant is also implemented which has the
> added benefit of getting the number of flush requests.
>
> Signed-off-by: Joao Martins 
> ---
>  configure.ac |   2 +-
>  src/libxl/libxl_driver.c | 420 
> +++
>  2 files changed, 421 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index ef7fbdb..56fb266 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -896,7 +896,7 @@ if test "$with_libxl" != "no" ; then
>  LIBS="$LIBS $LIBXL_LIBS"
>  AC_CHECK_LIB([xenlight], [libxl_ctx_alloc], [
>  with_libxl=yes
> -LIBXL_LIBS="$LIBXL_LIBS -lxenlight -lxenctrl"
> +LIBXL_LIBS="$LIBXL_LIBS -lxenlight -lxenctrl -lxenstore"
>  ],[
>  if test "$with_libxl" = "yes"; then
>  fail=1
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index dc83083..fd952a3 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -59,6 +59,7 @@
>  #include "network/bridge_driver.h"
>  #include "locking/domain_lock.h"
>  #include "virstats.h"
> +#include 
>  
>  #define VIR_FROM_THIS VIR_FROM_LIBXL
>  
> @@ -75,6 +76,7 @@ VIR_LOG_INIT("libxl.libxl_driver");
>  #define LIBXL_CONFIG_FORMAT_SEXPR "xen-sxpr"
>  
>  #define LIBXL_NB_TOTAL_CPU_STAT_PARAM 1
> +#define LIBXL_NB_TOTAL_BLK_STAT_PARAM 6
>  
>  #define HYPERVISOR_CAPABILITIES "/proc/xen/capabilities"
>  #define HYPERVISOR_XENSTORED "/dev/xen/xenstored"
> @@ -103,6 +105,25 @@ struct _libxlOSEventHookInfo {
>  int id;
>  };
>  
> +/* Object used to store disk statistics across multiple xen backends */
> +typedef struct _libxlBlockStats libxlBlockStats;
> +typedef libxlBlockStats *libxlBlockStatsPtr;
> +struct _libxlBlockStats {
> +long long rd_req;
> +long long rd_bytes;
> +long long wr_req;
> +long long wr_bytes;
> +long long f_req;
> +
> +char *backend;
> +union {
> +struct {
> +long long ds_req;
> +long long oo_req;
> +} vbd;
> +} u;
> +};
> +
>  /* Function declarations */
>  static int
>  libxlDomainManagedSaveLoad(virDomainObjPtr vm,
> @@ -4641,6 +4662,403 @@ libxlDomainIsUpdated(virDomainPtr dom)
>  }
>  
>  static int
> +libxlDiskPathMatches(const char *virtpath, const char *devtype,
> + int *index_r, int max_index,
> + int *partition_r, int max_partition)
> +{
> +const char *p;
> +char *ep;
> +int tl, c;
> +long pl;
> +
> +tl = strlen(devtype);
> +if (memcmp(virtpath, devtype, tl))
> +return 0;
> +
> +/* We decode the drive letter as if it were in base 52
> + * with digits a-zA-Z, more or less */
> +*index_r = -1;
> +p = virtpath + tl;
> +for (;;) {
> +c = *p++;
> +if (c >= 'a' && c <= 'z') {
> +c -= 'a';
> +} else {
> +--p;
> +break;
> +}
> +(*index_r)++;
> +(*index_r) *= 26;
> +(*index_r) += c;
> +
> +if (*index_r > max_index)
> +return 0;
> +}
> +
> +if (!*p) {
> +*partition_r = 0;
> +return 1;
> +}
> +
> +if (*p == '0')
> +return 0; /* leading zeroes not permitted in partition number */
> +
> +if (virStrToLong_l(p, , 10, ) < 0)
> +return 0;
> +
> +if (pl > max_partition || *ep)
> +return 0;
> +
> +*partition_r = pl;
> +return 1;
> +}
> +
> +static int
> +libxlDiskPathParse(const char *virtpath, int *pdisk, int *ppartition)
> +{
> +int disk, partition;
> +char *ep;
> +unsigned long ul;
> +int chrused;

[libvirt] [PATCH] Use daemon log facility for journald

[Guido Günther]

otherwise messages end up in /var/log/kern.log if journald forwards to
syslog.

Closes: #799633
---
 src/util/virlog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index b45ee91..627f4cb 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -920,6 +920,7 @@ virLogOutputToJournald(virLogSourcePtr source,
 journalAddString(, "MESSAGE", rawstr);
 journalAddInt(, "PRIORITY",
   virLogPrioritySyslog(priority));
+journalAddInt(, "SYSLOG_FACILITY", LOG_DAEMON);
 journalAddString(, "LIBVIRT_SOURCE", source->name);
 if (filename)
 journalAddString(, "CODE_FILE", filename);
-- 
2.1.4

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

Re: [libvirt] [PATCH v2] Close the source fd if the destination qemu exits during tunnelled migration

[John Ferlan]

On 09/22/2015 07:21 AM, Shivaprasad bhat wrote:
> On Mon, Sep 21, 2015 at 8:04 PM, John Ferlan  wrote:
>>
>>
>> On 09/21/2015 05:09 AM, Shivaprasad bhat wrote:
>>> Thanks John for the comments.
>>>
>>>
>>> On Fri, Sep 18, 2015 at 10:34 PM, John Ferlan  wrote:


 On 09/14/2015 10:44 AM, Shivaprasad G Bhat wrote:
> Tunnelled migration can hang if the destination qemu exits despite all the
> ABI checks. This happens whenever the destination qemu exits before the
> complete transfer is noticed by source qemu. The savevm state checks at
> runtime can fail at destination and cause qemu to error out.
> The source qemu cant notice it as the EPIPE is not propogated to it.
> The qemuMigrationIOFunc() notices the stream being broken from 
> virStreamSend()
> and it cleans up the stream alone. The qemuMigrationWaitForCompletion() 
> would
> never get to 100% transfer completion.
> The qemuMigrationWaitForCompletion() never breaks out as well since
> the ssh connection to destination is healthy, and the source qemu also 
> thinks
> the migration is ongoing as the Fd to which it transfers, is never
> closed or broken. So, the migration will hang forever. Even Ctrl-C on the
> virsh migrate wouldn't be honoured. Close the source side FD when there is
> an error in the stream. That way, the source qemu updates itself and
> qemuMigrationWaitForCompletion() notices the failure.
>
> Close the FD for all kinds of errors to be sure. The error message is not
> copied for EPIPE so that the destination error is copied instead later.
>
> Note:
> Reproducible with repeated migrations between Power hosts running in 
> different
> subcores-per-core modes.
>
> Changes from v1 -> v2:
>VIR_FORCE_CLOSE() was called twice for this use case which would log
>unneccessary warnings. So, move the fd close to qemuMigrationIOFunc
>so that there are no unnecessary duplicate attempts.(Would this trigger
>a Coverity error? I don't have a setup to check.)
>
> Signed-off-by: Shivaprasad G Bhat 
> ---
>  src/qemu/qemu_migration.c |8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index ff89ab5..9602fb2 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -4012,6 +4012,7 @@ static void qemuMigrationIOFunc(void *arg)
>  if (virStreamFinish(data->st) < 0)
>  goto error;
>
> +VIR_FORCE_CLOSE(data->sock);
>  VIR_FREE(buffer);
>
>  return;
> @@ -4029,7 +4030,11 @@ static void qemuMigrationIOFunc(void *arg)
>  }
>
>   error:
> -virCopyLastError(>err);
> +/* Let the source qemu know that the transfer cant continue anymore.
> + * Don't copy the error for EPIPE as destination has the actual 
> error. */
> +VIR_FORCE_CLOSE(data->sock);
> +if (!virLastErrorIsSystemErrno(EPIPE))
> +virCopyLastError(>err);
>  virResetLastError();
>  VIR_FREE(buffer);
>  }
> @@ -4397,7 +4402,6 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>  if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0)
>  ret = -1;
>  }
> -VIR_FORCE_CLOSE(fd);

 ^^^

 This causes Coverity to claim a RESOURCE_LEAK

 Feels like this was a mistake edit...

>>>
>>> The VIR_FORCE_CLOSE() inside qemuMigrationIOFunc() alone is sufficient.
>>> Having this again here would lead to Warning in the logs. I too thought 
>>> coverity
>>> would complain. Is there a way to force ignore a coverity warning?
>>>
>>
>> Typically a marker of sorts, such as
>>
>> /* coverity[leaked_handle]  */
>>
>> Although I'm not sure that's the best way to handle this either.
>>
>> The problem I see though is this is an "error path" issue and when
>> perhaps it's safe/required to close fd/io->sock/data->sock.
>>
>> Your commit comment notes that the issue is seen on a fairly specific
>> event (virStreamSend failure) for a specific type of migration.
> 
> I believe the failure can be seen for all types of migration with savestate
> mismatch.
> 

My thoughts were based mostly on your commit message comments:

"The qemuMigrationIOFunc() notices the stream being broken from
virStreamSend() and it cleans up the stream alone. The
qemuMigrationWaitForCompletion() would never get to 100% transfer
completion."

and the belief that the core issue you described is there's no way for
qemuMigrationCompleted to know the qemuMigrationIOFunc thread failed and
thus that's what caused the hang. Since we can pass the iothread "io"
object to the Completion function, I figured we could also use that to
check the status of the thread in the list of 

[libvirt] [PATCH] storage: Fix incorrect format for XML

[John Ferlan]

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

When creating a copy of the 'authdef', need to take into account the
slight variation between  and  before blindly copying
the 'authType' field. This ensures virStorageAuthDefFormat will
properly format the  XML for a .

Signed-off-by: John Ferlan 
---
 src/util/virstoragefile.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 2aa1d90..0b72cb3 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1522,7 +1522,12 @@ virStorageAuthDefCopy(const virStorageAuthDef *src)
 /* Not present for storage pool, but used for disk source */
 if (VIR_STRDUP(ret->secrettype, src->secrettype) < 0)
 goto error;
-ret->authType = src->authType;
+/* A  uses secrettype, while a  uses authType, so
+ * if we have a secrettype, then don't copy authType; otherwise,
+ * we will format the authType in 
+ */
+if (!ret->secrettype)
+ret->authType = src->authType;
 ret->secretType = src->secretType;
 if (ret->secretType == VIR_STORAGE_SECRET_TYPE_UUID) {
 memcpy(ret->secret.uuid, src->secret.uuid, sizeof(ret->secret.uuid));
-- 
2.1.0

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

[libvirt] [PATCH] domain: Fix migratable XML with graphics/@listen

[Jiri Denemark]

As of commit 6992994, we set graphics/@listen attribute according to the
first listen child element even if that element is of type='network'.
This was done for backward compatibility with applications which only
support the original listen attribute. However, by doing so we broke
migration to older libvirt which tried to check that the listen
attribute matches one of the listen child elements but which did not
take type='network' elements into account.

We are not concerned about compatibility with old applications when
formatting domain XML for migration for two reasons. The XML is consumed
only by libvirtd and the IP address associated with type='network'
listen address on the source host is just useless on the destination
host. Thus, we can safely avoid propagating the type='network' IP
address to graphics/@listen attribute when creating migratable XML.

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

Signed-off-by: Jiri Denemark 
---
 src/conf/domain_conf.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c890977..033ae46 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21020,19 +21020,17 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
 * . This is done to improve backward compatibility.
 */
 for (i = 0; i < def->nListens; i++) {
-virDomainGraphicsListenType listenType;
-
 if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE &&
 def->listens[i].fromConfig)
 continue;
-listenType = virDomainGraphicsListenGetType(def, i);
 
-if (listenType == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS ||
-(listenType == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK &&
- !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))) {
-if ((listenAddr = virDomainGraphicsListenGetAddress(def, i)))
-break;
-}
+if (def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK &&
+flags & (VIR_DOMAIN_DEF_FORMAT_INACTIVE |
+ VIR_DOMAIN_DEF_FORMAT_MIGRATABLE))
+continue;
+
+if ((listenAddr = virDomainGraphicsListenGetAddress(def, i)))
+break;
 }
 
 virBufferAsprintf(buf, "https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [libvirt-sandbox][PATCH 4/4] libvirt-sandbox-init-qemu: Check for fopen() return value

[Michal Privoznik]

There's a problem in mount_root(): the return value of fopen() is
not checked rather than used directly. Not only this interferes
with pattern laid out by other areas of the code, but it's
possibly dangerous too. If opening the config file fails, @fp may
be dereferenced directly.

Signed-off-by: Michal Privoznik 
---
 libvirt-sandbox/libvirt-sandbox-init-qemu.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c 
b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
index 054dd67..864db42 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
@@ -217,6 +217,8 @@ mount_entry(const char *source,
 }
 }
 
+#define MOUNTS_CONFIG_FILE SANDBOXCONFIGDIR "/mounts.cfg"
+
 static void
 mount_root(const char *path)
 {
@@ -226,7 +228,14 @@ mount_root(const char *path)
 mount_mkdir(SANDBOXCONFIGDIR, 0755);
 mount_9pfs("sandbox:config", SANDBOXCONFIGDIR, 0755, 1);
 
-FILE *fp = fopen(SANDBOXCONFIGDIR "/mounts.cfg", "r");
+FILE *fp = fopen(MOUNTS_CONFIG_FILE, "r");
+
+if (!fp) {
+fprintf(stderr, "libvirt-sandbox-init-qemu: %s: can't open %s: %s",
+__func__, MOUNTS_CONFIG_FILE, strerror(errno));
+exit_poweroff();
+}
+
 while (fgets(line, sizeof line, fp) && !foundRoot) {
 char *source = line;
 char *target = strchr(source, '\t');
-- 
2.4.9

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

[libvirt] [libvirt-sandbox][PATCH 2/4] libvirt-sandbox-init-common: Avoid calling fclose(NULL)

[Michal Privoznik]

The problem occurs in setup_disk_tags. Imagine that fopen()
called at the beginning of the function fails. This results in
jumping onto the 'cleanup' label where fclose() is called.
However, at this point @fp is NULL. And fclose() does not like
that.

Signed-off-by: Michal Privoznik 
---
 libvirt-sandbox/libvirt-sandbox-init-common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c 
b/libvirt-sandbox/libvirt-sandbox-init-common.c
index 42beadc..af7e457 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-common.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-common.c
@@ -109,7 +109,8 @@ static gboolean setup_disk_tags(void) {
 }
 ret = TRUE;
  cleanup:
-fclose(fp);
+if (fp)
+fclose(fp);
 return ret;
 }
 
-- 
2.4.9

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

[libvirt] [libvirt-sandbox][PATCH 0/4] Couple of coverity fixes

[Michal Privoznik]

*** BLURB HERE ***

Michal Privoznik (4):
  builder: Drop dead code in gvir_sandbox_builder_clean_post_stop
  libvirt-sandbox-init-common: Avoid calling fclose(NULL)
  libvirt-sandbox-config: Don't deref NULL
  libvirt-sandbox-init-qemu: Check for fopen() return value

 libvirt-sandbox/libvirt-sandbox-builder.c |  4 
 libvirt-sandbox/libvirt-sandbox-config.c  | 15 ---
 libvirt-sandbox/libvirt-sandbox-init-common.c |  3 ++-
 libvirt-sandbox/libvirt-sandbox-init-qemu.c   | 11 ++-
 4 files changed, 20 insertions(+), 13 deletions(-)

-- 
2.4.9

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

[libvirt] [libvirt-sandbox][PATCH 3/4] libvirt-sandbox-config: Don't deref NULL

[Michal Privoznik]

The problem is in gvir_sandbox_config_add_mount_opts. When
parsing disk string, "format=" may be within it. This is
supposed to change disk format from raw to the desired one.
However, due to bug in our implementation, we may end up
dereferencing a NULL pointer.

Signed-off-by: Michal Privoznik 
---
 libvirt-sandbox/libvirt-sandbox-config.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/libvirt-sandbox/libvirt-sandbox-config.c 
b/libvirt-sandbox/libvirt-sandbox-config.c
index 780d174..5a4aacb 100644
--- a/libvirt-sandbox/libvirt-sandbox-config.c
+++ b/libvirt-sandbox/libvirt-sandbox-config.c
@@ -1611,15 +1611,16 @@ gboolean 
gvir_sandbox_config_add_mount_opts(GVirSandboxConfig *config,
 *tmp = '\0';
 formatStr = tmp + 1;
 
-if ((strncmp(formatStr, "format=", 7) == 0) &&
-!(enum_value = g_enum_get_value_by_nick(enum_class, formatStr 
+ 7))) {
+if ((strncmp(formatStr, "format=", 7) == 0)) {
+if (!(enum_value = g_enum_get_value_by_nick(enum_class, 
formatStr + 7))) {
+g_type_class_unref(enum_class);
+g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0,
+_("Unknown disk image format: '%s'"), 
formatStr + 7);
+return FALSE;
+}
 g_type_class_unref(enum_class);
-g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0,
-_("Unknown disk image format: '%s'"), formatStr + 
7);
-return FALSE;
+format = enum_value->value;
 }
-g_type_class_unref(enum_class);
-format = enum_value->value;
 }
 
 mnt = GVIR_SANDBOX_CONFIG_MOUNT(g_object_new(type,
-- 
2.4.9

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

[libvirt] [libvirt-sandbox][PATCH 1/4] builder: Drop dead code in gvir_sandbox_builder_clean_post_stop

[Michal Privoznik]

At the 'cleanup' label we try to unref @child. However, whenever
the label is entered there's no chance for the variable to be
anything else than NULL rendering those two lines as dead code.
Drop it. And it's the same story with @info.

Signed-off-by: Michal Privoznik 
---
 libvirt-sandbox/libvirt-sandbox-builder.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c 
b/libvirt-sandbox/libvirt-sandbox-builder.c
index ea7d064..b4b4d77 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder.c
@@ -752,10 +752,6 @@ gboolean 
gvir_sandbox_builder_clean_post_stop(GVirSandboxBuilder *builder,
 ret = FALSE;
 
  cleanup:
-if (child)
-g_object_unref(child);
-if (info)
-g_object_unref(info);
 g_object_unref(enumerator);
 g_object_unref(libsFile);
 g_free(libsdir);
-- 
2.4.9

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

Re: [libvirt] [libvirt-sandbox][PATCH 2/4] libvirt-sandbox-init-common: Avoid calling fclose(NULL)

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 11:15:22AM +0200, Michal Privoznik wrote:
> The problem occurs in setup_disk_tags. Imagine that fopen()
> called at the beginning of the function fails. This results in
> jumping onto the 'cleanup' label where fclose() is called.
> However, at this point @fp is NULL. And fclose() does not like
> that.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  libvirt-sandbox/libvirt-sandbox-init-common.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

ACK


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] [libvirt-sandbox][PATCH 4/4] libvirt-sandbox-init-qemu: Check for fopen() return value

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 11:15:24AM +0200, Michal Privoznik wrote:
> There's a problem in mount_root(): the return value of fopen() is
> not checked rather than used directly. Not only this interferes
> with pattern laid out by other areas of the code, but it's
> possibly dangerous too. If opening the config file fails, @fp may
> be dereferenced directly.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  libvirt-sandbox/libvirt-sandbox-init-qemu.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

ACK


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] [libvirt-sandbox][PATCH 3/4] libvirt-sandbox-config: Don't deref NULL

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 11:15:23AM +0200, Michal Privoznik wrote:
> The problem is in gvir_sandbox_config_add_mount_opts. When
> parsing disk string, "format=" may be within it. This is
> supposed to change disk format from raw to the desired one.
> However, due to bug in our implementation, we may end up
> dereferencing a NULL pointer.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  libvirt-sandbox/libvirt-sandbox-config.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)

ACK


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] [libvirt-sandbox][PATCH 1/4] builder: Drop dead code in gvir_sandbox_builder_clean_post_stop

[Daniel P. Berrange]

On Wed, Sep 23, 2015 at 11:15:21AM +0200, Michal Privoznik wrote:
> At the 'cleanup' label we try to unref @child. However, whenever
> the label is entered there's no chance for the variable to be
> anything else than NULL rendering those two lines as dead code.
> Drop it. And it's the same story with @info.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  libvirt-sandbox/libvirt-sandbox-builder.c | 4 
>  1 file changed, 4 deletions(-)

ACK


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] [libvirt-guests] New feature ALWAYS_START

[Marek Lukács]

Hi Martin,

thank you for your reply. Please check my comments.

On Wed, Sep 23, 2015 at 10:50 AM, Martin Kletzander 
wrote:

> On Tue, Sep 22, 2015 at 07:33:30AM +0200, Marek Lukács wrote:
>
>> Hi,
>>
>> It will be nice feature to have configuration option
>> ALWAYS_START="$uri:$name $uri:name ..." in libvirt-guests
>> configuration file.
>>
>> If ON_BOOT is "start" and if ALWAYS_START is not empty, it iterates
>> over the ALWAYS_START and starts guests with same conditions (delays
>> etc.) before it starts guests from LISTFILE.
>>
>>
> To be honest, I don't think that's _exactly_ what you want _just_ from
> libvirt itself; let me explain.
>
> Benefits:
>> - guests are started with delays
>>
>
> Delays that are done due to the guests are not something we should
> handle.  Guests and mainly the applications inside them should handle
> this gracefully.  Just delaying the starts is still error-prone.
>

I fully understand this and fully agree with you. Production application
has to handle this gracefully. But still I see a space for doing it in
environments where it is not so necessary to configure application so
gracefully, like development or testing environments. Delays are fine for
many not production environments, where applications are not in production
state.


> - guests are started after host failure
>>
>
> That's what libvirt-guests does already.  And if you want some domains
> to be started on every start, there's the 'autostart' parameter for
> domains.
>

No, libvirt-guests only starts those domains which has been running before
"service" libvirt-guests has been stopped. In case of host failure, there
is no LISTFILE in filesystem, as it has not been generated by
libvirt-guests stop mechanism.

But probably I do not understand "script" libvirt-guests correctly and why
it is in libvirt. I will be happy, if you will give me more details, why
there is this script, even if libvirt has 'autostart' parameter for domains.

- For what usage is libvirt-guests designed?
- Why it supports delays?
- Why to have libvirt-guests if there is 'autostart' domain parameter?
- For who is libvirt-guests and who should use 'autostart' domain parameter?


> - guests are started in specific order (for example complex
>> environment, when DB should be started before other guest, etc.)
>>
>>
> Again, same as the first point.  This should be handled gracefully in
> the application itself or at least worked around in the guest (not
> starting DB-related app before DB is accessible).
>
> Anyway, if you *really* want this, then the easiest thing to do is
> creating a service that starts before libvirt-guests, but after
> libvirt, which is just something similar to "local", so it just runs a
> script that does:
>
>  for i in domain_one some_other_domain database_dom
>  do
>virsh start "$i"
>sleep 60 # or you can try connecting to make sure it started
>  done
>
> or something similar.  However, you might still propose a simple patch
> for the feature you described.
>

For me it is no problem to design my own script to handle my needs. I have
spent some time googling, if there is already a tool for it. I found only
similar questions, so I got feeling, that I am not the only one with
similar requirements.

Script libvirt-guests in my eyes handles very similar task. It starts
domains with delays, it starts domains what has been running at previous
stop, but do not handle situation in case of host failure and do not starts
domain in specific order. I prefer and I think, that it is better to not
create new script no one knows about, but to modify existing one everybody
knows about. But again, maybe I do not understand why and for what
libvirt-guests is.

Anyway, much easier in case of testing and development environments is to
set start sequence in /etc/rc.local and forget about script like
libvirt-guests, if it does not have features I described.


> Regards,
>>
>> Marek Lukács
>>
>> --
>> 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] qemuDomainDeviceDefPostParse: Drop useless check

[Martin Kletzander]

On Wed, Sep 23, 2015 at 10:05:13AM +0200, Michal Privoznik wrote:

Now that virQEMUDriverCreateXMLConf is never called with NULL
(after 086f37e97aab) we can safely drop useless check in
qemuDomainDeviceDefPostParse as we are guaranteed to be always
called with the driver initialized. Therefore checking if driver
is NULL makes no sense. Moreover, if we mix it with direct driver
dereference.

Signed-off-by: Michal Privoznik 
---
src/qemu/qemu_domain.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7d92f3a..8e3a7d3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1233,12 +1233,9 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
{
virQEMUDriverPtr driver = opaque;
virQEMUCapsPtr qemuCaps = NULL;
-virQEMUDriverConfigPtr cfg = NULL;
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
int ret = -1;

-if (driver)
-cfg = virQEMUDriverGetConfig(driver);
-


It's nice, but since you're cleaning it up, this makes another check
for (cfg) below useless as well.


qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);

if (dev->type == VIR_DOMAIN_DEVICE_NET &&
--
2.4.9

--
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 2/5] qemuDomainCreateXML: Make domain definition transient

[Michal Privoznik]

On 23.09.2015 03:53, Jim Fehlig wrote:
> On 09/22/2015 09:28 AM, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=871452
>>
>> So, you want to create a domain from XML. The domain already
>> exists in libvirt's database of domains. It's okay, because name
>> and UUID matches. However, on domain startup, internal
>> representation of the domain is overwritten with your XML even
>> though we claim that the XML you've provided is a transient one.
>> Le sigh.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>>   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 30d2d98..2a4b026 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -1745,6 +1745,7 @@ static virDomainPtr
>> qemuDomainCreateXML(virConnectPtr conn,
>> if (!(vm = virDomainObjListAdd(driver->domains, def,
>>  driver->xmlopt,
>> +   VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
>>  VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
>>  NULL)))
>>   goto cleanup;
> 
> It looks like the libxl driver should be fixed similarly, right?
> 

Oh right. Let me respin with more drivers fixed.

Michal

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

[libvirt] [PATCH] Use VIR_DIV_UP macro where possible

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
Will push as trivial in a second.

 src/rpc/virnetclientstream.c | 2 +-
 src/util/virbitmap.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c
index 1cc9002b0ad7..64e9cd221be2 100644
--- a/src/rpc/virnetclientstream.c
+++ b/src/rpc/virnetclientstream.c
@@ -279,7 +279,7 @@ int virNetClientStreamQueuePacket(virNetClientStreamPtr st,
 goto end;
 }

-pieces = (length + size - 1) / size;
+pieces = VIR_DIV_UP(length, size);
 for (piece = 0; piece < pieces; piece++) {
 if (size > length - offset)
 size = length - offset;
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 4d270a910c19..57135a09f71e 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -71,7 +71,7 @@ virBitmapNewQuiet(size_t size)
 if (SIZE_MAX - VIR_BITMAP_BITS_PER_UNIT < size || size == 0)
 return NULL;

-sz = (size + VIR_BITMAP_BITS_PER_UNIT - 1) / VIR_BITMAP_BITS_PER_UNIT;
+sz = VIR_DIV_UP(size, VIR_BITMAP_BITS_PER_UNIT);

 if (VIR_ALLOC_QUIET(bitmap) < 0)
 return NULL;
--
2.5.3

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

Re: [libvirt] [PATCH] conf: escape string for disk driver name attribute

[Martin Kletzander]

On Tue, Sep 22, 2015 at 04:13:53PM +0800, Luyao Huang wrote:

Just like e92e5ba1, this attribute was missed.

Signed-off-by: Luyao Huang 
---
src/conf/domain_conf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)



ACK && Pushed.


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

[libvirt] by libvirt api(virStoragePoolRefresh) to refresh pool, discovered the connection is very slow

[饶俊明]

Hello

In ours project,use libvirt(libvirt-0.10.2-46.el6) to manage storage 
pool(the type of pool is ceph rbd),
by libvirt api(virStoragePoolRefresh) to refresh pool, discovered the 
connection(libvirt to ceph) is very slow
libvirt is how to deal with pool refresh.
tks!!!




Jason rao

-邮件原件-
发件人: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] 代表 
libvir-list-requ...@redhat.com
发送时间: 2015年9月24日 7:19
收件人: libvir-list@redhat.com
主题: libvir-list Digest, Vol 118, Issue 146

Send libvir-list mailing list submissions to
libvir-list@redhat.com

To subscribe or unsubscribe via the World Wide Web, visit
https://www.redhat.com/mailman/listinfo/libvir-list
or, via email, send a message with subject or body 'help' to
libvir-list-requ...@redhat.com

You can reach the person managing the list at
libvir-list-ow...@redhat.com

When replying, please edit your Subject line so it is more specific than "Re: 
Contents of libvir-list digest..."


Today's Topics:

   1. [PATCH] domain: Fix migratable XML with graphics/@listen
  (Jiri Denemark)
   2. [PATCH] storage: Fix incorrect format for XML
  (John Ferlan)
   3. Re: [PATCH v2] qemuDomainDeviceDefPostParse: Drop useless
  checks (John Ferlan)
   4. Re: [PATCH RFC 4/7] libxl: implement virDomainBlockStats
  (Jim Fehlig)
   5. [PATCH 0/6] Fix some Coverity issues (John Ferlan)


--

Message: 1
Date: Wed, 23 Sep 2015 22:48:14 +0200
From: Jiri Denemark 
To: libvir-list@redhat.com
Subject: [libvirt] [PATCH] domain: Fix migratable XML with
graphics/@listen
Message-ID:

<1e763711f268f0f5d09f6a70aab87bf13dd459f9.1443041294.git.jdene...@redhat.com>


As of commit 6992994, we set graphics/@listen attribute according to the first 
listen child element even if that element is of type='network'.
This was done for backward compatibility with applications which only support 
the original listen attribute. However, by doing so we broke migration to older 
libvirt which tried to check that the listen attribute matches one of the 
listen child elements but which did not take type='network' elements into 
account.

We are not concerned about compatibility with old applications when formatting 
domain XML for migration for two reasons. The XML is consumed only by libvirtd 
and the IP address associated with type='network'
listen address on the source host is just useless on the destination host. 
Thus, we can safely avoid propagating the type='network' IP address to 
graphics/@listen attribute when creating migratable XML.

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

Signed-off-by: Jiri Denemark 
---
 src/conf/domain_conf.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 
c890977..033ae46 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21020,19 +21020,17 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
 * . This is done to improve backward compatibility.
 */
 for (i = 0; i < def->nListens; i++) {
-virDomainGraphicsListenType listenType;
-
 if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE &&
 def->listens[i].fromConfig)
 continue;
-listenType = virDomainGraphicsListenGetType(def, i);
 
-if (listenType == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS ||
-(listenType == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK &&
- !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))) {
-if ((listenAddr = virDomainGraphicsListenGetAddress(def, i)))
-break;
-}
+if (def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK &&
+flags & (VIR_DOMAIN_DEF_FORMAT_INACTIVE |
+ VIR_DOMAIN_DEF_FORMAT_MIGRATABLE))
+continue;
+
+if ((listenAddr = virDomainGraphicsListenGetAddress(def, i)))
+break;
 }
 
 virBufferAsprintf(buf, "
To: libvir-list@redhat.com
Subject: [libvirt] [PATCH] storage: Fix incorrect format for 
  XML
Message-ID: <1443041675-19738-1-git-send-email-jfer...@redhat.com>

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

When creating a copy of the 'authdef', need to take into account the
slight variation between  and  before blindly copying
the 'authType' field. This ensures virStorageAuthDefFormat will
properly format the  XML for a .

Signed-off-by: John Ferlan 
---
 src/util/virstoragefile.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 2aa1d90..0b72cb3 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1522,7 +1522,12 @@ virStorageAuthDefCopy(const virStorageAuthDef *src)
 /* Not present for storage pool, but used for disk source 

[libvirt] [PATCH 6/6] qemu: Resolve Coverity RESOURCE_LEAK

[John Ferlan]

This seemed to be more of a false positive as for some reason Coverity
was missing the "ret < 0" goto error condition and somehow believing that
event could be overwritten.  At first I thought it was just the ret != 0
condition difference, but it wasn't.

In any case, make use of the recent change to qemuDomainEventQueue to
check event == NULL and just pass it as a parameter directly in the
error path. That avoids the error.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_driver.c | 24 +++-
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2387cf3..3a98774 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3155,7 +3155,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
 virFileWrapperFdFree(wrapperFd);
 VIR_FREE(xml);
 
-if (ret != 0 && needUnlink)
+if (ret < 0 && needUnlink)
 unlink(path);
 
 return ret;
@@ -3174,7 +3174,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
 char *xml = NULL;
 bool was_running = false;
 int ret = -1;
-int rc;
 virObjectEventPtr event = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virCapsPtr caps;
@@ -3249,21 +3248,20 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
 /* Shut it down */
 qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SAVED, 0);
 virDomainAuditStop(vm, "saved");
-event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
- VIR_DOMAIN_EVENT_STOPPED_SAVED);
+event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
+  VIR_DOMAIN_EVENT_STOPPED_SAVED);
  endjob:
-if (ret != 0) {
+if (ret < 0) {
 if (was_running && virDomainObjIsActive(vm)) {
 virErrorPtr save_err = virSaveLastError();
-rc = qemuProcessStartCPUs(driver, vm, dom->conn,
-  VIR_DOMAIN_RUNNING_SAVE_CANCELED,
-  QEMU_ASYNC_JOB_SAVE);
-if (rc < 0) {
+if (qemuProcessStartCPUs(driver, vm, dom->conn,
+ VIR_DOMAIN_RUNNING_SAVE_CANCELED,
+ QEMU_ASYNC_JOB_SAVE) < 0) {
 VIR_WARN("Unable to resume guest CPUs after save failure");
-event = virDomainEventLifecycleNewFromObj(vm,
-  
VIR_DOMAIN_EVENT_SUSPENDED,
-  
VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR);
+qemuDomainEventQueue(driver,
+ virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_SUSPENDED,
+ 
VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR));
 }
 virSetError(save_err);
 virFreeError(save_err);
-- 
2.1.0

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

[libvirt] [PATCH 3/6] tests: Resolve Coverity RESOURCE_LEAK

[John Ferlan]

The cleanup path did not clear the reference for sk1 and sk2

Signed-off-by: John Ferlan 
---
 tests/virnetdaemontest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c
index fb8a6c0..24cbd54 100644
--- a/tests/virnetdaemontest.c
+++ b/tests/virnetdaemontest.c
@@ -125,6 +125,8 @@ testCreateServer(const char *host, int family)
 virObjectUnref(cln2);
 virObjectUnref(svc1);
 virObjectUnref(svc2);
+virObjectUnref(sk1);
+virObjectUnref(sk2);
 return srv;
 
  error:
-- 
2.1.0

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

[libvirt] [PATCH 5/6] qemu: Resolve Coverity CHECKED_RETURN

[John Ferlan]

Coverity complains that return from virHookCall is not checked in
one place in qemuProcessStop.  Since the comment notes that we cannot
stop the operation even it if fails, just added the ignore_value.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_process.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b961f40..eb04883 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5276,9 +5276,9 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
 
 /* we can't stop the operation even if the script raised an error */
-virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
-VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END,
-NULL, xml, NULL);
+ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
+ VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END,
+ NULL, xml, NULL));
 VIR_FREE(xml);
 }
 
-- 
2.1.0

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

[libvirt] [PATCH 4/6] virsh: Resolve Coverity DEADCODE

[John Ferlan]

Use 'dead_error_condition' instead of 'dead_error_begin'

Signed-off-by: John Ferlan 
---
 tools/virsh.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 76b5e9f..7484bed 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -118,7 +118,7 @@ virshCatchDisconnect(virConnectPtr conn,
 case VIR_CONNECT_CLOSE_REASON_KEEPALIVE:
 str = N_("Disconnected from %s due to keepalive timeout");
 break;
-/* coverity[dead_error_begin] */
+/* coverity[dead_error_condition] */
 case VIR_CONNECT_CLOSE_REASON_CLIENT:
 case VIR_CONNECT_CLOSE_REASON_LAST:
 break;
-- 
2.1.0

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

[libvirt] [PATCH 0/6] Fix some Coverity issues

[John Ferlan]

Newer Coverity (7.7.0) found a couple real issues and a few more
false positives. There's still a few more to be resolved, but still
trying to figure them out...

libxlDomainMigrationPrepare - claim is args is leaked. Although
it seems to be handled in libxlMigrateReceive or libxlDoMigrateReceive.
Don't know the code well enough to do proper triage.

qemuProcessStop - claim is usage of net->ifname in the condition
(vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)
is using a NULL net->ifname that would have been VIR_FREE()'d
when case VIR_DOMAIN_NET_TYPE_DIRECT a few lines above.

qemuDomainBlockRebase - claim is overwriting 'dest' after call to
   qemuDomainBlockCopyCommon causes resource leak.  Although reading
   code says otherwise, except of course if CopyCommon goes to cleanup
   rather than endjob.  Even changing the location doesn't resolve
   the issue.

Fix the Coverity tag for the DEADCODE... It's always a coin-flip either
dead_error_begin or dead_error_condition - I gave Jiri bad advice as
to which one to use it seems.

John Ferlan (6):
  Avoid Coverity FORWARD_NULL prior to strtok_r calls
  tests: Resolve Coverity RESOURCE_LEAK
  tests: Resolve Coverity RESOURCE_LEAK
  virsh: Resolve Coverity DEADCODE
  qemu: Resolve Coverity CHECKED_RETURN
  qemu: Resolve Coverity RESOURCE_LEAK

 src/esx/esx_vi.c  |  1 +
 src/libxl/libxl_conf.c|  1 +
 src/openvz/openvz_conf.c  |  2 ++
 src/qemu/qemu_driver.c| 24 +++-
 src/qemu/qemu_process.c   |  6 +++---
 src/xenapi/xenapi_utils.c |  1 +
 tests/qemucaps2xmltest.c  |  1 +
 tests/virnetdaemontest.c  |  2 ++
 tools/virsh.c |  2 +-
 9 files changed, 23 insertions(+), 17 deletions(-)

-- 
2.1.0

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

[libvirt] [PATCH 1/6] Avoid Coverity FORWARD_NULL prior to strtok_r calls

[John Ferlan]

The 'strtok_r' function requires passing a NULL as the first parameter
on subsequent calls in order to ensure the code picks up where it left
off on a previous call.  However, Coverity doesn't quite realize this
and points out that if a NULL was passed in as the third argument it would
result in a possible NULL deref because the strtok_r function will assign
the third argument to the first in the call is NULL.

Adding an sa_assert() prior to each initial call quiets Coverity

Signed-off-by: John Ferlan 
---
 src/esx/esx_vi.c  | 1 +
 src/libxl/libxl_conf.c| 1 +
 src/openvz/openvz_conf.c  | 2 ++
 src/xenapi/xenapi_utils.c | 1 +
 4 files changed, 5 insertions(+)

diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index af822b1..a57d753 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -1173,6 +1173,7 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context 
*ctx, const char *path)
 goto cleanup;
 
 /* Lookup Datacenter */
+sa_assert(tmp);
 item = strtok_r(tmp, "/", );
 
 if (!item) {
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 35fd495..ab588e8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -353,6 +353,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
 /* Split capabilities string into tokens. strtok_r is OK here because
  * we "own" the buffer.  Parse out the features from each token.
  */
+sa_assert(ver_info->capabilities);
 for (str = ver_info->capabilities, nr_guest_archs = 0;
  nr_guest_archs < sizeof(guest_archs) / sizeof(guest_archs[0])
  && (token = strtok_r(str, " ", )) != NULL;
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index db0a9a7..003272e 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -138,6 +138,7 @@ openvzParseBarrierLimit(const char* value,
 char *str;
 int ret = -1;
 
+sa_assert(value);
 if (VIR_STRDUP(str, value) < 0)
 goto error;
 
@@ -965,6 +966,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len)
 }
 }
 
+sa_assert(line);
 iden = strtok_r(line, " ", );
 uuidbuf = strtok_r(NULL, "\n", );
 
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index a80e084..6b710cd 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -301,6 +301,7 @@ getCpuBitMapfromString(char *mask, unsigned char *cpumap, 
int maplen)
 int max_bits = maplen * 8;
 char *num = NULL, *bp = NULL;
 bzero(cpumap, maplen);
+sa_assert(mask);
 num = strtok_r(mask, ",", );
 while (num != NULL) {
 if (virStrToLong_i(num, NULL, 10, ) < 0)
-- 
2.1.0

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

[libvirt] [PATCH 2/6] tests: Resolve Coverity RESOURCE_LEAK

[John Ferlan]

In the error path need to unref the 'caps' as well

Signed-off-by: John Ferlan 
---
 tests/qemucaps2xmltest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c
index 6a5811c..7adde24 100644
--- a/tests/qemucaps2xmltest.c
+++ b/tests/qemucaps2xmltest.c
@@ -113,6 +113,7 @@ testGetCaps(char *capsData, const testQemuData *data)
 
  error:
 virObjectUnref(qemuCaps);
+virObjectUnref(caps);
 return NULL;
 }
 
-- 
2.1.0

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

Re: [libvirt] [PATCH 0/3] Introduce new virtType enum item

[John Ferlan]

On 09/23/2015 12:51 AM, Shivangi Dhir wrote:

[...]


> 
> Thanks alot for your feedback.
> Should I modify and resend the patches after making the changes
> suggested above, if there are no other issues ?
> 
> 

I should be able to jiggle your patches - I'll work on those tomorrow
(for me).

I also found out from practice that using virDomainVirtType in the
capabilities.{h,c} functions & structures would result in many more
changes in order to pull in the definition from domain_conf.h.  So just
kept them as int, but added a comment to make it more obvious.

I'll end up with two patches, with the first getting the attached
squashed in... Unless someone comes up with a path that's using the
numerical representation - it just seemed the virtType would be (but I
can check that).


John

>From a27c58e42f95e2929ebf0ca0819ed187a513f63e Mon Sep 17 00:00:00 2001
From: John Ferlan 
Date: Wed, 23 Sep 2015 18:04:43 -0400
Subject: [PATCH 2/3] merge

Signed-off-by: John Ferlan 
---
 src/conf/capabilities.c | 11 ++-
 src/conf/capabilities.h |  6 +++---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index b420f9f..86ea212 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1,7 +1,7 @@
 /*
  * capabilities.c: hypervisor capabilities
  *
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -573,7 +573,7 @@ virCapsDomainDataCompare(virCapsGuestPtr guest,
  virCapsGuestMachinePtr machine,
  int ostype,
  virArch arch,
- int domaintype,
+ virDomainVirtType domaintype,
  const char *emulator,
  const char *machinetype)
 {
@@ -584,7 +584,8 @@ virCapsDomainDataCompare(virCapsGuestPtr guest,
 if ((arch != VIR_ARCH_NONE) && (guest->arch.id != arch))
 return false;
 
-if (domaintype != VIR_DOMAIN_VIRT_NONE && (!domain || domain->type != domaintype))
+if (domaintype != VIR_DOMAIN_VIRT_NONE &&
+(!domain || domain->type != domaintype))
 return false;
 
 if (emulator) {
@@ -611,7 +612,7 @@ static virCapsDomainDataPtr
 virCapabilitiesDomainDataLookupInternal(virCapsPtr caps,
 int ostype,
 virArch arch,
-int domaintype,
+virDomainVirtType domaintype,
 const char *emulator,
 const char *machinetype)
 {
@@ -678,7 +679,7 @@ virCapabilitiesDomainDataLookupInternal(virCapsPtr caps,
   virDomainOSTypeToString(ostype));
 if (arch)
 virBufferAsprintf(, "arch=%s ", virArchToString(arch));
-if (domaintype)
+if (domaintype > VIR_DOMAIN_VIRT_NONE)
 virBufferAsprintf(, "domaintype=%s ",
   virDomainVirtTypeToString(domaintype));
 if (emulator)
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index c14fcf6..1754b13 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -1,7 +1,7 @@
 /*
  * capabilities.h: hypervisor capabilities
  *
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -61,7 +61,7 @@ struct _virCapsGuestDomainInfo {
 typedef struct _virCapsGuestDomain virCapsGuestDomain;
 typedef virCapsGuestDomain *virCapsGuestDomainPtr;
 struct _virCapsGuestDomain {
-int type;
+int type; /* virDomainVirtType */
 virCapsGuestDomainInfo info;
 };
 
@@ -197,7 +197,7 @@ typedef virCapsDomainData *virCapsDomainDataPtr;
 struct _virCapsDomainData {
 int ostype;
 int arch;
-int domaintype;
+int domaintype; /* virDomainVirtType */
 const char *emulator;
 const char *machinetype;
 };
-- 
2.1.0

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