[libvirt] [PATCH RFC 2/2] libxl: add v3 migration w/o params

[Joao Martins]

This patch introduces migration v3 without ext. params. Most of
the changes are mechanical and most of it is moving code and handling
of the arguments in different way. Functional-wise it works same way
as its "Params" variants.

By having v3 migration we end up supporting older variants of
virDomainMigrateToURI which don't use v3 with params. The latter is
only supported by the latest virDomainMigrateToURI3, thus we
broaden the API support of migration in libxl e.g. ability to use
virDomainMigrateToURI2 and virDomainMigrate (in P2P mode).

Signed-off-by: Joao Martins 
---
 src/libxl/libxl_driver.c | 213 +++
 1 file changed, 213 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 4d6821f..afb8a7d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4711,6 +4711,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int 
feature)
 case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
 case VIR_DRV_FEATURE_MIGRATION_PARAMS:
 case VIR_DRV_FEATURE_MIGRATION_P2P:
+case VIR_DRV_FEATURE_MIGRATION_V3:
 return 1;
 default:
 return 0;
@@ -4890,6 +4891,42 @@ libxlNodeDeviceReset(virNodeDevicePtr dev)
 }
 
 static char *
+libxlDomainMigrateBegin3(virDomainPtr domain,
+ const char *xmlin,
+ char **cookieout ATTRIBUTE_UNUSED,
+ int *cookieoutlen ATTRIBUTE_UNUSED,
+ unsigned long flags,
+ const char *dname ATTRIBUTE_UNUSED,
+ unsigned long resource ATTRIBUTE_UNUSED)
+{
+virDomainObjPtr vm = NULL;
+
+#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME
+virReportUnsupportedError();
+return NULL;
+#endif
+
+virCheckFlags(LIBXL_MIGRATION_FLAGS, NULL);
+
+if (!(vm = libxlDomObjFromDomain(domain)))
+return NULL;
+
+if (virDomainMigrateBegin3EnsureACL(domain->conn, vm->def) < 0) {
+virObjectUnlock(vm);
+return NULL;
+}
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("domain is not running"));
+virObjectUnlock(vm);
+return NULL;
+}
+
+return libxlDomainMigrationBegin(domain->conn, vm, xmlin);
+}
+
+static char *
 libxlDomainMigrateBegin3Params(virDomainPtr domain,
virTypedParameterPtr params,
int nparams,
@@ -4939,6 +4976,46 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain,
 }
 
 static int
+libxlDomainMigratePrepare3(virConnectPtr dconn,
+   const char *cookiein ATTRIBUTE_UNUSED,
+   int cookieinlen ATTRIBUTE_UNUSED,
+   char **cookieout ATTRIBUTE_UNUSED,
+   int *cookieoutlen ATTRIBUTE_UNUSED,
+   const char *uri_in,
+   char **uri_out,
+   unsigned long flags,
+   const char *dname,
+   unsigned long resource ATTRIBUTE_UNUSED,
+   const char *dom_xml)
+{
+libxlDriverPrivatePtr driver = dconn->privateData;
+virDomainDefPtr def = NULL;
+
+#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME
+virReportUnsupportedError();
+return -1;
+#endif
+
+virCheckFlags(LIBXL_MIGRATION_FLAGS, -1);
+
+if (!(def = libxlDomainMigrationPrepareDef(driver, dom_xml, dname)))
+goto error;
+
+if (virDomainMigratePrepare3EnsureACL(dconn, def) < 0)
+goto error;
+
+if (libxlDomainMigrationPrepare(dconn, , uri_in, uri_out, flags) < 0)
+goto error;
+
+return 0;
+
+ error:
+virDomainDefFree(def);
+return -1;
+
+}
+
+static int
 libxlDomainMigratePrepare3Params(virConnectPtr dconn,
  virTypedParameterPtr params,
  int nparams,
@@ -4993,6 +5070,55 @@ libxlDomainMigratePrepare3Params(virConnectPtr dconn,
 }
 
 static int
+libxlDomainMigratePerform3(virDomainPtr dom,
+  const char *dom_xml,
+  const char *cookiein ATTRIBUTE_UNUSED,
+  int cookieinlen ATTRIBUTE_UNUSED,
+  char **cookieout ATTRIBUTE_UNUSED,
+  int *cookieoutlen ATTRIBUTE_UNUSED,
+  const char *dconnuri,
+  const char *uri,
+  unsigned long flags,
+  const char *dname,
+  unsigned long resource ATTRIBUTE_UNUSED)
+{
+libxlDriverPrivatePtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+int ret = -1;
+
+#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME
+virReportUnsupportedError();
+return -1;
+#endif
+
+virCheckFlags(LIBXL_MIGRATION_FLAGS, -1);
+
+if (!(vm = libxlDomObjFromDomain(dom)))
+goto 

[libvirt] [PATCH RFC 1/2] libxl: add p2p migration

[Joao Martins]

Introduce support for VIR_MIGRATE_PEER2PEER in libvirt migration.
Most of the changes occur at the source and no modifications
at the receiver.

In P2P mode there is only the Perform phase so we must handle
the connection with the destination and actually perform the
migration. libxlDomainPerformP2P implements the connection to
the destination and let libxlDoMigrateP2P implements the actual
migration logic with virConnectPtr. In this function we take
of doing all phases of migration in the destination similar to
virDomainMigrateVersion3Full. We appropriately save the last
error reported in each of the phases to provide proper
reporting. We don't yet support VIR_MIGRATE_TUNNELED yet and
we always use V3 with extensible params, thus it also makes the
implementation simpler.

It is worth noting that the receiver didn't have any changes,
and since it's still the v3 sequence thus it is possible to
migrate from a P2P to non-P2P host.

Signed-off-by: Joao Martins 
---
 src/libxl/libxl_driver.c|  13 ++-
 src/libxl/libxl_migration.c | 220 
 src/libxl/libxl_migration.h |  11 +++
 3 files changed, 241 insertions(+), 3 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 5f69b49..4d6821f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4710,6 +4710,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int 
feature)
 switch (feature) {
 case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
 case VIR_DRV_FEATURE_MIGRATION_PARAMS:
+case VIR_DRV_FEATURE_MIGRATION_P2P:
 return 1;
 default:
 return 0;
@@ -5036,9 +5037,15 @@ libxlDomainMigratePerform3Params(virDomainPtr dom,
 if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
-uri, dname, flags) < 0)
-goto cleanup;
+if (flags & VIR_MIGRATE_PEER2PEER) {
+if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml,
+   dconnuri, uri, dname, flags) < 0)
+goto cleanup;
+} else {
+if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
+uri, dname, flags) < 0)
+goto cleanup;
+}
 
 ret = 0;
 
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 0d23e5f..659f4d8 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -42,6 +42,7 @@
 #include "libxl_conf.h"
 #include "libxl_migration.h"
 #include "locking/domain_lock.h"
+#include "virtypedparam.h"
 
 #define VIR_FROM_THIS VIR_FROM_LIBXL
 
@@ -456,6 +457,225 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
 return ret;
 }
 
+/* This function is a simplification of virDomainMigrateVersion3Full
+ * excluding tunnel support and restricting it to migration v3
+ * with params since it was the first to be introduced in libxl.
+ */
+static int
+libxlDoMigrateP2P(libxlDriverPrivatePtr driver,
+  virDomainObjPtr vm,
+  const char *dom_xml,
+  virConnectPtr dconn,
+  const char *dconnuri ATTRIBUTE_UNUSED,
+  const char *dname,
+  const char *uri,
+  unsigned int flags)
+{
+virDomainPtr ddomain = NULL;
+virTypedParameterPtr params = NULL;
+int nparams = 0;
+int maxparams = 0;
+char *uri_out = NULL;
+unsigned long destflags;
+bool cancelled = true;
+virErrorPtr orig_err = NULL;
+int ret = -1;
+
+if (virTypedParamsAddString(, , ,
+VIR_MIGRATE_PARAM_DEST_XML, dom_xml) < 0)
+goto cleanup;
+
+if (dname &&
+virTypedParamsAddString(, , ,
+VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0)
+goto cleanup;
+
+if (uri &&
+virTypedParamsAddString(, , ,
+VIR_MIGRATE_PARAM_URI, uri) < 0)
+goto cleanup;
+
+/* We don't require the destination to have P2P support
+ * as it looks to be normal migration from the receiver perpective.
+ */
+destflags = flags & ~(VIR_MIGRATE_PEER2PEER);
+
+VIR_DEBUG("Prepare3");
+virObjectUnlock(vm);
+ret = dconn->driver->domainMigratePrepare3Params
+(dconn, params, nparams, NULL, 0, NULL, NULL, _out, destflags);
+virObjectLock(vm);
+
+if (ret == -1)
+goto cleanup;
+
+if (uri_out) {
+if (virTypedParamsReplaceString(, ,
+VIR_MIGRATE_PARAM_URI, uri_out) < 0) {
+orig_err = virSaveLastError();
+goto finish;
+}
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("domainMigratePrepare3 did not set uri"));
+goto finish;
+}
+
+VIR_DEBUG("Perform3 uri=%s", 

[libvirt] [PATCH RFC 0/2] libxl: migration v3 and p2p support

[Joao Martins]

Hey Jim,

These series introduce support for peer2peer and plain v3 migration without
extensible parameters. This patch series is organized as follows:

The first patch introduces the P2P where the source host must take care of 
connection handling with destination libvirtd and is responsible for performing
the migration. I introduced an additional method for the P2P Perform phase
since the default Perform of libxl is then reused. The sequence is the same as
v3 so it is possible to migrate to non-P2P host. libxlDoMigrateP2P actually
does the migration using a virConnectPtr to the destination and thus 

The second patch is mostly mechanical, and functional wise is very much the
same as the Params variant. The added value of introducing plain v3 is having
access to older APIs such virDomainMigrateToURI2 and virDomainMigrate (in P2P
mode) which are currently in use by Openstack/virt-manager.

My testing was performed in xen stable-4.5 and 4.4.3 (with the added patches in
the Xen CI loop) plus Openstack Kilo. In addition I backported the patches and
tested also on libvirt 1.2.14 (whic is also ran by the Xen CI Openstack loop).

Any comments or suggestions are welcome,

Thanks!
Joao

Joao Martins (2):
  libxl: add p2p migration
  libxl: add v3 migration w/o params

 src/libxl/libxl_driver.c| 226 +++-
 src/libxl/libxl_migration.c | 220 ++
 src/libxl/libxl_migration.h |  11 +++
 3 files changed, 454 insertions(+), 3 deletions(-)

-- 
2.1.4

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

Re: [libvirt] [PATCH] conf: fix crash when parse a disordered numa settings

[lhuang]

On 09/08/2015 04:40 PM, Michal Privoznik wrote:

On 08.09.2015 06:59, Luyao Huang wrote:

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

Introduced by 8fedbbdb, if we parse the disordered numa
cell, will get segfault. This is because we allow parse
the numa cell not in order and we alloc them before the
parse loop, the cpumask maybe NULL when parse each numa
cell.

Signed-off-by: Luyao Huang 
---
  src/conf/numa_conf.c   | 10 +---
  .../qemuxml2argv-cpu-numa-disordered.xml   | 26 +++
  .../qemuxml2xmlout-cpu-numa-disordered.xml | 29 ++
  tests/qemuxml2xmltest.c|  1 +
  4 files changed, 63 insertions(+), 3 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disordered.xml
  create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa-disordered.xml

I've reworded the commit message a bit, ACKed and pushed.


Thanks a lot for your quick review and help.


Michal


Luyao

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

Re: [libvirt] [PATCH] admin: Resolve leaked reference to private data

[Martin Kletzander]

On Mon, Sep 07, 2015 at 05:45:57PM +0200, Erik Skultety wrote:

Running valgrind on a very simplistic program consisting only from
opening and closing admin connection (virAdmConnect{Open,Close}) shows a
leak in remoteAdminPrivNew, because the last reference to privateData is
not decremented, thus the object won't be disposed. This patch unrefs
the privateData object once we closed the active connection to daemon,
making further use of this connection  useless.

==24577==at 0x4A089C7: calloc (in /usr/lib64/valgrind/vgpreload_***linux.so)
==24577==by 0x4E8835F: virAllocVar (viralloc.c:560)
==24577==by 0x4EDFA5C: virObjectNew (virobject.c:193)
==24577==by 0x4EDFBD4: virObjectLockableNew (virobject.c:219)
==24577==by 0x4C14DAF: remoteAdminPrivNew (libvirt-admin.c:152)
==24577==by 0x4C1537E: virAdmConnectOpen (libvirt-admin.c:308)
==24577==by 0x400BAD: main (listservers.c:39)

==24577== LEAK SUMMARY:
==24577==definitely lost: 80 bytes in 1 blocks
==24577==indirectly lost: 840 bytes in 6 blocks
==24577==  possibly lost: 0 bytes in 0 blocks
==24577==still reachable: 12,179 bytes in 199 blocks
==24577== suppressed: 0 bytes in 0 blocks
---
src/libvirt-admin.c | 1 +
1 file changed, 1 insertion(+)



ACK


diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
index b3fd0b3..5a4fc48 100644
--- a/src/libvirt-admin.c
+++ b/src/libvirt-admin.c
@@ -132,6 +132,7 @@ remoteAdminPrivFree(void *opaque)
virAdmConnectPtr conn = opaque;

remoteAdminConnectClose(conn);
+virObjectUnref(conn->privateData);
}

static remoteAdminPrivPtr
--
2.4.3

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

[libvirt] [PATCH RFC 5/7] libxl: implement virConnectGetAllDomainStats

[Joao Martins]

Introduce support for connectGetAllDomainStats call that
allow us to _all_ domain(s) statistics including network, block,
cpus and memory. Changes are rather mechanical and mostly
take care of the format to export the data.

Signed-off-by: Joao Martins 
---
 src/libxl/libxl_driver.c | 267 +++
 1 file changed, 267 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index fd952a3..01e97fd 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5284,6 +5284,272 @@ libxlDomainMemoryStats(virDomainPtr dom,
 
 #undef LIBXL_SET_MEMSTAT
 
+#define LIBXL_RECORD_UINT(error, key, value, ...) \
+do { \
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
+ key, ##__VA_ARGS__); \
+if (virTypedParamsAddUInt(>params, \
+  >nparams, \
+  , \
+  param_name, \
+  value) < 0) \
+goto error;   \
+} while (0)
+
+#define LIBXL_RECORD_LL(error, key, value, ...) \
+do { \
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
+ key, ##__VA_ARGS__); \
+if (virTypedParamsAddLLong(>params, \
+   >nparams, \
+   , \
+   param_name, \
+   value) < 0) \
+goto error; \
+} while (0)
+
+#define LIBXL_RECORD_ULL(error, key, value, ...) \
+do { \
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
+ key, ##__VA_ARGS__); \
+if (virTypedParamsAddULLong(>params, \
+>nparams, \
+, \
+param_name, \
+value) < 0) \
+goto error; \
+} while (0)
+
+#define LIBXL_RECORD_STR(error, key, value, ...) \
+do { \
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
+ key, ##__VA_ARGS__); \
+if (virTypedParamsAddString(>params, \
+>nparams, \
+, \
+param_name, \
+value) < 0) \
+goto error; \
+} while (0)
+
+static int
+libxlDomainGetStats(virConnectPtr conn,
+virDomainObjPtr dom,
+unsigned int stats,
+virDomainStatsRecordPtr *record)
+{
+libxlDriverPrivatePtr driver = conn->privateData;
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+virDomainStatsRecordPtr tmp;
+libxl_dominfo d_info;
+libxl_vcpuinfo *vcpuinfo = NULL;
+char *path;
+int maxcpu, hostcpus;
+unsigned long long mem, maxmem;
+int maxparams = 0;
+int ret = -1;
+size_t i, state;
+unsigned int domflags = stats & (VIR_DOMAIN_STATS_BALLOON |
+ VIR_DOMAIN_STATS_CPU_TOTAL);
+unsigned int vcpuflags = stats & VIR_DOMAIN_STATS_VCPU;
+unsigned int netflags = stats & VIR_DOMAIN_STATS_INTERFACE;
+unsigned int blkflags = stats & VIR_DOMAIN_STATS_BLOCK;
+
+if (VIR_ALLOC(tmp) < 0)
+return ret;
+
+mem = virDomainDefGetMemoryInitial(dom->def);
+maxmem = virDomainDefGetMemoryActual(dom->def);
+d_info.cpu_time = 0;
+
+if (domflags && virDomainObjIsActive(dom) &&
+!libxl_domain_info(cfg->ctx, _info, dom->def->id)) {
+mem = d_info.current_memkb;
+maxmem = d_info.max_memkb;
+}
+
+if (stats & VIR_DOMAIN_STATS_STATE) {
+LIBXL_RECORD_UINT(cleanup, "state.reason", dom->state.reason);
+LIBXL_RECORD_UINT(cleanup, "state.state", dom->state.state);
+}
+
+if (stats & VIR_DOMAIN_STATS_BALLOON) {
+LIBXL_RECORD_ULL(cleanup, "balloon.current", mem);
+LIBXL_RECORD_ULL(cleanup, "balloon.maximum", maxmem);
+}
+
+if (stats & VIR_DOMAIN_STATS_CPU_TOTAL)
+LIBXL_RECORD_ULL(cleanup, "cpu.time", d_info.cpu_time);
+
+if (vcpuflags)
+vcpuinfo = libxl_list_vcpu(cfg->ctx, dom->def->id, , );
+
+for (i = 0; i < dom->def->vcpus && vcpuflags; i++) {
+if (!vcpuinfo)
+state = VIR_VCPU_OFFLINE;
+else if (vcpuinfo[i].running)
+state = VIR_VCPU_RUNNING;
+else if (vcpuinfo[i].blocked)
+state = VIR_VCPU_BLOCKED;
+else
+state = VIR_VCPU_OFFLINE;
+
+LIBXL_RECORD_UINT(cleanup_vcpu, "vcpu.%zu.state", state, i);
+
+/* vcputime is shown only if the VM is active */
+if (!virDomainObjIsActive(dom))
+continue;
+
+LIBXL_RECORD_ULL(cleanup_vcpu, 

Re: [libvirt] [PATCH] conf: fix crash when parse a disordered numa settings

[Michal Privoznik]

On 08.09.2015 06:59, Luyao Huang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1260846
> 
> Introduced by 8fedbbdb, if we parse the disordered numa
> cell, will get segfault. This is because we allow parse
> the numa cell not in order and we alloc them before the
> parse loop, the cpumask maybe NULL when parse each numa
> cell.
> 
> Signed-off-by: Luyao Huang 
> ---
>  src/conf/numa_conf.c   | 10 +---
>  .../qemuxml2argv-cpu-numa-disordered.xml   | 26 +++
>  .../qemuxml2xmlout-cpu-numa-disordered.xml | 29 
> ++
>  tests/qemuxml2xmltest.c|  1 +
>  4 files changed, 63 insertions(+), 3 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disordered.xml
>  create mode 100644 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa-disordered.xml

I've reworded the commit message a bit, ACKed and pushed.

Michal

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

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

[Joao Martins]

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;
+}
+
+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 */
-- 
2.1.4

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

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

[Joao Martins]

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;
+
+chrused = -1;
+if ((sscanf(virtpath, "d%ip%i%n", , , )  >= 2
+ && chrused == strlen(virtpath) && disk < (1<<20) && partition < 256)
+||
+libxlDiskPathMatches(virtpath, "xvd",
+ , (1<<20)-1,
+ , 255)) {
+if (pdisk) *pdisk = disk;
+ 

[libvirt] [PATCH RFC 0/7] libxl: domain statistics support

[Joao Martins]

Hey Jim,

This series bring support for various statistics about domains
regarding CPU, Memory, Network Interfaces and BlockStats. Not all of
the statistics are implemented: qdisk support is missing in this series
and some of the memory statistics aren't available.

With this series we further implement 7 more functions of libvirt APIs.
It is organized as follows:

 * Patch 1, 2: implements cpu/memory statistics.
 * Patch 3, 4: implements (netback) network statistics and VBD block
   statistics. QDisk will follow up in a separate series
   regarding QEMU monitor integration.
 * Patch 5: implement fetching all domain statistics
 * Patch 6, 7: implements Job information.

Overall it looks big but 70% of the patch is due to #4 and #5 but doesn't
add necessarily more complexity to the driver I believe. Patch #6 and #7
are of special importance because GetJobInfo and GetJobStats are now used
in Openstack Kilo to monitor live-migration progress. This two patches
together with an earlier series [0] I sent before let us sucessfully
live-migrate with Openstack Kilo. Further with this series we get to
support nova diagnostics.

Tested this series on 4.4.3 and 4.5 setups plus Openstack Kilo.

Any comments or suggestions are welcome,

Thanks!
Joao

Joao Martins (7):
  libxl: implement virDomainGetCPUStats
  libxl: implement virDomainMemorystats
  libxl: implement virDomainInterfaceStats
  libxl: implement virDomainBlockStats
  libxl: implement virConnectGetAllDomainStats
  libxl: implement virDomainGetJobInfo
  libxl: implement virDomainGetJobStats

 configure.ac |2 +-
 src/libxl/libxl_domain.c |   29 ++
 src/libxl/libxl_domain.h |6 +
 src/libxl/libxl_driver.c | 1010 ++
 4 files changed, 1046 insertions(+), 1 deletion(-)

-- 
2.1.4

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

[libvirt] [PATCH RFC 7/7] libxl: implement virDomainGetJobStats

[Joao Martins]

Introduces support for domainGetJobStats which has the same
info as domainGetJobInfo but in a slightly different format.
Another difference is that virDomainGetJobStats can also
retrieve info on the most recently completed job. Though so
far this is only used in the source node to know if the
migration has been completed. But because we don't support
completed jobs we will deliver an error.

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 eaf6a67..31f0b39 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5319,6 +5319,58 @@ libxlDomainGetJobInfo(virDomainPtr dom,
 return ret;
 }
 
+static int
+libxlDomainGetJobStats(virDomainPtr dom,
+  int *type,
+  virTypedParameterPtr *params,
+  int *nparams,
+  unsigned int flags)
+{
+libxlDomainObjPrivatePtr priv;
+virDomainObjPtr vm;
+virDomainJobInfoPtr jobInfo;
+int ret = -1;
+int maxparams = 0;
+
+/* VIR_DOMAIN_JOB_STATS_COMPLETED not supported yet */
+virCheckFlags(0, -1);
+
+if (!(vm = libxlDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetJobStatsEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+priv = vm->privateData;
+jobInfo = priv->job.current;
+if (!priv->job.active) {
+*type = VIR_DOMAIN_JOB_NONE;
+*params = NULL;
+*nparams = 0;
+ret = 0;
+goto cleanup;
+}
+
+/* In libxl we don't have an estimed completion time
+ * thus we always set to unbounded and update time
+ * for the active job. */
+if (libxlDomainJobUpdateTime(>job) < 0)
+goto cleanup;
+
+if (virTypedParamsAddULLong(params, nparams, ,
+VIR_DOMAIN_JOB_TIME_ELAPSED,
+jobInfo->timeElapsed) < 0)
+goto cleanup;
+
+*type = jobInfo->type;
+ret = 0;
+
+ cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
 #undef LIBXL_SET_MEMSTAT
 
 #define LIBXL_RECORD_UINT(error, key, value, ...) \
@@ -6181,6 +6233,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
 .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
 .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
 .domainGetJobInfo = libxlDomainGetJobInfo, /* 1.2.20 */
+.domainGetJobStats = libxlDomainGetJobStats, /* 1.2.20 */
 .domainBlockStats = libxlDomainBlockStats, /* 1.2.20 */
 .domainBlockStatsFlags = libxlDomainBlockStatsFlags, /* 1.2.20 */
 .domainInterfaceStats = libxlDomainInterfaceStats, /* 1.2.20 */
-- 
2.1.4

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

[libvirt] [PATCH RFC 6/7] libxl: implement virDomainGetJobInfo

[Joao Martins]

Introduce support for domainGetJobInfo to get info about the
ongoing job. If the job is active it will update the
timeElapsed which is computed with the "started" field added to
struct libxlDomainJobObj.  For now we support just the very basic
info and all jobs have VIR_DOMAIN_JOB_UNBOUNDED (i.e. no completion
time estimation) plus timeElapsed computed.

Openstack Kilo uses the Job API to monitor live-migration
progress which is currently inexistent in libxl driver and
therefore leads to a crash in the nova compute node. Right
now, migration doesn't use jobs in the source node and will
return VIR_DOMAIN_JOB_NONE. Though nova handles this case and
will migrate it properly instead of crashing.

Signed-off-by: Joao Martins 
---
 src/libxl/libxl_domain.c | 29 +
 src/libxl/libxl_domain.h |  6 ++
 src/libxl/libxl_driver.c | 38 ++
 3 files changed, 73 insertions(+)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 40dcea1..d665615 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -74,6 +74,10 @@ libxlDomainObjInitJob(libxlDomainObjPrivatePtr priv)
 if (virCondInit(>job.cond) < 0)
 return -1;
 
+if (VIR_ALLOC(priv->job.current) < 0)
+return -1;
+
+memset(priv->job.current, 0, sizeof(*(priv->job.current)));
 return 0;
 }
 
@@ -84,12 +88,14 @@ libxlDomainObjResetJob(libxlDomainObjPrivatePtr priv)
 
 job->active = LIBXL_JOB_NONE;
 job->owner = 0;
+
 }
 
 static void
 libxlDomainObjFreeJob(libxlDomainObjPrivatePtr priv)
 {
 ignore_value(virCondDestroy(>job.cond));
+VIR_FREE(priv->job.current);
 }
 
 /* Give up waiting for mutex after 30 seconds */
@@ -131,6 +137,8 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver 
ATTRIBUTE_UNUSED,
 VIR_DEBUG("Starting job: %s", libxlDomainJobTypeToString(job));
 priv->job.active = job;
 priv->job.owner = virThreadSelfID();
+priv->job.started = now;
+priv->job.current->type = VIR_DOMAIN_JOB_UNBOUNDED;
 
 return 0;
 
@@ -179,6 +187,27 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver 
ATTRIBUTE_UNUSED,
 return virObjectUnref(obj);
 }
 
+int
+libxlDomainJobUpdateTime(struct libxlDomainJobObj *job)
+{
+virDomainJobInfoPtr jobInfo = job->current;
+unsigned long long now;
+
+if (!job->started)
+return 0;
+
+if (virTimeMillisNow() < 0)
+return -1;
+
+if (now < job->started) {
+job->started = 0;
+return 0;
+}
+
+jobInfo->timeElapsed = now - job->started;
+return 0;
+}
+
 static void *
 libxlDomainObjPrivateAlloc(void)
 {
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index 44b3e0b..1c1eba3 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -53,6 +53,8 @@ struct libxlDomainJobObj {
 virCond cond;   /* Use to coordinate jobs */
 enum libxlDomainJob active; /* Currently running job */
 int owner;  /* Thread which set current job */
+unsigned long long started; /* When the job started */
+virDomainJobInfoPtr current;/* Statistics for the current job */
 };
 
 typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate;
@@ -88,6 +90,10 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver,
  virDomainObjPtr obj)
 ATTRIBUTE_RETURN_CHECK;
 
+int
+libxlDomainJobUpdateTime(struct libxlDomainJobObj *job)
+ATTRIBUTE_RETURN_CHECK;
+
 void
 libxlDomainEventQueue(libxlDriverPrivatePtr driver,
   virObjectEventPtr event);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 01e97fd..eaf6a67 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5282,6 +5282,43 @@ libxlDomainMemoryStats(virDomainPtr dom,
 return ret;
 }
 
+static int
+libxlDomainGetJobInfo(virDomainPtr dom,
+  virDomainJobInfoPtr info)
+{
+libxlDomainObjPrivatePtr priv;
+virDomainObjPtr vm;
+int ret = -1;
+
+if (!(vm = libxlDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetJobInfoEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+priv = vm->privateData;
+if (!priv->job.active) {
+memset(info, 0, sizeof(*info));
+info->type = VIR_DOMAIN_JOB_NONE;
+ret = 0;
+goto cleanup;
+}
+
+/* In libxl we don't have an estimed completion time
+ * thus we always set to unbounded and update time
+ * for the active job. */
+if (libxlDomainJobUpdateTime(>job) < 0)
+goto cleanup;
+
+memcpy(info, priv->job.current, sizeof(virDomainJobInfo));
+ret = 0;
+
+ cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
 #undef LIBXL_SET_MEMSTAT
 
 #define LIBXL_RECORD_UINT(error, key, value, ...) \
@@ -6143,6 +6180,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
 #endif
 .nodeGetFreeMemory = 

[libvirt] [PATCH RFC 2/7] libxl: implement virDomainMemorystats

[Joao Martins]

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;
+}
+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 */
-- 
2.1.4

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

[libvirt] [PATCH RFC 1/7] libxl: implement virDomainGetCPUStats

[Joao Martins]

Introduce support for domainGetCPUStats API call and consequently
allow us to use `virsh cpu-stats`. The latter returns a more brief
output than the one provided by`virsh vcpuinfo`.

Signed-off-by: Joao Martins 
---
 src/libxl/libxl_driver.c | 111 +++
 1 file changed, 111 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 5f69b49..101e4c7 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -73,6 +73,8 @@ VIR_LOG_INIT("libxl.libxl_driver");
 #define LIBXL_CONFIG_FORMAT_XM "xen-xm"
 #define LIBXL_CONFIG_FORMAT_SEXPR "xen-sxpr"
 
+#define LIBXL_NB_TOTAL_CPU_STAT_PARAM 1
+
 #define HYPERVISOR_CAPABILITIES "/proc/xen/capabilities"
 #define HYPERVISOR_XENSTORED "/dev/xen/xenstored"
 
@@ -4638,6 +4640,114 @@ libxlDomainIsUpdated(virDomainPtr dom)
 }
 
 static int
+libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
+virDomainObjPtr vm,
+virTypedParameterPtr params,
+unsigned int nparams)
+{
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+libxl_dominfo d_info;
+int ret = -1;
+
+if (nparams == 0)
+return LIBXL_NB_TOTAL_CPU_STAT_PARAM;
+
+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 ret;
+}
+
+if (virTypedParameterAssign([0], VIR_DOMAIN_CPU_STATS_CPUTIME,
+VIR_TYPED_PARAM_ULLONG, d_info.cpu_time) < 0)
+return ret;
+
+return nparams;
+}
+
+static int
+libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
+virDomainObjPtr vm,
+virTypedParameterPtr params,
+unsigned int nparams,
+int start_cpu,
+unsigned int ncpus)
+{
+libxl_vcpuinfo *vcpuinfo;
+int maxcpu, hostcpus;
+size_t i;
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+int ret = -1;
+
+if (nparams == 0 && ncpus != 0)
+return LIBXL_NB_TOTAL_CPU_STAT_PARAM;
+else if (nparams == 0)
+return vm->def->maxvcpus;
+
+if ((vcpuinfo = libxl_list_vcpu(cfg->ctx, vm->def->id, ,
+)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to list vcpus for domain '%d' with 
libxenlight"),
+   vm->def->id);
+goto cleanup;
+}
+
+for (i = start_cpu; i < maxcpu && i < ncpus; ++i) {
+if (virTypedParameterAssign([(i-start_cpu)],
+VIR_DOMAIN_CPU_STATS_CPUTIME,
+VIR_TYPED_PARAM_ULLONG,
+vcpuinfo[i].vcpu_time) < 0)
+goto cleanup;
+
+libxl_vcpuinfo_dispose([i]);
+}
+ret = nparams;
+
+ cleanup:
+VIR_FREE(vcpuinfo);
+return ret;
+}
+
+static int
+libxlDomainGetCPUStats(virDomainPtr dom,
+   virTypedParameterPtr params,
+   unsigned int nparams,
+   int start_cpu,
+   unsigned int ncpus,
+   unsigned int flags)
+{
+libxlDriverPrivatePtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+int ret = -1;
+
+virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+if (!(vm = libxlDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetCPUStatsEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("domain is not running"));
+goto cleanup;
+}
+
+if (start_cpu == -1)
+ret = libxlDomainGetTotalCPUStats(driver, vm, params, nparams);
+else
+ret = libxlDomainGetPerCPUStats(driver, vm, params, nparams,
+  start_cpu, ncpus);
+
+ cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
+static int
 libxlConnectDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, int 
eventID,
virConnectDomainEventGenericCallback 
callback,
void *opaque, virFreeCallback freecb)
@@ -5230,6 +5340,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
 #endif
 .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
 .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
+.domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.20 */
 .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */
 .connectDomainEventDeregister = libxlConnectDomainEventDeregister, /* 
0.9.0 */
 .domainManagedSave = 

Re: [libvirt] [PATCH] admin: Resolve leaked reference to private data

[Erik Skultety]

On 08/09/15 10:04, Martin Kletzander wrote:
> On Mon, Sep 07, 2015 at 05:45:57PM +0200, Erik Skultety wrote:
>> Running valgrind on a very simplistic program consisting only from
>> opening and closing admin connection (virAdmConnect{Open,Close}) shows a
>> leak in remoteAdminPrivNew, because the last reference to privateData is
>> not decremented, thus the object won't be disposed. This patch unrefs
>> the privateData object once we closed the active connection to daemon,
>> making further use of this connection  useless.
>>
>> ==24577==at 0x4A089C7: calloc (in
>> /usr/lib64/valgrind/vgpreload_***linux.so)
>> ==24577==by 0x4E8835F: virAllocVar (viralloc.c:560)
>> ==24577==by 0x4EDFA5C: virObjectNew (virobject.c:193)
>> ==24577==by 0x4EDFBD4: virObjectLockableNew (virobject.c:219)
>> ==24577==by 0x4C14DAF: remoteAdminPrivNew (libvirt-admin.c:152)
>> ==24577==by 0x4C1537E: virAdmConnectOpen (libvirt-admin.c:308)
>> ==24577==by 0x400BAD: main (listservers.c:39)
>>
>> ==24577== LEAK SUMMARY:
>> ==24577==definitely lost: 80 bytes in 1 blocks
>> ==24577==indirectly lost: 840 bytes in 6 blocks
>> ==24577==  possibly lost: 0 bytes in 0 blocks
>> ==24577==still reachable: 12,179 bytes in 199 blocks
>> ==24577== suppressed: 0 bytes in 0 blocks
>> ---
>> src/libvirt-admin.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
> 
> ACK

Thanks, pushed now.

Erik

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

Re: [libvirt] [PATCH] docs: Remove last use of double semicolon in Makefile

[Pavel Hrdina]

On Mon, Sep 07, 2015 at 10:36:18AM +0200, Martin Kletzander wrote:
> Double semicolons have special meaning in makefiles, but they would have
> to be combined with other rules witch such separators in order to be
> used as intended.  Since there are no other rules like that, let's
> clean it up.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  docs/Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

ACK, yes, we don't need the double-colon rule here.

Pavel

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

Re: [libvirt] [PATCHv2] qemu: Validate address type when attaching a disk device.

[Ján Tomko]

On Mon, Sep 07, 2015 at 07:22:01PM +0800, Ruifeng Bian wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1257844
> 
> Attach-device can hotplug a virtio disk device with any address type now,
> it need to validate the address type before the attachment.
> 
> Coldplug a scsi disk device without checking the address type in current
> version, this patch also fix the scsi disk problem.
> ---
>  src/qemu/qemu_driver.c  |  8 
>  src/qemu/qemu_hotplug.c | 18 ++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 91eb661..af926fc 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8050,6 +8050,14 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
>  if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
>  if (virDomainDefAddImplicitControllers(vmdef) < 0)
>  return -1;
> +/* scsi disk should have an address type of driver */
> +if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI &&
> +(disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("scsi disk cannot have an address of type 
> '%s'"),
> +   
> virDomainDeviceAddressTypeToString(disk->info.type));
> +return -1;
> +}

This is the check for DISK_BUS_SCSI, qemuDomainAssignAddresses does the
check for DISK_BUS_VIRTIO.

Should we check other bus types as well?

I think the most important part of the bug is that we allow hotplugging
a device with broken configuration, then fail to unplug it because it is
broken. Here detach-device works with --config even with an incorrect
address type.

>  if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
>  return -1;
>  break;
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 63fafa6..4226650 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -335,6 +335,24 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
>  if (!qemuCheckCCWS390AddressSupport(vm->def, disk->info, 
> priv->qemuCaps,
>  disk->dst))
>  goto cleanup;
> +
> +/* virtio device should either have a ccw or pci address */
> +if (qemuDomainMachineIsS390CCW(vm->def) &&
> +virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
> +if (!virDomainDeviceAddressIsValid(>info,
> +   
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) {
> +virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +   _("device cannot be attached without a valid 
> CCW address"));
> +goto cleanup;
> +}

Is hotplugging a device with a S390 type address impossible on a s390-ccw
machine?

> +} else {
> +if (!virDomainDeviceAddressIsValid(>info,
> +   
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
> +virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +   _("device cannot be attached without a valid 
> PCI address"));

These are not the only two options. At this point, the address type can
also be VIRTIO_S390 for non-CCW S390 machines, and ADDRESS_TYPE_NONE
(and a PCI address will be generated by virDomainPCIAddressEnsureAddr).

The checks seem to be copied from qemuDomainDetachVirtioDiskDevice. For
ancient QEMUs which did not support -device, we use the PCI address to
uniquely identify the device. In that case, detaching a virtio disk with
no address is not possible.

For all reasonable versions of QEMU, the device alias is used, so
detaching devices with other address types is possible.
But we explicitly require a PCI or CCW address, to make sure we will
unplug only the device requested by user. S390 seems to be missing here,
but that might be on purpose.

Allowing S390 addresses on hotplug without allowing them on hotunplug
won't fix the linked bug for S390 machines, but attaching a device with
no PCI address is a valid use case - libvirt will just generate one.

> +goto cleaup;

Please make sure 'make check' and 'make syntax-check' pass before
sending a patch to the list, as described on our hacking page:
http://libvirt.org/hacking.html

Jan


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

[libvirt] [PATCH] migration: support all toURI and proto combos

[Nikolay Shirokovskiy]

Current implementation of 'toURI' migration interfaces does not support all
combinations of interface versions and protocol versions. For example 'toURI2'
with p2p flag will not migrate if driver supports only v3params proto.

This is not convinient as drivers that starts to support migration have to
manually support older versions of protocol. I guess this should be done in
one place, namely here.

Another issue is that there are a lot of code duplication in implementation of
toURI interfaces and it is not obvious from code how are they related.

This implementation uses extensible parameters as intermediate parameters
representation. This is possible as interfaces are done backward compatible in
terms of parameters and later versions supports all parameters of former
versions.

Another change that is convinient to add here is reusing the code for p2p and
direct migration. The only major difference between them is how URIs are passed
in places that supports both connection and migration uri. Now this difference
is plain in code. See comments in code for details.

Minor p2p vs direct difference is that direct not support proto with extensible
parameters. This behaviour is saved.

Unobvious check of migration dconnuri in case of p2p migration is actually
protection from migration to localhost (from cover letter to patch 59d13aae
that introduced it). So reformat this check to make it goal obvious.

==Behaviour changes

Previous implementation of direct migration demands that proto v1 is
implemented even if proto v3 is used. As this is somewhat against the purpuse
of this patch this behaviour is dropped.

Another behavior change that introduced is that now offline flag examined
against driver migration capabilities for all api version not only for toURI1.
It was odd taking into account that all toURI uses common peer2peer and direct
implementation.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/libvirt-domain.c |  564 +-
 1 files changed, 232 insertions(+), 332 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cbf08fc..721d820 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3291,181 +3291,209 @@ virDomainMigrateVersion3Params(virDomainPtr domain,
 params, nparams, true, flags);
 }
 
-
-/*
- * In normal migration, the libvirt client co-ordinates communication
- * between the 2 libvirtd instances on source & dest hosts.
- *
- * In this peer-2-peer migration alternative, the libvirt client
- * only talks to the source libvirtd instance. The source libvirtd
- * then opens its own connection to the destination and co-ordinates
- * migration itself.
- *
- * If useParams is true, params and nparams contain migration parameters and
- * we know it's safe to call the API which supports extensible parameters.
- * Otherwise, we have to use xmlin, dname, uri, and bandwidth and pass them
- * to the old-style APIs.
- */
 static int
-virDomainMigratePeer2PeerFull(virDomainPtr domain,
-  const char *dconnuri,
-  const char *xmlin,
-  const char *dname,
-  const char *uri,
-  unsigned long long bandwidth,
-  virTypedParameterPtr params,
-  int nparams,
-  bool useParams,
-  unsigned int flags)
+virDomainMigrateURIproto3(virDomainPtr domain,
+  const char *dconnuri,
+  virTypedParameterPtr params,
+  int nparams,
+  unsigned int flags)
 {
-virURIPtr tempuri = NULL;
+const char *compatParams[] = { VIR_MIGRATE_PARAM_URI,
+   VIR_MIGRATE_PARAM_DEST_NAME,
+   VIR_MIGRATE_PARAM_DEST_XML,
+   VIR_MIGRATE_PARAM_BANDWIDTH };
+const char *uri = NULL;
+const char *dname = NULL;
+const char *dxml = NULL;
+unsigned long long bandwidth = 0;
 
-VIR_DOMAIN_DEBUG(domain,
- "dconnuri=%s, xmlin=%s, dname=%s, uri=%s, bandwidth=%llu "
- "params=%p, nparams=%d, useParams=%d, flags=%x",
- dconnuri, NULLSTR(xmlin), NULLSTR(dname), NULLSTR(uri),
- bandwidth, params, nparams, useParams, flags);
-VIR_TYPED_PARAMS_DEBUG(params, nparams);
+if (!virTypedParamsCheck(params, nparams, compatParams,
+ ARRAY_CARDINALITY(compatParams))) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+   _("Migration does not support some of parameters."));
+return -1;
+}
 
-if ((useParams && !domain->conn->driver->domainMigratePerform3Params) ||
-(!useParams &&
- 

Re: [libvirt] [PATCH] util: Add win32 version of virFileUnlink

[Pavel Hrdina]

On Mon, Sep 07, 2015 at 10:25:24AM +0200, Martin Kletzander wrote:
> Commit 35847860f65f Added the virFileUnlink function, but failed to add
> a version for mingw build, causing the following error:
> 
>   Cannot export virFileUnlink: symbol not defined
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/util/virfile.c | 14 ++
>  1 file changed, 14 insertions(+)
> 

ACK, it's a build breaker fix for MinGW builds.

Pavel

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

Re: [libvirt] [PATCH v2 2/2] virsh: Enhance the detailed output of domblklist for networked source

[Peter Krempa]

On Wed, Sep 02, 2015 at 17:58:19 +0200, Michal Privoznik wrote:
> From: Lin Ma 
> 
> Format & output more detailed information about networked source
> 
> e.g: The output without the patch:
> $ virsh domblklist $DOMAIN --details
> Type   Device Target Source
> 
> networkdisk   vdatest-pool/image
> networkdisk   vdbiqn.2015-08.org.example:sn01/0
> networkdisk   vdc/image.raw
> networkdisk   vdd-
> networkdisk   vde-
> networkdisk   vdfimage1
> networkdisk   vdgtest-volume/image.raw
> 
> The output with the patch:
> $ virsh domblklist $DOMAIN --details
> Type   Device Target Source
> 
> networkdisk   vda
> rbd://monitor1.example.org:6321/test-pool/image

One other thing to note is that RBD volumes may have multiple hosts,
which is not taken into account by the above format ...

> networkdisk   vdb
> iscsi://192.168.124.200:3260/iqn.2015-08.org.example:sn01/0
> networkdisk   vdchttp://192.168.124.200:80/image.raw
> networkdisk   vddnbd+unix:///var/run/nbdsock
> networkdisk   vdenbd://192.168.124.200:12345
> networkdisk   vdfsheepdog://192.168.124.200:6000/image1
> networkdisk   vdg
> gluster://192.168.124.200/test-volume/image.raw

... and gluster volumes will possibly have multiple sources too.

> 
> Signed-off-by: Lin Ma 
> Signed-off-by: Michal Privoznik 
> ---
>  tools/virsh-domain-monitor.c | 64 
> 
>  1 file changed, 59 insertions(+), 5 deletions(-)

Peter


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

[libvirt] [PATCH] Ignore virtio-mmio disks in qemuAssignDevicePCISlots()

[Pavel Fedin]

Fixes the following error when attempting to add a disk with bus='virtio':

virtio only support device address type 'PCI'

Signed-off-by: Pavel Fedin 
---
 src/qemu/qemu_command.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 38104da..408b249 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2189,12 +2189,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
 if (def->disks[i]->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
 continue;
 
-/* don't touch s390 devices */
+/* don't touch s390 and virtio-mmio devices */
 if (def->disks[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI ||
 def->disks[i]->info.type ==
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 ||
 def->disks[i]->info.type ==
-VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
+VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW ||
+def->disks[i]->info.type ==
+VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
 continue;
 
 if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
-- 
1.9.5.msysgit.0


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

Re: [libvirt] [PATCH] migration: support all toURI and proto combos

[Jiri Denemark]

On Tue, Sep 08, 2015 at 13:43:09 +0300, Nikolay Shirokovskiy wrote:
> Current implementation of 'toURI' migration interfaces does not support all
> combinations of interface versions and protocol versions. For example 'toURI2'
> with p2p flag will not migrate if driver supports only v3params proto.

In general, this patch is pretty large and hard to review, I suggest
splitting it into a series of several shorter patches. They all need to
compile and work, but it shouldn't be too hard in this case.

> This is not convinient as drivers that starts to support migration have to
> manually support older versions of protocol. I guess this should be done in
> one place, namely here.

Well, the thing is all the code you're changing runs on a *client* and
then the appropriate API calls are sent as RPC to a daemon. So just
checking what APIs are implemented by the *client's* remote driver to
choose what API will be called on the daemon is wrong. The client can be
new enough to support all migration APIs while the daemon may be pretty
old supporting only a few of them. Thus, for backward compatibility, the
client has to either check what API is supported by the daemon (via
VIR_DRV_SUPPORTS_FEATURE(..., VIR_DRV_FEATURE_MIGRATION_...)) or it has
to be conservative and call the oldest API which supports the parameters
passed by the application/user.

Moreover, various versions of virDomainMigrateToURI are not really about
the migration protocol to be used. They differ only in the set of
parameters, the actual migration protocol will be decided by the daemon
itself after making a connection to the destination daemon. That said,
it should be fairly easy to implement all the entry points in a driver
while still allowing only protocol version 3.

> Another issue is that there are a lot of code duplication in implementation of
> toURI interfaces and it is not obvious from code how are they related.
> 
> This implementation uses extensible parameters as intermediate parameters
> representation. This is possible as interfaces are done backward compatible in
> terms of parameters and later versions supports all parameters of former
> versions.
> 
> Another change that is convinient to add here is reusing the code for p2p and
> direct migration. The only major difference between them is how URIs are 
> passed
> in places that supports both connection and migration uri. Now this difference
> is plain in code. See comments in code for details.

Sounds like this could be separated.

> Minor p2p vs direct difference is that direct not support proto with 
> extensible
> parameters. This behaviour is saved.
> 
> Unobvious check of migration dconnuri in case of p2p migration is actually
> protection from migration to localhost (from cover letter to patch 59d13aae
> that introduced it). So reformat this check to make it goal obvious.

Another very good candidate for a separate patch.

> ==Behaviour changes
> 
> Previous implementation of direct migration demands that proto v1 is
> implemented even if proto v3 is used. As this is somewhat against the purpuse
> of this patch this behaviour is dropped.

Overall, if you see a way to refactor and improve the code so that it's
more readable and maintainable, just go ahead, but be careful about this
kind of changes in behavior.

Jirka

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

Re: [libvirt] [PATCH] qemu: migration: Relax enforcement of memory hotplug support

[Jiri Denemark]

On Tue, Sep 08, 2015 at 15:09:37 +0200, Peter Krempa wrote:
> If the current live definition does not have memory hotplug enabled, but
> the persistent one does libvirt would reject migration if the
> destination does not support memory hotplug even if the user didn't want
> to persist the VM at the destination and thus the XML containing the
> memory hotplug definition would not be used. To fix this corner case the
> code will check for memory hotplug in the newDef only if
> VIR_MIGRATE_PERSIST_DEST was used.
> ---
>  src/qemu/qemu_migration.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index d50d367..1846239 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2989,7 +2989,8 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
>  }
> 
>  if (vm->def->mem.max_memory ||
> -(vm->newDef &&
> +((flags & VIR_MIGRATE_PERSIST_DEST) &&
> + vm->newDef &&
>   vm->newDef->mem.max_memory))
>  cookieFlags |= QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG;
> 

ACK

Jirka

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

Re: [libvirt] [PATCH] Allow execute access to /var/lib/libvirt/qemu/ for others

[Jiri Denemark]

On Tue, Sep 08, 2015 at 19:07:09 +0200, Martin Kletzander wrote:
> Commit f1f68ca33433 tried fixing running multiple domains under various
> users, but if the user can't browse the directory, it's hard for the
> qemu running under that user to create the monitor socket.
> 
> The permissions need to be fixed in two places due to support for both
> installations with and without driver modules.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146886
> 
> Signed-off-by: Martin Kletzander 
> ---
> This is not a problem for non-rpm installs because normal make install
> will not change the permissions, it will just create the directory, so
> it has 0755, but that difference is not something I'm trying to fix in
> this patch.
> 
>  libvirt.spec.in | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index bb8bfc3c25c1..48461e865dc8 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -2002,7 +2002,7 @@ exit 0
>  %config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu
>  %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/
>  %ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/
> -%dir %attr(0750, %{qemu_user}, %{qemu_group}) 
> %{_localstatedir}/lib/libvirt/qemu/
> +%dir %attr(0751, %{qemu_user}, %{qemu_group}) 
> %{_localstatedir}/lib/libvirt/qemu/

Seems OK, but are we sure every file created in that directory uses 007
mask? Otherwise, we would be opening a hole here...

Jirka

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

Re: [libvirt] [PATCH] Distribute only generated virkeymaps.h

[Daniel P. Berrange]

On Tue, Sep 08, 2015 at 03:06:41PM +0200, Martin Kletzander wrote:
> We are distributing virkeymaps.h and all the tools needed to rebuild
> that file.  On top of that, we are generating that file into the
> $(srcdir) and that sometimes fails for me when trying to do make dist in
> VPATH on rawhide fedora.  And we don't clean the file when
> maintainer-clean make target is requested.

I'd suggest we do the opposite - don't distribute virkeymaps.h - we
already require users to have python, so we should be easily able
to generate it at build time.

> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/Makefile.am | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 429137561c6f..c2784af299dc 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -173,15 +173,13 @@ UTIL_SOURCES =  
> \
>   $(NULL)
> 
> 
> -EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
> - $(srcdir)/util/virkeycode-mapgen.py

Keep keymaps.csv + virkeycode-mapgen.py

> -
>  BUILT_SOURCES += util/virkeymaps.h
> +MAINTAINERCLEANFILES += util/virkeymaps.h

Change to CLEANFILES

> 
>  util/virkeymaps.h: $(srcdir)/util/keymaps.csv\
>   $(srcdir)/util/virkeycode-mapgen.py
>   $(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py \
> -   <$(srcdir)/util/keymaps.csv >$(srcdir)/util/virkeymaps.h
> +   <$(srcdir)/util/keymaps.csv >util/virkeymaps.h
> 
>  # Internal generic driver infrastructure
>  NODE_INFO_SOURCES = nodeinfo.h nodeinfo.c nodeinfopriv.h



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] Ignore virtio-mmio disks in qemuAssignDevicePCISlots()

[Martin Kletzander]

[Please don't Cc random people when they did not request it, all
developers are subscribed to the list]

On Tue, Sep 08, 2015 at 03:23:27PM +0300, Pavel Fedin wrote:

Fixes the following error when attempting to add a disk with bus='virtio':

virtio only support device address type 'PCI'



How did you manage to do that?  I think we are not handlind
virtio-mmio addressing properly as we won't add some controller qemu
will then be missing.  Shouldn't that be fixed as well?  And isn't
virtio-mmio only supported on some platforms?

Otherwise the change makes sense.


Signed-off-by: Pavel Fedin 
---
src/qemu/qemu_command.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 38104da..408b249 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2189,12 +2189,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
if (def->disks[i]->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
continue;

-/* don't touch s390 devices */
+/* don't touch s390 and virtio-mmio devices */
if (def->disks[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI ||
def->disks[i]->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 ||
def->disks[i]->info.type ==
-VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
+VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW ||
+def->disks[i]->info.type ==
+VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
continue;

if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
--
1.9.5.msysgit.0




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

[libvirt] [PATCH] Distribute only generated virkeymaps.h

[Martin Kletzander]

We are distributing virkeymaps.h and all the tools needed to rebuild
that file.  On top of that, we are generating that file into the
$(srcdir) and that sometimes fails for me when trying to do make dist in
VPATH on rawhide fedora.  And we don't clean the file when
maintainer-clean make target is requested.

Signed-off-by: Martin Kletzander 
---
 src/Makefile.am | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 429137561c6f..c2784af299dc 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -173,15 +173,13 @@ UTIL_SOURCES =
\
$(NULL)


-EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
-   $(srcdir)/util/virkeycode-mapgen.py
-
 BUILT_SOURCES += util/virkeymaps.h
+MAINTAINERCLEANFILES += util/virkeymaps.h

 util/virkeymaps.h: $(srcdir)/util/keymaps.csv  \
$(srcdir)/util/virkeycode-mapgen.py
$(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py \
- <$(srcdir)/util/keymaps.csv >$(srcdir)/util/virkeymaps.h
+ <$(srcdir)/util/keymaps.csv >util/virkeymaps.h

 # Internal generic driver infrastructure
 NODE_INFO_SOURCES = nodeinfo.h nodeinfo.c nodeinfopriv.h
-- 
2.5.1

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

Re: [libvirt] [PATCH] migration: support all toURI and proto combos

[Nikolay Shirokovskiy]

On 08.09.2015 15:14, Jiri Denemark wrote:
> On Tue, Sep 08, 2015 at 13:43:09 +0300, Nikolay Shirokovskiy wrote:
>> Current implementation of 'toURI' migration interfaces does not support all
>> combinations of interface versions and protocol versions. For example 
>> 'toURI2'
>> with p2p flag will not migrate if driver supports only v3params proto.
> 
> In general, this patch is pretty large and hard to review, I suggest
> splitting it into a series of several shorter patches. They all need to
> compile and work, but it shouldn't be too hard in this case.
> 
>> This is not convinient as drivers that starts to support migration have to
>> manually support older versions of protocol. I guess this should be done in
>> one place, namely here.
> 
> Well, the thing is all the code you're changing runs on a *client* and
> then the appropriate API calls are sent as RPC to a daemon. So just
> checking what APIs are implemented by the *client's* remote driver to
> choose what API will be called on the daemon is wrong. The client can be
> new enough to support all migration APIs while the daemon may be pretty
> old supporting only a few of them. Thus, for backward compatibility, the
> client has to either check what API is supported by the daemon (via
> VIR_DRV_SUPPORTS_FEATURE(..., VIR_DRV_FEATURE_MIGRATION_...)) or it has
> to be conservative and call the oldest API which supports the parameters
> passed by the application/user.
This approach(namely checking driver capabilities beforehand) is already
present in code and i dont change it.

> 
> Overall, if you see a way to refactor and improve the code so that it's
> more readable and maintainable, just go ahead, but be careful about this
> kind of changes in behavior.
> 
Ok. I'll split into a more managable patches.
> Jirka
> 

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

Re: [libvirt] [PATCH] util: Add win32 version of virFileUnlink

[John Ferlan]

On 09/07/2015 04:25 AM, Martin Kletzander wrote:
> Commit 35847860f65f Added the virFileUnlink function, but failed to add
> a version for mingw build, causing the following error:
> 
>   Cannot export virFileUnlink: symbol not defined
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/util/virfile.c | 14 ++
>  1 file changed, 14 insertions(+)
> 

Ugh... Sorry about this one... Too bad there wasn't a way to have some
sort of make check rule - it wasn't the first and won't be the last time
 ;-)

John
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 408d2d912f13..75819d9c8bd7 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -2637,6 +2637,20 @@ virDirCreate(const char *path ATTRIBUTE_UNUSED,
> 
>  return -ENOSYS;
>  }
> +
> +int
> +virFileUnlink(const char *path,
> +  uid_t uid ATTRIBUTE_UNUSED,
> +  gid_t gid ATTRIBUTE_UNUSED)
> +{
> +if (unlink(path) < 0) {
> +virReportSystemError(errno, _("Unable to unlink path '%s'"),
> + path);
> +return -1;
> +}
> +
> +return 0;
> +}
>  #endif /* WIN32 */
> 
>  /**
> 

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

[libvirt] [PATCH] qemu: migration: Relax enforcement of memory hotplug support

[Peter Krempa]

If the current live definition does not have memory hotplug enabled, but
the persistent one does libvirt would reject migration if the
destination does not support memory hotplug even if the user didn't want
to persist the VM at the destination and thus the XML containing the
memory hotplug definition would not be used. To fix this corner case the
code will check for memory hotplug in the newDef only if
VIR_MIGRATE_PERSIST_DEST was used.
---
 src/qemu/qemu_migration.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d50d367..1846239 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2989,7 +2989,8 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
 }

 if (vm->def->mem.max_memory ||
-(vm->newDef &&
+((flags & VIR_MIGRATE_PERSIST_DEST) &&
+ vm->newDef &&
  vm->newDef->mem.max_memory))
 cookieFlags |= QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG;

-- 
2.4.5

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

Re: [libvirt] [PATCH 1/2] qemuProcessStart: Parse backingStore for all disks

[Peter Krempa]

On Tue, Sep 08, 2015 at 17:17:18 +0200, Michal Privoznik wrote:

Subject is misleading. Backing store is detected from disk images not
parsed from XML.

> Later, in the next commit, we are going to need backing chain
> for all the disks within the domain. Let's parse them at domain
> startup phase.

Why do we need this patch? Currently qemuDomainDetermineDiskChain still
gets called on all disks where it makes sense. Exceptions are:
1) empty drives (this is also redundant, since
qemuDomainDetermineDiskChain will check it again).

2) disk source formats that don't support backing chains, where we just
check that the file exists.

This patch would make us call that on those too.

NACK,

Peter


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

[libvirt] [PATCH 0/2] Recursively lock backing chain

[Michal Privoznik]

This is how I tested the feature:

Create couple of disks:

qemu-img create -f qcow2 /var/lib/libvirt/images/test1.qcow2 1G
qemu-img create -f qcow2 /var/lib/libvirt/images/test2.qcow2 -b 
/var/lib/libvirt/images/test1.qcow2
qemu-img create -f qcow2 /var/lib/libvirt/images/test3.qcow2 -b 
/var/lib/libvirt/images/test1.qcow2

Then adjust configuration of two domains:

virsh # domblklist gentoo
Target Source

fda/var/lib/libvirt/images/fd.img
vda/var/lib/libvirt/images/gentoo.qcow2
vdb/var/lib/libvirt/images/test1.qcow2
hdc/home/zippy/tmp/install-amd64-minimal-20150402.iso

virsh # domblklist fedora
Target Source

sda/var/lib/libvirt/images/fedora.qcow2
sdb/var/lib/libvirt/images/test2.qcow2
hda-


With this configuration, fedora and gentoo should not be able to run at the 
same time:

virsh # start fedora
Domain fedora started

virsh # start gentoo
error: Failed to start domain gentoo
error: resource busy: Lockspace resource '/var/lib/libvirt/images/test1.qcow2' 
is locked

Cool!

But with slight change, gentoo should be able to run:

virsh # domblklist gentoo
Target Source

fda/var/lib/libvirt/images/fd.img
vda/var/lib/libvirt/images/gentoo.qcow2
vdb/var/lib/libvirt/images/test3.qcow2
hdc/home/zippy/tmp/install-amd64-minimal-20150402.iso

virsh # start gentoo
Domain gentoo started

virsh # list
 IdName   State

 10fedora running
 11gentoo running


Awesome!

Michal Privoznik (2):
  qemuProcessStart: Parse backingStore for all disks
  virDomainLockManagerAddImage: Recursively lock backing chain

 src/locking/domain_lock.c | 53 +--
 src/qemu/qemu_domain.c| 11 +++---
 src/qemu/qemu_process.c   |  6 +-
 3 files changed, 41 insertions(+), 29 deletions(-)

-- 
2.4.6

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

Re: [libvirt] [PATCH 2/2] virDomainLockManagerAddImage: Recursively lock backing chain

[Daniel P. Berrange]

On Tue, Sep 08, 2015 at 05:59:45PM +0200, Peter Krempa wrote:
> On Tue, Sep 08, 2015 at 17:17:19 +0200, Michal Privoznik wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1192399
> > 
> > It's known fact for a while now that we should not only lock the
> > top level layers of backing chain but the rest of it too. And
> > well known too that we are not doing that. Well, up until this
> > commit. The reason is that while one guest can have for instance
> > the following backing chain: A (top) -> B -> C (bottom), the
> > other can be started just over B or C. But libvirt should prevent
> > that as it will almost certainly mangle the backing chain for the
> > former guest. On the other hand, we want to allow guest with the
> 
> Well, the corruption may still happen since if you run the guest that
> uses image 'B' for writing, without starting the one that uses 'A' the
> image 'A' will be invalidated anyways.

Yep, the lock manager won't protect against you doing stupid
stuff to the base image, which invalidates children, but that's
out of scope really. We only need to consider the concurrent
running VM problem here.


> > following backing chain: D (top) -> B -> C (bottom), because in
> > both cases B and C are there just for reading. In order to
> > achieve that we must lock the rest of backing chain with
> > VIR_LOCK_MANAGER_RESOURCE_SHARED flag.
> 
> See below ...

> > +
> > +if (!src->path)
> > +break;
> > +
> > +VIR_DEBUG("Add disk %s", src->path);
> > +if (virLockManagerAddResource(lock,
> > +  VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK,
> > +  src->path,
> > +  0,
> > +  NULL,
> > +  diskFlags) < 0) {
> > +VIR_DEBUG("Failed add disk %s", src->path);
> > +goto cleanup;
> > +}
> > +
> > +/* Now that we've added the first disk (top layer of backing chain)
> > + * into the lock manager, let's traverse the rest of the backing 
> > chain
> > + * and lock each layer for RO. This should prevent accidentally
> > + * starting a domain with RW disk from the middle of the chain. */
> > +diskFlags = VIR_LOCK_MANAGER_RESOURCE_SHARED;
> 
> The VIR_LOCK_MANAGER_RESOURCE_SHARED flag means "shared, writable" mode
> according to lock_driver.h, I think you want to lock it with
> VIR_LOCK_MANAGER_RESOURCE_READONLY.

Yep, backing files should be marked readonly - 'shared' will allow
multiple VMs to write to the image at once. So you've only provided
mutual exclusion between an exclusive writer, vs a shared writer.

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 2/2] virsh: Enhance the detailed output of domblklist for networked source

[Lin Ma]

在 2015年09月08日 20:10, Peter Krempa 写道:

On Wed, Sep 02, 2015 at 17:58:19 +0200, Michal Privoznik wrote:

From: Lin Ma 

Format & output more detailed information about networked source

e.g: The output without the patch:
$ virsh domblklist $DOMAIN --details
Type   Device Target Source

networkdisk   vdatest-pool/image
networkdisk   vdbiqn.2015-08.org.example:sn01/0
networkdisk   vdc/image.raw
networkdisk   vdd-
networkdisk   vde-
networkdisk   vdfimage1
networkdisk   vdgtest-volume/image.raw

The output with the patch:
$ virsh domblklist $DOMAIN --details
Type   Device Target Source

networkdisk   vdarbd://monitor1.example.org:6321/test-pool/image

One other thing to note is that RBD volumes may have multiple hosts,
which is not taken into account by the above format ...


Yes, It may have multiple hosts for rbd. My original idea was adding 
more specific

info to details option for networked source and make the output more nice.

It has nothing to do with functional changes, So it just shows the first 
host.

Should we bring all of hosts to here?


networkdisk   vdb
iscsi://192.168.124.200:3260/iqn.2015-08.org.example:sn01/0
networkdisk   vdchttp://192.168.124.200:80/image.raw
networkdisk   vddnbd+unix:///var/run/nbdsock
networkdisk   vdenbd://192.168.124.200:12345
networkdisk   vdfsheepdog://192.168.124.200:6000/image1
networkdisk   vdggluster://192.168.124.200/test-volume/image.raw

... and gluster volumes will possibly have multiple sources too.


Signed-off-by: Lin Ma 
Signed-off-by: Michal Privoznik 
---
  tools/virsh-domain-monitor.c | 64 
  1 file changed, 59 insertions(+), 5 deletions(-)

Peter


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

[libvirt] [PATCH sandbox v5 15/20] Image: man file for virt-sandbox-image

[Daniel P. Berrange]

From: Eren Yagdiran 

Signed-off-by: Daniel P. Berrange 
---
 bin/Makefile.am|   5 ++
 bin/virt-sandbox-image.pod | 169 +
 libvirt-sandbox.spec.in|   1 +
 3 files changed, 175 insertions(+)
 create mode 100644 bin/virt-sandbox-image.pod

diff --git a/bin/Makefile.am b/bin/Makefile.am
index deedcf6..398e90c 100644
--- a/bin/Makefile.am
+++ b/bin/Makefile.am
@@ -20,6 +20,7 @@ POD_FILES = \
virt-sandbox-service-delete.pod \
virt-sandbox-service-reload.pod \
virt-sandbox-service-upgrade.pod \
+   virt-sandbox-image.pod \
$(NULL)
 EXTRA_DIST = $(bin_SCRIPTS) $(POD_FILES) 
virt-sandbox-service-bash-completion.sh virt-sandbox-service.logrotate
 EXTRA_DIST += virt-sandbox-service-bash-completion.sh
@@ -34,6 +35,7 @@ man1_MANS = \
virt-sandbox-service-delete.1 \
virt-sandbox-service-reload.1 \
virt-sandbox-service-upgrade.1 \
+   virt-sandbox-image.1 \
$(NULL)
 
 POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)"
@@ -65,6 +67,9 @@ virt-sandbox-service-reload.1: 
virt-sandbox-service-reload.pod Makefile
 virt-sandbox-service-upgrade.1: virt-sandbox-service-upgrade.pod Makefile
$(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@
 
+virt-sandbox-image.1: virt-sandbox-image.pod Makefile
+   $(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@
+
 CLEANFILES = $(man1_MANS)
 
 virt_sandbox_SOURCES = virt-sandbox.c
diff --git a/bin/virt-sandbox-image.pod b/bin/virt-sandbox-image.pod
new file mode 100644
index 000..85954b0
--- /dev/null
+++ b/bin/virt-sandbox-image.pod
@@ -0,0 +1,169 @@
+=head1 NAME
+
+virt-sandbox-image - Sandbox Container Image Tool
+
+=head1 SYNOPSIS
+
+  {download,create,run,delete}
+
+  commands:
+
+download  Download template data
+
+createCreate image from template data
+
+run   Run an already built image
+
+deleteDelete template data
+
+=head1 DESCRIPTION
+
+virt-sandbox-image.py is a sandbox container image tool developed in python.
+This tool can download,create,run and delete templates which are provided by
+different sources. This tool comes with Docker source by default. Other sources
+can be implemented by extending source class
+
+=head1 OPTIONS
+
+=over 4
+
+=item B
+
+Download a template by given name with a specified source.
+
+=over 6
+
+=item B
+
+Template name to download
+
+=item B<-s or --source>
+
+Source parameter will try load source module under sources/ directory. Each 
source has to implement Source.py base class and register itself with a proper 
name
+Default source is Docker.
+
+=item B<-r or --registry>
+
+Custom registry url for downloading data. This might need privileged 
credentials which can be specified by --username and --password parameters.
+
+=item B<-u or --username>
+
+Username for custom registry authentication
+
+=item B<-p or --password>
+
+Password for custom registry authentication
+
+=item B<-t or --template-dir>
+
+Custom directory for downloading template data
+
+=back
+
+=item B
+
+Create already downloaded template into image with given format.
+
+=over 5
+
+=item B
+
+Template name to download.
+
+=item B
+
+Image format e.g qcow2
+
+=item B<-s or --source>
+
+Source parameter will try load source module under sources/ directory. Each 
source has to implement Source.py base class and register itself with a proper 
name
+Default source is Docker.
+
+=item B<-c or --connect>
+
+Driver parameter can be specified with only supported driver by 
libvirt-sandbox. These are lxc:///, qemu:///session, qemu:///system.
+
+=back
+
+=item B
+
+Run already built image. If B is not specified, the default defined
+command for the image will be run.
+
+=over 6
+
+=item B
+
+Template name to download.
+
+=item B
+
+Image path where template image will be stored.
+
+=item B<-n or --name>
+
+The sandbox guest name
+
+=item B<-N or --network>
+
+Network params will be passed directly to the virt-sandbox. More information 
about network params, See C
+
+=item B<-v or --volume>
+
+Volume params are for binding host-paths to the guest. E.g -v /home:/home will 
map /home directory from host to the guest.
+
+=item B<-c or --connect>
+
+Driver parameter can be specified with only supported driver by 
libvirt-sandbox. These are lxc:///, qemu:///session, qemu:///system.
+
+=back
+
+=item B
+
+Delete downloaded template data and its built image.
+
+=over 3
+
+=item B
+
+Template name to delete.
+
+=item B
+
+Image path where template data or image stays.
+
+=item B<-s or --source>
+
+Source parameter will try load source module under sources/ directory. Each 
source has to implement Source.py base class and register itself with a proper 
name
+Default source is Docker.
+
+=back
+
+=back
+
+=head1 SEE ALSO
+
+C
+
+=head1 FILES
+
+Container content will be stored in subdirectories of

[libvirt] [PATCH sandbox v5 01/20] Add virt-sandbox-image

[Daniel P. Berrange]

From: Daniel P Berrange 

virt-sandbox-image.py is a python script that lets you download Docker
images easily. It is a proof of concept code and consumes Docker Rest API.

Signed-off-by: Daniel P. Berrange 
---
 bin/Makefile.am   |   3 +-
 bin/virt-sandbox-image|   8 +
 configure.ac  |   2 +
 libvirt-sandbox.spec.in   |   2 +
 libvirt-sandbox/Makefile.am   |   2 +-
 libvirt-sandbox/image/Makefile.am |   8 +
 libvirt-sandbox/image/__init__.py |   0
 libvirt-sandbox/image/cli.py  | 394 ++
 po/POTFILES.in|   1 +
 9 files changed, 418 insertions(+), 2 deletions(-)
 create mode 100644 bin/virt-sandbox-image
 create mode 100644 libvirt-sandbox/image/Makefile.am
 create mode 100644 libvirt-sandbox/image/__init__.py
 create mode 100644 libvirt-sandbox/image/cli.py

diff --git a/bin/Makefile.am b/bin/Makefile.am
index 416f86f..deedcf6 100644
--- a/bin/Makefile.am
+++ b/bin/Makefile.am
@@ -3,7 +3,8 @@ bin_PROGRAMS = virt-sandbox
 
 libexec_PROGRAMS = virt-sandbox-service-util
 
-bin_SCRIPTS = virt-sandbox-service
+bin_SCRIPTS = virt-sandbox-service \
+   virt-sandbox-image
 
 virtsandboxcompdir = $(datarootdir)/bash-completion/completions/
 
diff --git a/bin/virt-sandbox-image b/bin/virt-sandbox-image
new file mode 100644
index 000..7e0d76b
--- /dev/null
+++ b/bin/virt-sandbox-image
@@ -0,0 +1,8 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+from libvirt_sandbox.image import cli
+import sys
+
+if __name__ == '__main__':
+   sys.exit(cli.main())
diff --git a/configure.ac b/configure.ac
index 71c4392..a8376ca 100644
--- a/configure.ac
+++ b/configure.ac
@@ -124,10 +124,12 @@ dnl Should be in m4/virt-gettext.m4 but intltoolize is too
 dnl dumb to find it there
 IT_PROG_INTLTOOL([0.35.0])
 
+AM_PATH_PYTHON
 
 AC_OUTPUT(Makefile
   libvirt-sandbox/Makefile
   libvirt-sandbox/tests/Makefile
+  libvirt-sandbox/image/Makefile
   bin/Makefile
   examples/Makefile
   docs/Makefile
diff --git a/libvirt-sandbox.spec.in b/libvirt-sandbox.spec.in
index 4978242..54fde55 100644
--- a/libvirt-sandbox.spec.in
+++ b/libvirt-sandbox.spec.in
@@ -98,7 +98,9 @@ rm -rf $RPM_BUILD_ROOT
 %dir %{_sysconfdir}/libvirt-sandbox/services
 %{_bindir}/virt-sandbox
 %{_bindir}/virt-sandbox-service
+%{_bindir}/virt-sandbox-image
 %{_libexecdir}/virt-sandbox-service-util
+%{python_sitelib}/libvirt_sandbox
 %{_mandir}/man1/virt-sandbox.1*
 %{_mandir}/man1/virt-sandbox-service.1*
 %{_mandir}/man1/virt-sandbox-service-*.1*
diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am
index 597803e..b303078 100644
--- a/libvirt-sandbox/Makefile.am
+++ b/libvirt-sandbox/Makefile.am
@@ -2,7 +2,7 @@
 EXTRA_DIST = libvirt-sandbox.sym
 CLEANFILES =
 
-SUBDIRS = tests
+SUBDIRS = tests image
 
 rundir = $(localstatedir)/run
 
diff --git a/libvirt-sandbox/image/Makefile.am 
b/libvirt-sandbox/image/Makefile.am
new file mode 100644
index 000..7c8da51
--- /dev/null
+++ b/libvirt-sandbox/image/Makefile.am
@@ -0,0 +1,8 @@
+
+pythonsandboxdir = $(pythondir)/libvirt_sandbox
+pythonsandbox_DATA = __init__.py
+
+pythonimagedir = $(pythondir)/libvirt_sandbox/image
+pythonimage_DATA = __init__.py cli.py
+
+EXTRA_DIST = $(pythonimage_DATA)
diff --git a/libvirt-sandbox/image/__init__.py 
b/libvirt-sandbox/image/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
new file mode 100644
index 000..ec96c7e
--- /dev/null
+++ b/libvirt-sandbox/image/cli.py
@@ -0,0 +1,394 @@
+#!/usr/bin/python -Es
+#
+# Authors: Daniel P. Berrange 
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+#
+
+import argparse
+import gettext
+import hashlib
+import json
+import os
+import os.path
+import shutil
+import sys
+import urllib2
+import subprocess
+
+default_index_server = "index.docker.io"
+default_template_dir = "/var/lib/libvirt/templates"
+
+debug = True
+verbose = True
+
+gettext.bindtextdomain("libvirt-sandbox", "/usr/share/locale")
+gettext.textdomain("libvirt-sandbox")
+try:
+gettext.install("libvirt-sandbox",
+localedir="/usr/share/locale",
+  

[libvirt] [PATCH sandbox v5 16/20] Add config for environment variables

[Daniel P. Berrange]

From: Eren Yagdiran 

Add the config gobject to store custom environment variables.
This will allow creating custom environment variables on a sandbox
with a parameter formatted like --env key1=val1

Add testcase for custom environment variables

"make check" now includes testcase for environment variables

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/libvirt-sandbox-config.c | 171 ++-
 libvirt-sandbox/libvirt-sandbox-config.h |  13 +++
 libvirt-sandbox/libvirt-sandbox.sym  |   9 ++
 libvirt-sandbox/tests/test-config.c  |  10 ++
 4 files changed, 202 insertions(+), 1 deletion(-)

diff --git a/libvirt-sandbox/libvirt-sandbox-config.c 
b/libvirt-sandbox/libvirt-sandbox-config.c
index 2506072..780d174 100644
--- a/libvirt-sandbox/libvirt-sandbox-config.c
+++ b/libvirt-sandbox/libvirt-sandbox-config.c
@@ -64,6 +64,7 @@ struct _GVirSandboxConfigPrivate
 GList *networks;
 GList *mounts;
 GList *disks;
+GHashTable* envs;
 
 gchar *secLabel;
 gboolean secDynamic;
@@ -276,6 +277,8 @@ static void gvir_sandbox_config_finalize(GObject *object)
 g_list_foreach(priv->networks, (GFunc)g_object_unref, NULL);
 g_list_free(priv->networks);
 
+g_hash_table_destroy(priv->envs);
+
 g_list_foreach(priv->disks, (GFunc)g_object_unref, NULL);
 g_list_free(priv->disks);
 
@@ -483,6 +486,7 @@ static void gvir_sandbox_config_init(GVirSandboxConfig 
*config)
 priv->arch = g_strdup(uts.machine);
 priv->secDynamic = TRUE;
 
+priv->envs = g_hash_table_new(g_str_hash, g_str_equal);
 priv->uid = geteuid();
 priv->gid = getegid();
 priv->username = g_strdup(g_get_user_name());
@@ -1144,6 +1148,102 @@ gboolean 
gvir_sandbox_config_has_networks(GVirSandboxConfig *config)
 return priv->networks ? TRUE : FALSE;
 }
 
+/**
+ * gvir_sandbox_config_add_env:
+ * @config: (transfer none): the sandbox config
+ * @key: (transfer none): the key for environment variable
+ * @value: (transfer none): the value for environment variable
+ *
+ * Adds a new environment variable to the sandbox
+ *
+ */
+void gvir_sandbox_config_add_env(GVirSandboxConfig *config,
+ gchar *k,
+ gchar *v)
+{
+gchar * key = g_strdup(k);
+gchar * value = g_strdup(v);
+GVirSandboxConfigPrivate *priv = config->priv;
+g_hash_table_insert(priv->envs,key,value);
+}
+
+/**
+ * gvir_sandbox_config_get_envs:
+ * @config: (transfer none): the sandbox config
+ *
+ * Retrieves the hashtable of custom environment variables in the sandbox
+ *
+ * Returns: (transfer full) (element-type gchar gchar): the hashtable of 
environment variables
+ */
+GHashTable *gvir_sandbox_config_get_envs(GVirSandboxConfig *config)
+{
+GVirSandboxConfigPrivate *priv = config->priv;
+return priv->envs;
+}
+
+/**
+ * gvir_sandbox_config_add_env_strv:
+ * @config: (transfer none): the sandbox config
+ * @envs: (transfer none)(array zero-terminated=1): the list of environment 
variables
+ *
+ * Parses @envs whose elements are in the format KEY=VALUE
+ *
+ * --env KEY=VALUE
+ */
+gboolean gvir_sandbox_config_add_env_strv(GVirSandboxConfig *config,
+  gchar **envs,
+  GError **error)
+{
+gsize i = 0;
+while (envs && envs[i]) {
+if (!gvir_sandbox_config_add_env_opts(config,
+   envs[i],
+   error))
+return FALSE;
+i++;
+}
+return TRUE;
+}
+
+/**
+ * gvir_sandbox_config_add_env_opts:
+ * @config: (transfer none): the sandbox config
+ * @env: (transfer none): the env config
+ *
+ * Parses @env in the format KEY=VALUE
+ * creating #GVirSandboxConfigEnv instances for each element. For
+ * example
+ *
+ * --env KEY=VALUE
+ */
+
+gboolean gvir_sandbox_config_add_env_opts(GVirSandboxConfig *config,
+   const char *opt,
+   GError **error)
+{
+gchar *tmp = NULL;
+gchar *key = NULL;
+gchar *value = NULL;
+
+if (!(tmp = g_strdup(opt)))
+return FALSE;
+
+key  = strchr(tmp, '=');
+
+if (!key) {
+g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0,
+_("Wrong environment format on %s"), opt);
+return FALSE;
+}
+
+*key = '\0';
+value = key + 1;
+
+gvir_sandbox_config_add_env(config, tmp, value);
+
+g_free(tmp);
+return TRUE;
+}
 
 /**
  * gvir_sandbox_config_add_disk:
@@ -1163,7 +1263,6 @@ void gvir_sandbox_config_add_disk(GVirSandboxConfig 
*config,
 priv->disks = g_list_append(priv->disks, dsk);
 }
 
-
 /**
  * gvir_sandbox_config_get_disks:
  * @config: (transfer none): the sandbox config
@@ -1172,6 +1271,7 @@ void gvir_sandbox_config_add_disk(GVirSandboxConfig 
*config,
  *
  * Returns: (transfer full) 

[libvirt] [PATCH sandbox v5 03/20] Fix docker authentication handling

[Daniel P. Berrange]

From: Eren Yagdiran 

Authentication fix for Docker REST API.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 89eef1a..ea04820 100644
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -1,8 +1,10 @@
 #!/usr/bin/python -Es
 #
 # Authors: Daniel P. Berrange 
+#  Eren Yagdiran 
 #
 # Copyright (C) 2013 Red Hat, Inc.
+# Copyright (C) 2015 Universitat Politècnica de Catalunya.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -166,7 +168,7 @@ def download_template(name, server, destdir):
 # or more parents, in a linear stack. Here we are getting the list
 # of layers for the image with the tag we used.
 (data, res) = get_json(registryserver, "/v1/images/" + imagetagid + 
"/ancestry",
-   { "Cookie": cookie })
+   { "Authorization": "Token " + token })
 
 if data[0] != imagetagid:
 raise ValueError(["Expected first layer id '%s' to match image id 
'%s'",
@@ -188,9 +190,9 @@ def download_template(name, server, destdir):
 if not os.path.exists(jsonfile) or not os.path.exists(datafile):
 # The '/json' URL gives us some metadata about the layer
 res = save_data(registryserver, "/v1/images/" + layerid + 
"/json",
-{ "Cookie": cookie }, jsonfile)
+{ "Authorization": "Token " + token }, 
jsonfile)
 createdFiles.append(jsonfile)
-layersize = int(res.info().getheader("x-docker-size"))
+layersize = int(res.info().getheader("Content-Length"))
 
 datacsum = None
 if layerid in checksums:
@@ -199,7 +201,7 @@ def download_template(name, server, destdir):
 # and the '/layer' URL is the actual payload, provided
 # as a tar.gz archive
 save_data(registryserver, "/v1/images/" + layerid + "/layer",
-  { "Cookie": cookie }, datafile, datacsum, layersize)
+  { "Authorization": "Token " + token }, datafile, 
datacsum, layersize)
 createdFiles.append(datafile)
 
 # Strangely the 'json' data for a layer doesn't include
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 13/20] Image: Add run function

[Daniel P. Berrange]

From: Eren Yagdiran 

Run an already-built template
If there is no execution command specified by user, source.get_command will
find the command to invoke

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py | 33 +
 1 file changed, 33 insertions(+)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 2672a20..a06eb9c 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -31,6 +31,8 @@ import shutil
 import sys
 import urllib2
 import subprocess
+import random
+import string
 
 if os.geteuid() == 0:
 default_template_dir = "/var/lib/libvirt/templates"
@@ -97,6 +99,37 @@ def create(args):
 except Exception,e:
 print "Create Error %s" % str(e)
 
+def run(args):
+try:
+if args.connect is not None:
+check_connect(args.connect)
+source = dynamic_source_loader(args.source)
+name = args.name
+if name is None:
+randomid = ''.join(random.choice(string.lowercase) for i in 
range(10))
+name = args.template + ":" + randomid
+
+diskfile = source.get_disk(templatename=args.template,
+   templatedir=args.template_dir,
+   imagedir=args.image_dir,
+   sandboxname=name)
+
+format = "qcow2"
+commandToRun = source.get_command(args.template, 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)]
+cmd = cmd + params + ['--'] + commandToRun
+subprocess.call(cmd)
+os.unlink(diskfile)
+
+except Exception,e:
+print "Run Error %s" % str(e)
+
 def requires_template(parser):
 parser.add_argument("template",
 help=_("name of the template"))
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 08/20] Image: Add delete function

[Daniel P. Berrange]

From: Eren Yagdiran 

Refactoring delete function from virt-sandbox-image to DockerSource. Delete 
function
can delete templates by name.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py  | 58 ---
 libvirt-sandbox/image/sources/DockerSource.py | 50 +++
 libvirt-sandbox/image/sources/Source.py   | 10 +
 3 files changed, 67 insertions(+), 51 deletions(-)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index f3c0ab7..5490c4b 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -69,55 +69,6 @@ def debug(msg):
 def info(msg):
 sys.stdout.write(msg)
 
-def delete_template(name, destdir):
-imageusage = {}
-imageparent = {}
-imagenames = {}
-imagedirs = os.listdir(destdir)
-for imagetagid in imagedirs:
-indexfile = destdir + "/" + imagetagid + "/index.json"
-if os.path.exists(indexfile):
-with open(indexfile, "r") as f:
-index = json.load(f)
-imagenames[index["name"]] = imagetagid
-jsonfile = destdir + "/" + imagetagid + "/template.json"
-if os.path.exists(jsonfile):
-with open(jsonfile, "r") as f:
-template = json.load(f)
-
-parent = template.get("parent", None)
-if parent:
-if parent not in imageusage:
-imageusage[parent] = []
-imageusage[parent].append(imagetagid)
-imageparent[imagetagid] = parent
-
-if not name in imagenames:
-raise ValueError(["Image %s does not exist locally" % name])
-
-imagetagid = imagenames[name]
-while imagetagid != None:
-debug("Remove %s\n" %  imagetagid)
-parent = imageparent.get(imagetagid, None)
-
-indexfile = destdir + "/" + imagetagid + "/index.json"
-if os.path.exists(indexfile):
-os.remove(indexfile)
-jsonfile = destdir + "/" + imagetagid + "/template.json"
-if os.path.exists(jsonfile):
-os.remove(jsonfile)
-datafile = destdir + "/" + imagetagid + "/template.tar.gz"
-if os.path.exists(datafile):
-os.remove(datafile)
-imagedir = destdir + "/" + imagetagid
-os.rmdir(imagedir)
-
-if parent:
-if len(imageusage[parent]) != 1:
-debug("Parent %s is shared\n" % parent)
-parent = None
-imagetagid = parent
-
 def download(args):
 try:
 
dynamic_source_loader(args.source).download_template(templatename=args.template,
@@ -131,8 +82,11 @@ def download(args):
 print "Download Error %s" % str(e)
 
 def delete(args):
-info("Deleting %s from %s\n" % (args.template, default_template_dir))
-delete_template(args.template, default_template_dir)
+try:
+
dynamic_source_loader(args.source).delete_template(templatename=args.template,
+   
templatedir=args.template_dir)
+except Exception,e:
+print "Delete Error %s", str(e)
 
 def create(args):
 try:
@@ -183,6 +137,8 @@ def gen_delete_args(subparser):
 parser = subparser.add_parser("delete",
help=_("Delete template data"))
 requires_template(parser)
+requires_source(parser)
+requires_template_dir(parser)
 parser.set_defaults(func=delete)
 
 def gen_create_args(subparser):
diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
b/libvirt-sandbox/image/sources/DockerSource.py
index c1c8a7d..ab18b52 100644
--- a/libvirt-sandbox/image/sources/DockerSource.py
+++ b/libvirt-sandbox/image/sources/DockerSource.py
@@ -295,5 +295,55 @@ class DockerSource(Source):
 cmd = cmd + params
 subprocess.call(cmd)
 
+def delete_template(self, templatename, templatedir):
+imageusage = {}
+imageparent = {}
+imagenames = {}
+imagedirs = os.listdir(templatedir)
+for imagetagid in imagedirs:
+indexfile = templatedir + "/" + imagetagid + "/index.json"
+if os.path.exists(indexfile):
+with open(indexfile,"r") as f:
+index = json.load(f)
+imagenames[index["name"]] = imagetagid
+jsonfile = templatedir + "/" + imagetagid + "/template.json"
+if os.path.exists(jsonfile):
+with open(jsonfile,"r") as f:
+template = json.load(f)
+
+parent = template.get("parent",None)
+if parent:
+if parent not in imageusage:
+imageusage[parent] = []
+imageusage[parent].append(imagetagid)
+imageparent[imagetagid] = parent
+
+
+if not templatename in imagenames:
+raise ValueError(["Image %s does not exist locally" 

Re: [libvirt] [sandbox PATCH v4 11/21] Image: Add run args

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:39PM +, Eren Yagdiran wrote:
> Commandline parameters for running a template
> ---
>  virt-sandbox-image/virt-sandbox-image.py | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/virt-sandbox-image/virt-sandbox-image.py 
> b/virt-sandbox-image/virt-sandbox-image.py
> index 1da5150..d6b682f 100755
> --- a/virt-sandbox-image/virt-sandbox-image.py
> +++ b/virt-sandbox-image/virt-sandbox-image.py
> @@ -178,6 +178,18 @@ def gen_create_args(subparser):
>  help=_("format format for image"))
>  parser.set_defaults(func=create)
>  
> +def gen_run_args(subparser):
> +parser = subparser.add_parser("run",
> +  help=_("Run a already built image"))
> +requires_name(parser)
> +requires_source(parser)
> +requires_connect(parser)
> +parser.add_argument("-t","--template-dir",
> +help=_("Template directory for saving templates"))
> +parser.add_argument("-i","--igniter",
> +help=_("Igniter command for image"))

Rather than having an --igniter arg, which only allows a single binary
name, we should just allow an arbitrary number of positional args, so
the user can pass full arg lists, eg 

   virt-sandbox-image run ubuntu /bin/somefile somearg otherarg 

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 PATCH v4 10/21] Image: Add get_command function to Source

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:38PM +, Eren Yagdiran wrote:
> Provide a way to know how a template can be started depending on the used 
> source
> DockerSource will need to parse the topmost config file in order to find the 
> igniter command
> ---
>  virt-sandbox-image/sources/DockerSource.py | 14 ++
>  virt-sandbox-image/sources/Source.py   |  4 
>  2 files changed, 18 insertions(+)
> 
> diff --git a/virt-sandbox-image/sources/DockerSource.py 
> b/virt-sandbox-image/sources/DockerSource.py
> index 760ba6c..3e0362b 100644
> --- a/virt-sandbox-image/sources/DockerSource.py
> +++ b/virt-sandbox-image/sources/DockerSource.py
> @@ -29,6 +29,15 @@ import os
>  import subprocess
>  import shutil
>  
> +class DockerConfParser():
> +
> +def __init__(self,jsonfile):
> +with open(jsonfile) as json_file:
> +self.json_data = json.load(json_file)
> +def getRunCommand(self):
> +cmd = self.json_data['container_config']['Cmd'][2]
> +return cmd[cmd.index('"') + 1:cmd.rindex('"')]

In testing, I found out we should be using 'config' here
not 'container_config', since the latter refers to the
configuration used when the image was built, which is
subtly different from the config that is intended to be
used when in it deployed.

In particular the 'Cmd' field does not need this string
munging when we use 'config' instead of 'container_config'.

In researching this, I found out that docker also has an
optional 'Entrypoints' array and if both Entrypoints and
Cmd are specified, we should concatenate them. If the
user provides command line args, then should replace
the 'Cmd' list, but not the 'Entrypoints' list. Finally
some images don'pt define any 'Cmd' or 'Entrypoints' at
all, in which case it seems we should just try using
/bin/sh, in the absence of any user supplied command.

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 PATCH v4 16/21] Image: Add Volume Support

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:44PM +, Eren Yagdiran wrote:
> Volumes let user to map host-paths into sandbox. Docker containers
> need volumes for data persistence.

I'm a little bit puzzelled about how this feature is supposed
to be used. IIUC, in the Docker json file we have something
like

{
"/var/my-app-data/": {},
"/etc/some-config.d/": {},
}

> +def getVolumes(self):
> +volumes = self.json_data['container_config']['Volumes']
> +volumelist = []
> +if isinstance(volumes,collections.Iterable):
> +  for key,value in volumes.iteritems():
> +volumelist.append(key)
> +return volumelist

This will just return a python list

  ["/var/my-app-data/", "/etc/some-config.d"]


> diff --git a/virt-sandbox-image/virt-sandbox-image.py 
> b/virt-sandbox-image/virt-sandbox-image.py
> index 058738a..79f8d8c 100755
> --- a/virt-sandbox-image/virt-sandbox-image.py

> @@ -150,6 +151,25 @@ def run(args):
>  if networkArgs is not None:
>  params.append('-N')
>  params.append(networkArgs)
> +allVolumes = source.get_volume(configfile)
> +volumeArgs = args.volume
> +if volumeArgs is not None:
> +allVolumes = allVolumes + volumeArgs
> +for volume in allVolumes:
> +volumeSplit = volume.split(":")

We don't have any ':' in our returned list from getVolumes()


> +volumelen = len(volumeSplit)
> +if volumelen == 2:
> +hostPath = volumeSplit[0]
> +guestPath = volumeSplit[1]
> +elif volumelen == 1:
> +guestPath = volumeSplit[0]
> +hostPath = storage_dir + guestPath
> +if not os.path.exists(hostPath):
> +os.makedirs(hostPath)
> +else:
> +pass

So we seem to just skip this ?

> +params.append("--mount")
> +params.append("host-bind:%s=%s" %(guestPath,hostPath))
>  params.append('--')
>  params.append(commandToRun)
>  cmd = cmd + params

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] [PATCH sandbox v5 07/20] Image: Refactor create function

[Daniel P. Berrange]

From: Eren Yagdiran 

Move the docker-related code to the DockerSource and use
the Source mechanism

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py  | 76 +-
 libvirt-sandbox/image/sources/DockerSource.py | 90 +++
 libvirt-sandbox/image/sources/Source.py   | 15 +
 3 files changed, 122 insertions(+), 59 deletions(-)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 7af617e..f3c0ab7 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -118,59 +118,6 @@ def delete_template(name, destdir):
 parent = None
 imagetagid = parent
 
-
-def get_image_list(name, destdir):
-imageparent = {}
-imagenames = {}
-imagedirs = os.listdir(destdir)
-for imagetagid in imagedirs:
-indexfile = destdir + "/" + imagetagid + "/index.json"
-if os.path.exists(indexfile):
-with open(indexfile, "r") as f:
-index = json.load(f)
-imagenames[index["name"]] = imagetagid
-jsonfile = destdir + "/" + imagetagid + "/template.json"
-if os.path.exists(jsonfile):
-with open(jsonfile, "r") as f:
-template = json.load(f)
-
-parent = template.get("parent", None)
-if parent:
-imageparent[imagetagid] = parent
-
-if not name in imagenames:
-raise ValueError(["Image %s does not exist locally" % name])
-
-imagetagid = imagenames[name]
-imagelist = []
-while imagetagid != None:
-imagelist.append(imagetagid)
-parent = imageparent.get(imagetagid, None)
-imagetagid = parent
-
-return imagelist
-
-def create_template(name, imagepath, format, destdir):
-if not format in ["qcow2"]:
-raise ValueError(["Unsupported image format %s" % format])
-
-imagelist = get_image_list(name, destdir)
-imagelist.reverse()
-
-parentImage = None
-for imagetagid in imagelist:
-templateImage = destdir + "/" + imagetagid + "/template." + format
-cmd = ["qemu-img", "create", "-f", "qcow2"]
-if parentImage is not None:
-cmd.append("-o")
-cmd.append("backing_fmt=qcow2,backing_file=%s" % parentImage)
-cmd.append(templateImage)
-if parentImage is None:
-cmd.append("10G")
-debug("Run %s\n" % " ".join(cmd))
-subprocess.call(cmd)
-parentImage = templateImage
-
 def download(args):
 try:
 
dynamic_source_loader(args.source).download_template(templatename=args.template,
@@ -188,8 +135,13 @@ def delete(args):
 delete_template(args.template, default_template_dir)
 
 def create(args):
-info("Creating %s from %s in format %s\n" % (args.imagepath, 
args.template, args.format))
-create_template(args.template, args.imagepath, args.format, 
default_template_dir)
+try:
+
dynamic_source_loader(args.source).create_template(templatename=args.template,
+   
templatedir=args.template_dir,
+   
connect=args.connect,
+   format=args.format)
+except Exception,e:
+print "Create Error %s" % str(e)
 
 def requires_template(parser):
 parser.add_argument("template",
@@ -200,6 +152,10 @@ def requires_source(parser):
 default="docker",
 help=_("name of the template"))
 
+def requires_connect(parser):
+parser.add_argument("-c","--connect",
+help=_("Connect string for libvirt"))
+
 def requires_auth_conn(parser):
 parser.add_argument("-r","--registry",
 help=_("Url of the custom registry"))
@@ -233,10 +189,12 @@ def gen_create_args(subparser):
 parser = subparser.add_parser("create",
help=_("Create image from template data"))
 requires_template(parser)
-parser.add_argument("imagepath",
-help=_("path for image"))
-parser.add_argument("format",
-help=_("format"))
+requires_source(parser)
+requires_connect(parser)
+requires_template_dir(parser)
+parser.add_argument("-f","--format",
+default="qcow2",
+help=_("format format for image"))
 parser.set_defaults(func=create)
 
 def main():
diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
b/libvirt-sandbox/image/sources/DockerSource.py
index 37b40dc..c1c8a7d 100644
--- a/libvirt-sandbox/image/sources/DockerSource.py
+++ b/libvirt-sandbox/image/sources/DockerSource.py
@@ -205,5 +205,95 @@ class DockerSource(Source):
 debug("FAIL %s\n" % str(e))
 raise
 
+def create_template(self, templatename, templatedir, connect=None, 

[libvirt] [PATCH sandbox v5 19/20] Image: Add custom environment support

[Daniel P. Berrange]

From: Eren Yagdiran 

Any custom key=value pair can be used as a custom environment variable
in virt-sandbox-image.
e.g virt-sandbox-image run ubuntu /var/lib/libvirt/templates -c lxc:/// -i 
/bin/bash -e key1=val1

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py  | 16 
 libvirt-sandbox/image/sources/DockerSource.py | 11 +++
 libvirt-sandbox/image/sources/Source.py   | 10 ++
 3 files changed, 37 insertions(+)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 30e2558..c17b577 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -129,6 +129,19 @@ def run(args):
 params.append('-N')
 params.append(networkArgs)
 
+allEnvs = source.get_env(args.template, 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)
@@ -222,6 +235,9 @@ def gen_run_args(subparser):
 help=_("command arguments to run"))
 parser.add_argument("-N","--network",
 help=_("Network params for running template"))
+parser.add_argument("-e","--env",action="append",
+help=_("Environment params for running template"))
+
 parser.set_defaults(func=run)
 
 def main():
diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
b/libvirt-sandbox/image/sources/DockerSource.py
index 31c1d80..4455198 100644
--- a/libvirt-sandbox/image/sources/DockerSource.py
+++ b/libvirt-sandbox/image/sources/DockerSource.py
@@ -38,6 +38,12 @@ class DockerConfParser():
 return self.json_data['config']['Cmd']
 def getEntrypoint(self):
 return self.json_data['config']['Entrypoint']
+def getEnvs(self):
+lst = self.json_data['config']['Env']
+if lst is not None and isinstance(lst,list):
+  return lst
+else:
+  return []
 
 class DockerSource(Source):
 
@@ -388,5 +394,10 @@ class DockerSource(Source):
 else:
 return entrypoint + cmd
 
+def get_env(self, templatename, templatedir):
+configfile, diskfile = self._get_template_data(templatename, 
templatedir)
+configParser = DockerConfParser(configfile)
+return configParser.getEnvs()
+
 def debug(msg):
 sys.stderr.write(msg)
diff --git a/libvirt-sandbox/image/sources/Source.py 
b/libvirt-sandbox/image/sources/Source.py
index a5d3844..8a21f90 100644
--- a/libvirt-sandbox/image/sources/Source.py
+++ b/libvirt-sandbox/image/sources/Source.py
@@ -95,3 +95,13 @@ class Source():
 of a template.
 """
 pass
+
+@abstractmethod
+def get_env(self,templatename, templatedir):
+"""
+:param templatename: name of the template image to download
+:param templatedir: local directory path in which to find template
+
+Get the dict of environment variables to set
+"""
+pass
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 17/20] Add environment parameter to virt-sandbox

[Daniel P. Berrange]

From: Eren Yagdiran 

Allow users to add custom environment variables to their sandbox.

Signed-off-by: Daniel P. Berrange 
---
 bin/virt-sandbox.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c
index 332e53e..4c400d5 100644
--- a/bin/virt-sandbox.c
+++ b/bin/virt-sandbox.c
@@ -80,6 +80,7 @@ int main(int argc, char **argv) {
 GError *error = NULL;
 gchar *name = NULL;
 gchar **disks = NULL;
+gchar **envs = NULL;
 gchar **mounts = NULL;
 gchar **includes = NULL;
 gchar *includefile = NULL;
@@ -111,6 +112,8 @@ int main(int argc, char **argv) {
   N_("root directory of the sandbox"), "DIR" },
 { "disk", ' ', 0, G_OPTION_ARG_STRING_ARRAY, ,
   N_("add a disk in the guest"), "TYPE:TAGNAME=SOURCE,format=FORMAT" },
+{ "env", 'e', 0, G_OPTION_ARG_STRING_ARRAY, ,
+  N_("add a environment variable for the sandbox"), "KEY=VALUE" },
 { "mount", 'm', 0, G_OPTION_ARG_STRING_ARRAY, ,
   N_("mount a filesystem in the guest"), "TYPE:TARGET=SOURCE" },
 { "include", 'i', 0, G_OPTION_ARG_STRING_ARRAY, ,
@@ -201,6 +204,13 @@ int main(int argc, char **argv) {
 gvir_sandbox_config_set_username(cfg, "root");
 }
 
+if (envs &&
+!gvir_sandbox_config_add_env_strv(cfg, envs, )) {
+g_printerr(_("Unable to parse custom environment variables: %s\n"),
+   error && error->message ? error->message : _("Unknown 
failure"));
+goto cleanup;
+}
+
 if (disks &&
 !gvir_sandbox_config_add_disk_strv(cfg, disks, )) {
 g_printerr(_("Unable to parse disks: %s\n"),
@@ -350,6 +360,10 @@ inheriting the host's root filesystem.
 NB. C must contain a matching install of the libvirt-sandbox
 package. This restriction may be lifted in a future version.
 
+=item B<--env key=value>
+
+Sets up a custom environment variable on a running sandbox.
+
 =item B<--disk TYPE:TAGNAME=SOURCE,format=FORMAT>
 
 Sets up a disk inside the sandbox by using B with a symlink named as 
B
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 12/20] Image: Add get_disk function to Source

[Daniel P. Berrange]

From: Eren Yagdiran 

Provide a way to know which disk image to use for the sandbox depending on the 
used source
DockerSource will need to locate the topmost disk image among all the layers 
images

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/sources/DockerSource.py | 12 
 libvirt-sandbox/image/sources/Source.py   | 12 
 2 files changed, 24 insertions(+)

diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
b/libvirt-sandbox/image/sources/DockerSource.py
index 2607011..31c1d80 100644
--- a/libvirt-sandbox/image/sources/DockerSource.py
+++ b/libvirt-sandbox/image/sources/DockerSource.py
@@ -362,6 +362,18 @@ class DockerSource(Source):
 configfile = templatedir + "/" + toplayer + "/template.json"
 return configfile, diskfile
 
+def get_disk(self,templatename, templatedir, imagedir, sandboxname):
+configfile, diskfile = self._get_template_data(templatename, 
templatedir)
+tempfile = imagedir + "/" + sandboxname + ".qcow2"
+if not os.path.exists(imagedir):
+os.makedirs(imagedir)
+cmd = ["qemu-img","create","-q","-f","qcow2"]
+cmd.append("-o")
+cmd.append("backing_fmt=qcow2,backing_file=%s" % diskfile)
+cmd.append(tempfile)
+subprocess.call(cmd)
+return tempfile
+
 def get_command(self, templatename, templatedir, userargs):
 configfile, diskfile = self._get_template_data(templatename, 
templatedir)
 configParser = DockerConfParser(configfile)
diff --git a/libvirt-sandbox/image/sources/Source.py 
b/libvirt-sandbox/image/sources/Source.py
index 4305d0b..a5d3844 100644
--- a/libvirt-sandbox/image/sources/Source.py
+++ b/libvirt-sandbox/image/sources/Source.py
@@ -83,3 +83,15 @@ class Source():
 is specified, then this should override the default args in
 the image"""
 pass
+
+@abstractmethod
+def get_disk(self,templatename, templatedir, imagedir, sandboxname):
+"""
+:param templatename: name of the template image to download
+:param templatedir: local directory path in which to find template
+:param imagedir: local directory in which to storage disk image
+
+Creates an instance private disk image, backed by the content
+of a template.
+"""
+pass
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 14/20] Image: Add network support

[Daniel P. Berrange]

From: Eren Yagdiran 

Virt-sandbox-image will pass exact network arguments to virt-sandbox

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index a06eb9c..30e2558 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -123,6 +123,12 @@ def run(args):
 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)
+
 cmd = cmd + params + ['--'] + commandToRun
 subprocess.call(cmd)
 os.unlink(diskfile)
@@ -214,6 +220,8 @@ def gen_run_args(subparser):
 parser.add_argument("args",
 nargs=argparse.REMAINDER,
 help=_("command arguments to run"))
+parser.add_argument("-N","--network",
+help=_("Network params for running template"))
 parser.set_defaults(func=run)
 
 def main():
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 00/20] Docker image support

[Daniel P. Berrange]

This is an update of Eren's v4 patch series to provide
Docker image support. In this v5 I have incorporated
fixes for all of the feedback I gave Eren against the
v4, so I think this series is ready for merging now.

One thing I still want to look at separately is how
applicable this design is to other container formats,
in particular the 'appc' specification

https://github.com/appc/spec/blob/master/spec/discovery.md

Before we make a release containing the docker impl, I
want to be confident we've not done anything silly
which will cause us compat problems if we want to
extend to cover appc later.

Daniel P Berrange (1):
  Add virt-sandbox-image

Daniel P. Berrange (1):
  Rename 'name' to 'template' to disambiguate

Eren Yagdiran (18):
  Fix docker authentication handling
  Image: Add Hooking Mechanism
  Image: virt-sandbox-image default dir constants
  Image: Add download function
  Image: Refactor create function
  Image: Add delete function
  Image: Add get_command function to Source
  Image: Add run args
  Image: Add check_connect function
  Image: Add get_disk function to Source
  Image: Add run function
  Image: Add network support
  Image: man file for virt-sandbox-image
  Add config for environment variables
  Add environment parameter to virt-sandbox
  init-common: Exporting custom environment variables
  Image: Add custom environment support
  Image: Add Volume Support

 .gitignore|   1 +
 bin/Makefile.am   |   8 +-
 bin/virt-sandbox-image|   8 +
 bin/virt-sandbox-image.pod| 169 +++
 bin/virt-sandbox.c|  14 +
 configure.ac  |   3 +
 libvirt-sandbox.spec.in   |   3 +
 libvirt-sandbox/Makefile.am   |   2 +-
 libvirt-sandbox/image/Makefile.am |  10 +
 libvirt-sandbox/image/__init__.py |   0
 libvirt-sandbox/image/cli.py  | 293 ++
 libvirt-sandbox/image/sources/DockerSource.py | 416 ++
 libvirt-sandbox/image/sources/Makefile.am |   9 +
 libvirt-sandbox/image/sources/Source.py   | 117 
 libvirt-sandbox/image/sources/__init__.py |   0
 libvirt-sandbox/libvirt-sandbox-config.c  | 171 ++-
 libvirt-sandbox/libvirt-sandbox-config.h  |  13 +
 libvirt-sandbox/libvirt-sandbox-init-common.c |  18 ++
 libvirt-sandbox/libvirt-sandbox.sym   |   9 +
 libvirt-sandbox/tests/test-config.c   |  10 +
 po/POTFILES.in|   1 +
 21 files changed, 1272 insertions(+), 3 deletions(-)
 create mode 100644 bin/virt-sandbox-image
 create mode 100644 bin/virt-sandbox-image.pod
 create mode 100644 libvirt-sandbox/image/Makefile.am
 create mode 100644 libvirt-sandbox/image/__init__.py
 create mode 100755 libvirt-sandbox/image/cli.py
 create mode 100644 libvirt-sandbox/image/sources/DockerSource.py
 create mode 100644 libvirt-sandbox/image/sources/Makefile.am
 create mode 100644 libvirt-sandbox/image/sources/Source.py
 create mode 100644 libvirt-sandbox/image/sources/__init__.py

-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 20/20] Image: Add Volume Support

[Daniel P. Berrange]

From: Eren Yagdiran 

Volumes let user to map host-paths into sandbox. Docker containers
need volumes for data persistence.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py  | 24 +++-
 libvirt-sandbox/image/sources/DockerSource.py | 13 +
 libvirt-sandbox/image/sources/Source.py   | 10 ++
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index c17b577..3f1ab0d 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -101,6 +101,7 @@ def create(args):
 
 def run(args):
 try:
+global image_dir
 if args.connect is not None:
 check_connect(args.connect)
 source = dynamic_source_loader(args.source)
@@ -142,6 +143,26 @@ def run(args):
 else:
 pass
 
+allVolumes = source.get_volumes(args.template, args.template_dir)
+volumeArgs = args.volume
+if volumeArgs is not None:
+allVolumes = allVolumes + volumeArgs
+for volume in allVolumes:
+volumeSplit = volume.split(":")
+volumelen = len(volumeSplit)
+if volumelen == 2:
+hostPath = volumeSplit[0]
+guestPath = volumeSplit[1]
+elif volumelen == 1:
+guestPath = volumeSplit[0]
+hostPath = image_dir + guestPath
+if not os.path.exists(hostPath):
+os.makedirs(hostPath)
+else:
+pass
+params.append("--mount")
+params.append("host-bind:%s=%s" %(guestPath,hostPath))
+
 cmd = cmd + params + ['--'] + commandToRun
 subprocess.call(cmd)
 os.unlink(diskfile)
@@ -237,7 +258,8 @@ def gen_run_args(subparser):
 help=_("Network params for running template"))
 parser.add_argument("-e","--env",action="append",
 help=_("Environment params for running template"))
-
+parser.add_argument("--volume",action="append",
+help=_("Volume params for running template"))
 parser.set_defaults(func=run)
 
 def main():
diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
b/libvirt-sandbox/image/sources/DockerSource.py
index 4455198..2c358fe 100644
--- a/libvirt-sandbox/image/sources/DockerSource.py
+++ b/libvirt-sandbox/image/sources/DockerSource.py
@@ -28,6 +28,7 @@ import traceback
 import os
 import subprocess
 import shutil
+import collections
 
 class DockerConfParser():
 
@@ -44,6 +45,13 @@ class DockerConfParser():
   return lst
 else:
   return []
+def getVolumes(self):
+volumes = self.json_data['config']['Volumes']
+volumelist = []
+if isinstance(volumes,collections.Iterable):
+for key,value in volumes.iteritems():
+volumelist.append(key)
+return volumelist
 
 class DockerSource(Source):
 
@@ -399,5 +407,10 @@ class DockerSource(Source):
 configParser = DockerConfParser(configfile)
 return configParser.getEnvs()
 
+def get_volumes(self, templatename, templatedir):
+configfile, diskfile = self._get_template_data(templatename, 
templatedir)
+configParser = DockerConfParser(configfile)
+return configParser.getVolumes()
+
 def debug(msg):
 sys.stderr.write(msg)
diff --git a/libvirt-sandbox/image/sources/Source.py 
b/libvirt-sandbox/image/sources/Source.py
index 8a21f90..f1dd3e7 100644
--- a/libvirt-sandbox/image/sources/Source.py
+++ b/libvirt-sandbox/image/sources/Source.py
@@ -105,3 +105,13 @@ class Source():
 Get the dict of environment variables to set
 """
 pass
+
+@abstractmethod
+def get_volumes(self,templatename, templatedir):
+"""
+:param templatename: name of the template image to download
+:param templatedir: local directory path in which to find template
+
+Get the list of volumes associated with the template
+"""
+pass
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 18/20] init-common: Exporting custom environment variables

[Daniel P. Berrange]

From: Eren Yagdiran 

Common-init reads config file and exports custom environment
variables from config file and applies them to the running sandbox.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/libvirt-sandbox-init-common.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c 
b/libvirt-sandbox/libvirt-sandbox-init-common.c
index e3e14a3..42beadc 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-common.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-common.c
@@ -348,6 +348,21 @@ static gboolean setup_network(GVirSandboxConfig *config, 
GError **error)
 }
 
 
+static gboolean setup_custom_env(GVirSandboxConfig *config, GError **error)
+{
+gboolean ret = FALSE;
+GHashTableIter iter;
+gpointer key, value;
+g_hash_table_iter_init (, gvir_sandbox_config_get_envs(config));
+while (g_hash_table_iter_next (, , )){
+if(setenv(key,value,1)!=0)
+goto cleanup;
+}
+ret = TRUE;
+  cleanup:
+return ret;
+}
+
 static int change_user(const gchar *user,
uid_t uid,
gid_t gid,
@@ -1436,6 +1451,9 @@ int main(int argc, char **argv) {
 if (!setup_disk_tags())
 exit(EXIT_FAILURE);
 
+if (!setup_custom_env(config, ))
+goto error;
+
 if (!setup_network(config, ))
 goto error;
 
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 10/20] Image: Add run args

[Daniel P. Berrange]

From: Eren Yagdiran 

Commandline parameters for running a template

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py | 25 +
 1 file changed, 25 insertions(+)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 5490c4b..6f9a5e7 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -101,6 +101,10 @@ def requires_template(parser):
 parser.add_argument("template",
 help=_("name of the template"))
 
+def requires_name(parser):
+parser.add_argument("-n","--name",
+help=_("Name of the running sandbox"))
+
 def requires_source(parser):
 parser.add_argument("-s","--source",
 default="docker",
@@ -124,6 +128,12 @@ def requires_template_dir(parser):
 default=default_template_dir,
 help=_("Template directory for saving templates"))
 
+def requires_image_dir(parser):
+global default_image_dir
+parser.add_argument("-I","--image-dir",
+default=default_image_dir,
+help=_("Image directory for saving images"))
+
 def gen_download_args(subparser):
 parser = subparser.add_parser("download",
help=_("Download template data"))
@@ -153,6 +163,20 @@ def gen_create_args(subparser):
 help=_("format format for image"))
 parser.set_defaults(func=create)
 
+def gen_run_args(subparser):
+parser = subparser.add_parser("run",
+  help=_("Run a already built image"))
+requires_name(parser)
+requires_template(parser)
+requires_source(parser)
+requires_connect(parser)
+requires_template_dir(parser)
+requires_image_dir(parser)
+parser.add_argument("args",
+nargs=argparse.REMAINDER,
+help=_("command arguments to run"))
+parser.set_defaults(func=run)
+
 def main():
 parser = argparse.ArgumentParser(description='Sandbox Container Image 
Tool')
 
@@ -160,6 +184,7 @@ def main():
 gen_download_args(subparser)
 gen_delete_args(subparser)
 gen_create_args(subparser)
+gen_run_args(subparser)
 
 try:
 args = parser.parse_args()
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 06/20] Image: Add download function

[Daniel P. Berrange]

From: Eren Yagdiran 

Refactor download function from virt-sandbox-image to use
the newly introduced Source abstract class. The docker-specific
download code is moved to a new DockerSource class.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py  | 204 -
 libvirt-sandbox/image/sources/DockerSource.py | 209 ++
 libvirt-sandbox/image/sources/Makefile.am |   1 +
 libvirt-sandbox/image/sources/Source.py   |  15 ++
 4 files changed, 257 insertions(+), 172 deletions(-)
 create mode 100644 libvirt-sandbox/image/sources/DockerSource.py

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index de34321..7af617e 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -69,176 +69,6 @@ def debug(msg):
 def info(msg):
 sys.stdout.write(msg)
 
-def get_url(server, path, headers):
-url = "https://; + server + path
-debug("  Fetching %s..." % url)
-req = urllib2.Request(url=url)
-
-if json:
-req.add_header("Accept", "application/json")
-
-for h in headers.keys():
-req.add_header(h, headers[h])
-
-return urllib2.urlopen(req)
-
-def get_json(server, path, headers):
-try:
-res = get_url(server, path, headers)
-data = json.loads(res.read())
-debug("OK\n")
-return (data, res)
-except Exception, e:
-debug("FAIL %s\n" % str(e))
-raise
-
-def save_data(server, path, headers, dest, checksum=None, datalen=None):
-try:
-res = get_url(server, path, headers)
-
-csum = None
-if checksum is not None:
-csum = hashlib.sha256()
-
-pattern = [".", "o", "O", "o"]
-patternIndex = 0
-donelen = 0
-
-with open(dest, "w") as f:
-while 1:
-buf = res.read(1024*64)
-if not buf:
-break
-if csum is not None:
-csum.update(buf)
-f.write(buf)
-
-if datalen is not None:
-donelen = donelen + len(buf)
-debug("\x1b[s%s (%5d Kb of %5d Kb)\x1b8" % (
-pattern[patternIndex], (donelen/1024), (datalen/1024)
-))
-patternIndex = (patternIndex + 1) % 4
-
-debug("\x1b[K")
-if csum is not None:
-csumstr = "sha256:" + csum.hexdigest()
-if csumstr != checksum:
-debug("FAIL checksum '%s' does not match '%s'" % (csumstr, 
checksum))
-os.remove(dest)
-raise IOError("Checksum '%s' for data does not match '%s'" % 
(csumstr, checksum))
-debug("OK\n")
-return res
-except Exception, e:
-debug("FAIL %s\n" % str(e))
-raise
-
-
-def download_template(name, server, destdir):
-tag = "latest"
-
-offset = name.find(':')
-if offset != -1:
-tag = name[offset + 1:]
-name = name[0:offset]
-
-# First we must ask the index server about the image name. THe
-# index server will return an auth token we can use when talking
-# to the registry server. We need this token even when anonymous
-try:
-(data, res) = get_json(server, "/v1/repositories/" + name + "/images",
-   {"X-Docker-Token": "true"})
-except urllib2.HTTPError, e:
-raise ValueError(["Image '%s' does not exist" % name])
-
-registryserver = res.info().getheader('X-Docker-Endpoints')
-token = res.info().getheader('X-Docker-Token')
-checksums = {}
-for layer in data:
-pass
-# XXX Checksums here don't appear to match the data in
-# image download later. Find out what these are sums of
-#checksums[layer["id"]] = layer["checksum"]
-
-# Now we ask the registry server for the list of tags associated
-# with the image. Tags usually reflect some kind of version of
-# the image, but they aren't officially "versions". There is
-# always a "latest" tag which is the most recent upload
-#
-# We must pass in the auth token from the index server. This
-# token can only be used once, and we're given a cookie back
-# in return to use for later RPC calls.
-(data, res) = get_json(registryserver, "/v1/repositories/" + name + 
"/tags",
-   { "Authorization": "Token " + token })
-
-cookie = res.info().getheader('Set-Cookie')
-
-if not tag in data:
-raise ValueError(["Tag '%s' does not exist for image '%s'" % (tag, 
name)])
-imagetagid = data[tag]
-
-# Only base images are self-contained, most images reference one
-# or more parents, in a linear stack. Here we are getting the list
-# of layers for the image with the tag we used.
-(data, res) = get_json(registryserver, "/v1/images/" + imagetagid + 
"/ancestry",
-

[libvirt] [PATCH sandbox v5 05/20] Image: virt-sandbox-image default dir constants

[Daniel P. Berrange]

From: Eren Yagdiran 

Define some constants to refer to the default image and
template storage directories

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 3bf7d58..de34321 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -32,6 +32,16 @@ import sys
 import urllib2
 import subprocess
 
+if os.geteuid() == 0:
+default_template_dir = "/var/lib/libvirt/templates"
+default_image_dir = "/var/lib/libvirt/images"
+else:
+default_template_dir = os.environ['HOME'] + 
"/.local/share/libvirt/templates"
+default_image_dir = os.environ['HOME'] + "/.local/share/libvirt/images"
+
+debug = False
+verbose = False
+
 import importlib
 def dynamic_source_loader(name):
 name = name[0].upper() + name[1:]
-- 
2.4.3

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

Re: [libvirt] [PATCH 2/2] virDomainLockManagerAddImage: Recursively lock backing chain

[Peter Krempa]

On Tue, Sep 08, 2015 at 17:17:19 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1192399
> 
> It's known fact for a while now that we should not only lock the
> top level layers of backing chain but the rest of it too. And
> well known too that we are not doing that. Well, up until this
> commit. The reason is that while one guest can have for instance
> the following backing chain: A (top) -> B -> C (bottom), the
> other can be started just over B or C. But libvirt should prevent
> that as it will almost certainly mangle the backing chain for the
> former guest. On the other hand, we want to allow guest with the

Well, the corruption may still happen since if you run the guest that
uses image 'B' for writing, without starting the one that uses 'A' the
image 'A' will be invalidated anyways.

> following backing chain: D (top) -> B -> C (bottom), because in
> both cases B and C are there just for reading. In order to
> achieve that we must lock the rest of backing chain with
> VIR_LOCK_MANAGER_RESOURCE_SHARED flag.

See below ...

> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/locking/domain_lock.c | 53 
> +--
>  1 file changed, 33 insertions(+), 20 deletions(-)
> 
> diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c
> index 705b132..faf73f2 100644
> --- a/src/locking/domain_lock.c
> +++ b/src/locking/domain_lock.c
> @@ -69,35 +69,48 @@ static int virDomainLockManagerAddLease(virLockManagerPtr 
> lock,
>  
>  
>  static int virDomainLockManagerAddImage(virLockManagerPtr lock,
> -virStorageSourcePtr src)
> +virStorageSourcePtr disk)

This function is used both in cases where you are adding the whole disk
to the lock manager but also in cases where we are adding individual
members of the backing chain that may already contain shared portions of
the backing chain (eg. when creating snapshots) which is not desired.

virDomainLockDiskAttach should recurse through the backing chain, but
virDomainLockImageAttach should not.

While this wouldn't pose a big problem for adding disks, when you are
removing a single image file from the locking, the code would unlock the
whole backing chain.

>  {
>  unsigned int diskFlags = 0;
> -int type = virStorageSourceGetActualType(src);
> -
> -if (!src->path)
> -return 0;
> -
> -if (!(type == VIR_STORAGE_TYPE_BLOCK ||
> -  type == VIR_STORAGE_TYPE_FILE ||
> -  type == VIR_STORAGE_TYPE_DIR))
> -return 0;
> +virStorageSourcePtr src = disk;
> +int ret = -1;
>  
> +/* The top layer of backing chain should have the correct flags set.
> + * The rest should be locked with VIR_LOCK_MANAGER_RESOURCE_SHARED
> + * for it can be shared among other domains. */
>  if (src->readonly)
>  diskFlags |= VIR_LOCK_MANAGER_RESOURCE_READONLY;
>  if (src->shared)
>  diskFlags |= VIR_LOCK_MANAGER_RESOURCE_SHARED;
>  
> -VIR_DEBUG("Add disk %s", src->path);
> -if (virLockManagerAddResource(lock,
> -  VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK,
> -  src->path,
> -  0,
> -  NULL,
> -  diskFlags) < 0) {
> -VIR_DEBUG("Failed add disk %s", src->path);
> -return -1;
> +while (src)  {
> +if (!virStorageSourceIsLocalStorage(src))
> +break;

Also note that once you create a snapshot on a remote storage device on
top of the existing local backing chain, the backing chain of that image
won't get unlocked.

> +
> +if (!src->path)
> +break;
> +
> +VIR_DEBUG("Add disk %s", src->path);
> +if (virLockManagerAddResource(lock,
> +  VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK,
> +  src->path,
> +  0,
> +  NULL,
> +  diskFlags) < 0) {
> +VIR_DEBUG("Failed add disk %s", src->path);
> +goto cleanup;
> +}
> +
> +/* Now that we've added the first disk (top layer of backing chain)
> + * into the lock manager, let's traverse the rest of the backing 
> chain
> + * and lock each layer for RO. This should prevent accidentally
> + * starting a domain with RW disk from the middle of the chain. */
> +diskFlags = VIR_LOCK_MANAGER_RESOURCE_SHARED;

The VIR_LOCK_MANAGER_RESOURCE_SHARED flag means "shared, writable" mode
according to lock_driver.h, I think you want to lock it with
VIR_LOCK_MANAGER_RESOURCE_READONLY.

> +src = src->backingStore;
>  }
> -return 0;
> +ret = 0;
> + cleanup:
> +return ret;
>  }

Peter


signature.asc
Description: Digital signature
--

Re: [libvirt] [sandbox PATCH v4 07/21] Image: Add download function

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:35PM +, Eren Yagdiran wrote:
> Refactor download function from virt-sandbox-image to use
> the newly introduced Source abstract class. The docker-specific
> download code is moved to a new DockerSource class.
> ---
>  virt-sandbox-image/Makefile.am |   1 +
>  virt-sandbox-image/sources/DockerSource.py | 214 
> +
>  virt-sandbox-image/sources/Source.py   |   4 +
>  virt-sandbox-image/virt-sandbox-image.py   | 204 +--
>  4 files changed, 251 insertions(+), 172 deletions(-)
>  create mode 100644 virt-sandbox-image/sources/DockerSource.py
> diff --git a/virt-sandbox-image/sources/DockerSource.py 
> b/virt-sandbox-image/sources/DockerSource.py
> new file mode 100644
> index 000..f3cf5f3
> --- /dev/null
> +++ b/virt-sandbox-image/sources/DockerSource.py
> +
> +def download_template(self,**args):
> +name = args['name']
> +registry = args['registry'] if args['registry'] is not None else 
> self.default_index_server
> +username = args['username']
> +password = args['password']
> +templatedir = args['templatedir']
> +self._download_template(name,registry,username,password,templatedir)

Using '**args' is a bit of a wierd way todo named parameters in python. We 
should
jsut list the named parameters explicitly as you've done in the folloiwing
_download_template method. In fact we don't need separate download_template
and _download_template methods at all.

> +def _download_template(self,name, server,username,password,destdir):
> +

> diff --git a/virt-sandbox-image/sources/Source.py 
> b/virt-sandbox-image/sources/Source.py
> index 508ca80..8751689 100644
> --- a/virt-sandbox-image/sources/Source.py
> +++ b/virt-sandbox-image/sources/Source.py
> @@ -25,3 +25,7 @@ class Source():
>  __metaclass__ = ABCMeta
>  def __init__(self):
>  pass
> +
> +@abstractmethod
> +def download_template(self,**args):
> +pass

We actually want to list the named parameters here, as the base class
is defining the API contract that the subclass must satisfy. It is
also worth having python docs inline.



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] Distribute only generated virkeymaps.h

[Martin Kletzander]

On Tue, Sep 08, 2015 at 02:10:18PM +0100, Daniel P. Berrange wrote:

On Tue, Sep 08, 2015 at 03:06:41PM +0200, Martin Kletzander wrote:

We are distributing virkeymaps.h and all the tools needed to rebuild
that file.  On top of that, we are generating that file into the
$(srcdir) and that sometimes fails for me when trying to do make dist in
VPATH on rawhide fedora.  And we don't clean the file when
maintainer-clean make target is requested.


I'd suggest we do the opposite - don't distribute virkeymaps.h - we
already require users to have python, so we should be easily able
to generate it at build time.



Oh, I just noticed one thing.  Since the virkeymaps.h is mentioned in
the list of files needed for libvirt_util sources, well basically
*any* file being built, it will get generated anyway.  So in this case
there is no point in regenerating it.  And it won't regenerate when
it's not needed, so I would keep the patch as it is.



Signed-off-by: Martin Kletzander 
---
 src/Makefile.am | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 429137561c6f..c2784af299dc 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -173,15 +173,13 @@ UTIL_SOURCES =
\
$(NULL)


-EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
-   $(srcdir)/util/virkeycode-mapgen.py


Keep keymaps.csv + virkeycode-mapgen.py


-
 BUILT_SOURCES += util/virkeymaps.h
+MAINTAINERCLEANFILES += util/virkeymaps.h


Change to CLEANFILES



 util/virkeymaps.h: $(srcdir)/util/keymaps.csv  \
$(srcdir)/util/virkeycode-mapgen.py
$(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py \
- <$(srcdir)/util/keymaps.csv >$(srcdir)/util/virkeymaps.h
+ <$(srcdir)/util/keymaps.csv >util/virkeymaps.h

 # Internal generic driver infrastructure
 NODE_INFO_SOURCES = nodeinfo.h nodeinfo.c nodeinfopriv.h




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

[libvirt] [PATCH sandbox v5 02/20] Rename 'name' to 'template' to disambiguate

[Daniel P. Berrange]

Multiple objects have names, and it is desirable to reserve
the 'name' arg to refer to the name of the sandbox instance,
so rename 'name' to 'template'.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index ec96c7e..89eef1a 100644
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -327,37 +327,37 @@ def create_template(name, imagepath, format, destdir):
 parentImage = templateImage
 
 def download(args):
-info("Downloading %s from %s to %s\n" % (args.name, default_index_server, 
default_template_dir))
-download_template(args.name, default_index_server, default_template_dir)
+info("Downloading %s from %s to %s\n" % (args.template, 
default_index_server, default_template_dir))
+download_template(args.template, default_index_server, 
default_template_dir)
 
 def delete(args):
-info("Deleting %s from %s\n" % (args.name, default_template_dir))
-delete_template(args.name, default_template_dir)
+info("Deleting %s from %s\n" % (args.template, default_template_dir))
+delete_template(args.template, default_template_dir)
 
 def create(args):
-info("Creating %s from %s in format %s\n" % (args.imagepath, args.name, 
args.format))
-create_template(args.name, args.imagepath, args.format, 
default_template_dir)
+info("Creating %s from %s in format %s\n" % (args.imagepath, 
args.template, args.format))
+create_template(args.template, args.imagepath, args.format, 
default_template_dir)
 
-def requires_name(parser):
-parser.add_argument("name",
+def requires_template(parser):
+parser.add_argument("template",
 help=_("name of the template"))
 
 def gen_download_args(subparser):
 parser = subparser.add_parser("download",
help=_("Download template data"))
-requires_name(parser)
+requires_template(parser)
 parser.set_defaults(func=download)
 
 def gen_delete_args(subparser):
 parser = subparser.add_parser("delete",
help=_("Delete template data"))
-requires_name(parser)
+requires_template(parser)
 parser.set_defaults(func=delete)
 
 def gen_create_args(subparser):
 parser = subparser.add_parser("create",
help=_("Create image from template data"))
-requires_name(parser)
+requires_template(parser)
 parser.add_argument("imagepath",
 help=_("path for image"))
 parser.add_argument("format",
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 04/20] Image: Add Hooking Mechanism

[Daniel P. Berrange]

From: Eren Yagdiran 

Define a 'Source' class which is an abstract base to use for
different template repository sources. Initially there will be
a docker source which can talk to the Docker repository REST
API, but others may follow.

Signed-off-by: Daniel P. Berrange 
---
 .gitignore|  1 +
 configure.ac  |  1 +
 libvirt-sandbox/image/Makefile.am |  2 ++
 libvirt-sandbox/image/cli.py  | 15 --
 libvirt-sandbox/image/sources/Makefile.am |  8 
 libvirt-sandbox/image/sources/Source.py   | 33 +++
 libvirt-sandbox/image/sources/__init__.py |  0
 7 files changed, 54 insertions(+), 6 deletions(-)
 mode change 100644 => 100755 libvirt-sandbox/image/cli.py
 create mode 100644 libvirt-sandbox/image/sources/Makefile.am
 create mode 100644 libvirt-sandbox/image/sources/Source.py
 create mode 100644 libvirt-sandbox/image/sources/__init__.py

diff --git a/.gitignore b/.gitignore
index f77ea12..390d65c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -42,6 +42,7 @@ libvirt-sandbox/LibvirtSandbox-1.0.typelib
 *.lo
 *.la
 *.o
+*.pyc
 *~
 libvirt-sandbox/libvirt-sandbox-init-lxc
 libvirt-sandbox/libvirt-sandbox-init-common
diff --git a/configure.ac b/configure.ac
index a8376ca..92d65f4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -130,6 +130,7 @@ AC_OUTPUT(Makefile
   libvirt-sandbox/Makefile
   libvirt-sandbox/tests/Makefile
   libvirt-sandbox/image/Makefile
+  libvirt-sandbox/image/sources/Makefile
   bin/Makefile
   examples/Makefile
   docs/Makefile
diff --git a/libvirt-sandbox/image/Makefile.am 
b/libvirt-sandbox/image/Makefile.am
index 7c8da51..344a881 100644
--- a/libvirt-sandbox/image/Makefile.am
+++ b/libvirt-sandbox/image/Makefile.am
@@ -1,4 +1,6 @@
 
+SUBDIRS = sources
+
 pythonsandboxdir = $(pythondir)/libvirt_sandbox
 pythonsandbox_DATA = __init__.py
 
diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
old mode 100644
new mode 100755
index ea04820..3bf7d58
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -1,5 +1,5 @@
 #!/usr/bin/python -Es
-#
+# -*- coding: utf-8 -*-
 # Authors: Daniel P. Berrange 
 #  Eren Yagdiran 
 #
@@ -32,11 +32,14 @@ import sys
 import urllib2
 import subprocess
 
-default_index_server = "index.docker.io"
-default_template_dir = "/var/lib/libvirt/templates"
-
-debug = True
-verbose = True
+import importlib
+def dynamic_source_loader(name):
+name = name[0].upper() + name[1:]
+modname = "libvirt_sandbox.image.sources." + name + "Source"
+mod = importlib.import_module(modname)
+classname = name + "Source"
+classimpl = getattr(mod, classname)
+return classimpl()
 
 gettext.bindtextdomain("libvirt-sandbox", "/usr/share/locale")
 gettext.textdomain("libvirt-sandbox")
diff --git a/libvirt-sandbox/image/sources/Makefile.am 
b/libvirt-sandbox/image/sources/Makefile.am
new file mode 100644
index 000..48d0f33
--- /dev/null
+++ b/libvirt-sandbox/image/sources/Makefile.am
@@ -0,0 +1,8 @@
+
+pythonimagedir = $(pythondir)/libvirt_sandbox/image/sources
+pythonimage_DATA = \
+   __init__.py \
+   Source.py \
+   $(NULL)
+
+EXTRA_DIST = $(pythonimage_DATA)
diff --git a/libvirt-sandbox/image/sources/Source.py 
b/libvirt-sandbox/image/sources/Source.py
new file mode 100644
index 000..f12b0eb
--- /dev/null
+++ b/libvirt-sandbox/image/sources/Source.py
@@ -0,0 +1,33 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2015 Universitat Politècnica de Catalunya.
+#
+# 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: Eren Yagdiran 
+
+from abc import ABCMeta, abstractmethod
+
+class Source():
+'''The Source class defines the base interface for
+all image provider source implementations. An image
+provide source is able to download templates from
+a repository, convert them to a host specific image
+format and report commands used to invoke them.'''
+
+__metaclass__ = ABCMeta
+def __init__(self):
+pass
diff --git a/libvirt-sandbox/image/sources/__init__.py 

[libvirt] [PATCH sandbox v5 11/20] Image: Add check_connect function

[Daniel P. Berrange]

From: Eren Yagdiran 

Check if user-specified connect argument is valid

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/cli.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 6f9a5e7..2672a20 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -105,6 +105,12 @@ def requires_name(parser):
 parser.add_argument("-n","--name",
 help=_("Name of the running sandbox"))
 
+def check_connect(connectstr):
+supportedDrivers = ['lxc:///','qemu:///session','qemu:///system']
+if not connectstr in supportedDrivers:
+raise ValueError("URI '%s' is not supported by virt-sandbox-image" % 
connectstr)
+return True
+
 def requires_source(parser):
 parser.add_argument("-s","--source",
 default="docker",
-- 
2.4.3

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

[libvirt] [PATCH sandbox v5 09/20] Image: Add get_command function to Source

[Daniel P. Berrange]

From: Eren Yagdiran 

Provide a way to know how a template can be started depending on the used source
DockerSource will need to parse the topmost config file in order to find the 
igniter command

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/sources/DockerSource.py | 31 +++
 libvirt-sandbox/image/sources/Source.py   | 12 +++
 2 files changed, 43 insertions(+)

diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
b/libvirt-sandbox/image/sources/DockerSource.py
index ab18b52..2607011 100644
--- a/libvirt-sandbox/image/sources/DockerSource.py
+++ b/libvirt-sandbox/image/sources/DockerSource.py
@@ -29,6 +29,16 @@ import os
 import subprocess
 import shutil
 
+class DockerConfParser():
+
+def __init__(self,jsonfile):
+with open(jsonfile) as json_file:
+self.json_data = json.load(json_file)
+def getCommand(self):
+return self.json_data['config']['Cmd']
+def getEntrypoint(self):
+return self.json_data['config']['Entrypoint']
+
 class DockerSource(Source):
 
 www_auth_username = None
@@ -345,5 +355,26 @@ class DockerSource(Source):
 parent = None
 imagetagid = parent
 
+def _get_template_data(self, templatename, templatedir):
+imageList = self._get_image_list(templatename, templatedir)
+toplayer = imageList[0]
+diskfile = templatedir + "/" + toplayer + "/template.qcow2"
+configfile = templatedir + "/" + toplayer + "/template.json"
+return configfile, diskfile
+
+def get_command(self, templatename, templatedir, userargs):
+configfile, diskfile = self._get_template_data(templatename, 
templatedir)
+configParser = DockerConfParser(configfile)
+cmd = configParser.getCommand()
+entrypoint = configParser.getEntrypoint()
+if entrypoint is None:
+entrypoint = []
+if cmd is None:
+cmd = []
+if userargs is not None and len(userargs) > 0:
+return entrypoint + userargs
+else:
+return entrypoint + cmd
+
 def debug(msg):
 sys.stderr.write(msg)
diff --git a/libvirt-sandbox/image/sources/Source.py 
b/libvirt-sandbox/image/sources/Source.py
index 4aea5c9..4305d0b 100644
--- a/libvirt-sandbox/image/sources/Source.py
+++ b/libvirt-sandbox/image/sources/Source.py
@@ -71,3 +71,15 @@ class Source():
 Delete all local files associated with the template
 """
 pass
+
+@abstractmethod
+def get_command(self, templatename, templatedir, userargs):
+"""
+:param templatename: name of the template image to query
+:param templatedir: local directory path in which templates are stored
+:param userargs: user specified arguments to run
+
+Get the command line to invoke in the container. If userargs
+is specified, then this should override the default args in
+the image"""
+pass
-- 
2.4.3

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

[libvirt] [PATCH 2/2] virDomainLockManagerAddImage: Recursively lock backing chain

[Michal Privoznik]

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

It's known fact for a while now that we should not only lock the
top level layers of backing chain but the rest of it too. And
well known too that we are not doing that. Well, up until this
commit. The reason is that while one guest can have for instance
the following backing chain: A (top) -> B -> C (bottom), the
other can be started just over B or C. But libvirt should prevent
that as it will almost certainly mangle the backing chain for the
former guest. On the other hand, we want to allow guest with the
following backing chain: D (top) -> B -> C (bottom), because in
both cases B and C are there just for reading. In order to
achieve that we must lock the rest of backing chain with
VIR_LOCK_MANAGER_RESOURCE_SHARED flag.

Signed-off-by: Michal Privoznik 
---
 src/locking/domain_lock.c | 53 +--
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c
index 705b132..faf73f2 100644
--- a/src/locking/domain_lock.c
+++ b/src/locking/domain_lock.c
@@ -69,35 +69,48 @@ static int virDomainLockManagerAddLease(virLockManagerPtr 
lock,
 
 
 static int virDomainLockManagerAddImage(virLockManagerPtr lock,
-virStorageSourcePtr src)
+virStorageSourcePtr disk)
 {
 unsigned int diskFlags = 0;
-int type = virStorageSourceGetActualType(src);
-
-if (!src->path)
-return 0;
-
-if (!(type == VIR_STORAGE_TYPE_BLOCK ||
-  type == VIR_STORAGE_TYPE_FILE ||
-  type == VIR_STORAGE_TYPE_DIR))
-return 0;
+virStorageSourcePtr src = disk;
+int ret = -1;
 
+/* The top layer of backing chain should have the correct flags set.
+ * The rest should be locked with VIR_LOCK_MANAGER_RESOURCE_SHARED
+ * for it can be shared among other domains. */
 if (src->readonly)
 diskFlags |= VIR_LOCK_MANAGER_RESOURCE_READONLY;
 if (src->shared)
 diskFlags |= VIR_LOCK_MANAGER_RESOURCE_SHARED;
 
-VIR_DEBUG("Add disk %s", src->path);
-if (virLockManagerAddResource(lock,
-  VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK,
-  src->path,
-  0,
-  NULL,
-  diskFlags) < 0) {
-VIR_DEBUG("Failed add disk %s", src->path);
-return -1;
+while (src)  {
+if (!virStorageSourceIsLocalStorage(src))
+break;
+
+if (!src->path)
+break;
+
+VIR_DEBUG("Add disk %s", src->path);
+if (virLockManagerAddResource(lock,
+  VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK,
+  src->path,
+  0,
+  NULL,
+  diskFlags) < 0) {
+VIR_DEBUG("Failed add disk %s", src->path);
+goto cleanup;
+}
+
+/* Now that we've added the first disk (top layer of backing chain)
+ * into the lock manager, let's traverse the rest of the backing chain
+ * and lock each layer for RO. This should prevent accidentally
+ * starting a domain with RW disk from the middle of the chain. */
+diskFlags = VIR_LOCK_MANAGER_RESOURCE_SHARED;
+src = src->backingStore;
 }
-return 0;
+ret = 0;
+ cleanup:
+return ret;
 }
 
 
-- 
2.4.6

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

[libvirt] [PATCH 1/2] qemuProcessStart: Parse backingStore for all disks

[Michal Privoznik]

Later, in the next commit, we are going to need backing chain
for all the disks within the domain. Let's parse them at domain
startup phase.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_domain.c  | 11 +++
 src/qemu/qemu_process.c |  6 +-
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 91c632c..54bcce9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2788,17 +2788,12 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
 virFileExists(virDomainDiskGetSource(disk)))
 continue;
 
-if (qemuDomainDetermineDiskChain(driver, vm, disk, true, true) >= 0)
-continue;
-
 if (disk->startupPolicy &&
 qemuDomainCheckDiskStartupPolicy(driver, vm, idx,
- cold_boot) >= 0) {
-virResetLastError();
-continue;
-}
+ cold_boot) < 0)
+goto error;
 
-goto error;
+virResetLastError();
 }
 
 ret = 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f7eb2b6..693ec33 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4661,7 +4661,11 @@ int qemuProcessStart(virConnectPtr conn,
  * cgroup and security setting.
  */
 for (i = 0; i < vm->def->ndisks; i++) {
-if (virStorageTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0)
+virDomainDiskDefPtr disk = vm->def->disks[i];
+if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
+goto cleanup;
+
+if (qemuDomainDetermineDiskChain(driver, vm, disk, true, true) < 0)
 goto cleanup;
 }
 
-- 
2.4.6

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

Re: [libvirt] [PATCH] Distribute only generated virkeymaps.h

[Martin Kletzander]

On Tue, Sep 08, 2015 at 02:10:18PM +0100, Daniel P. Berrange wrote:

On Tue, Sep 08, 2015 at 03:06:41PM +0200, Martin Kletzander wrote:

We are distributing virkeymaps.h and all the tools needed to rebuild
that file.  On top of that, we are generating that file into the
$(srcdir) and that sometimes fails for me when trying to do make dist in
VPATH on rawhide fedora.  And we don't clean the file when
maintainer-clean make target is requested.


I'd suggest we do the opposite - don't distribute virkeymaps.h - we
already require users to have python, so we should be easily able
to generate it at build time.



That's another option.  OK.  By the way, while I was looking at the
makefile, I saw that when we are in VPATH, we're generating way too
much stuff into the $(srcdir) even though it should be in $(builddir)
as I believe we shouldn't touch anything outside VPATH when building
in one.  Or am I wrong?



Signed-off-by: Martin Kletzander 
---
 src/Makefile.am | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 429137561c6f..c2784af299dc 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -173,15 +173,13 @@ UTIL_SOURCES =
\
$(NULL)


-EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
-   $(srcdir)/util/virkeycode-mapgen.py


Keep keymaps.csv + virkeycode-mapgen.py


-
 BUILT_SOURCES += util/virkeymaps.h
+MAINTAINERCLEANFILES += util/virkeymaps.h


Change to CLEANFILES



 util/virkeymaps.h: $(srcdir)/util/keymaps.csv  \
$(srcdir)/util/virkeycode-mapgen.py
$(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py \
- <$(srcdir)/util/keymaps.csv >$(srcdir)/util/virkeymaps.h
+ <$(srcdir)/util/keymaps.csv >util/virkeymaps.h

 # Internal generic driver infrastructure
 NODE_INFO_SOURCES = nodeinfo.h nodeinfo.c nodeinfopriv.h




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] [PATCH] util: Add win32 version of virFileUnlink

[Martin Kletzander]

On Tue, Sep 08, 2015 at 09:12:11AM -0400, John Ferlan wrote:



On 09/07/2015 04:25 AM, Martin Kletzander wrote:

Commit 35847860f65f Added the virFileUnlink function, but failed to add
a version for mingw build, causing the following error:

  Cannot export virFileUnlink: symbol not defined

Signed-off-by: Martin Kletzander 
---
 src/util/virfile.c | 14 ++
 1 file changed, 14 insertions(+)



Ugh... Sorry about this one... Too bad there wasn't a way to have some
sort of make check rule - it wasn't the first and won't be the last time
 ;-)



That's not easy without having such build.  I'm sure there are many of
those as the matrix of #ifs and #ifndefs etc. grows exponentially.  If
we had some kind of check for that, it would take way too much time.
And the mingw build is still broken because other things I'm trying to
investigate, so don't worry about that ;)


John

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 408d2d912f13..75819d9c8bd7 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2637,6 +2637,20 @@ virDirCreate(const char *path ATTRIBUTE_UNUSED,

 return -ENOSYS;
 }
+
+int
+virFileUnlink(const char *path,
+  uid_t uid ATTRIBUTE_UNUSED,
+  gid_t gid ATTRIBUTE_UNUSED)
+{
+if (unlink(path) < 0) {
+virReportSystemError(errno, _("Unable to unlink path '%s'"),
+ path);
+return -1;
+}
+
+return 0;
+}
 #endif /* WIN32 */

 /**



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

Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo

[Fabio Kung]

On Mon, Sep 7, 2015 at 8:55 AM, Serge Hallyn  wrote:
>
> Ah, my memory was failing me, so took a bit of searching, but
>
> http://fabiokung.com/2014/03/13/memory-inside-linux-containers/
>
> I can't find anything called 'libmymem', and in 2014 he said
>
> https://github.com/docker/docker/issues/8427#issuecomment-58255159
>
> so maybe this never went anywhere.

Correct, unfortunately.


> For the same reasons you cited above, and because everyeone is rolling
> their own at fuse level, I still think that a libresource and patches
> to proc tools to use them, is the right way to go.  We have no shortage
> of sample code for the functions doing the actual work, between libvirt,
> lxc, docker, etc :)
>
> Should we just go ahead and start a libresource github project?

+1, if there's momentum on this I believe I will be able to contribute
some cycles. Maybe now is the right time?

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

[libvirt] [PATCH] qemu: Report error if per-VM directory cannot be created

[Martin Kletzander]

Commit f1f68ca33433 did not report an error if virFileMakePath()
returned -1.  Well, who would've guessed function with name starting
with 'vir' sets an errno instead of reporting an error the libvirt way.
Anyway, let's fix it, so the output changes from:

  $ virsh start arm
  error: Failed to start domain arm
  error: An error occurred, but the cause is unknown

to:

  $ virsh start arm
  error: Failed to start domain arm
  error: Cannot create directory '/var/lib/libvirt/qemu/domain-arm': Not
  a directory

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

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_process.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f7eb2b609437..d9a0942c4dfd 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4734,8 +4734,10 @@ int qemuProcessStart(virConnectPtr conn,
 if (virAsprintf(, "%s/domain-%s", cfg->libDir, vm->def->name) < 0)
 goto cleanup;

-if (virFileMakePath(tmppath) < 0)
+if (virFileMakePath(tmppath) < 0) {
+virReportSystemError(errno, _("Cannot create directory '%s'"), 
tmppath);
 goto cleanup;
+}

 if (virSecurityManagerDomainSetDirLabel(driver->securityManager,
 vm->def, tmppath) < 0)
@@ -4747,8 +4749,10 @@ int qemuProcessStart(virConnectPtr conn,
 cfg->channelTargetDir, vm->def->name) < 0)
 goto cleanup;

-if (virFileMakePath(tmppath) < 0)
+if (virFileMakePath(tmppath) < 0) {
+virReportSystemError(errno, _("Cannot create directory '%s'"), 
tmppath);
 goto cleanup;
+}

 if (virSecurityManagerDomainSetDirLabel(driver->securityManager,
 vm->def, tmppath) < 0)
-- 
2.5.1

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

Re: [libvirt] [PATCH sandbox v5 00/20] Docker image support

[Cole Robinson]

On 09/08/2015 12:29 PM, Daniel P. Berrange wrote:

> 
> Daniel P Berrange (1):
>   Add virt-sandbox-image
> 
> Daniel P. Berrange (1):
>   Rename 'name' to 'template' to disambiguate

Two different .gitconfig ?

- Cole

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

[libvirt] [PATCH] Allow execute access to /var/lib/libvirt/qemu/ for others

[Martin Kletzander]

Commit f1f68ca33433 tried fixing running multiple domains under various
users, but if the user can't browse the directory, it's hard for the
qemu running under that user to create the monitor socket.

The permissions need to be fixed in two places due to support for both
installations with and without driver modules.

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

Signed-off-by: Martin Kletzander 
---
This is not a problem for non-rpm installs because normal make install
will not change the permissions, it will just create the directory, so
it has 0755, but that difference is not something I'm trying to fix in
this patch.

 libvirt.spec.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index bb8bfc3c25c1..48461e865dc8 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -2002,7 +2002,7 @@ exit 0
 %config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu
 %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/
 %ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/
-%dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/
+%dir %attr(0751, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/
 %dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/cache/libvirt/qemu/
 %{_datadir}/augeas/lenses/libvirtd_qemu.aug
 %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug
@@ -2110,7 +2110,7 @@ exit 0
 %config(noreplace) %{_sysconfdir}/libvirt/qemu-lockd.conf
 %config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu
 %ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/
-%dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/
+%dir %attr(0751, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/
 %dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/cache/libvirt/qemu/
 %{_datadir}/augeas/lenses/libvirtd_qemu.aug
 %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug
--
2.5.1

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

Re: [libvirt] [PATCH] docs: Drop unused rule for internals/%.html.tmp target

[Guido Günther]

On Tue, Sep 08, 2015 at 09:58:12AM +0200, Martin Kletzander wrote:
[..snip..]
> No message like that for me.  I completes with 2 test failures (namely
> qemuxml2argvtest and virsh-uriprecedence), but no problem with any

That's caused by:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781232

if the maintainer doesn't act before that it will be fixed with my
libxml2 NMU:

https://ftp-master.debian.org/deferred.html

> html files being generated.  And I tried this on both stable and
> unstable debian installations.
> 
> Would you mind also trying one patch I proposed upstream?  I know it
> will seem a bit far-fetched, desperate times call for desperate
> measures, I guess.  I'm talking about this one:
> 
> https://www.redhat.com/archives/libvir-list/2015-September/msg00194.html

Nope, same error. I'll try to have another look later this week.
Cheers,
 -- Guido

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

Re: [libvirt] [PATCH] qemu: Report error if per-VM directory cannot be created

[Jiri Denemark]

On Tue, Sep 08, 2015 at 19:24:15 +0200, Martin Kletzander wrote:
> Commit f1f68ca33433 did not report an error if virFileMakePath()
> returned -1.  Well, who would've guessed function with name starting
> with 'vir' sets an errno instead of reporting an error the libvirt way.
> Anyway, let's fix it, so the output changes from:
> 
>   $ virsh start arm
>   error: Failed to start domain arm
>   error: An error occurred, but the cause is unknown
> 
> to:
> 
>   $ virsh start arm
>   error: Failed to start domain arm
>   error: Cannot create directory '/var/lib/libvirt/qemu/domain-arm': Not
>   a directory
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146886
> 
> Signed-off-by: Martin Kletzander 

ACK

Jirka

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

Re: [libvirt] [PATCH v2 2/2] virsh: Enhance the detailed output of domblklist for networked source

[Lin Ma]

在 2015年09月04日 18:53, John Ferlan 写道:


On 09/04/2015 04:26 AM, Michal Privoznik wrote:

On 03.09.2015 22:47, John Ferlan wrote:


On 09/02/2015 11:58 AM, Michal Privoznik wrote:

From: Lin Ma 

Format & output more detailed information about networked source

e.g: The output without the patch:
$ virsh domblklist $DOMAIN --details
Type   Device Target Source

networkdisk   vdatest-pool/image
networkdisk   vdbiqn.2015-08.org.example:sn01/0
networkdisk   vdc/image.raw
networkdisk   vdd-
networkdisk   vde-
networkdisk   vdfimage1
networkdisk   vdgtest-volume/image.raw

The output with the patch:
$ virsh domblklist $DOMAIN --details
Type   Device Target Source

networkdisk   vdarbd://monitor1.example.org:6321/test-pool/image
networkdisk   vdb
iscsi://192.168.124.200:3260/iqn.2015-08.org.example:sn01/0
networkdisk   vdchttp://192.168.124.200:80/image.raw
networkdisk   vddnbd+unix:///var/run/nbdsock
networkdisk   vdenbd://192.168.124.200:12345
networkdisk   vdfsheepdog://192.168.124.200:6000/image1
networkdisk   vdggluster://192.168.124.200/test-volume/image.raw

Is the goal to just format in some "standard" format or to use the
format that would be used by qemu in the command line?

While it's the former, I think we should at least cover asses and
document that these strings have no special meaning and can change later
if we find a better way of expressing them. Or should we?


Since the Source path is provided whether or not --details is supplied,
I guess I'd be concerned if we've ever made any "guarantees" about the
format of the output for default displays and what a change like this
could break. I can tell you that when there was a change to add an extra
leading space to some display output, there were virt-tests that just
began failing because the difference between seeing "# ..." and " # ..."
in the output wasn't handled properly.

Anyway, the man page says:

Print a table showing the brief information of all block devices
associated with I. If I<--inactive> is specified, query the
block devices that will be used on the next boot, rather than those
currently in use by a running domain. If I<--details> is specified,
disk type and device value will also be printed. Other contexts
that require a block device name (such as I or
I for disk snapshots) will accept either target
or unique source names printed by this command.


For networked storage, The domblkinfo or snapshot-create won't work
with target or source name, Should we add something to mention it?



Which a naive user could be led to believe that by grabbing the value in
the "Source" column (such as "nbd://192.168.124.200:12345") they could
feed that into "virsh domblkinfo $dom $Source" and it would work. In
fact, someone that can write scripts better than I could be currently
stripping that last field off and using it as input to their domblkinfo
command in order to get the Capacity, Allocation, Physical values in
some form. Yes, of course those would be "broken" today for network.
Since the test environment is already set up, Lin Ma can you at least
see what happens for the various formats if one just cut-n-pasted that
column for domblkinfo?

One option/adjustment perhaps is we only print the "details" of the
network source if --details is provided (and document it).  Or we could
add something like the following after the first sentence to virsh.pod
(for the CYA needs):

For networked storage the Source displayed uses the domain XML to
extract source protocol, transport, host name, host port, and source
name or socket data in order to format and display in a standard manner
starting with the protocol, such as:

"$protocol[+$transport]://[$host:$port][{/$name|:$socket}]"



I tend to :
1. Add above explaination to virsh.pod.
2. Mention in virsh.pod that the command require a block device name 
says domblkinfo or snapshot-create can't

accept the target or source name for networked storage.
3. Print the whole url info for networked storage if --details is 
provided.  [Done]


May I have your ideas? or anyelse suggestions?

That could be ugly difficult to display and I don't see any other
current example in a quick scan through virsh.pod.

John

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