Re: [libvirt] [sandbox PATCH v3 01/22] Add virt-sandbox-image

[Daniel P. Berrange]

On Tue, Aug 18, 2015 at 06:53:22AM +, Eren Yagdiran wrote:
> 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.
> ---
>  po/POTFILES.in   |   1 +
>  virt-sandbox-image/virt-sandbox-image.py | 394 
> +++
>  2 files changed, 395 insertions(+)
>  create mode 100644 virt-sandbox-image/virt-sandbox-image.py

ACK

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

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

Re: [libvirt] [sandbox PATCH v4 01/21] Add virt-sandbox-image

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:29PM +, Eren Yagdiran wrote:
> 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.
> ---
>  po/POTFILES.in   |   1 +
>  virt-sandbox-image/virt-sandbox-image.py | 394 
> +++
>  2 files changed, 395 insertions(+)
>  create mode 100644 virt-sandbox-image/virt-sandbox-image.py

ACK


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

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

Re: [libvirt] [PATCH 0/4] Several virsh initialization adjustments

[Erik Skultety]

On 04/09/15 13:47, Ján Tomko wrote:
> On Fri, Sep 04, 2015 at 01:10:02PM +0200, Erik Skultety wrote:
>> When looking at commit 4fdd873f, I've come to notice, that after my changes 
>> to
>> virsh (834c5720), vshInit always calls vshReadlineInit and that is because
>> client mode defaults to interactive which might be changed after command line
>> arguments are parsed. So this series addresses this minor issue and provides
>> some small tweaks and adjustments.
>>
>> Erik Skultety (4):
>>   virsh: Do not make interactive mode default
>>   vsh: adjust vshInit signature and remove redundant error label
>>   vsh: Introduce vshInitReload
>>   vsh: Make vshInitDebug static
>>
>>  tools/virsh.c | 12 +++-
>>  tools/vsh.c   | 32 ++--
>>  tools/vsh.h   |  4 ++--
>>  3 files changed, 31 insertions(+), 17 deletions(-)
> 
> ACK series
> 
> Jan
> 

Thanks for review, I fixed 3/4, moved 1/4 after 3/4 and pushed the series.

Erik

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

Re: [libvirt] [sandbox PATCH v4 02/21] Fix virt-sandbox-image

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:30PM +, Eren Yagdiran wrote:
> Authentication fix for Docker REST API.
> ---
>  virt-sandbox-image/virt-sandbox-image.py | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)

ACK


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

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

[libvirt] [PATCH 3/4] vsh: Introduce vshInitReload

[Erik Skultety]

Commit a0b6a36f separated vshInitDebug from the original vshInit
(before virsh got split and vshInit became virshInit - commit 834c5720)
in order to be able to debug command line parsing.
After the parsing is finished, debugging is reinitialized to work properly.
There might as well be other features that require re-initialization as
the command line could specify parameters that override our defaults which
had been set prior to calling vshArgvParse.
---
 tools/virsh.c |  5 +++--
 tools/vsh.c   | 16 
 tools/vsh.h   |  1 +
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 5317be8..bb12dec 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -347,8 +347,9 @@ virshInit(vshControl *ctl)
 virshControlPtr priv = ctl->privData;
 
 /* Since we have the commandline arguments parsed, we need to
- * re-initialize all the debugging to make it work properly */
-vshInitDebug(ctl);
+ * reload our initial settings to make debugging and readline
+ * work properly */
+vshInitReload(ctl);
 
 if (priv->conn)
 return false;
diff --git a/tools/vsh.c b/tools/vsh.c
index e6ecc03..d4059fc 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2738,6 +2738,22 @@ vshInit(vshControl *ctl, const vshCmdGrp *groups, const 
vshCmdDef *set)
 return true;
 }
 
+bool
+vshInitReload(vshControl *ctl)
+{
+if (!cmdGroups && !cmdSet) {
+vshError(ctl, "%s", _("command groups and command are both NULL "
+  "run vshInit before reloading"));
+return false;
+}
+
+vshInitDebug(ctl);
+if (ctl->imode && vshReadlineInit(ctl) < 0)
+return false;
+
+return true;
+}
+
 void
 vshDeinit(vshControl *ctl)
 {
diff --git a/tools/vsh.h b/tools/vsh.h
index e2e33ba..b687604 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -303,6 +303,7 @@ int vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd 
*cmd, int *timeout);
 void vshPrintExtra(vshControl *ctl, const char *format, ...)
 ATTRIBUTE_FMT_PRINTF(2, 3);
 bool vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set);
+bool vshInitReload(vshControl *ctl);
 void vshDeinit(vshControl *ctl);
 void vshInitDebug(vshControl *ctl);
 void vshDebug(vshControl *ctl, int level, const char *format, ...)
-- 
2.4.3

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

Re: [libvirt] [sandbox PATCH v4 05/21] Image: Discard caching bytecode

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:33PM +, Eren Yagdiran wrote:
> ---
>  virt-sandbox-image/virt-sandbox-image.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/virt-sandbox-image/virt-sandbox-image.py 
> b/virt-sandbox-image/virt-sandbox-image.py
> index 55aea6a..9e98bf2 100755
> --- a/virt-sandbox-image/virt-sandbox-image.py
> +++ b/virt-sandbox-image/virt-sandbox-image.py
> @@ -40,6 +40,8 @@ default_unprivileged_storage_dir = 
> default_unprivileged_template_dir + "/storage
>  debug = False
>  verbose = False
>  
> +sys.dont_write_byte_code = True
> +

As I've said before, do *not* do this.

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 2/4] Sync and unmount filesystems during shutdown

[Daniel P. Berrange]

To ensure that all pending I/O for filesytems backed by block
devices is flushed to disk, it is important to sync and unmount
the filesystems during shutdown. To avoid relying on the kernel
reboot-on-panic behaviour, we also explicitly call reboot to
power off the guest.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/libvirt-sandbox-init-common.c | 166 ++
 libvirt-sandbox/libvirt-sandbox-init-qemu.c   |   1 +
 2 files changed, 167 insertions(+)

diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c 
b/libvirt-sandbox/libvirt-sandbox-init-common.c
index 760509f..a3b4687 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-common.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-common.c
@@ -44,6 +44,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "libvirt-sandbox-rpcpacket.h"
 
@@ -52,6 +54,8 @@ static gboolean verbose = FALSE;
 static int sigwrite;
 
 #define ATTR_UNUSED __attribute__((__unused__))
+static void sync_data(void);
+static void umount_fs(void);
 
 static void sig_child(int sig ATTR_UNUSED)
 {
@@ -598,6 +602,14 @@ static GVirSandboxRPCPacket *gvir_sandbox_encode_exit(int 
status,
 pkt->header.type = GVIR_SANDBOX_PROTOCOL_TYPE_MESSAGE;
 pkt->header.serial = serial;
 
+/* The host may destroy the guest any time after receiving
+ * the exit code messages. So although the main() has code
+ * to sync + unmount we can't rely on that running. So we
+ * opportunistically sync + unmount here too.
+ */
+sync_data();
+umount_fs();
+
 if (!gvir_sandbox_rpcpacket_encode_header(pkt, error))
 goto error;
 if (!gvir_sandbox_rpcpacket_encode_payload_msg(pkt,
@@ -613,7 +625,151 @@ static GVirSandboxRPCPacket *gvir_sandbox_encode_exit(int 
status,
 return NULL;
 }
 
+/* Copied & adapted from libguestfs daemon/sync.c under LGPLv2+ */
+static void sync_data(void)
+{
+  DIR *dir;
+  struct dirent *d;
+  char *dev_path;
+  int fd;
+
+  if (debug)
+  fprintf(stderr, "Syncing data\n");
+  sync();
+
+  /* On Linux, sync(2) doesn't perform a barrier, so qemu (which may
+   * have a writeback cache, even with cache=none) will still have
+   * some unwritten data.  Force the data out of any qemu caches, by
+   * calling fsync on all block devices.  Note we still need the
+   * call to sync above in order to schedule the writes.
+   * Thanks to: Avi Kivity, Kevin Wolf.
+   */
+
+  if (!(dir = opendir("/dev"))) {
+  fprintf(stderr, "opendir: /dev failed %s\n", strerror(errno));
+  return;
+  }
+
+  for (;;) {
+  errno = 0;
+  d = readdir(dir);
+  if (!d)
+  break;
+
+  if (!(g_str_has_prefix(d->d_name, "sd") ||
+g_str_has_prefix(d->d_name, "hd") ||
+g_str_has_prefix(d->d_name, "ubd") ||
+g_str_has_prefix(d->d_name, "vd") ||
+g_str_has_prefix(d->d_name, "sr"))) {
+  continue;
+  }
+
+  dev_path = g_strdup_printf("/dev/%s", d->d_name);
+
+  if (debug)
+  fprintf(stderr, "Syncing fd %s\n", dev_path);
+  if ((fd = open(dev_path, O_RDONLY)) < 0) {
+  fprintf(stderr, "cannot open %s: %s\n", dev_path,
+  strerror(errno));
+  g_free(dev_path);
+  continue;
+  }
+
+  /* fsync the device. */
+  if (debug) {
+  fprintf(stderr, "fsync %s\n", dev_path);
+  }
+
+  if (fsync(fd) < 0) {
+  fprintf(stderr, "failed to fsync %s: %s\n",
+  dev_path, strerror(errno));
+  }
+  if (close(fd) < 0) {
+  fprintf(stderr, "failed to close %s: %s\n",
+  dev_path, strerror(errno));
+  }
+  g_free(dev_path);
+  }
+
+  /* Check readdir didn't fail */
+  if (errno != 0) {
+  fprintf(stderr, "Failed to read /dev: %s\n",
+  strerror(errno));
+  }
+
+  /* Close the directory handle */
+  if (closedir(dir) < 0) {
+  fprintf(stderr, "Failed to block /dev: %s\n",
+  strerror(errno));
+  }
+
+  if (debug)
+  fprintf(stderr, "Syncing complete\n");
+}
+
+
+static int
+compare_longest_first (gconstpointer vp1, gconstpointer vp2)
+{
+int n1 = strlen(vp1);
+int n2 = strlen(vp2);
+return n2 - n1;
+}
+
+
+/* Copied & adapted from libguestfs daemon/sync.c under LGPLv2+ */
+static void umount_fs(void)
+{
+FILE *fp;
+struct mntent *m;
+GList *mounts = NULL, *tmp;
+
+if (debug)
+fprintf(stderr, "Unmounting all filesystems\n");
+if (!(fp = setmntent ("/proc/mounts", "r"))) {
+fprintf(stderr, "Failed to open /proc/mounts: %s\n",
+strerror(errno));
+return;
+}
+
+while ((m = getmntent (fp)) != NULL) {
+if (debug)
+fprintf(stderr, "Got fsname=%s dir=%s type=%s opts=%s freq=%d 
passno=%d\n",
+m->mnt_fsname, m->mnt_dir, m->mnt_type, m->mnt_opts,
+m->mnt_freq, m->mnt_passno);
+
+mounts = g_list_append(mounts, 

[libvirt] [PATCH sandbox 3/4] Fix passing of strace option to guest kernel

[Daniel P. Berrange]

The libvirt-sandbox-init-qemu command expects to see 'strace='
or 'strace=some,list,of,syscalls' but we only passed 'strace'.
This meant strace could never be enabled.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/libvirt-sandbox-builder-machine.c | 7 ++-
 libvirt-sandbox/libvirt-sandbox-init-qemu.c   | 2 +-
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c 
b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
index a458882..a142f68 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
@@ -239,11 +239,8 @@ static gchar 
*gvir_sandbox_builder_machine_cmdline(GVirSandboxConfig *config G_G
 g_string_append(str, " quiet loglevel=0");
 
 if ((tmp = getenv("LIBVIRT_SANDBOX_STRACE"))) {
-g_string_append(str, " strace");
-if (!g_str_equal(tmp, "1")) {
-g_string_append(str, "=");
-g_string_append(str, tmp);
-}
+g_string_append(str, " strace=");
+g_string_append(str, tmp);
 }
 
 /* These make boot a little bit faster */
diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c 
b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
index 8bde224..bbe70ad 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c
@@ -414,7 +414,7 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED)
 args[narg++] = "/tmp/sandbox.log";
 args[narg++] = "-f";
 args[narg++] = "-ff";
-if (strace) {
+if (strace && STRNEQ(strace, "1")) {
 args[narg++] = "-e";
 args[narg++] = strace;
 }
-- 
2.4.3

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

[libvirt] [PATCH 0/2] Change name of the domain upon successful rename

[Martin Kletzander]

Try running the example added in 1/2 before and after applying 2/2.

Martin Kletzander (2):
  Add example that renames domain there and back
  Change name of the domain upon successful rename

 .gitignore   |  1 +
 configure.ac |  1 +
 examples/rename/Makefile.am  | 24 +++
 examples/rename/rename.c | 73 
 src/remote/remote_driver.c   | 41 +
 src/remote/remote_protocol.x |  2 +-
 6 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 examples/rename/Makefile.am
 create mode 100644 examples/rename/rename.c

--
2.5.1

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

[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   vda
>>> rbd://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   vdg
>>> gluster://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.


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}]"


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

Re: [libvirt] [sandbox PATCH v4 04/21] Image: virt-sandbox-image default dir constants

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:32PM +, Eren Yagdiran wrote:
> Conflicts:
>   virt-sandbox-image/virt-sandbox-image.py
> ---
>  virt-sandbox-image/virt-sandbox-image.py | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/virt-sandbox-image/virt-sandbox-image.py 
> b/virt-sandbox-image/virt-sandbox-image.py
> index fa9e1c8..55aea6a 100755
> --- a/virt-sandbox-image/virt-sandbox-image.py
> +++ b/virt-sandbox-image/virt-sandbox-image.py
> @@ -32,6 +32,14 @@ import sys
>  import urllib2
>  import subprocess
>  
> +default_privileged_template_dir = "/var/lib/libvirt/templates"
> +default_home_dir = os.environ['HOME']
> +default_unprivileged_template_dir = default_home_dir + 
> "/.local/share/libvirt/templates"
> +default_privileged_storage_dir = default_privileged_template_dir + "/storage"

This should really be  "/var/lib/libvirt/images"

> +default_unprivileged_storage_dir = default_unprivileged_template_dir + 
> "/storage"

And default_home_dir + "/.local/share/libvirt/images"


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 03/21] Image: Add Hooking Mechanism

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:31PM +, Eren Yagdiran wrote:
> Any custom source provider can be added to virt-sandbox-image as a source
> ---
>  .gitignore   |  1 +
>  bin/Makefile.am  | 16 
>  bin/virt-sandbox-image.in|  3 +++
>  configure.ac |  2 ++
>  virt-sandbox-image/Makefile.am   | 13 +
>  virt-sandbox-image/sources/Source.py | 27 +++
>  virt-sandbox-image/sources/__init__.py   | 26 ++
>  virt-sandbox-image/virt-sandbox-image.py | 15 +--
>  8 files changed, 93 insertions(+), 10 deletions(-)
>  create mode 100644 bin/virt-sandbox-image.in
>  create mode 100644 virt-sandbox-image/Makefile.am
>  create mode 100644 virt-sandbox-image/sources/Source.py
>  create mode 100644 virt-sandbox-image/sources/__init__.py
>  mode change 100644 => 100755 virt-sandbox-image/virt-sandbox-image.py

ACK


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

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

Re: [libvirt] [PATCH v2 0/2] Need to perform address checks for ccw/s390

[Ján Tomko]

On Thu, Sep 03, 2015 at 02:51:54PM -0400, John Ferlan wrote:
> Assumptions were made that if someone provided an address type ccw or
> s390 that it would occur only if using an enabled emulator.  Turns out
> that premise isn't necessarily true and it leads to libvirtd crashing
> for hotplugs and qemu start errors for config paths.
> 
> These patches will make the checks prior to crashes or qemu process
> starts in order to avoid the situation.
> 
> v1:
> http://www.redhat.com/archives/libvir-list/2015-August/msg01043.html
> 
> Changes since v1...
> 
> ... Implement a function to handle the s390-ccw check using STRPREFIX
> instead of a mix of STRPREFIX and STREQLEN
> 
> ... Create qemuCheckCCWS390AddressSupport to handle checking address
> type if defined on entry to disk, controller, and rng device additions
> whether through hotplug or config options.
> 
> NB: It doesn't seem network devices are afflicted, although perhaps
> I read the code wrong. It seems for a network device there is/was
> none of the set the default address if undefined code added.
> 
> John Ferlan (2):
>   qemu: Introduce qemuDomainMachineIsS390CCW
>   qemu: Need to check for machine.os when using ADDRESS_TYPE_CCW
> 
>  src/qemu/qemu_command.c | 55 
> +++--
>  src/qemu/qemu_command.h |  5 +
>  src/qemu/qemu_domain.c  |  6 ++
>  src/qemu/qemu_domain.h  |  1 +
>  src/qemu/qemu_hotplug.c | 24 +++--
>  5 files changed, 83 insertions(+), 8 deletions(-)
> 

ACK series.

Jan


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

Re: [libvirt] [PATCH 1/2] Add example that renames domain there and back

[Daniel P. Berrange]

On Fri, Sep 04, 2015 at 02:10:07PM +0200, Martin Kletzander wrote:
> And in the middle it prints out its name to demonstrate changes in later
> patch(es).
> 
> Signed-off-by: Martin Kletzander 
> ---
>  .gitignore  |  1 +
>  configure.ac|  1 +
>  examples/rename/Makefile.am | 24 +++
>  examples/rename/rename.c| 73 
> +
>  4 files changed, 99 insertions(+)
>  create mode 100644 examples/rename/Makefile.am
>  create mode 100644 examples/rename/rename.c

ACK

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

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

Re: [libvirt] [PATCH 2/2] Change name of the domain upon successful rename

[Daniel P. Berrange]

On Fri, Sep 04, 2015 at 02:10:08PM +0200, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander 
> ---
>  src/remote/remote_driver.c   | 41 +
>  src/remote/remote_protocol.x |  2 +-
>  2 files changed, 42 insertions(+), 1 deletion(-)

ACK


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

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

Re: [libvirt] [sandbox PATCH v4 06/21] Image: Add check_writable and runtime resolver

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:34PM +, Eren Yagdiran wrote:
> These helper functions are for selecting right directories according
> to running user privileges
> ---
>  virt-sandbox-image/virt-sandbox-image.py | 27 ++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/virt-sandbox-image/virt-sandbox-image.py 
> b/virt-sandbox-image/virt-sandbox-image.py
> index 9e98bf2..5917dd6 100755
> --- a/virt-sandbox-image/virt-sandbox-image.py
> +++ b/virt-sandbox-image/virt-sandbox-image.py
> @@ -32,6 +32,8 @@ import sys
>  import urllib2
>  import subprocess
>  
> +template_dir = None
> +storage_dir = None
>  default_privileged_template_dir = "/var/lib/libvirt/templates"
>  default_home_dir = os.environ['HOME']
>  default_unprivileged_template_dir = default_home_dir + 
> "/.local/share/libvirt/templates"
> @@ -40,6 +42,29 @@ default_unprivileged_storage_dir = 
> default_unprivileged_template_dir + "/storage
>  debug = False
>  verbose = False
>  
> +def check_dir_writable(path):
> +if not os.access(path,os.W_OK):
> +return False
> +return True
> +
> +def runtime_dir_resolver():
> +global default_privileged_template_dir
> +global default_privileged_storage_dir
> +global default_unprivileged_template_dir
> +global default_unprivileged_storage_dir
> +global template_dir
> +global storage_dir
> +if(check_dir_writable(default_privileged_template_dir)):
> +template_dir = default_privileged_template_dir
> +storage_dir = default_privileged_storage_dir
> +return
> +template_dir = default_unprivileged_template_dir
> +storage_dir = default_unprivileged_storage_dir
> +if not os.path.exists(template_dir):
> +os.makedirs(template_dir)
> +if not os.path.exists(storage_dir):
> +os.makedirs(storage_dir)

This code is called in early startup before  we've even decided
what command to run. We should not be calling os.makedirs() at
this point. eg if the user asks  virt-sandbox-image --help
they would not expect us to create any files/dirs.

We should only create the directories when we actually need to
storage some data in them, eg for the 'download' command, but
not for the 'delete' command for example.

> +
>  sys.dont_write_byte_code = True
>  
>  import importlib
> @@ -380,8 +405,8 @@ def gen_create_args(subparser):
>  parser.set_defaults(func=create)
>  
>  if __name__ == '__main__':
> +runtime_dir_resolver()
>  parser = argparse.ArgumentParser(description='Sandbox Container Image 
> Tool')
> -
>  subparser = parser.add_subparsers(help=_("commands"))
>  gen_download_args(subparser)
>  gen_delete_args(subparser)

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 0/4] Several virsh initialization adjustments

[Erik Skultety]

When looking at commit 4fdd873f, I've come to notice, that after my changes to
virsh (834c5720), vshInit always calls vshReadlineInit and that is because
client mode defaults to interactive which might be changed after command line
arguments are parsed. So this series addresses this minor issue and provides
some small tweaks and adjustments.

Erik Skultety (4):
  virsh: Do not make interactive mode default
  vsh: adjust vshInit signature and remove redundant error label
  vsh: Introduce vshInitReload
  vsh: Make vshInitDebug static

 tools/virsh.c | 12 +++-
 tools/vsh.c   | 32 ++--
 tools/vsh.h   |  4 ++--
 3 files changed, 31 insertions(+), 17 deletions(-)

-- 
2.4.3

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

[libvirt] [PATCH 4/4] vsh: Make vshInitDebug static

[Erik Skultety]

There's no reason why debug initialization could not be made completely
hidden, just like readline initialization is. The point of the global
initializer vshInit is to make initialization of smaller features transparent
to the user/caller.
---
 tools/vsh.c | 2 +-
 tools/vsh.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index d4059fc..1564d84 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2680,7 +2680,7 @@ vshReadline(vshControl *ctl, const char *prompt)
 /*
  * Initialize debug settings.
  */
-void
+static void
 vshInitDebug(vshControl *ctl)
 {
 const char *debugEnv;
diff --git a/tools/vsh.h b/tools/vsh.h
index b687604..d4e9710 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -305,7 +305,6 @@ void vshPrintExtra(vshControl *ctl, const char *format, ...)
 bool vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set);
 bool vshInitReload(vshControl *ctl);
 void vshDeinit(vshControl *ctl);
-void vshInitDebug(vshControl *ctl);
 void vshDebug(vshControl *ctl, int level, const char *format, ...)
 ATTRIBUTE_FMT_PRINTF(3, 4);
 
-- 
2.4.3

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

[libvirt] [PATCH 2/4] vsh: adjust vshInit signature and remove redundant error label

[Erik Skultety]

As part of the effort to stay consistent, change the vshInit signature
from returning int to returning bool. Moreover, remove the
unnecessary error label as there is no cleanup that would make use of
it.
---
 tools/virsh.c |  2 +-
 tools/vsh.c   | 14 +-
 tools/vsh.h   |  2 +-
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index a71cd47..5317be8 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -905,7 +905,7 @@ main(int argc, char **argv)
 if ((defaultConn = virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI")))
 ctl->connname = vshStrdup(ctl, defaultConn);
 
-if (vshInit(ctl, cmdGroups, NULL) < 0)
+if (!vshInit(ctl, cmdGroups, NULL))
 exit(EXIT_FAILURE);
 
 if (!virshParseArgv(ctl, argc, argv) ||
diff --git a/tools/vsh.c b/tools/vsh.c
index 54c4614..e6ecc03 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2714,20 +2714,18 @@ vshInitDebug(vshControl *ctl)
 /*
  * Initialize global data
  */
-int
+bool
 vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set)
 {
-int ret = -1;
-
 if (!ctl->hooks) {
 vshError(ctl, "%s", _("client hooks cannot be NULL"));
-goto error;
+return false;
 }
 
 if (!groups && !set) {
 vshError(ctl, "%s", _("command groups and command set "
   "cannot both be NULL"));
-goto error;
+return false;
 }
 
 cmdGroups = groups;
@@ -2735,11 +2733,9 @@ vshInit(vshControl *ctl, const vshCmdGrp *groups, const 
vshCmdDef *set)
 vshInitDebug(ctl);
 
 if (ctl->imode && vshReadlineInit(ctl) < 0)
-goto error;
+return false;
 
-ret = 0;
- error:
-return ret;
+return true;
 }
 
 void
diff --git a/tools/vsh.h b/tools/vsh.h
index 37416c7..e2e33ba 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -302,7 +302,7 @@ int vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd 
*cmd, int *timeout);
 
 void vshPrintExtra(vshControl *ctl, const char *format, ...)
 ATTRIBUTE_FMT_PRINTF(2, 3);
-int vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set);
+bool vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set);
 void vshDeinit(vshControl *ctl);
 void vshInitDebug(vshControl *ctl);
 void vshDebug(vshControl *ctl, int level, const char *format, ...)
-- 
2.4.3

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

Re: [libvirt] [PATCH 3/4] vsh: Introduce vshInitReload

[Ján Tomko]

On Fri, Sep 04, 2015 at 01:10:05PM +0200, Erik Skultety wrote:
> Commit a0b6a36f separated vshInitDebug from the original vshInit
> (before virsh got split and vshInit became virshInit - commit 834c5720)
> in order to be able to debug command line parsing.
> After the parsing is finished, debugging is reinitialized to work properly.
> There might as well be other features that require re-initialization as
> the command line could specify parameters that override our defaults which
> had been set prior to calling vshArgvParse.
> ---
>  tools/virsh.c |  5 +++--
>  tools/vsh.c   | 16 
>  tools/vsh.h   |  1 +
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 5317be8..bb12dec 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -347,8 +347,9 @@ virshInit(vshControl *ctl)
>  virshControlPtr priv = ctl->privData;
>  
>  /* Since we have the commandline arguments parsed, we need to
> - * re-initialize all the debugging to make it work properly */
> -vshInitDebug(ctl);
> + * reload our initial settings to make debugging and readline
> + * work properly */
> +vshInitReload(ctl);
>  
>  if (priv->conn)
>  return false;
> diff --git a/tools/vsh.c b/tools/vsh.c
> index e6ecc03..d4059fc 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -2738,6 +2738,22 @@ vshInit(vshControl *ctl, const vshCmdGrp *groups, 
> const vshCmdDef *set)
>  return true;
>  }
>  
> +bool
> +vshInitReload(vshControl *ctl)
> +{
> +if (!cmdGroups && !cmdSet) {
> +vshError(ctl, "%s", _("command groups and command are both NULL "
> +  "run vshInit before reloading"));
> +return false;
> +}
> +
> +vshInitDebug(ctl);
> +if (ctl->imode && vshReadlineInit(ctl) < 0)

You add another vshReadlineInit call here, but do not remove the one in
vshInit. Just calling vshInit might make sense for callers that know
upfront they are running in interactive mode. But we should call
vshReadlineDeInit to prevent a memory leak if they call vshInitReload
too.

Jan


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

[libvirt] [PATCH 1/2] Add example that renames domain there and back

[Martin Kletzander]

And in the middle it prints out its name to demonstrate changes in later
patch(es).

Signed-off-by: Martin Kletzander 
---
 .gitignore  |  1 +
 configure.ac|  1 +
 examples/rename/Makefile.am | 24 +++
 examples/rename/rename.c| 73 +
 4 files changed, 99 insertions(+)
 create mode 100644 examples/rename/Makefile.am
 create mode 100644 examples/rename/rename.c

diff --git a/.gitignore b/.gitignore
index 6bd41be9db89..19402f5b8cd2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -83,6 +83,7 @@
 /examples/domtop/domtop
 /examples/hellolibvirt/hellolibvirt
 /examples/openauth/openauth
+/examples/rename/test
 /gnulib/lib/*
 /gnulib/m4/*
 /gnulib/tests/*
diff --git a/configure.ac b/configure.ac
index 8471a4659464..de31486792fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2806,6 +2806,7 @@ AC_CONFIG_FILES([\
 examples/domtop/Makefile \
 examples/openauth/Makefile \
 examples/hellolibvirt/Makefile \
+examples/rename/Makefile \
 examples/systemtap/Makefile \
 examples/xml/nwfilter/Makefile \
 examples/lxcconvert/Makefile \
diff --git a/examples/rename/Makefile.am b/examples/rename/Makefile.am
new file mode 100644
index ..1b3484c1f30e
--- /dev/null
+++ b/examples/rename/Makefile.am
@@ -0,0 +1,24 @@
+## Copyright (C) 2005-2013 Red Hat, Inc.
+##
+## 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, see
+## .
+
+INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include
+LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la \
+   $(COVERAGE_LDFLAGS)
+
+noinst_PROGRAMS=rename
+
+rename_SOURCES=rename.c
+rename_LDADD= $(LDADDS)
diff --git a/examples/rename/rename.c b/examples/rename/rename.c
new file mode 100644
index ..85f18e9df32d
--- /dev/null
+++ b/examples/rename/rename.c
@@ -0,0 +1,73 @@
+/*
+ * rename.c
+ *
+ * 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, see
+ * .
+ */
+
+#include 
+#include 
+#include 
+
+int main(int argc, char **argv)
+{
+virConnectPtr conn = NULL; /* the hypervisor connection */
+virDomainPtr dom = NULL;   /* the domain being checked */
+int ret = EXIT_FAILURE;
+
+if (argc != 3) {
+fprintf(stderr, "Usage: %s  \n",
+argv[0]);
+goto error;
+}
+
+conn = virConnectOpen(NULL);
+if (conn == NULL) {
+fprintf(stderr, "Failed to connect to hypervisor\n");
+goto error;
+}
+
+dom = virDomainLookupByName(conn, argv[1]);
+if (dom == NULL) {
+fprintf(stderr, "Failed to find domain\n");
+goto error;
+}
+
+printf("Before first rename: %s\n", virDomainGetName(dom));
+
+/* Get the information */
+ret = virDomainRename(dom, argv[2], 0);
+if (ret < 0) {
+fprintf(stderr, "Failed to rename domain\n");
+goto error;
+}
+
+printf("After first rename: %s\n", virDomainGetName(dom));
+
+/* Get the information */
+ret = virDomainRename(dom, argv[1], 0);
+if (ret < 0) {
+fprintf(stderr, "Failed to rename domain\n");
+goto error;
+}
+
+printf("After second rename: %s\n", virDomainGetName(dom));
+
+ error:
+if (dom != NULL)
+virDomainFree(dom);
+if (conn != NULL)
+virConnectClose(conn);
+return ret;
+}
-- 
2.5.1

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

[libvirt] [PATCH 2/2] Change name of the domain upon successful rename

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 src/remote/remote_driver.c   | 41 +
 src/remote/remote_protocol.x |  2 +-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index aca00c0974e5..a1dd64087b3b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8038,6 +8038,47 @@ remoteDomainInterfaceAddresses(virDomainPtr dom,
 }


+static int
+remoteDomainRename(virDomainPtr dom, const char *new_name, unsigned int flags)
+{
+int rv = -1;
+struct private_data *priv = dom->conn->privateData;
+remote_domain_rename_args args;
+remote_domain_rename_ret ret;
+char *tmp = NULL;
+
+if (VIR_STRDUP(tmp, new_name) < 0)
+return -1;
+
+remoteDriverLock(priv);
+
+make_nonnull_domain(, dom);
+args.new_name = new_name ? (char **)_name : NULL;
+args.flags = flags;
+
+memset(, 0, sizeof(ret));
+
+if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_RENAME,
+ (xdrproc_t)xdr_remote_domain_rename_args, (char *),
+ (xdrproc_t)xdr_remote_domain_rename_ret, (char *)) == -1) {
+goto done;
+}
+
+rv = ret.retcode;
+
+if (rv == 0) {
+VIR_FREE(dom->name);
+dom->name = tmp;
+tmp = NULL;
+}
+
+ done:
+remoteDriverUnlock(priv);
+VIR_FREE(tmp);
+return rv;
+}
+
+
 /* get_nonnull_domain and get_nonnull_network turn an on-wire
  * (name, uuid) pair into virDomainPtr or virNetworkPtr object.
  * These can return NULL if underlying memory allocations fail,
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 92a92e2bfa24..80f4a8b3a181 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -5708,7 +5708,7 @@ enum remote_procedure {
 REMOTE_PROC_DOMAIN_SET_USER_PASSWORD = 357,

 /**
- * @generate: both
+ * @generate: server
  * @acl: domain:write
  * @acl: domain:save
  */
-- 
2.5.1

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

Re: [libvirt] [PATCH 0/4] Several virsh initialization adjustments

[Ján Tomko]

On Fri, Sep 04, 2015 at 01:10:02PM +0200, Erik Skultety wrote:
> When looking at commit 4fdd873f, I've come to notice, that after my changes to
> virsh (834c5720), vshInit always calls vshReadlineInit and that is because
> client mode defaults to interactive which might be changed after command line
> arguments are parsed. So this series addresses this minor issue and provides
> some small tweaks and adjustments.
> 
> Erik Skultety (4):
>   virsh: Do not make interactive mode default
>   vsh: adjust vshInit signature and remove redundant error label
>   vsh: Introduce vshInitReload
>   vsh: Make vshInitDebug static
> 
>  tools/virsh.c | 12 +++-
>  tools/vsh.c   | 32 ++--
>  tools/vsh.h   |  4 ++--
>  3 files changed, 31 insertions(+), 17 deletions(-)

ACK series

Jan


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

Re: [libvirt] [PATCH 1/4] virsh: Do not make interactive mode default

[Ján Tomko]

On Fri, Sep 04, 2015 at 01:10:03PM +0200, Erik Skultety wrote:
> Currently, we set interactive mode as default possibly reverting the
> setting after we parse the command line arguments. There's nothing
> particulary wrong with that, but a call to vshReadlineInit is performed
> always in the global initializer just because the default mode is interactive.
> Rather than moving vshReadlineInit call somewhere else (because another client
> might want to implement interactive mode only), we could make the decision
> if we're about to run in interactive mode once the command line is parsed.
> ---
>  tools/virsh.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 

Moving this patch after the introduction of vshInitReload would make
sure that vshReadlineInit is called correctly in every intermediate
commit.

Jan


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

[libvirt] [PATCH v2] examples: Add example polkit ACL rules

[Jiri Denemark]

Creating ACL rules is not exactly easy and existing examples are pretty
simple. This patch adds a somewhat complex example which defines several
roles. Admins can do everything, operators can do basic operations
on any domain and several groups of users who act as operators but only
on a limited set of domains.

Signed-off-by: Jiri Denemark 
---
 Makefile.am   |   2 +-
 configure.ac  |   1 +
 examples/polkit/Makefile.am   |  17 ++
 examples/polkit/libvirt-acl.rules | 115 ++
 libvirt.spec.in   |   3 +
 5 files changed, 137 insertions(+), 1 deletion(-)
 create mode 100644 examples/polkit/Makefile.am
 create mode 100644 examples/polkit/libvirt-acl.rules

diff --git a/Makefile.am b/Makefile.am
index 91b943b..d338d5a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -23,7 +23,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs 
gnulib/tests \
   tests po examples/object-events examples/hellolibvirt \
   examples/dominfo examples/domsuspend examples/apparmor \
   examples/xml/nwfilter examples/openauth examples/systemtap \
-  tools/wireshark examples/dommigrate \
+  tools/wireshark examples/dommigrate examples/polkit \
   examples/lxcconvert examples/domtop
 
 ACLOCAL_AMFLAGS = -I m4
diff --git a/configure.ac b/configure.ac
index 8471a46..136c2e7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2809,6 +2809,7 @@ AC_CONFIG_FILES([\
 examples/systemtap/Makefile \
 examples/xml/nwfilter/Makefile \
 examples/lxcconvert/Makefile \
+examples/polkit/Makefile \
 tools/wireshark/Makefile \
 tools/wireshark/src/Makefile])
 AC_OUTPUT
diff --git a/examples/polkit/Makefile.am b/examples/polkit/Makefile.am
new file mode 100644
index 000..4d213e8
--- /dev/null
+++ b/examples/polkit/Makefile.am
@@ -0,0 +1,17 @@
+## Copyright (C) 2015 Red Hat, Inc.
+##
+## 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, see
+## .
+
+EXTRA_DIST = libvirt-acl.rules
diff --git a/examples/polkit/libvirt-acl.rules 
b/examples/polkit/libvirt-acl.rules
new file mode 100644
index 000..5c26593
--- /dev/null
+++ b/examples/polkit/libvirt-acl.rules
@@ -0,0 +1,115 @@
+function Role(name) {
+this.name = name;
+
+this.users = [];
+this.groups = [];
+
+this.check = function(subject, api, domain) {
+var validUser = false
+
+if (this.users.indexOf(subject.user) >= 0) {
+validUser = true;
+} else {
+for (var i = 0; i < subject.groups.length; i++) {
+if (this.groups.indexOf(subject.groups[i]) >= 0) {
+validUser = true;
+break;
+}
+}
+}
+
+if (validUser &&
+(this.name == "admin" ||
+ !domain ||
+ (this.domains && domain.match(this.domains {
+var msg = "Access granted: " +
+  "user = " + subject.user +
+  ", groups = [" + subject.groups + "]" +
+  ", role = " + this.name +
+  ", api = " + api;
+if (domain)
+msg += ", domain = " + domain;
+polkit.log(msg);
+return true
+}
+
+return false;
+};
+}
+
+
+/* Basic operations and monitoring on a limited set of domains. */
+var userA = new Role("userA");
+userA.domains = /^a/;
+userA.users = ["userA1", "userA2", "userA3", "multiUser"];
+userA.groups = ["groupA1", "groupA2"];
+
+var userB = new Role("userB");
+userB.domains = /^b/;
+userB.users = ["userB1", "userB2", "userB3", "multiUser"];
+userB.groups = ["groupB1", "groupB2", "multiGroup"];
+
+var userC = new Role("userC");
+userC.domains = /^c/;
+userC.users = ["userC1", "userC2", "userC3"];
+userC.groups = ["groupC1", "groupC2", "multiGroup"];
+
+/* Same as users but on any domain. */
+var operator = new Role("operator");
+operator.domains = /.*/;
+operator.users = ["powerUser1", "powerUser2"];
+operator.groups = ["powerGroup1", "powerGroup2", "powerGroup3"];
+
+var users = [operator, userA, userB, userC];
+
+/* Full access. */
+var admin = new Role("admin");
+admin.users = ["adminUser1"];
+admin.groups = ["adminGroup1"];
+
+
+restrictedActions = [
+"domain.core-dump",
+"domain.fs-freeze",
+"domain.fs-trim",
+ 

Re: [libvirt] [PATCH v2] examples: Add example polkit ACL rules

[Daniel P. Berrange]

On Fri, Sep 04, 2015 at 02:19:09PM +0200, Jiri Denemark wrote:
> Creating ACL rules is not exactly easy and existing examples are pretty
> simple. This patch adds a somewhat complex example which defines several
> roles. Admins can do everything, operators can do basic operations
> on any domain and several groups of users who act as operators but only
> on a limited set of domains.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  Makefile.am   |   2 +-
>  configure.ac  |   1 +
>  examples/polkit/Makefile.am   |  17 ++
>  examples/polkit/libvirt-acl.rules | 115 
> ++
>  libvirt.spec.in   |   3 +
>  5 files changed, 137 insertions(+), 1 deletion(-)
>  create mode 100644 examples/polkit/Makefile.am
>  create mode 100644 examples/polkit/libvirt-acl.rules
> 
> diff --git a/Makefile.am b/Makefile.am
> index 91b943b..d338d5a 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -23,7 +23,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs 
> gnulib/tests \
>tests po examples/object-events examples/hellolibvirt \
>examples/dominfo examples/domsuspend examples/apparmor \
>examples/xml/nwfilter examples/openauth examples/systemtap \
> -  tools/wireshark examples/dommigrate \
> +  tools/wireshark examples/dommigrate examples/polkit \
>examples/lxcconvert examples/domtop
>  
>  ACLOCAL_AMFLAGS = -I m4
> diff --git a/configure.ac b/configure.ac
> index 8471a46..136c2e7 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2809,6 +2809,7 @@ AC_CONFIG_FILES([\
>  examples/systemtap/Makefile \
>  examples/xml/nwfilter/Makefile \
>  examples/lxcconvert/Makefile \
> +examples/polkit/Makefile \
>  tools/wireshark/Makefile \
>  tools/wireshark/src/Makefile])
>  AC_OUTPUT
> diff --git a/examples/polkit/Makefile.am b/examples/polkit/Makefile.am
> new file mode 100644
> index 000..4d213e8
> --- /dev/null
> +++ b/examples/polkit/Makefile.am
> @@ -0,0 +1,17 @@
> +## Copyright (C) 2015 Red Hat, Inc.
> +##
> +## 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, see
> +## .
> +
> +EXTRA_DIST = libvirt-acl.rules
> diff --git a/examples/polkit/libvirt-acl.rules 
> b/examples/polkit/libvirt-acl.rules
> new file mode 100644
> index 000..5c26593
> --- /dev/null
> +++ b/examples/polkit/libvirt-acl.rules
> @@ -0,0 +1,115 @@


It would be beneficial to put some docs in this file header here
to explain to people what this example is achieving.

> +function Role(name) {
> +this.name = name;
> +
> +this.users = [];
> +this.groups = [];
> +
> +this.check = function(subject, api, domain) {
> +var validUser = false
> +
> +if (this.users.indexOf(subject.user) >= 0) {
> +validUser = true;
> +} else {
> +for (var i = 0; i < subject.groups.length; i++) {
> +if (this.groups.indexOf(subject.groups[i]) >= 0) {
> +validUser = true;
> +break;
> +}
> +}
> +}
> +
> +if (validUser &&
> +(this.name == "admin" ||
> + !domain ||
> + (this.domains && domain.match(this.domains {
> +var msg = "Access granted: " +
> +  "user = " + subject.user +
> +  ", groups = [" + subject.groups + "]" +
> +  ", role = " + this.name +
> +  ", api = " + api;
> +if (domain)
> +msg += ", domain = " + domain;
> +polkit.log(msg);
> +return true
> +}
> +
> +return false;
> +};
> +}
> +
> +
> +/* Basic operations and monitoring on a limited set of domains. */
> +var userA = new Role("userA");
> +userA.domains = /^a/;
> +userA.users = ["userA1", "userA2", "userA3", "multiUser"];
> +userA.groups = ["groupA1", "groupA2"];
> +
> +var userB = new Role("userB");
> +userB.domains = /^b/;
> +userB.users = ["userB1", "userB2", "userB3", "multiUser"];
> +userB.groups = ["groupB1", "groupB2", "multiGroup"];
> +
> +var userC = new Role("userC");
> +userC.domains = /^c/;
> +userC.users = ["userC1", "userC2", "userC3"];
> +userC.groups = ["groupC1", "groupC2", "multiGroup"];
> +
> +/* Same as users but on any domain. 

Re: [libvirt] [PATCH v4 5/6] vz: support misc migration options

[Nikolay Shirokovskiy]

On 04.09.2015 11:44, Daniel P. Berrange wrote:
> On Fri, Sep 04, 2015 at 10:42:00AM +0300, Nikolay Shirokovskiy wrote:
>>
>>
>> On 03.09.2015 20:04, Daniel P. Berrange wrote:
>>> On Wed, Sep 02, 2015 at 03:09:26PM +0300, Nikolay Shirokovskiy wrote:
 From: nshirokovs...@virtuozzo.com 

 Migration API has a lot of options. This patch intention is to provide
 support for those options that can be trivially supported and give
 estimation for other options support in this commit message.

 I. Supported.

 1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain
 memory'. It is supported but quite uncommon way: vz migration demands that 
 this
 option should be set. This is due to vz is hardcoded to moving VMs memory 
 using
 compression. So anyone who wants to migrate vz domain should set this 
 option
 thus declaring it knows it uses compression.

 Why bother? May be just support this option and ignore if it is not set or
 don't support at all as we can't change behaviour in this aspect.  Well I
 believe that this option is, first, inherent to hypervisor implementation 
 as
 we have a task of moving domain memory to different place and, second, we 
 have
 a tradeoff here between cpu and network resources and some managment should
 choose the stratery via this option. If we choose ignoring or unsupporting
 implementation than this option has a too vague meaning. Let's go into more
 detail.

 First if we ignore situation where option is not set than we put user into
 fallacy that vz hypervisor don't use compression and thus have lower cpu
 usage. Second approach is to not support the option. The main reason not to
 follow this way is that 'not supported and not set' is indistinguishable 
 from
 'supported and not set' and thus again fool the user.
>>>
>>> I think it is reasonable to say that the absence of 'VIR_MIGRATE_COMPRESSED'
>>> does not mean compression should be turned off.
>> For me saing so is possible only if impact of compressing/not compressing
>> is rather insignificant. Otherwise it would be strange that turning on/off
>> this option does not change anything. For example for live migration
>> option we can't afford go this way.
>>
>> So for clarity i would rather raise an error. One day may be vz
>> will turn to supporting this option too or default will change.
> 
> Sure, if you prefer to raise an error that's fine with me.. It just
> means users will need to always pass --compressed to virsh, but that
> probably isn't a big deal.
> 
 3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE. This two comes
 together. Vz domain are always persistent so we have to demand that options
 VIR_MIGRATE_PERSIST_DEST and VIR_MIGRATE_UNDEFINE_SOURCE are both set (and
 this is done just by unsupporting it).
>>>
>>> This is a tricky one. It would be nice if we could have just "done the
>>> right thing" by default with the migrate API, since the default behaviour
>>> is kind of stupid honestly, but we have to live with backwards compatible
>>> API semantics now :(
>>>
>>> To avoid forcing people to always specify these flags, could you "undo"
>>> by VZ does by default ?
>>>
>>> ie, if VIR_MIGRATE_PERSIST_DEST is *not* passed, then once migration
>>> has completed, you could use the domainMigrateFinish step to delete
>>> the config that VZ just created.
>> This is *not* possible. Vz domains are always persistent. Technically
>> vz has it own domain configuration file and we convert it to libvirt
>> xml when asked.
> 
> Oh right, I understand now.
> 
>>> Similarly if VIR_MIGRATE_UNDEFINE_SOURCE is *not* passed, then once
>>> migration completed, you could use the domainMigrateConfirm step
>>> to re-create the config on the src that VZ just deleted.
>> This is possible. Eventually this means that we just need to 
>> define domain with the same configuration as migrated one
>> right after the migration.
>>
>> So as one option is possible and the other is not and they are somewhat
>> paired may be not bother? Or should we take the ignore path as
>> for compressed so to finish *no-demanding options* strategy?
> 
> I don't mind which way you choose really. I think allowing the user
> to not pass UNDEFINE_SOURCE is fairly low priority, since most really
> /want/ to use UNDEFINE_SOURCE. It only really causes pain for people
> using the low level virsh tool. For serious apps like OpenStack the
> default VZ behaviour is what they want all the time.

After all that said I'll keep this patch unchanged then. The only
thing that can be done is the support of non-live migration, but
as it is an elaborated I'd prefer to delay it to the moment when there
will be a need to do it.




> 
> Regards,
> Daniel
> 

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

Re: [libvirt] [PATCH v4 2/6] vz: add migration backbone code

[Nikolay Shirokovskiy]

On 04.09.2015 11:40, Daniel P. Berrange wrote:
> On Fri, Sep 04, 2015 at 10:56:52AM +0300, Nikolay Shirokovskiy wrote:
 @@ -1396,6 +1585,9 @@ static virHypervisorDriver vzDriver = {
  .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */
  .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */
  .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */
 +.connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */
 +.domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 
 1.2.20 */
 +.domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 
 1.2.20 */
>>>
>>> Somewhat annoyingly you also need to implement the callbacks for
>>> .domainMigratePrepare3 and .domainMigratePerform3, as we don't
>>> automatically convert non-params usage to the params based
>>> method AFAICT.
>>>
>>> Your impl of .domainMigratePerform3 could pack the values into a
>>> virTypedParams array and then call .domainMigratePerform3Params,
>>> or do the reverse.
>> Yes, without plain(non-params) callbacks we get working only toURI3
>> API function and I create a patch not included in this patchset
>> to make toURI{1,2} work too. I take this approach of converting
>> parameters and use one common worker function but patch a different
>> place - API implementaion itself. So I'll include this patch
>> in next version of the set.
>>
>> As in this case I need to patch 2 different sets of API implementation
>> *migrate{N} and *migrateURI{N} I'd rather put direct managed support
>> to a different patchset. Is it ok?
> 
> Yeah, that'd be fine as long as we still compile at each step it isn't
> a problem if the impl is not final.
Ok. Then I'll send patch supporting toURI{1, 2} in a different patchset as it 
will
be a new thing that could lead to meaningless ping-ponging of parts 
already areed upon.
> 
> 
> Regards,
> Daniel
> 

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

[libvirt] [PATCH 1/4] virsh: Do not make interactive mode default

[Erik Skultety]

Currently, we set interactive mode as default possibly reverting the
setting after we parse the command line arguments. There's nothing
particulary wrong with that, but a call to vshReadlineInit is performed
always in the global initializer just because the default mode is interactive.
Rather than moving vshReadlineInit call somewhere else (because another client
might want to implement interactive mode only), we could make the decision
if we're about to run in interactive mode once the command line is parsed.
---
 tools/virsh.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 391c155..a71cd47 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -786,7 +786,9 @@ virshParseArgv(vshControl *ctl, int argc, char **argv)
 longindex = -1;
 }
 
-if (argc > optind) {
+if (argc == optind) {
+ctl->imode = true;
+} else {
 /* parse command */
 ctl->imode = false;
 if (argc - optind == 1) {
@@ -846,7 +848,6 @@ main(int argc, char **argv)
 memset(ctl, 0, sizeof(vshControl));
 memset(, 0, sizeof(virshControl));
 ctl->name = "virsh";/* hardcoded name of the binary */
-ctl->imode = true;  /* default is interactive mode */
 ctl->log_fd = -1;   /* Initialize log file descriptor */
 ctl->debug = VSH_DEBUG_DEFAULT;
 ctl->hooks = 
-- 
2.4.3

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

[libvirt] [PATCH sandbox 4/4] Don't close immediately when getting EOF on RPC console

[Daniel P. Berrange]

The RPC console is closed when the libvirt-sandbox-init-common
binary reports the exit of the guest process. We still have
some cleanup code that runs in the guest, for example, syncing
and ummounting filesystems. We want to be able to see debug
and/or error messages from this code, so we should not quit
until we get a close on that console. This should happen a
few ms after the close on the RPC console, but just in case
something causes shutdown to hang, we have a delayed timer
registered.

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

diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c
index 195515f..332e53e 100644
--- a/bin/virt-sandbox.c
+++ b/bin/virt-sandbox.c
@@ -34,6 +34,22 @@ static gboolean do_close(GVirSandboxConsole *con 
G_GNUC_UNUSED,
 return FALSE;
 }
 
+static gboolean do_delayed_close(gpointer opaque)
+{
+GMainLoop *loop = opaque;
+g_main_loop_quit(loop);
+return FALSE;
+}
+
+static gboolean do_pending_close(GVirSandboxConsole *con G_GNUC_UNUSED,
+ gboolean error G_GNUC_UNUSED,
+ gpointer opaque)
+{
+GMainLoop *loop = opaque;
+g_timeout_add(2000, do_delayed_close, loop);
+return FALSE;
+}
+
 static gboolean do_exited(GVirSandboxConsole *con G_GNUC_UNUSED,
   int status,
   gpointer opaque)
@@ -256,7 +272,12 @@ int main(int argc, char **argv) {
error && error->message ? error->message : _("Unknown 
failure"));
 goto cleanup;
 }
-g_signal_connect(con, "closed", (GCallback)do_close, loop);
+/* We don't close right away - we want to ensure we read any
+ * final debug info from the log console. We should get an
+ * EOF on that console which will trigger the real close,
+ * but we schedule a timer just in case.
+ */
+g_signal_connect(con, "closed", (GCallback)do_pending_close, loop);
 g_signal_connect(con, "exited", (GCallback)do_exited, );
 
 if (!(gvir_sandbox_console_attach_stdio(con, ))) {
-- 
2.4.3

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

[libvirt] [PATCH sandbox 0/4] Fix data retention with QEMU sandboxes

[Daniel P. Berrange]

In testing the docker sandbox patches I found that with QEMU
sandboxes, we would randomly loose data written to the block
device. This meant that after untar'ing the image contents into
the qcow2 the filesystem was left empty!

Eventually I realized that there was no where we are calling sync()
to flush data out to disk. We never noticed this before because
the sandbox code mostly used 9p filesystems with QEMU which do not
need this sync().

Daniel P. Berrange (4):
  push changing of user ID down into child process
  Sync and unmount filesystems during shutdown
  Fix passing of strace option to guest kernel
  Don't close immediately when getting EOF on RPC console

 bin/virt-sandbox.c|  23 ++-
 libvirt-sandbox/libvirt-sandbox-builder-machine.c |   7 +-
 libvirt-sandbox/libvirt-sandbox-init-common.c | 226 +++---
 libvirt-sandbox/libvirt-sandbox-init-qemu.c   |   3 +-
 4 files changed, 225 insertions(+), 34 deletions(-)

-- 
2.4.3

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

[libvirt] [PATCH sandbox 1/4] push changing of user ID down into child process

[Daniel P. Berrange]

When running interactive sandboxes, don't drop privileges in the
long running libvirt-sandbox-init-common process. This needs to
be privileged in order to sync, unmount and shutdown the guest
when the user command is finished. Push changing of user ID into
the child process, between fork & exec.

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

diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c 
b/libvirt-sandbox/libvirt-sandbox-init-common.c
index d35f760..760509f 100644
--- a/libvirt-sandbox/libvirt-sandbox-init-common.c
+++ b/libvirt-sandbox/libvirt-sandbox-init-common.c
@@ -390,30 +390,43 @@ static int change_user(const gchar *user,
 return 0;
 }
 
-static gboolean run_command(gboolean interactive, gchar **argv,
+static gboolean run_command(GVirSandboxConfig *config,
 pid_t *child,
 int *appin, int *appout, int *apperr)
 {
+GVirSandboxConfigInteractive *iconfig = 
GVIR_SANDBOX_CONFIG_INTERACTIVE(config);
 int pid;
 int master = -1;
 int slave = -1;
 int pipein[2] = { -1, -1};
 int pipeout[2] = { -1, -1};
 int pipeerr[2] = { -1, -1};
+gchar **appargv = gvir_sandbox_config_get_command(config);
+gboolean wanttty = gvir_sandbox_config_interactive_get_tty(iconfig);
 
 if (debug)
 fprintf(stderr, "libvirt-sandbox-init-common: running command %s %d\n",
-argv[0], interactive);
+appargv[0], wanttty);
 
 *appin = *appout = -1;
 
-if (interactive) {
+if (wanttty) {
 if (openpty(, , NULL, NULL, NULL) < 0) {
 fprintf(stderr, "Cannot setup slave for child: %s\n",
 strerror(errno));
 goto error;
 }
 
+if (!g_str_equal(gvir_sandbox_config_get_username(config), "root")) {
+if (fchown(slave,
+   gvir_sandbox_config_get_userid(config),
+   gvir_sandbox_config_get_groupid(config)) < 0) {
+fprintf(stderr, "Cannot make PTY available to user: %s\n",
+strerror(errno));
+goto error;
+}
+}
+
 *appin = master;
 *appout = master;
 *apperr = master;
@@ -436,7 +449,13 @@ static gboolean run_command(gboolean interactive, gchar 
**argv,
 }
 
 if (pid == 0) {
-if (interactive) {
+if (change_user(gvir_sandbox_config_get_username(config),
+gvir_sandbox_config_get_userid(config),
+gvir_sandbox_config_get_groupid(config),
+gvir_sandbox_config_get_homedir(config)) < 0)
+exit(EXIT_FAILURE);
+
+if (wanttty) {
 close(master);
 close(STDIN_FILENO);
 close(STDOUT_FILENO);
@@ -469,24 +488,25 @@ static gboolean run_command(gboolean interactive, gchar 
**argv,
 abort();
 }
 
-execv(argv[0], argv);
-fprintf(stderr, "Cannot execute '%s': %s\n", argv[0], strerror(errno));
+execv(appargv[0], appargv);
+fprintf(stderr, "Cannot execute '%s': %s\n", appargv[0], 
strerror(errno));
 exit(EXIT_FAILURE);
 }
 
-if (interactive)
+if (wanttty) {
 close(slave);
-else {
+} else {
 close(pipein[0]);
 close(pipeout[1]);
 close(pipeerr[1]);
 }
 
 *child = pid;
+g_strfreev(appargv);
 return TRUE;
 
  error:
-if (interactive) {
+if (wanttty) {
 if (master != -1)
 close(master);
 if (slave != -1)
@@ -506,6 +526,7 @@ static gboolean run_command(gboolean interactive, gchar 
**argv,
 close(pipeerr[1]);
 }
 *appin = *appout = *apperr = -1;
+g_strfreev(appargv);
 return FALSE;
 }
 
@@ -639,8 +660,7 @@ typedef enum {
 GVIR_SANDBOX_CONSOLE_STATE_RUNNING,
 } GVirSandboxConsoleState;
 
-static gboolean eventloop(gboolean interactive,
-  gchar **appargv,
+static gboolean eventloop(GVirSandboxConfig *config,
   int sigread,
   int host)
 {
@@ -825,8 +845,7 @@ static gboolean eventloop(gboolean interactive,
 if (rx->buffer[0] == 
GVIR_SANDBOX_PROTOCOL_HANDSHAKE_SYNC) {
 if (debug)
 fprintf(stderr, "Running 
command\n");
-if (!run_command(interactive,
- appargv,
+if (!run_command(config,
  ,
  ,
  ,
@@ -1097,13 +1116,11 @@ static 

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

This should have been requires_template(parser) otherwise
we miss out the default value setting


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 v5 2/5] vz: add migration backbone code

[Nikolay Shirokovskiy]

From: nshirokovs...@virtuozzo.com 

This patch makes basic vz migration possible. For example by virsh:

virsh -c vz:///system migrate $NAME vz+ssh://$DST/system --p2p

Vz migration is implemented as p2p migration. The reason
is that vz sdk do all the job. The question may arise then
why don't implement it as a direct migration. The reason
is that we want to leverage rich libvirt authentication abilities
we lack in vz sdk. We can do it because vz sdk can use tokens to
factor out authentication from migration command.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/vz/vz_driver.c |  257 
 src/vz/vz_sdk.c|   33 +++
 src/vz/vz_sdk.h|2 +
 3 files changed, 292 insertions(+), 0 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 8fa7957..36c64e9 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -146,6 +146,9 @@ vzBuildCapabilities(void)
 
 caps->host.cpu = cpu;
 
+if (virCapabilitiesAddHostMigrateTransport(caps, "vzmigr") < 0)
+goto error;
+
 if (!(data = cpuNodeData(cpu->arch))
 || cpuDecode(cpu, data, NULL, 0, NULL) < 0) {
 goto cleanup;
@@ -1343,6 +1346,257 @@ vzDomainMemoryStats(virDomainPtr domain,
 return ret;
 }
 
+static char*
+vzFormatCookie(const unsigned char *session_uuid)
+{
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAddLit(, "\n");
+virUUIDFormat(session_uuid, uuidstr);
+virBufferAsprintf(, "%s\n", uuidstr);
+virBufferAddLit(, "\n");
+
+if (virBufferCheckError() < 0)
+return NULL;
+
+return virBufferContentAndReset();
+}
+
+static int
+vzParseCookie(const char *xml, unsigned char *session_uuid)
+{
+xmlDocPtr doc = NULL;
+xmlXPathContextPtr ctx = NULL;
+char *tmp = NULL;
+int ret = -1;
+
+if (!(doc = virXMLParseStringCtxt(xml, _("(_migration_cookie)"), )))
+goto cleanup;
+
+if (!(tmp = virXPathString("string(./session_uuid[1])", ctx))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s", _("missing session_uuid element in migration 
data"));
+goto cleanup;
+}
+if (virUUIDParse(tmp, session_uuid) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s", _("malformed session_uuid element in migration 
data"));
+goto cleanup;
+}
+ret = 0;
+
+ cleanup:
+xmlXPathFreeContext(ctx);
+xmlFreeDoc(doc);
+VIR_FREE(tmp);
+
+return ret;
+}
+
+#define VZ_MIGRATION_FLAGS (VIR_MIGRATE_PEER2PEER)
+
+#define VZ_MIGRATION_PARAMETERS \
+VIR_MIGRATE_PARAM_URI,  VIR_TYPED_PARAM_STRING, \
+NULL
+
+static char*
+vzCreateMigrationURI(void)
+{
+char *hostname = NULL;
+char *uri = NULL;
+
+if ((hostname = virGetHostname()) == NULL)
+goto cleanup;
+
+if (STRPREFIX(hostname, "localhost")) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("hostname on destination resolved to localhost,"
+ " but migration requires an FQDN"));
+goto cleanup;
+}
+
+if (virAsprintf(, "vzmigr://%s", hostname) < 0)
+goto cleanup;
+
+ cleanup:
+VIR_FREE(hostname);
+return uri;
+}
+
+static int
+vzDomainMigratePrepare3Params(virConnectPtr conn,
+  virTypedParameterPtr params,
+  int nparams,
+  const char *cookiein ATTRIBUTE_UNUSED,
+  int cookieinlen ATTRIBUTE_UNUSED,
+  char **cookieout,
+  int *cookieoutlen,
+  char **uri_out,
+  unsigned int flags)
+{
+vzConnPtr privconn = conn->privateData;
+const char *miguri = NULL;
+int ret = -1;
+
+virCheckFlags(VZ_MIGRATION_FLAGS, -1);
+
+if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0)
+goto cleanup;
+
+if (virTypedParamsGetString(params, nparams,
+VIR_MIGRATE_PARAM_URI, ) < 0)
+goto cleanup;
+
+/* This check is of par with direct managed imlementation */
+if (miguri == NULL)
+*uri_out = vzCreateMigrationURI();
+
+if (!(*cookieout = vzFormatCookie(privconn->session_uuid)))
+goto cleanup;
+*cookieoutlen = strlen(*cookieout) + 1;
+ret = 0;
+
+ cleanup:
+if (ret != 0) {
+VIR_FREE(*cookieout);
+*cookieoutlen = 0;
+VIR_FREE(*uri_out);
+}
+
+return ret;
+}
+
+static int
+vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature)
+{
+switch (feature) {
+case VIR_DRV_FEATURE_MIGRATION_PARAMS:
+case VIR_DRV_FEATURE_MIGRATION_P2P:
+return 1;
+default:
+return 0;
+}
+}
+
+virURIPtr
+vzParseVzURI(const char 

[libvirt] [PATCH v5 1/5] vz: save session uuid on login

[Nikolay Shirokovskiy]

This session uuid acts as authN token for different multihost vz operations one
of which is migration. Unfortunately we can't get it from server at any time
thus we need to save it at login.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/vz/vz_sdk.c   |   39 +--
 src/vz/vz_utils.h |2 +-
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 744b58a..f7253de 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -37,6 +37,9 @@
 #define VIR_FROM_THIS VIR_FROM_PARALLELS
 #define JOB_INFINIT_WAIT_TIMEOUT UINT_MAX
 
+static int
+prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid);
+
 VIR_LOG_INIT("parallels.sdk");
 
 /*
@@ -228,24 +231,40 @@ prlsdkDeinit(void)
 int
 prlsdkConnect(vzConnPtr privconn)
 {
-PRL_RESULT ret;
+int ret = -1;
+PRL_RESULT pret;
 PRL_HANDLE job = PRL_INVALID_HANDLE;
+PRL_HANDLE result = PRL_INVALID_HANDLE;
+PRL_HANDLE response = PRL_INVALID_HANDLE;
+char session_uuid[VIR_UUID_STRING_BUFLEN + 2];
+PRL_UINT32 buflen = ARRAY_CARDINALITY(session_uuid);
 
-ret = PrlSrv_Create(>server);
-if (PRL_FAILED(ret)) {
-logPrlError(ret);
-return -1;
-}
+pret = PrlSrv_Create(>server);
+prlsdkCheckRetGoto(pret, cleanup);
 
 job = PrlSrv_LoginLocalEx(privconn->server, NULL, 0,
   PSL_HIGH_SECURITY, PACF_NON_INTERACTIVE_MODE);
+if (PRL_FAILED(getJobResult(job, )))
+goto cleanup;
 
-if (waitJob(job)) {
+pret = PrlResult_GetParam(result, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlLoginResponse_GetSessionUuid(response, session_uuid, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+if (prlsdkUUIDParse(session_uuid, privconn->session_uuid) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+if (ret < 0)
 PrlHandle_Free(privconn->server);
-return -1;
-}
+PrlHandle_Free(result);
+PrlHandle_Free(response);
 
-return 0;
+return ret;
 }
 
 void
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index db09647..fe54b25 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -60,7 +60,7 @@ struct _vzConn {
 
 /* Immutable pointer, self-locking APIs */
 virDomainObjListPtr domains;
-
+unsigned char session_uuid[VIR_UUID_BUFLEN];
 PRL_HANDLE server;
 virStoragePoolObjList pools;
 virNetworkObjListPtr networks;
-- 
1.7.1

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

[libvirt] [PATCH v5 0/5] vz: add migration support

[Nikolay Shirokovskiy]

NOTE that minimal command to migrate vz domain is like next:

virsh -c vz:///system migrate 200 vz+ssh://shiny0/system --p2p
  --live --compressed --persistent --undefinesource 

Difference from v4: 

1. move preparation of the migration uri from src to dst as dst intended to do 
it.
2. change hypervisor migration scheme from 'tcp' to 'vzmigr'
3. declare this scheme in host caps

 src/vz/vz_driver.c |  356 
 src/vz/vz_sdk.c|   86 +++--
 src/vz/vz_sdk.h|6 +
 src/vz/vz_utils.h  |4 +-
 4 files changed, 437 insertions(+), 15 deletions(-)

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

Re: [libvirt] [PATCH v2] examples: Add example polkit ACL rules

[Jiri Denemark]

On Fri, Sep 04, 2015 at 14:19:09 +0200, Jiri Denemark wrote:
> Creating ACL rules is not exactly easy and existing examples are pretty
> simple. This patch adds a somewhat complex example which defines several
> roles. Admins can do everything, operators can do basic operations
> on any domain and several groups of users who act as operators but only
> on a limited set of domains.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  Makefile.am   |   2 +-
>  configure.ac  |   1 +
>  examples/polkit/Makefile.am   |  17 ++
>  examples/polkit/libvirt-acl.rules | 115 
> ++
>  libvirt.spec.in   |   3 +
>  5 files changed, 137 insertions(+), 1 deletion(-)
>  create mode 100644 examples/polkit/Makefile.am
>  create mode 100644 examples/polkit/libvirt-acl.rules

Consider the following addition to aclpolkit.html squashed in:

diff --git i/docs/aclpolkit.html.in w/docs/aclpolkit.html.in
index e5a9b16..dae0814 100644
--- i/docs/aclpolkit.html.in
+++ w/docs/aclpolkit.html.in
@@ -348,6 +348,12 @@
   lookup method.
 
 
+
+See
+http://libvirt.org/git/?p=libvirt.git;a=tree;f=examples/polkit;hb=HEAD;>source
 code
+for a more complex example.
+
+
 Example: restricting ability to connect to 
drivers
 
 

Jirka

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

[libvirt] [PATCH 5/5] conf: Remove need for a couple of sa_asserts

[John Ferlan]

Remove the need for a couple of sa_asserts.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f95190f..6df1618 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -24067,10 +24067,11 @@ virDomainObjListCollect(virDomainObjListPtr domlist,
 unsigned int flags)
 {
 struct virDomainListData data = { NULL, 0 };
+ssize_t hash_size;
 
 virObjectLock(domlist);
-sa_assert(domlist->objs);
-if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) {
+if ((hash_size = virHashSize(domlist->objs)) < 0 ||
+(VIR_ALLOC_N(data.vms, hash_size) < 0)) {
 virObjectUnlock(domlist);
 return -1;
 }
@@ -24132,8 +24133,8 @@ virDomainObjListConvert(virDomainObjListPtr domlist,
 }
 virObjectUnlock(domlist);
 
-sa_assert(*vms);
-virDomainObjListFilter(vms, nvms, conn, filter, flags);
+if (*vms)
+virDomainObjListFilter(vms, nvms, conn, filter, flags);
 
 return 0;
 
-- 
2.1.0

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

[libvirt] [PATCH 2/5] lxc: Avoid Coverity SIZEOF_MISMATCH

[John Ferlan]

Commit id '692e9fac7' used virProcessSetNamespaces instead of inlining
the similar functionality; however, Coverity notes that the function
prototype expects a size_t value and not an enum and complains. So,
just pass as a size_t to avoid the noise.

Signed-off-by: John Ferlan 
---
 src/lxc/lxc_container.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index a433552..062da68 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2150,8 +2150,8 @@ static int lxcContainerDropCapabilities(virDomainDefPtr 
def ATTRIBUTE_UNUSED,
  */
 static int lxcAttachNS(int *ns_fd)
 {
-if (ns_fd &&
-virProcessSetNamespaces(VIR_LXC_DOMAIN_NAMESPACE_LAST, ns_fd) < 0)
+size_t nfdlist = VIR_LXC_DOMAIN_NAMESPACE_LAST;
+if (ns_fd && virProcessSetNamespaces(nfdlist, ns_fd) < 0)
 return -1;
 return 0;
 }
-- 
2.1.0

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

[libvirt] [PATCH 1/5] qemu: Check virGetLastError return value for migration finish failure

[John Ferlan]

Commit id '2e7cea243' added a check for an error from Finish instead
of 'unexpected error'; however, if for some reason there wasn't an
error, then virGetLastError could return NULL resulting in the
NULL pointer deref to err->domain.

Signed-off-by: John Ferlan 
---
 src/libvirt-domain.c  | 3 ++-
 src/qemu/qemu_migration.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cbf08fc..964a4d7 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3195,7 +3195,8 @@ virDomainMigrateVersion3Full(virDomainPtr domain,
 orig_err->domain == VIR_FROM_QEMU &&
 orig_err->code == VIR_ERR_OPERATION_FAILED) {
 virErrorPtr err = virGetLastError();
-if (err->domain == VIR_FROM_QEMU &&
+if (err &&
+err->domain == VIR_FROM_QEMU &&
 err->code != VIR_ERR_MIGRATE_FINISH_OK) {
 virFreeError(orig_err);
 orig_err = NULL;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ff89ab5..d50d367 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5023,7 +5023,8 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver,
 orig_err->domain == VIR_FROM_QEMU &&
 orig_err->code == VIR_ERR_OPERATION_FAILED) {
 virErrorPtr err = virGetLastError();
-if (err->domain == VIR_FROM_QEMU &&
+if (err &&
+err->domain == VIR_FROM_QEMU &&
 err->code != VIR_ERR_MIGRATE_FINISH_OK) {
 virFreeError(orig_err);
 orig_err = NULL;
-- 
2.1.0

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

[libvirt] [PATCH 4/5] util: Avoid Coverity FORWARD_NULL

[John Ferlan]

Coverity claims it could be possible to call virDBusTypeStackFree with
*stack == NULL and although the two API's that call it don't appear to
allow that - I suppose it's better to be safe than sorry

Signed-off-by: John Ferlan 
---
 src/util/virdbus.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 1cf1eef..78fb795 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -544,6 +544,10 @@ static void virDBusTypeStackFree(virDBusTypeStack **stack,
  size_t *nstack)
 {
 size_t i;
+
+if (!*stack)
+return;
+
 /* The iter in the first level of the stack is the
  * root iter which must not be freed
  */
-- 
2.1.0

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

[libvirt] [PATCH] cpu: Introduce IvyBridge CPU model

[Jiri Denemark]

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

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu_map.xml | 49 +
 1 file changed, 49 insertions(+)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index d2469ee..51aa899 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -869,6 +869,55 @@
   
 
 
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+
 
   
   
-- 
2.5.1

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

[libvirt] [PATCH v5 4/5] vz: support misc migration options

[Nikolay Shirokovskiy]

From: nshirokovs...@virtuozzo.com 

Migration API has a lot of options. This patch intention is to provide
support for those options that can be trivially supported and give
estimation for other options support in this commit message.

I. Supported.

1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain
memory'. It is supported but quite uncommon way: vz migration demands that this
option should be set. This is due to vz is hardcoded to moving VMs memory using
compression. So anyone who wants to migrate vz domain should set this option
thus declaring it knows it uses compression.

Why bother? May be just support this option and ignore if it is not set or
don't support at all as we can't change behaviour in this aspect.  Well I
believe that this option is, first, inherent to hypervisor implementation as
we have a task of moving domain memory to different place and, second, we have
a tradeoff here between cpu and network resources and some managment should
choose the stratery via this option. If we choose ignoring or unsupporting
implementation than this option has a too vague meaning. Let's go into more
detail.

First if we ignore situation where option is not set than we put user into
fallacy that vz hypervisor don't use compression and thus have lower cpu
usage. Second approach is to not support the option. The main reason not to
follow this way is that 'not supported and not set' is indistinguishable from
'supported and not set' and thus again fool the user.

2. VIR_MIGRATE_LIVE. Means 'don't pause domain before migration'.
Vz doesn give this option but we can emulate it easily making
pause/unpause in libvirt itself. However as this is rather
unnatural addon its implementation is delayed until futher
request. So this option is demanded.

One may be fooled by vz sdk flags of migration api: PVMT_HOT_MIGRATION(aka
live) and PVMT_WARM_MIGRATION(aka normal). Current implementation ignore these
flags and always use live migration.

3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE. This two comes
together. Vz domain are always persistent so we have to demand that options
VIR_MIGRATE_PERSIST_DEST and VIR_MIGRATE_UNDEFINE_SOURCE are both set.

4. VIR_MIGRATE_PAUSED. Means 'don't resume domain on destination'. This is
trivially supported as we have a corresponding option in vz migration.

5. VIR_MIGRATE_OFFLINE. Means 'migrate only XML definition of a domain'. It is
a forcing option that is it is ignored if domain is running and must be set
to migrate stopped domain. Vz implemenation follows this unformal definition
with one exception: non-shared disks will be migrated too. This desicion is on
par with VIR_MIGRATE_NON_SHARED_DISK condideration(see last part of this
notes).

All that said the minimal command to migrate vz domain looks like next:

migrate $DOMAIN $DESTINATION --compressed --live --persistent --undefinesource

Not good. Say if you want to just migrate a domain without further
details you will get error messages until you add these options to
command line. I think there is a lack of notion 'default' behaviour
in all these aspects. If we have it we could just issue:

migrate $DOMAIN $DESTINATION

For vz this would give default compression for example, for qemu - default
no-compression. Then we could have flags --compressed and -no-compressed
and for vz the latter would give unsupported error.

II. Unsupported.

1. VIR_MIGRATE_UNSAFE. Vz disks are always have 'cache=none' set (this
is not reflected in current version of vz driver and will be fixed
soon). So we need not to support this option.

2. VIR_MIGRATE_CHANGE_PROTECTION. Unsupported as we have no appopriate
support from vz sdk. Although we have locks they are advisory and
cant help us.

3. VIR_MIGRATE_TUNNELLED. Means 'use libvirtd to libvirtd connection
to pass hypervisor migration traffic'. Unsupported as not among
vz hypervisor usecases.

4. Direct migration. Which is exposed via *toURI* interface with
VIR_MIGRATE_PEER2PEER flag unset. Means 'migrate without using
libvirtd on the other side'. To support it we should add authN
means to vz driver as mentioned in 'backbone patch' which looks
ugly.

5. VIR_MIGRATE_ABORT_ON_ERROR, VIR_MIGRATE_AUTO_CONVERGE,
VIR_MIGRATE_RDMA_PIN_ALL, VIR_MIGRATE_NON_SHARED_INC,
VIR_MIGRATE_PARAM_DEST_XML, VIR_MIGRATE_PARAM_BANDWIDTH,
VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_MIGRATE_PARAM_LISTEN_ADDRESS,
VIR_MIGRATE_PARAM_MIGRATE_DISKS.
Without further discussion. They are just not usecases of vz hypevisor.

III. Incompatible options.

6. VIR_MIGRATE_NON_SHARED_DISK. Means 'force to migrate if domain has
   non-shared disks'. Without this option we should refuse to migrate with
non-shared disks. Unfortunately this behaviour is incompatible with vz as vz
doesn't demand any user awareness of disk sharedness.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/vz/vz_driver.c |  100 +--
 

[libvirt] [PATCH v5 5/5] vz: cleanup: define vz format of uuids

[Nikolay Shirokovskiy]

vz puts uuids into curly braces. Simply introduce new contstant to reflect this
and get rid of magic +2 in code.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/vz/vz_sdk.c   |   12 ++--
 src/vz/vz_utils.h |2 ++
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 9a2b5df..bafc6e4 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -236,7 +236,7 @@ prlsdkConnect(vzConnPtr privconn)
 PRL_HANDLE job = PRL_INVALID_HANDLE;
 PRL_HANDLE result = PRL_INVALID_HANDLE;
 PRL_HANDLE response = PRL_INVALID_HANDLE;
-char session_uuid[VIR_UUID_STRING_BUFLEN + 2];
+char session_uuid[VZ_UUID_STRING_BUFLEN];
 PRL_UINT32 buflen = ARRAY_CARDINALITY(session_uuid);
 
 pret = PrlSrv_Create(>server);
@@ -316,7 +316,7 @@ prlsdkUUIDFormat(const unsigned char *uuid, char *uuidstr)
 static PRL_HANDLE
 prlsdkSdkDomainLookupByUUID(vzConnPtr privconn, const unsigned char *uuid)
 {
-char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+char uuidstr[VZ_UUID_STRING_BUFLEN];
 PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
 
 prlsdkUUIDFormat(uuid, uuidstr);
@@ -365,7 +365,7 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
char **name,
unsigned char *uuid)
 {
-char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+char uuidstr[VZ_UUID_STRING_BUFLEN];
 PRL_UINT32 len;
 PRL_RESULT pret;
 
@@ -1722,7 +1722,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR 
opaque)
 vzConnPtr privconn = opaque;
 PRL_RESULT pret = PRL_ERR_FAILURE;
 PRL_HANDLE_TYPE handleType;
-char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+char uuidstr[VZ_UUID_STRING_BUFLEN];
 unsigned char uuid[VIR_UUID_BUFLEN];
 PRL_UINT32 bufsize = ARRAY_CARDINALITY(uuidstr);
 PRL_EVENT_TYPE prlEventType;
@@ -3480,7 +3480,7 @@ prlsdkDoApplyConfig(virConnectPtr conn,
 {
 PRL_RESULT pret;
 size_t i;
-char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+char uuidstr[VZ_UUID_STRING_BUFLEN];
 bool needBoot = true;
 char *mask = NULL;
 
@@ -4070,7 +4070,7 @@ int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri,
 int ret = -1;
 vzDomObjPtr privdom = dom->privateData;
 PRL_HANDLE job = PRL_INVALID_HANDLE;
-char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+char uuidstr[VZ_UUID_STRING_BUFLEN];
 PRL_UINT32 vzflags = PRLSDK_MIGRATION_FLAGS;
 
 if (flags & VIR_MIGRATE_PAUSED)
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index fe54b25..2a59426 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -55,6 +55,8 @@
 # define PARALLELS_REQUIRED_BRIDGED_NETWORK  "Bridged"
 # define PARALLELS_BRIDGED_NETWORK_TYPE  "bridged"
 
+# define VZ_UUID_STRING_BUFLEN (VIR_UUID_STRING_BUFLEN + 2)
+
 struct _vzConn {
 virMutex lock;
 
-- 
1.7.1

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

[libvirt] [PATCH v5 3/5] vz: support domain rename on migrate

[Nikolay Shirokovskiy]

From: nshirokovs...@virtuozzo.com 

Signed-off-by: Nikolay Shirokovskiy 
---
 src/vz/vz_driver.c |9 -
 src/vz/vz_sdk.c|   16 +---
 src/vz/vz_sdk.h|5 -
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 36c64e9..e9ca611 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1398,6 +1398,7 @@ vzParseCookie(const char *xml, unsigned char 
*session_uuid)
 
 #define VZ_MIGRATION_PARAMETERS \
 VIR_MIGRATE_PARAM_URI,  VIR_TYPED_PARAM_STRING, \
+VIR_MIGRATE_PARAM_DEST_NAME,VIR_TYPED_PARAM_STRING, \
 NULL
 
 static char*
@@ -1541,12 +1542,18 @@ vzDomainMigratePerform3Params(virDomainPtr domain,
 char *cookie = NULL;
 int cookielen = 0;
 const char *miguri = NULL;
+const char *dname = NULL;
 
 virCheckFlags(VZ_MIGRATION_FLAGS, -1);
 
 if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0)
 goto cleanup;
 
+if (virTypedParamsGetString(params, nparams,
+VIR_MIGRATE_PARAM_DEST_NAME,
+) < 0)
+goto cleanup;
+
 if (!(dom = vzDomObjFromDomain(domain)))
 goto cleanup;
 
@@ -1579,7 +1586,7 @@ vzDomainMigratePerform3Params(virDomainPtr domain,
 if (!(vzuri = vzParseVzURI(miguri)))
 goto cleanup;
 
-if (prlsdkMigrate(dom, vzuri, session_uuid) < 0)
+if (prlsdkMigrate(dom, vzuri, session_uuid, dname) < 0)
 goto cleanup;
 
 virDomainObjListRemove(privconn->domains, dom);
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 783438d..89a2429 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4064,7 +4064,8 @@ prlsdkGetMemoryStats(virDomainObjPtr dom,
 #define PRLSDK_MIGRATION_FLAGS (PSL_HIGH_SECURITY)
 
 int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri,
-  const unsigned char *session_uuid)
+  const unsigned char *session_uuid,
+  const char *dname)
 {
 int ret = -1;
 vzDomObjPtr privdom = dom->privateData;
@@ -4072,12 +4073,13 @@ int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri,
 char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
 
 prlsdkUUIDFormat(session_uuid, uuidstr);
-job = PrlVm_MigrateEx(privdom->sdkdom, uri->server, uri->port, uuidstr,
-  "", /* use default dir for migrated instance bundle 
*/
-  PRLSDK_MIGRATION_FLAGS,
-  0, /* reserved flags */
-  PRL_TRUE /* don't ask for confirmations */
-  );
+job = PrlVm_MigrateWithRenameEx(privdom->sdkdom, uri->server, uri->port, 
uuidstr,
+dname == NULL ? "" : dname,
+"", /* use default dir for migrated 
instance bundle */
+PRLSDK_MIGRATION_FLAGS,
+0, /* reserved flags */
+PRL_TRUE /* don't ask for confirmations */
+);
 
 if (PRL_FAILED(waitJob(job)))
 goto cleanup;
diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
index d3f0caf..0aa70b3 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -77,4 +77,7 @@ prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned 
long long *time);
 int
 prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats, 
unsigned int nr_stats);
 int
-prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, const char unsigned 
*session_uuid);
+prlsdkMigrate(virDomainObjPtr dom,
+  virURIPtr uri,
+  const char unsigned *session_uuid,
+  const char *dname);
-- 
1.7.1

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

Re: [libvirt] [sandbox PATCH v4 13/21] Image: Add get_disk function to Source

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:41PM +, Eren Yagdiran wrote:
> 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
> ---
>  virt-sandbox-image/sources/DockerSource.py | 16 
>  virt-sandbox-image/sources/Source.py   |  4 
>  virt-sandbox-image/virt-sandbox-image.py   |  9 +
>  3 files changed, 29 insertions(+)
> 
> diff --git a/virt-sandbox-image/sources/DockerSource.py 
> b/virt-sandbox-image/sources/DockerSource.py
> index 3e0362b..87fbcf3 100644
> --- a/virt-sandbox-image/sources/DockerSource.py
> +++ b/virt-sandbox-image/sources/DockerSource.py
> @@ -372,6 +372,22 @@ class DockerSource(Source):
>  parent = None
>  imagetagid = parent
>  
> +def get_disk(self,**args):
> +name = args['name']
> +destdir = args['templatedir']
> +sandboxid = args['id']
> +imageList = self._get_image_list(name,destdir)
> +toplayer = imageList[0]
> +diskfile = destdir + "/" + toplayer + "/template.qcow2"
> +configfile = destdir + "/" + toplayer + "/template.json"
> +tempfile = destdir + "/" + toplayer + "/" + sandboxid + ".qcow2"

This is storing the per-sandbox image file inside the
template directory which is not what we want. We should
be using the storage_dir instead of template_dir for the
per instance image.

> +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,configfile)
> +
>  def get_command(self,configfile):
>  configParser = DockerConfParser(configfile)
>  commandToRun = configParser.getRunCommand()

> +def requires_id(parser):
> +randomid = ''.join(random.choice(string.lowercase) for i in range(10))
> +parser.add_argument("-d","--id",
> +default=randomid,
> +help=_("id of the running sandbox"))

This is currently just used to form the per-instance image filename.
We should also use this for the libvirt guest name too. So I'd
suggest we just call this  '-n' / '--name' to match the virt-sandbox
arg syntax and have a prefix on it eg  sandboxXXX where X
is the random suffix

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 14/21] Image: Add run function

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:42PM +, Eren Yagdiran wrote:
> Run an already-built template
> If there is no execution command specified by user, source.get_command will
> find the command to invoke
> ---
>  virt-sandbox-image/virt-sandbox-image.py | 25 +
>  1 file changed, 25 insertions(+)
> 
> diff --git a/virt-sandbox-image/virt-sandbox-image.py 
> b/virt-sandbox-image/virt-sandbox-image.py
> index a73619c..c182874 100755
> --- a/virt-sandbox-image/virt-sandbox-image.py
> +++ b/virt-sandbox-image/virt-sandbox-image.py
> @@ -130,6 +130,31 @@ def check_connect(connectstr):
>  raise ValueError("%s is not supported by Virt-sandbox" 
> %connectstr)
>  return True
>  
> +def run(args):
> +try:
> +if args.connect is not None:
> +check_connect(args.connect)
> +source = dynamic_source_loader(args.source)
> +diskfile,configfile = 
> source.get_disk(name=args.name,templatedir=args.template_dir,id=args.id)
> +
> +format = "qcow2"
> +commandToRun = args.igniter
> +if commandToRun is None:
> +commandToRun = source.get_command(configfile)
> +cmd = ['virt-sandbox']
> +if args.connect is not None:
> +cmd.append("-c")
> +cmd.append(args.connect)

We must also pass '-n' to give the instance name, otherwise we'll
never be able to run more than one docker image at once

> +params = ['-m','host-image:/=%s,format=%s' %(diskfile,format),
> +   '--',
> +   commandToRun]
> +cmd = cmd + params
> +subprocess.call(cmd)
> +subprocess.call(["rm", "-rf", diskfile])

We should just use  os.unlink(diskfile) - no need to shell out to
the rm command to just delete a single file


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 3/5] virfile: Avoid Coverity IDENTICAL_BRANCHES error

[John Ferlan]

In virFileNBDDeviceFindUnused if virFileNBDDeviceIsBusy returns 0,
then both branches jumped to cleanup, so just use ignore_value
since the function returns NULL or some memory and the caller
handles the error.

Signed-off-by: John Ferlan 
---
 src/util/virfile.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index e3c00ef..408d2d9 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -797,8 +797,7 @@ virFileNBDDeviceFindUnused(void)
 if (rv < 0)
 goto cleanup;
 if (rv == 0) {
-if (virAsprintf(, "/dev/%s", de->d_name) < 0)
-goto cleanup;
+ignore_value(virAsprintf(, "/dev/%s", de->d_name));
 goto cleanup;
 }
 }
-- 
2.1.0

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

[libvirt] [PATCH 0/5] Resolve some Coverity related errors

[John Ferlan]

Patches 1, 3, & 4 show up periodically in my Coverity runs, but not
always.  Usually only when some sort of self inflicted build error only
gets a partial build followed by a complete analysis phase.

Patch 2 is related to a Coverity error seen in private integration
testing as well. Although it could be deemed Coverity noise - it's
just easier to adjust our code to use the right type than try to
generate a reproducer or figure out exactly what the error is.

Patch 5 is based on some investigation I started in July to remove
some sa_asserts. I could split into two patches if really desired,
but it was just easier to keep as one.

John Ferlan (5):
  qemu: Check virGetLastError return value for migration finish failure
  lxc: Avoid Coverity SIZEOF_MISMATCH
  virfile: Avoid Coverity IDENTICAL_BRANCHES error
  util: Avoid Coverity FORWARD_NULL
  conf: Remove need for a couple of sa_asserts

 src/conf/domain_conf.c| 9 +
 src/libvirt-domain.c  | 3 ++-
 src/lxc/lxc_container.c   | 4 ++--
 src/qemu/qemu_migration.c | 3 ++-
 src/util/virdbus.c| 4 
 src/util/virfile.c| 3 +--
 6 files changed, 16 insertions(+), 10 deletions(-)

-- 
2.1.0

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

Re: [libvirt] [PATCH v5 0/5] vz: add migration support

[Maxim Nestratov]

04.09.2015 17:18, Nikolay Shirokovskiy пишет:

NOTE that minimal command to migrate vz domain is like next:

virsh -c vz:///system migrate 200 vz+ssh://shiny0/system --p2p
   --live --compressed --persistent --undefinesource

Difference from v4:

1. move preparation of the migration uri from src to dst as dst intended to do 
it.
2. change hypervisor migration scheme from 'tcp' to 'vzmigr'
3. declare this scheme in host caps

  src/vz/vz_driver.c |  356 
  src/vz/vz_sdk.c|   86 +++--
  src/vz/vz_sdk.h|6 +
  src/vz/vz_utils.h  |4 +-
  4 files changed, 437 insertions(+), 15 deletions(-)

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

Have you forgotten to send the #2 patch of the series?

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

Re: [libvirt] [PATCH v2] examples: Add example polkit ACL rules

[Jiri Denemark]

On Fri, Sep 04, 2015 at 13:26:17 +0100, Daniel P. Berrange wrote:
> On Fri, Sep 04, 2015 at 02:19:09PM +0200, Jiri Denemark wrote:
> > Creating ACL rules is not exactly easy and existing examples are pretty
> > simple. This patch adds a somewhat complex example which defines several
> > roles. Admins can do everything, operators can do basic operations
> > on any domain and several groups of users who act as operators but only
> > on a limited set of domains.
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  Makefile.am   |   2 +-
> >  configure.ac  |   1 +
> >  examples/polkit/Makefile.am   |  17 ++
> >  examples/polkit/libvirt-acl.rules | 115 
> > ++
> >  libvirt.spec.in   |   3 +
> >  5 files changed, 137 insertions(+), 1 deletion(-)
> >  create mode 100644 examples/polkit/Makefile.am
> >  create mode 100644 examples/polkit/libvirt-acl.rules
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 91b943b..d338d5a 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -23,7 +23,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs 
> > gnulib/tests \
> >tests po examples/object-events examples/hellolibvirt \
> >examples/dominfo examples/domsuspend examples/apparmor \
> >examples/xml/nwfilter examples/openauth examples/systemtap \
> > -  tools/wireshark examples/dommigrate \
> > +  tools/wireshark examples/dommigrate examples/polkit \
> >examples/lxcconvert examples/domtop
> >  
> >  ACLOCAL_AMFLAGS = -I m4
> > diff --git a/configure.ac b/configure.ac
> > index 8471a46..136c2e7 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -2809,6 +2809,7 @@ AC_CONFIG_FILES([\
> >  examples/systemtap/Makefile \
> >  examples/xml/nwfilter/Makefile \
> >  examples/lxcconvert/Makefile \
> > +examples/polkit/Makefile \
> >  tools/wireshark/Makefile \
> >  tools/wireshark/src/Makefile])
> >  AC_OUTPUT
> > diff --git a/examples/polkit/Makefile.am b/examples/polkit/Makefile.am
> > new file mode 100644
> > index 000..4d213e8
> > --- /dev/null
> > +++ b/examples/polkit/Makefile.am
> > @@ -0,0 +1,17 @@
> > +## Copyright (C) 2015 Red Hat, Inc.
> > +##
> > +## 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, see
> > +## .
> > +
> > +EXTRA_DIST = libvirt-acl.rules
> > diff --git a/examples/polkit/libvirt-acl.rules 
> > b/examples/polkit/libvirt-acl.rules
> > new file mode 100644
> > index 000..5c26593
> > --- /dev/null
> > +++ b/examples/polkit/libvirt-acl.rules
> > @@ -0,0 +1,115 @@
> 
> 
> It would be beneficial to put some docs in this file header here
> to explain to people what this example is achieving.
... 
> ACK

Thanks, I added the documentation and pushed this patch.

Jirka

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

[libvirt] [PATCH sandbox] Require libvirt-glib >= 0.2.2 for LXC fsdriver format fix

[Daniel P. Berrange]

Versions of libvirt-glib < 0.2.2 are buggy when configuring the
 format/driver attributes, causing the disk to be
setup as a plain volume instead of nbd.

Signed-off-by: Daniel P. Berrange 
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 69b5870..9677b24 100644
--- a/configure.ac
+++ b/configure.ac
@@ -14,7 +14,7 @@ GIO_UNIX_REQUIRED=2.28.0
 GOBJECT_REQUIRED=2.32.0
 LIBVIRT_REQUIRED=1.0.2
 LIBVIRT_GCONFIG_REQUIRED=0.2.1
-LIBVIRT_GLIB_REQUIRED=0.1.7
+LIBVIRT_GLIB_REQUIRED=0.2.2
 LIBVIRT_GOBJECT_REQUIRED=0.1.7
 GOBJECT_INTROSPECTION_REQUIRED=0.10.8
 LZMA_REQUIRED=5.0.0
-- 
2.4.3

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

Re: [libvirt] [PATCH v5 0/5] vz: add migration support

[Maxim Nestratov]

04.09.2015 17:48, Maxim Nestratov пишет:

04.09.2015 17:18, Nikolay Shirokovskiy пишет:

NOTE that minimal command to migrate vz domain is like next:

virsh -c vz:///system migrate 200 vz+ssh://shiny0/system --p2p
   --live --compressed --persistent --undefinesource

Difference from v4:

1. move preparation of the migration uri from src to dst as dst 
intended to do it.

2. change hypervisor migration scheme from 'tcp' to 'vzmigr'
3. declare this scheme in host caps

  src/vz/vz_driver.c |  356 


  src/vz/vz_sdk.c|   86 +++--
  src/vz/vz_sdk.h|6 +
  src/vz/vz_utils.h  |4 +-
  4 files changed, 437 insertions(+), 15 deletions(-)

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

Have you forgotten to send the #2 patch of the series?



Please disregard. Just got it, though with a long delay.


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


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

Re: [libvirt] [PATCH sandbox] Require libvirt-glib >= 0.2.2 for LXC fsdriver format fix

[Cedric Bosdonnat]

On Fri, 2015-09-04 at 16:30 +0100, Daniel P. Berrange wrote:
> Versions of libvirt-glib < 0.2.2 are buggy when configuring the
>  format/driver attributes, causing the disk to be
> setup as a plain volume instead of nbd.

ACK
--
Cedric

> Signed-off-by: Daniel P. Berrange 
> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 69b5870..9677b24 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -14,7 +14,7 @@ GIO_UNIX_REQUIRED=2.28.0
>  GOBJECT_REQUIRED=2.32.0
>  LIBVIRT_REQUIRED=1.0.2
>  LIBVIRT_GCONFIG_REQUIRED=0.2.1
> -LIBVIRT_GLIB_REQUIRED=0.1.7
> +LIBVIRT_GLIB_REQUIRED=0.2.2
>  LIBVIRT_GOBJECT_REQUIRED=0.1.7
>  GOBJECT_INTROSPECTION_REQUIRED=0.10.8
>  LZMA_REQUIRED=5.0.0


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

Re: [libvirt] [PATCH 1/5] qemu: Check virGetLastError return value for migration finish failure

[Laine Stump]

On 09/04/2015 10:30 AM, John Ferlan wrote:

Commit id '2e7cea243' added a check for an error from Finish instead
of 'unexpected error'; however, if for some reason there wasn't an
error, then virGetLastError could return NULL resulting in the
NULL pointer deref to err->domain.


ACK.


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

Re: [libvirt] [PATCH libvirt master v2] interface type: add udp socket support

[Jonathan Toppins]

On 09/02/2015 04:23 AM, Ján Tomko wrote:

On Sat, Aug 29, 2015 at 04:19:10PM -0400, Jonathan Toppins wrote:

Adds a new interface type using UDP sockets, this seems only applicable
to QEMU but have edited tree-wide to support the new interface type.

The interface type required the addition of a "localaddr" (local
address), this then maps into the following xml and qemu call.








...

QEMU call:
-net socket,udp=127.0.0.1:2,localaddr=127.0.0.1:2

Notice the xml "local" entry becomes the "localaddr" for the qemu call.

reference:
http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg00629.html

Signed-off-by: Jonathan Toppins 
---

since v1:
  I expect there will be one more round, thanks for the comments thus far. 
Wanted
  to go a head and send this out since it has been a little to long to get to
  this point. Some final issues I am seeing:
  * there seems to be some trouble with adding a new UDP type interface to a
running VM. Stanley who is CC'ed and helping me test has more details.
  * unittests pass even though qemuxml2argvtest still fails, this appears to be
due to disk-drive-network-gluster failing - analysis looks to be the URI is
incorrect, not enough slashes - cuz more is better ;)


Works fine for me, what is your libxml version?
This could be related to
http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=8f17d0e

commit 8f17d0eaae7ee2fa3e214b79b188fc14ed5aa1eb
 util: Prepare URI formatting for libxml2 >= 2.9.2


jtoppins@penguin:~$ apt-cache policy libxml2
libxml2:
  Installed: 2.9.1+dfsg1-5
  Candidate: 2.9.1+dfsg1-5
  Version table:
 *** 2.9.1+dfsg1-5 0
500 http://ftp.us.debian.org/debian/ jessie/main amd64 Packages
100 /var/lib/dpkg/status
jtoppins@penguin:~$ lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:Debian GNU/Linux 8.1 (jessie)
Release:8.1
Codename:   jessie

So looks like its related to the commit listed.




  * please verify I have added the schema correctly, was kinda confusing

  Code Comments:
  [Laine Stump]
  * [DONE] change to using "local" as a nested element inside the "source" 
element
  * [DONE] enhance schema to validate the new formatting of the source and 
local elements
  [Ján Tomko]
  * [DONE] implement unit tests in tests/qemuxml2argvtest.c and 
tests/qemuxml2xmltest.c
  * [DONE] increase verbosity and note when the feature was added in 
formatdomain.html.in

  docs/formatdomain.html.in| 24 
  docs/schemas/domaincommon.rng| 27 +
  src/conf/domain_conf.c   | 74 ++--
  src/conf/domain_conf.h   |  3 +
  src/conf/netdev_bandwidth_conf.h |  1 +
  src/libxl/libxl_conf.c   |  1 +
  src/lxc/lxc_controller.c |  1 +
  src/lxc/lxc_process.c|  1 +
  src/qemu/qemu_command.c  | 12 
  src/qemu/qemu_hotplug.c  |  1 +
  src/qemu/qemu_interface.c|  2 +
  src/uml/uml_conf.c   |  5 ++
  src/xenconfig/xen_sxpr.c |  1 +
  tests/qemuxml2argvdata/qemuxml2argv-net-udp.args |  6 ++
  tests/qemuxml2argvdata/qemuxml2argv-net-udp.xml  | 34 +++
  tests/qemuxml2argvtest.c |  1 +
  tests/qemuxml2xmltest.c  |  1 +
  tools/virsh-domain.c |  1 +
  18 files changed, 190 insertions(+), 6 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-udp.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-udp.xml



ACK

I have fixed the nits mentioned below and pushed the patch.


Thanks Ján. Sorry it was still a little messy.

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

Re: [libvirt] [PATCH 5/5] conf: Remove need for a couple of sa_asserts

[Laine Stump]

On 09/04/2015 10:31 AM, John Ferlan wrote:

Remove the need for a couple of sa_asserts.

Signed-off-by: John Ferlan 
---
  src/conf/domain_conf.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f95190f..6df1618 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -24067,10 +24067,11 @@ virDomainObjListCollect(virDomainObjListPtr domlist,
  unsigned int flags)
  {
  struct virDomainListData data = { NULL, 0 };
+ssize_t hash_size;
  
  virObjectLock(domlist);

-sa_assert(domlist->objs);
-if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) {
+if ((hash_size = virHashSize(domlist->objs)) < 0 ||
+(VIR_ALLOC_N(data.vms, hash_size) < 0)) {
  virObjectUnlock(domlist);
  return -1;
  }
@@ -24132,8 +24133,8 @@ virDomainObjListConvert(virDomainObjListPtr domlist,
  }
  virObjectUnlock(domlist);
  
-sa_assert(*vms);

-virDomainObjListFilter(vms, nvms, conn, filter, flags);
+if (*vms)
+virDomainObjListFilter(vms, nvms, conn, filter, flags);
  
  return 0;
  


ACK.

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

Re: [libvirt] [PATCH 2/5] lxc: Avoid Coverity SIZEOF_MISMATCH

[Laine Stump]

On 09/04/2015 10:30 AM, John Ferlan wrote:

Commit id '692e9fac7' used virProcessSetNamespaces instead of inlining
the similar functionality; however, Coverity notes that the function
prototype expects a size_t value and not an enum and complains. So,
just pass as a size_t to avoid the noise.

Signed-off-by: John Ferlan 
---
  src/lxc/lxc_container.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c


This file needs the copyright date updated to include 2015 (no I don't 
go around checking for that; I have an emacs hook that complains about 
it any time I save a file.)



index a433552..062da68 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2150,8 +2150,8 @@ static int lxcContainerDropCapabilities(virDomainDefPtr 
def ATTRIBUTE_UNUSED,
   */
  static int lxcAttachNS(int *ns_fd)
  {
-if (ns_fd &&
-virProcessSetNamespaces(VIR_LXC_DOMAIN_NAMESPACE_LAST, ns_fd) < 0)
+size_t nfdlist = VIR_LXC_DOMAIN_NAMESPACE_LAST;
+if (ns_fd && virProcessSetNamespaces(nfdlist, ns_fd) < 0)


Doesn't it work if you just add a typecast in the call:

   virProcessSetNamespaces((size_t)VIR_LXC_DOMAIN_NAMESPACE_LAST, ns_fd)

?

Assuming there's a reason for the way you did it vs. this, ACK with the 
copyright updated.



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

Re: [libvirt] [PATCH 3/5] virfile: Avoid Coverity IDENTICAL_BRANCHES error

[Laine Stump]

On 09/04/2015 10:31 AM, John Ferlan wrote:

In virFileNBDDeviceFindUnused if virFileNBDDeviceIsBusy returns 0,
then both branches jumped to cleanup, so just use ignore_value
since the function returns NULL or some memory and the caller
handles the error.

Signed-off-by: John Ferlan 
---
  src/util/virfile.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index e3c00ef..408d2d9 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -797,8 +797,7 @@ virFileNBDDeviceFindUnused(void)
  if (rv < 0)
  goto cleanup;
  if (rv == 0) {
-if (virAsprintf(, "/dev/%s", de->d_name) < 0)
-goto cleanup;
+ignore_value(virAsprintf(, "/dev/%s", de->d_name));
  goto cleanup;
  }
  }


ACK.

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

Re: [libvirt] [PATCH 4/5] util: Avoid Coverity FORWARD_NULL

[Laine Stump]

On 09/04/2015 10:31 AM, John Ferlan wrote:

Coverity claims it could be possible to call virDBusTypeStackFree with
*stack == NULL and although the two API's that call it don't appear to
allow that - I suppose it's better to be safe than sorry

Signed-off-by: John Ferlan 
---
  src/util/virdbus.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 1cf1eef..78fb795 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -544,6 +544,10 @@ static void virDBusTypeStackFree(virDBusTypeStack **stack,
   size_t *nstack)
  {
  size_t i;
+
+if (!*stack)
+return;
+
  /* The iter in the first level of the stack is the
   * root iter which must not be freed
   */


ACK.

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


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?
Because China are in special holidays during these days, It's not 
convenient to access my storage environment
that in server room, So I'll test domblkinfo for $Source next week. but 
I guess the result are negative because

of the unrecognized url format of $Source.


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}]"


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

Re: [libvirt] [PATCH 2/5] lxc: Avoid Coverity SIZEOF_MISMATCH

[John Ferlan]

On 09/04/2015 01:41 PM, Laine Stump wrote:
> On 09/04/2015 10:30 AM, John Ferlan wrote:
>> Commit id '692e9fac7' used virProcessSetNamespaces instead of inlining
>> the similar functionality; however, Coverity notes that the function
>> prototype expects a size_t value and not an enum and complains. So,
>> just pass as a size_t to avoid the noise.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>   src/lxc/lxc_container.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> 
> This file needs the copyright date updated to include 2015 (no I don't
> go around checking for that; I have an emacs hook that complains about
> it any time I save a file.)
> 
>> index a433552..062da68 100644
>> --- a/src/lxc/lxc_container.c
>> +++ b/src/lxc/lxc_container.c
>> @@ -2150,8 +2150,8 @@ static int
>> lxcContainerDropCapabilities(virDomainDefPtr def ATTRIBUTE_UNUSED,
>>*/
>>   static int lxcAttachNS(int *ns_fd)
>>   {
>> -if (ns_fd &&
>> -virProcessSetNamespaces(VIR_LXC_DOMAIN_NAMESPACE_LAST, ns_fd)
>> < 0)
>> +size_t nfdlist = VIR_LXC_DOMAIN_NAMESPACE_LAST;
>> +if (ns_fd && virProcessSetNamespaces(nfdlist, ns_fd) < 0)
> 
> Doesn't it work if you just add a typecast in the call:
> 
>virProcessSetNamespaces((size_t)VIR_LXC_DOMAIN_NAMESPACE_LAST, ns_fd)
> 
> ?
> 

Yes typecasting works - some like it, some don't - so I played it safe
with a new variable.

John
> Assuming there's a reason for the way you did it vs. this, ACK with the
> copyright updated.
> 
> 

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

Re: [libvirt] [PATCH v4 5/6] vz: support misc migration options

[Nikolay Shirokovskiy]

On 03.09.2015 20:04, Daniel P. Berrange wrote:
> On Wed, Sep 02, 2015 at 03:09:26PM +0300, Nikolay Shirokovskiy wrote:
>> From: nshirokovs...@virtuozzo.com 
>>
>> Migration API has a lot of options. This patch intention is to provide
>> support for those options that can be trivially supported and give
>> estimation for other options support in this commit message.
>>
>> I. Supported.
>>
>> 1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain
>> memory'. It is supported but quite uncommon way: vz migration demands that 
>> this
>> option should be set. This is due to vz is hardcoded to moving VMs memory 
>> using
>> compression. So anyone who wants to migrate vz domain should set this option
>> thus declaring it knows it uses compression.
>>
>> Why bother? May be just support this option and ignore if it is not set or
>> don't support at all as we can't change behaviour in this aspect.  Well I
>> believe that this option is, first, inherent to hypervisor implementation as
>> we have a task of moving domain memory to different place and, second, we 
>> have
>> a tradeoff here between cpu and network resources and some managment should
>> choose the stratery via this option. If we choose ignoring or unsupporting
>> implementation than this option has a too vague meaning. Let's go into more
>> detail.
>>
>> First if we ignore situation where option is not set than we put user into
>> fallacy that vz hypervisor don't use compression and thus have lower cpu
>> usage. Second approach is to not support the option. The main reason not to
>> follow this way is that 'not supported and not set' is indistinguishable from
>> 'supported and not set' and thus again fool the user.
> 
> I think it is reasonable to say that the absence of 'VIR_MIGRATE_COMPRESSED'
> does not mean compression should be turned off.
For me saing so is possible only if impact of compressing/not compressing
is rather insignificant. Otherwise it would be strange that turning on/off
this option does not change anything. For example for live migration
option we can't afford go this way.

So for clarity i would rather raise an error. One day may be vz
will turn to supporting this option too or default will change.
> 
> IOW, if VZ does compression by default, that's fine. You don't need to
> raise an error if VIR_MIGRATE_COMPRESSED is not passed.
> 
> 
>> 2. VIR_MIGRATE_LIVE. Means 'reduce domain downtime by suspending it as lately
>> as possible' which technically means 'migrate as much domain memory as 
>> possible
>> before suspending'. Supported in same manner as VIR_MIGRATE_COMPRESSED as 
>> both
>> vz VMs and CTs are always migrated via live scheme.
>>
>> One may be fooled by vz sdk flags of migration api: PVMT_HOT_MIGRATION(aka
>> live) and PVMT_WARM_MIGRATION(aka normal). Current implementation ignore 
>> these
>> flags and always use live migration.
> 
> Actually that's not quite right interpretation.  VIR_MIGRATE_LIVE
> means that the guest CPUs should remain running throughout migration.
> 
> ie, if VIR_MIGRATE_LIVE is *not* provided by the caller, then the
> guest CPUs /must/ be paused while migration takes place. ie the
We are talking about the same thing. 
> migration is non-live / coma migration.  If VZ can't do this
> automatically as part of its migration API, you can fake it by
> just pausing the guest before hand & unpausing after.
Good idea, i really could go this way and support the option.
> 
> Or if you don't wish to support coma-migration, you could raise
> an error if VIR_MIGRATE_LIVE is not specified by caller.
> 
>> 3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE. This two comes
>> together. Vz domain are always persistent so we have to demand that options
>> VIR_MIGRATE_PERSIST_DEST and VIR_MIGRATE_UNDEFINE_SOURCE are both set (and
>> this is done just by unsupporting it).
> 
> This is a tricky one. It would be nice if we could have just "done the
> right thing" by default with the migrate API, since the default behaviour
> is kind of stupid honestly, but we have to live with backwards compatible
> API semantics now :(
> 
> To avoid forcing people to always specify these flags, could you "undo"
> by VZ does by default ?
> 
> ie, if VIR_MIGRATE_PERSIST_DEST is *not* passed, then once migration
> has completed, you could use the domainMigrateFinish step to delete
> the config that VZ just created.
This is *not* possible. Vz domains are always persistent. Technically
vz has it own domain configuration file and we convert it to libvirt
xml when asked.
> 
> Similarly if VIR_MIGRATE_UNDEFINE_SOURCE is *not* passed, then once
> migration completed, you could use the domainMigrateConfirm step
> to re-create the config on the src that VZ just deleted.
This is possible. Eventually this means that we just need to 
define domain with the same configuration as migrated one
right after the migration.

So as one option is possible and the other is not and they are somewhat
paired 

Re: [libvirt] [PATCH v4 2/6] vz: add migration backbone code

[Nikolay Shirokovskiy]

On 03.09.2015 19:45, Daniel P. Berrange wrote:
> On Wed, Sep 02, 2015 at 03:09:23PM +0300, Nikolay Shirokovskiy wrote:
>> From: nshirokovs...@virtuozzo.com 
>>
>> This patch makes basic vz migration possible. For example by virsh:
>>
>> virsh -c vz:///system migrate $NAME vz+ssh://$DST/system --p2p
>>
>> Vz migration is implemented as p2p migration. The reason
>> is that vz sdk do all the job. The question may arise then
>> why don't implement it as a direct migration. The reason
>> is that we want to leverage rich libvirt authentication abilities
>> we lack in vz sdk. We can do it because vz sdk can use tokens to
>> factor out authentication from migration command.
>>
>> Signed-off-by: Nikolay Shirokovskiy 
>> ---
>>  src/vz/vz_driver.c |  192 
>> 
>>  src/vz/vz_sdk.c|   33 +
>>  src/vz/vz_sdk.h|2 +
>>  3 files changed, 227 insertions(+), 0 deletions(-)
>>
> 
>> +static int
>> +vzDomainMigratePrepare3Params(virConnectPtr conn,
>> +virTypedParameterPtr params 
>> ATTRIBUTE_UNUSED,
>> +int nparams ATTRIBUTE_UNUSED,
>> +const char *cookiein ATTRIBUTE_UNUSED,
>> +int cookieinlen ATTRIBUTE_UNUSED,
>> +char **cookieout,
>> +int *cookieoutlen,
>> +char **uri_out ATTRIBUTE_UNUSED,
>> +unsigned int flags)
>> +{
>> +vzConnPtr privconn = conn->privateData;
>> +int ret = -1;
>> +
>> +virCheckFlags(0, -1);
>> +
>> +if (!(*cookieout = vzFormatCookie(privconn->session_uuid)))
>> +goto cleanup;
>> +*cookieoutlen = strlen(*cookieout) + 1;
> 
> As mentioned before, you should fill in uri_out here with the
> hypervisor URI to use for migration, by filling in the URI of
> the current host (ie dest host). eg
> 
>  char *thishost = virGetHostname();
>  virAsprintf(uri_out, "tcp://%s", thishost);
>  VIR_FREE(thishost);
> 
>> +ret = 0;
>> +
>> + cleanup:
>> +if (ret != 0) {
>> +VIR_FREE(*cookieout);
>> +*cookieoutlen = 0;
>> +}
>> +
>> +return ret;
>> +}
>> +
> 
>> +static int
>> +vzDomainMigratePerform3Params(virDomainPtr domain,
>> +  const char *dconnuri,
>> +  virTypedParameterPtr params,
>> +  int nparams,
>> +  const char *cookiein ATTRIBUTE_UNUSED,
>> +  int cookieinlen ATTRIBUTE_UNUSED,
>> +  char **cookieout ATTRIBUTE_UNUSED,
>> +  int *cookieoutlen ATTRIBUTE_UNUSED,
>> +  unsigned int flags)
>> +{
>> +int ret = -1;
>> +virDomainObjPtr dom = NULL;
>> +virConnectPtr dconn = NULL;
>> +virURIPtr vzuri = NULL;
>> +unsigned char session_uuid[VIR_UUID_BUFLEN];
>> +vzConnPtr privconn = domain->conn->privateData;
>> +char *cookie = NULL;
>> +int cookielen = 0;
>> +
>> +virCheckFlags(VZ_MIGRATION_FLAGS, -1);
>> +
>> +if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 
>> 0)
>> +goto cleanup;
>> +
>> +if (!(vzuri = vzMakeVzUri(dconnuri)))
>> +goto cleanup;
> 
> This is not right - you can't use the libvirt URI to form the
> hypervisor migration URI, since the libvirt URI may not in
> fact refer to the hypervisor host.
> 
> eg people may be accessing libvirt(d) via a SSH tunnel in
> which case the dconnuri would include 'localhost' and not
> the actual target host. This is why you must fill in the
> uri_out parameter in the Prepare() method and use that,
> if the "migrateuri" parameter is not provided in the
> virTypedParams array.
> 
>> +
>> +if (!(dom = vzDomObjFromDomain(domain)))
>> +goto cleanup;
>> +
>> +dconn = virConnectOpen(dconnuri);
>> +if (dconn == NULL) {
>> +virReportError(VIR_ERR_OPERATION_FAILED,
>> +   _("Failed to connect to remote libvirt URI %s: %s"),
>> +   dconnuri, virGetLastErrorMessage());
>> +goto cleanup;
>> +}
>> +
>> +/* NULL and zero elements are unused */
>> +if (virDomainMigratePrepare3Params(dconn, NULL, 0, NULL, 0,
>> +   , , NULL, 0) < 0)
>> +goto cleanup;
> 
> Since this is implementing v3, I'd prefer to see you provide the
> full set of 5 callbacks, even though they will currently be no-ops.
> This provides better future proofing for the migration impl in case
> those become neccessary later.
> 
> You can also then trivially implement the non-p2p plain migration
> in this method, by checking whether or not the PEER2PEER flag
> is set or not. If it is not set, you can just skip the connect
> open & prepare calls on 

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

[Lin Ma]

Just tested the patch, It works well.
The changes makes more sense, Thanks for helping me to improve the patch!

在 2015年09月02日 23:58, Michal Privoznik 写道:

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

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

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index d4e500b..6446549 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -486,10 +486,17 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
  xmlNodePtr *disks = NULL;
  size_t i;
  bool details = false;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
  char *type = NULL;
  char *device = NULL;
  char *target = NULL;
  char *source = NULL;
+char *protocol = NULL;
+char *transport = NULL;
+char *host_name = NULL;
+char *host_port = NULL;
+char *socket = NULL;
+char *name = NULL;
  
  if (vshCommandOptBool(cmd, "inactive"))

  flags |= VIR_DOMAIN_XML_INACTIVE;
@@ -536,11 +543,45 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
  vshError(ctl, "unable to query block list");
  goto cleanup;
  }
-source = virXPathString("string(./source/@file"
-"|./source/@dev"
-"|./source/@dir"
-"|./source/@name"
-"|./source/@volume)", ctxt);
+if (type && STREQ(type, "network")) {
+protocol = virXPathString("string(./source/@protocol)", ctxt);
+name = virXPathString("string(./source/@name)", ctxt);
+transport = virXPathString("string(./source/host/@transport)", 
ctxt);
+socket = virXPathString("string(./source/host/@socket)", ctxt);
+host_name = virXPathString("string(./source/host/@name)", ctxt);
+host_port = virXPathString("string(./source/host/@port)", ctxt);
+
+virBufferAddStr(, protocol);
+
+if (transport)
+virBufferAsprintf(, "+%s", transport);
+virBufferAddLit(, "://");
+if (host_name) {
+virBufferAddStr(, host_name);
+if (host_port)
+virBufferAsprintf(, ":%s", host_port);
+}
+if (name) {
+if (!STRPREFIX(name, "/"))
+virBufferAddChar(, '/');
+virBufferAddStr(, name);
+} else if (socket) {
+if (!STRPREFIX(socket, "/"))
+virBufferAddChar(, '/');
+virBufferAddStr(, socket);
+}
+if (virBufferError()) {
+virReportOOMError();
+goto cleanup;
+}
+source = virBufferContentAndReset();
+} else {
+source = virXPathString("string(./source/@file"
+"|./source/@dev"
+"|./source/@dir"
+"|./source/@name"
+"|./source/@volume)", ctxt);
+}
  if (details) {
  vshPrint(ctl, "%-10s %-10s %-10s %s\n", type, device,
   target, source ? source : "-");
@@ -548,6 +589,12 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
  vshPrint(ctl, "%-10s %s\n", target, source ? source : "-");
  }
  
+VIR_FREE(name);

+VIR_FREE(socket);
+VIR_FREE(host_port);
+VIR_FREE(host_name);
+VIR_FREE(transport);
+VIR_FREE(protocol);
  

Re: [libvirt] [RFC PATCH 0/3] Implement mockup capabilities cache in QEMU tests

[Pavel Fedin]

 Hello!

> It's just an assert.  We have those for various reasons and extra
> carefulness cannot hurt.  I don't think it needs differentiating
> between whether the code is ran in tests or not. 

 It would need differentiating because in tests we should not talk to the real 
filesystem, while in
real environment it's OK. So we would need some "extern bool qemuTestMode".
 Well, anyway, take a look at my new implementation, i sent it yesterday. It 
uses a similar global
variable, with only a 2-line change in the library code. The basic idea is to 
override 'binary' name
in virQEMUCapsCacheLookup(), so that we can force it to return particular 
capability set no matter
what def->emulator is. This very simple change actually allows us to do lots of 
nice things, and
improve our test suite even further by removing hardcoded capabilities at all.
 I cc'ed to you, but just in case:
http://www.redhat.com/archives/libvir-list/2015-September/msg00084.html

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


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

Re: [libvirt] [PATCH V2] libxl: don't overwrite error from virNetSocketNewConnectTCP()

[Michal Privoznik]

On 03.09.2015 22:55, John Ferlan wrote:
> 
> 
> On 09/03/2015 01:40 PM, Jim Fehlig wrote:
>> Remove redundant error reporting in libxlDomainMigrationPerform().
>> virNetSocketNewConnectTCP() is perfectly capable of reporting
>> sensible errors.
>>
>> Signed-off-by: Jim Fehlig 
>> ---
>>
>> V2:
>> Actually try to compile the code and find saved_errno is no
>> longer used - remove it.
>>
>>  src/libxl/libxl_migration.c | 7 +--
>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>
> 
> ACK
> 
> John
> 
> Also noteworthy that virNetSocketNewConnectTCP could overwrite errno
> when freeaddrinfo is run in error: or when VIR_FORCE_CLOSE() did it's
> thing...
> 

I don't think that either of those overwrites errno. Here's
freeaddrinfo() impl that I've found in glibc:

void
freeaddrinfo (struct addrinfo *ai)
{
  struct addrinfo *p;

  while (ai != NULL)
{
  p = ai;
  ai = ai->ai_next;
  free (p->ai_canonname);
  free (p);
}
}

And VIR_FORCE_CLOSE() preserves errno too. But nevertheless, ACK stands.

Michal

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

[Michal Privoznik]

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   vda
>> rbd://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   vdg
>> gluster://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?

Michal

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

Re: [libvirt] [PATCH v4 2/6] vz: add migration backbone code

[Daniel P. Berrange]

On Fri, Sep 04, 2015 at 10:56:52AM +0300, Nikolay Shirokovskiy wrote:
> >> @@ -1396,6 +1585,9 @@ static virHypervisorDriver vzDriver = {
> >>  .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */
> >>  .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */
> >>  .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */
> >> +.connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */
> >> +.domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 
> >> 1.2.20 */
> >> +.domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 
> >> 1.2.20 */
> > 
> > Somewhat annoyingly you also need to implement the callbacks for
> > .domainMigratePrepare3 and .domainMigratePerform3, as we don't
> > automatically convert non-params usage to the params based
> > method AFAICT.
> > 
> > Your impl of .domainMigratePerform3 could pack the values into a
> > virTypedParams array and then call .domainMigratePerform3Params,
> > or do the reverse.
> Yes, without plain(non-params) callbacks we get working only toURI3
> API function and I create a patch not included in this patchset
> to make toURI{1,2} work too. I take this approach of converting
> parameters and use one common worker function but patch a different
> place - API implementaion itself. So I'll include this patch
> in next version of the set.
> 
> As in this case I need to patch 2 different sets of API implementation
> *migrate{N} and *migrateURI{N} I'd rather put direct managed support
> to a different patchset. Is it ok?

Yeah, that'd be fine as long as we still compile at each step it isn't
a problem if the impl is not final.


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 v4 5/6] vz: support misc migration options

[Daniel P. Berrange]

On Fri, Sep 04, 2015 at 10:42:00AM +0300, Nikolay Shirokovskiy wrote:
> 
> 
> On 03.09.2015 20:04, Daniel P. Berrange wrote:
> > On Wed, Sep 02, 2015 at 03:09:26PM +0300, Nikolay Shirokovskiy wrote:
> >> From: nshirokovs...@virtuozzo.com 
> >>
> >> Migration API has a lot of options. This patch intention is to provide
> >> support for those options that can be trivially supported and give
> >> estimation for other options support in this commit message.
> >>
> >> I. Supported.
> >>
> >> 1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain
> >> memory'. It is supported but quite uncommon way: vz migration demands that 
> >> this
> >> option should be set. This is due to vz is hardcoded to moving VMs memory 
> >> using
> >> compression. So anyone who wants to migrate vz domain should set this 
> >> option
> >> thus declaring it knows it uses compression.
> >>
> >> Why bother? May be just support this option and ignore if it is not set or
> >> don't support at all as we can't change behaviour in this aspect.  Well I
> >> believe that this option is, first, inherent to hypervisor implementation 
> >> as
> >> we have a task of moving domain memory to different place and, second, we 
> >> have
> >> a tradeoff here between cpu and network resources and some managment should
> >> choose the stratery via this option. If we choose ignoring or unsupporting
> >> implementation than this option has a too vague meaning. Let's go into more
> >> detail.
> >>
> >> First if we ignore situation where option is not set than we put user into
> >> fallacy that vz hypervisor don't use compression and thus have lower cpu
> >> usage. Second approach is to not support the option. The main reason not to
> >> follow this way is that 'not supported and not set' is indistinguishable 
> >> from
> >> 'supported and not set' and thus again fool the user.
> > 
> > I think it is reasonable to say that the absence of 'VIR_MIGRATE_COMPRESSED'
> > does not mean compression should be turned off.
> For me saing so is possible only if impact of compressing/not compressing
> is rather insignificant. Otherwise it would be strange that turning on/off
> this option does not change anything. For example for live migration
> option we can't afford go this way.
> 
> So for clarity i would rather raise an error. One day may be vz
> will turn to supporting this option too or default will change.

Sure, if you prefer to raise an error that's fine with me.. It just
means users will need to always pass --compressed to virsh, but that
probably isn't a big deal.

> >> 3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE. This two comes
> >> together. Vz domain are always persistent so we have to demand that options
> >> VIR_MIGRATE_PERSIST_DEST and VIR_MIGRATE_UNDEFINE_SOURCE are both set (and
> >> this is done just by unsupporting it).
> > 
> > This is a tricky one. It would be nice if we could have just "done the
> > right thing" by default with the migrate API, since the default behaviour
> > is kind of stupid honestly, but we have to live with backwards compatible
> > API semantics now :(
> > 
> > To avoid forcing people to always specify these flags, could you "undo"
> > by VZ does by default ?
> > 
> > ie, if VIR_MIGRATE_PERSIST_DEST is *not* passed, then once migration
> > has completed, you could use the domainMigrateFinish step to delete
> > the config that VZ just created.
> This is *not* possible. Vz domains are always persistent. Technically
> vz has it own domain configuration file and we convert it to libvirt
> xml when asked.

Oh right, I understand now.

> > Similarly if VIR_MIGRATE_UNDEFINE_SOURCE is *not* passed, then once
> > migration completed, you could use the domainMigrateConfirm step
> > to re-create the config on the src that VZ just deleted.
> This is possible. Eventually this means that we just need to 
> define domain with the same configuration as migrated one
> right after the migration.
> 
> So as one option is possible and the other is not and they are somewhat
> paired may be not bother? Or should we take the ignore path as
> for compressed so to finish *no-demanding options* strategy?

I don't mind which way you choose really. I think allowing the user
to not pass UNDEFINE_SOURCE is fairly low priority, since most really
/want/ to use UNDEFINE_SOURCE. It only really causes pain for people
using the low level virsh tool. For serious apps like OpenStack the
default VZ behaviour is what they want all the time.

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