[libvirt] [PATCH] storage: remove a redundant NULL assignment

[Chen Hanxiao]

We already did this in virSecretDefFree.

Signed-off-by: Chen Hanxiao 
---
 src/storage/storage_backend.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 98720f6..440f8b1 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -558,7 +558,6 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
 goto cleanup;
 xml = virSecretDefFormat(def);
 virSecretDefFree(def);
-def = NULL;
 if (xml == NULL)
 goto cleanup;
 
-- 
1.9.3

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

Re: [libvirt] [PATCH 3/5] ip link needs 'name' in 3.16 to create the veth pair

[Martin Kletzander]

On Tue, Nov 25, 2014 at 04:19:48PM +0100, Richard Weinberger wrote:

On Tue, Nov 25, 2014 at 9:21 AM, Cedric Bosdonnat  wrote:

On Tue, 2014-11-25 at 08:42 +0100, Martin Kletzander wrote:

On Mon, Nov 24, 2014 at 09:54:44PM +0100, Cédric Bosdonnat wrote:
>Due to a change (or bug?) in ip link implementation, the command
>'ip link add vnet0...'
>is forced into
>'ip link add name vnet0...'
>The changed command also works on older versions of iproute2, just the
>'name' parameter has been made mandatory.
>---
> src/util/virnetdevveth.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
>index e9d6f9c..ad30e1d 100644
>--- a/src/util/virnetdevveth.c
>+++ b/src/util/virnetdevveth.c
>@@ -89,7 +89,7 @@ static int virNetDevVethGetFreeNum(int startDev)
>  * @veth2: pointer to return name for container end of veth pair
>  *
>  * Creates a veth device pair using the ip command:
>- * ip link add veth1 type veth peer name veth2
>+ * ip link add name veth1 type veth peer name veth2
>  * If veth1 points to NULL on entry, it will be a valid interface on
>  * return.  veth2 should point to NULL on entry.
>  *
>@@ -146,7 +146,7 @@ int virNetDevVethCreate(char** veth1, char** veth2)
> }
>
> cmd = virCommandNew("ip");
>-virCommandAddArgList(cmd, "link", "add",
>+virCommandAddArgList(cmd, "link", "add", "name",
>  *veth1 ? *veth1 : veth1auto,
>  "type", "veth", "peer", "name",
>  *veth2 ? *veth2 : veth2auto,
>--
>2.1.2
>

I agree, the 'name' was always there, just optional.  But what version
of iproute2 do you have that requires it?  I checked the current HEAD
and it's still optional.  This must be a bug in that particular
implementation.

ACK if you can argue with the version or platform this is required
on.


At least the 3.16 shipped on openSUSE 13.2 has that problem... though I
think it's just a side effect of another change in iproute2. It worked
fine with version 3.12.


Instead of papering over the issue in libvirt better ship a non-broken iproute2
in openSUSE 13.2.
real fix: 
https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=f1b66ff



Oh, thank you for finding that, I should've done my homework!  Since
it really is just a bug on iproute2 side in openSUSE, I'd rather keep
it in its original state.  And since the patch is already pushed, I'm
inclining to reverting it.

Other opinions?

Martin


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

Re: [libvirt] [sandbox][PATCH 1/2] AppArmor support

[Martin Kletzander]

On Tue, Nov 25, 2014 at 02:29:25PM +0100, Cédric Bosdonnat wrote:

Implement construction of apparmor security labels. The choice between
selinux and apparmor model isn't exposed to the user, but guessed
depending on what the host supports.
---
bin/virt-sandbox-service  | 15 ---
libvirt-sandbox/libvirt-sandbox-builder.c | 32 +++
2 files changed, 40 insertions(+), 7 deletions(-)


[...]

diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c 
b/libvirt-sandbox/libvirt-sandbox-builder.c
index 48fc9bc..bcad652 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder.c
@@ -358,6 +358,31 @@ static gboolean 
gvir_sandbox_builder_construct_security_selinux (GVirSandboxBuil
return TRUE;
}

+static gboolean 
gvir_sandbox_builder_construct_security_apparmor(GVirSandboxBuilder *builder,
+ 
GVirSandboxConfig *config,
+ 
GVirConfigDomain *domain,
+ GError 
**error)
+{
+GVirConfigDomainSeclabel *sec = gvir_config_domain_seclabel_new();
+const char *label = gvir_sandbox_config_get_security_label(config);
+
+gvir_config_domain_seclabel_set_model(sec, "apparmor");
+if (gvir_sandbox_config_get_security_dynamic(config)) {
+gvir_config_domain_seclabel_set_type(sec,
+ 
GVIR_CONFIG_DOMAIN_SECLABEL_DYNAMIC);


There probably isn't something like a 'baselabel' in apparmor, right?
Because that could be set if there is a label in the config.

Anyway, ACK,

Martin


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

Re: [libvirt] [sandbox][PATCH 2/2] virt-sandbox-service: mount /var after all other file systems

[Martin Kletzander]

On Tue, Nov 25, 2014 at 02:29:26PM +0100, Cédric Bosdonnat wrote:

When creating a sandbox with an image file, the /var folder contains
the mounted image. If we mount it before other file systems, how
could we possibly mount them? The new /var won't contain the mounted
image.
---
bin/virt-sandbox-service | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 7f72107..701bd6e 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -658,7 +658,7 @@ WantedBy=multi-user.target
self.config.add_mount(mount)

for d in self.BIND_SYSTEM_DIRS:
-if os.path.exists(d):
+if d != "/var" and os.path.exists(d):
source = "%s%s" % ( self.dest, d)
self.add_bind_mount(source, d)

@@ -677,6 +677,10 @@ WantedBy=multi-user.target
if not found:
source = "%s%s" % ( self.dest, d)
self.add_bind_mount(source, d)
+
+# /var contains the mounted image if there is an image: should be the
+# last thing to mount
+self.add_bind_mount(source, "/var")


You have the source set from a random value from self.dirs, ACK if you
use "%s/var" % self.dist instead of source.

Martin


self.add_mounts()

def get_expanded_unit_template(self, unit):
--
2.1.2

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


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

Re: [libvirt] [GIT PULL] namespace updates for v3.17-rc1

[Matthias Maier]

[just replying on libvir-list]

Am 26. Nov 2014, 00:15 schrieb Richard Weinberger 
:

>> I plan to send the pull request to Linus as soon as I have caught my
>> breath (from all of the conferences this week) that I can be certain I
>> am thinking clearly and not rushing things.
>
> Today I've upgraded my LXC testbed to the most recent kernel and found
> libvirt-lxc broken again (sic!).
> Remounting /proc/sys/ is failing.
> Investigating into the issue showed that commit "mnt: Implicitly add
> MNT_NODEV on remount as we do on mount"
> is not mainline.
> Why did you left out this patch? In my previous mails I explicitly
> stated that exactly this commit unbreaks libvirt-lxc.
>
> Now the userspace breaking changes are mainline and hit users hard. :-(

I can confirm that userns with libvirt-lxc and 3.17-rc* is broken and
the (kernel) commit in question fixes the issue.


Best,
Matthias

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

Re: [libvirt] [GIT PULL] namespace updates for v3.17-rc1

[Richard Weinberger]

Eric,

On Thu, Aug 21, 2014 at 4:09 PM, Eric W. Biederman
 wrote:
> Richard Weinberger  writes:
>
>> Am 21.08.2014 15:12, schrieb Christoph Hellwig:
>>> On Wed, Aug 20, 2014 at 09:53:49PM -0700, Eric W. Biederman wrote:
 Richard Weinberger  writes:

> On Wed, Aug 6, 2014 at 2:57 AM, Eric W. Biederman  
> wrote:
>
> This commit breaks libvirt-lxc.
> libvirt does in lxcContainerMountBasicFS():

 The bugs fixed are security issues, so if we have to break a small
 number of userspace applications we will.  Anything that we can
 reasonably do to avoid regressions will be done.
>>>
>>> Can you explain the security issues in detail?  Breaking common
>>> userspace like libvirt-lxc with just a little bit of handwaiving is
>>> entirely unacceptable.
>>
>> It looks like commit 87b47932f40a11280584bce260cbdb3b5f9e8b7d in
>> git.kernel.org/cgit/linux/kernel/git/ebiederm/user-namespace.git for-next
>> unbreaks libvirt-lxc.
>> I hope it hits Linus tree and -stable before the offending commit hits users.
>
> I plan to send the pull request to Linus as soon as I have caught my
> breath (from all of the conferences this week) that I can be certain I
> am thinking clearly and not rushing things.

Today I've upgraded my LXC testbed to the most recent kernel and found
libvirt-lxc broken again (sic!).
Remounting /proc/sys/ is failing.
Investigating into the issue showed that commit "mnt: Implicitly add
MNT_NODEV on remount as we do on mount"
is not mainline.
Why did you left out this patch? In my previous mails I explicitly
stated that exactly this commit unbreaks libvirt-lxc.

Now the userspace breaking changes are mainline and hit users hard. :-(

-- 
Thanks,
//richard

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

[libvirt] [PATCH v3 repost] network: Bring netdevs online later

[Matthew Rosato]

Currently, MAC registration occurs during device creation, which is
early enough that, during live migration, you end up with duplicate
MAC addresses on still-running source and target devices, even though
the target device isn't actually being used yet.
This patch proposes to defer MAC registration until right before
the guest can actually use the device -- In other words, right
before starting guest CPUs.

Signed-off-by: Matthew Rosato 
---
 src/Makefile.am |3 +-
 src/conf/domain_conf.h  |2 ++
 src/lxc/lxc_process.c   |4 ++-
 src/qemu/qemu_command.c |3 ++
 src/qemu/qemu_hotplug.c |8 +
 src/qemu/qemu_interface.c   |   76 +++
 src/qemu/qemu_interface.h   |   32 ++
 src/qemu/qemu_process.c |7 
 src/util/virnetdevmacvlan.c |8 +++--
 src/util/virnetdevmacvlan.h |2 ++
 10 files changed, 140 insertions(+), 5 deletions(-)
 create mode 100644 src/qemu/qemu_interface.c
 create mode 100644 src/qemu/qemu_interface.h

diff --git a/src/Makefile.am b/src/Makefile.am
index d8fe624..88125c6 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -733,7 +733,8 @@ QEMU_DRIVER_SOURCES =   
\
qemu/qemu_monitor_text.h\
qemu/qemu_monitor_json.c\
qemu/qemu_monitor_json.h\
-   qemu/qemu_driver.c qemu/qemu_driver.h
+   qemu/qemu_driver.c qemu/qemu_driver.h   \
+   qemu/qemu_interface.c qemu/qemu_interface.h
 
 XENAPI_DRIVER_SOURCES =\
xenapi/xenapi_driver.c xenapi/xenapi_driver.h   \
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index dcb30bc..3603f7c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -985,6 +985,8 @@ struct _virDomainNetDef {
 virNetDevVlan vlan;
 int trustGuestRxFilters; /* enum virTristateBool */
 int linkstate;
+/* vmOp value saved if deferring interface start */
+virNetDevVPortProfileOp vmOp;
 };
 
 /* Used for prefix of ifname of any network name generated dynamically
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 9208f02..57df90c 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -297,6 +297,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
 virNetDevVPortProfilePtr prof;
 virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
 const char *linkdev = virDomainNetGetActualDirectDev(net);
+unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_IFUP;
 
 /* XXX how todo bandwidth controls ?
  * Since the 'net-ifname' is about to be moved to a different
@@ -332,7 +333,8 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
 prof,
 &res_ifname,
 VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
-cfg->stateDir, 0) < 0)
+cfg->stateDir,
+macvlan_create_flags) < 0)
 goto cleanup;
 
 ret = res_ifname;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4ed6506..6ccd17d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -200,6 +200,9 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
 net->ifname = res_ifname;
 }
 
+/* Save vport profile op for later */
+net->vmOp = vmop;
+
 virObjectUnref(cfg);
 return rc;
 }
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b00fd8f..5893ec8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -30,6 +30,7 @@
 #include "qemu_domain.h"
 #include "qemu_command.h"
 #include "qemu_hostdev.h"
+#include "qemu_interface.h"
 #include "domain_audit.h"
 #include "netdev_bandwidth_conf.h"
 #include "domain_nwfilter.h"
@@ -922,6 +923,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
 priv->qemuCaps, tapfd, &tapfdSize) < 0)
 goto cleanup;
 iface_connected = true;
+/* Set device online immediately */
+qemuInterfaceStartDevice(net);
 if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, 
&vhostfdSize) < 0)
 goto cleanup;
 } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
@@ -937,6 +940,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
  
VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) < 0)
 goto cleanup;
 iface_connected = true;
+/* Set device online immediately */
+qemuInterfaceStartDevice(net);
 if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, 
&vhostfdSize) < 0)
 goto cleanup;
 } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
@@ -2087,6 +2092,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
 goto cleanup;
 }
 
+/* Set device online immediat

[libvirt] [PATCH v3 repost] network: Bring netdevs online later

[Matthew Rosato]

Repost of a patch that got lost in the shuffle.  The last version 
(v3) was based on review comments from Martin Kletzander but needs 
additional review.  
Here's a link back to the v2 post, which was the last to receive 
comments:  
http://www.redhat.com/archives/libvir-list/2014-August/msg01332.html

This repost is identical in content to the previous v3 submission, 
save for retrofit needed.

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

Changes for v3:
 * Some minor formatting fixes.
 * in qemuNetworkIfaceConnect, set VIR_NETDEV_TAP_CREATE_IFUP 
   unconditionally.
 * in qemuDomainAttachNetDevice, call qemuInterfaceStartDevice on for 
   VIR_DOMAIN_NET_TYPE_DIRECT, _BRIDGE and _NETWORK. 
 * in qemuProcessStartCPUs, use 'reason' to determine whether or not 
   qemuInterfaceStartDevices needs to be called.  Basically, it needs 
   to be called for any reason that the system would be initializing, 
   as well as potentially after a failed migration.

Matthew Rosato (1):
  network: Bring netdevs online later

 src/Makefile.am |3 +-
 src/conf/domain_conf.h  |2 ++
 src/lxc/lxc_process.c   |4 ++-
 src/qemu/qemu_command.c |3 ++
 src/qemu/qemu_hotplug.c |8 +
 src/qemu/qemu_interface.c   |   76 +++
 src/qemu/qemu_interface.h   |   32 ++
 src/qemu/qemu_process.c |7 
 src/util/virnetdevmacvlan.c |8 +++--
 src/util/virnetdevmacvlan.h |2 ++
 10 files changed, 140 insertions(+), 5 deletions(-)
 create mode 100644 src/qemu/qemu_interface.c
 create mode 100644 src/qemu/qemu_interface.h

-- 
1.7.9.5

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

[libvirt] [PATCH] network: don't allow multiple dhcp sections

[Kyle DeFrancia]

This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=907779

A  element can exist in only one IPv4 address and one IPv6
address per network.  This patch enforces that in virNetworkUpdate.
---
 src/conf/network_conf.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 067334e..a914962 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3472,6 +3472,31 @@ virNetworkIpDefByIndex(virNetworkDefPtr def, int
parentIndex) }

 static int
+virNetworkDefUpdateCheckMultiDHCP(virNetworkDefPtr def,
+  virNetworkIpDefPtr ipdef)
+{
+int family = VIR_SOCKET_ADDR_FAMILY(&ipdef->address);
+size_t i;
+virNetworkIpDefPtr ip;
+
+for (i = 0;
+ (ip = virNetworkDefGetIpByIndex(def, family, i));
+ i++) {
+if (ip != ipdef) {
+if (ip->nranges || ip->nhosts) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _("dhcp is supported only for a "
+ "single %s address on each network"),
+   (family == AF_INET) ? "IPv4" : "IPv6");
+return -1;
+}
+}
+}
+
+return 0;
+}
+
+static int
 virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def,
   unsigned int command,
   int parentIndex,
@@ -3536,6 +3561,9 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr
 def, } else if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
(command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {

+if (virNetworkDefUpdateCheckMultiDHCP(def, ipdef) < 0)
+goto cleanup;
+
 /* log error if an entry with same name/address/ip already
  exists */ for (i = 0; i < ipdef->nhosts; i++) {
 if ((host.mac &&
@@ -3643,6 +3671,9 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr
  def, if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
 (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {

+if (virNetworkDefUpdateCheckMultiDHCP(def, ipdef) < 0)
+goto cleanup;
+
 if (i < ipdef->nranges) {
 char *startip = virSocketAddrFormat(&range.start);
 char *endip = virSocketAddrFormat(&range.end);
-- 
2.1.0

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

Re: [libvirt] [PATCH] qemu: output error when try to hotplug unsupport console

[Pavel Hrdina]

On 11/17/2014 03:31 PM, Luyao Huang wrote:

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

When try to use attach-device to hotplug a qemu
unsupport sonsole, command will return success,
and add a console in vm's xml.But this doesn't work
in qemu, qemu doesn't support these console.Add
a error output when try to hotplug a unsupport console
in qemuBuildConsoleChrDeviceStr.

Signed-off-by: Luyao Huang 
---
  src/qemu/qemu_command.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1399ce4..2bf4a83 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10069,13 +10069,17 @@ qemuBuildConsoleChrDeviceStr(char **deviceStr,
  break;

  case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL:
+break;
  case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE:
  case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN:
  case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML:
  case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC:
  case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ:
  case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST:
-break;
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+  _("unsupported console target type %s"),
+   
NULLSTR(virDomainChrConsoleTargetTypeToString(chr->targetType)));


Wrong indentation


+return ret;
  }

  ret = 0;



Otherwise it seems good, bud what about the case that you will attach
unsupported device to offline domain? It is still possible but the
domain cannot be started afterwards. You should probably take care of
that too.

Pavel

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

Re: [libvirt] [PATCH 1/2] virsh: document block.n.allocation stat

[Eric Blake]

On 11/25/2014 01:59 AM, Peter Krempa wrote:
> On 11/25/14 06:21, Eric Blake wrote:
>> Commit 7557ddf added some additional block.* stats to
>> virDomainListGetStats, but failed to document them in 'man
>> virsh'.  Also, I noticed some inconsistent use of commas.
>>
>> * tools/virsh.pod (domstats): Tweak commas, add missing stats.
>>
>> Signed-off-by: Eric Blake 
>> ---
>>  tools/virsh.pod | 9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
> 
> ACK,

This one is pushed.  Later, I'll send a v2 with the adjustments based on
your suggestions on 2/2, as well as further patches to make more
progress towards backing chain stat reporting.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH RFC] storage: perform btrfs clone if possible

[Eric Blake]

On 11/25/2014 03:19 AM, Chen, Hanxiao wrote:
>> I think it makes sense to expose this functionality; although I suspect
>> it is better if we do so by having the user pass an explicit new flag
>> value to existing API instead of doing it automatically.
>>
> Thanks for your clarification.
> 
> But we've already had nocow in virStorageSource and  tags.
> So I think if we do not specify  tags in XML,
> we should try it according to 'nocow' in codes.
> 
> Or do we need a new flags such as --reflink
> for tools like virt-clone?

It's a design trade-off - do we make the default operation efficient
when possible (but with possible surprises due to overcommit of
storage), or safe (but slow)?  Regardless of the default, is it possible
to explicitly request the opposite action?  To me, it seems backwards
compatible to make an operation faster by using underlying clone hooks
as long as the end result is the same, and as long as a user NOT
specifying a specific use of clone or avoidance of clone does not get an
error message when clone is attempted but not supported.  So at this
point, I could go either way for the default as long as I can still have
full control over the behavior, and we don't break existing users.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [python PATCH] event: Add bindings for agent lifecycle event

[Eric Blake]

On 11/24/2014 08:42 AM, Peter Krempa wrote:
> Also add the example.
> ---
>  examples/event-test.py | 13 +++
>  libvirt-override.c | 60 
> ++
>  2 files changed, 73 insertions(+)
>  mode change 100644 => 100755 examples/event-test.py
> 
> +++ b/libvirt-override.c
> @@ -6566,6 +6566,61 @@ 
> libvirt_virConnectDomainEventTunableCallback(virConnectPtr conn 
> ATTRIBUTE_UNUSED
>  }
>  #endif /* LIBVIR_CHECK_VERSION(1, 2, 9) */
> 
> +#if LIBVIR_CHECK_VERSION(1, 2, 11)

Not a problem with this patch, but at some point, we ought to revisit
these #if checks to instead gate them on whether libvirt.h provided a
given callback id (since it is possible to backport events to earlier
libvirt builds without breaking .so compatibility in downstream distros,
and those distors right now have to tweak these defines to match up with
their backports instead of having it happen automatically).  It's on my
todo list of something to experiment with when I get time.

ACK; it matches the patterns used in other events.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] dbus: fix arrays of bools

[Eric Blake]

On 11/21/2014 05:12 PM, Eric Blake wrote:
> Commit 2aa167ca tried to fix the DBus interaction code to allow
> callers to use native types instead of 4-byte bools.  But in
> fixing the issue, I missed the case of an arrayref; Conrad Meyer
> shows the following valid complaint issued by clang:
> 

> 
> * src/util/virdbus.c (GET_NEXT_VAL): Also fix array cases.
> (SET_NEXT_VAL): Fix uses of sub-int arrays.
> * tests/virdbustest.c (testMessageArray, testMessageArrayRef):
> Test it.
> 

Now pushed, after Conrad's off-list review and Ian's on-list review in a
different thread.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH 2/2] getstats: add block.n.source stat

[Peter Krempa]

On 11/25/14 16:43, Eric Blake wrote:
> On 11/25/2014 02:10 AM, Peter Krempa wrote:
>> On 11/25/14 06:21, Eric Blake wrote:
>>> I noticed that for an offline domain, 'virsh domstats --block $dom'
>>> was producing just the domain name, with no stats.  But the older
>>> 'virsh domblkinfo' works just fine on offline domains.  Furthermore,
>>> I'm about to make block stats optionally more complex to cover
>>> backing chains, where block.count will no longer equal the number
>>> of  for a domain.  For these reasons, it is nicer if the
>>> statistics output always includes minimal information on every
>>> resource being described, with enough to correlate back to host
>>> resources, and even when some statistics are available only on a
>>> running domain.
>>>
> 
>>> + * "block..source" - string describing the source of block device 
>>> ,
>>> + *such as the host path of a file or device.
>>
>> Fair enough as long as we document that we only return it for
>> non-network sources. We had quite a few problems with network sources so
>> I'd rather not expose it for those due to possible ambiguity.
> 
> Sure, I can do that.  Maybe I should then name it block..path or
> block..file, to make it obvious that it is only a file name?  And I
> suppose we could always add other fields later if it turns out to be
> useful on network devices, but I won't worry about it for now.

I'd go with path. File is not quite right with physical devices/lvs used
as source.

> 
>>>   *
>>>   * Specifying VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS as @flags 
>>> makes
>>>   * the function return error in case some of the stat types in @stats were
>>> - * not recognized by the daemon.
>>> + * not recognized by the daemon.  However, even with this flag, a 
>>> hypervisor
>>> + * may omit individual fields within a group if the information is not
>>> + * available; as an extreme example, a supported group may produce zero
>>> + * fields for offline domains if the statistics are meaningful only for a
>>> + * running domain.
>>>   *
>>
>> The code changes are not entirely related to this doc change.
> 
> Should I split it into a separate patch, then?

I think it would make more sense.

> 
>>> -QEMU_ADD_NAME_PARAM(record, maxparams, "block", i, disk->dst);
>>> +QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", i, 
>>> disk->dst);
>>> +if (disk->src && disk->src->path)
>>
>> && virStorageSourceIsLocalStorage(disk->src)
> 
> Indeed, that fits with your request to limit to files.
> 

Peter



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

Re: [libvirt] [PATCH 2/2] getstats: add block.n.source stat

[Eric Blake]

On 11/25/2014 02:10 AM, Peter Krempa wrote:
> On 11/25/14 06:21, Eric Blake wrote:
>> I noticed that for an offline domain, 'virsh domstats --block $dom'
>> was producing just the domain name, with no stats.  But the older
>> 'virsh domblkinfo' works just fine on offline domains.  Furthermore,
>> I'm about to make block stats optionally more complex to cover
>> backing chains, where block.count will no longer equal the number
>> of  for a domain.  For these reasons, it is nicer if the
>> statistics output always includes minimal information on every
>> resource being described, with enough to correlate back to host
>> resources, and even when some statistics are available only on a
>> running domain.
>>

>> + * "block..source" - string describing the source of block device 
>> ,
>> + *such as the host path of a file or device.
> 
> Fair enough as long as we document that we only return it for
> non-network sources. We had quite a few problems with network sources so
> I'd rather not expose it for those due to possible ambiguity.

Sure, I can do that.  Maybe I should then name it block..path or
block..file, to make it obvious that it is only a file name?  And I
suppose we could always add other fields later if it turns out to be
useful on network devices, but I won't worry about it for now.

>>   *
>>   * Specifying VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS as @flags 
>> makes
>>   * the function return error in case some of the stat types in @stats were
>> - * not recognized by the daemon.
>> + * not recognized by the daemon.  However, even with this flag, a hypervisor
>> + * may omit individual fields within a group if the information is not
>> + * available; as an extreme example, a supported group may produce zero
>> + * fields for offline domains if the statistics are meaningful only for a
>> + * running domain.
>>   *
> 
> The code changes are not entirely related to this doc change.

Should I split it into a separate patch, then?

>> -QEMU_ADD_NAME_PARAM(record, maxparams, "block", i, disk->dst);
>> +QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", i, 
>> disk->dst);
>> +if (disk->src && disk->src->path)
> 
> && virStorageSourceIsLocalStorage(disk->src)

Indeed, that fits with your request to limit to files.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH 3/5] ip link needs 'name' in 3.16 to create the veth pair

[Richard Weinberger]

On Tue, Nov 25, 2014 at 9:21 AM, Cedric Bosdonnat  wrote:
> On Tue, 2014-11-25 at 08:42 +0100, Martin Kletzander wrote:
>> On Mon, Nov 24, 2014 at 09:54:44PM +0100, Cédric Bosdonnat wrote:
>> >Due to a change (or bug?) in ip link implementation, the command
>> >'ip link add vnet0...'
>> >is forced into
>> >'ip link add name vnet0...'
>> >The changed command also works on older versions of iproute2, just the
>> >'name' parameter has been made mandatory.
>> >---
>> > src/util/virnetdevveth.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
>> >index e9d6f9c..ad30e1d 100644
>> >--- a/src/util/virnetdevveth.c
>> >+++ b/src/util/virnetdevveth.c
>> >@@ -89,7 +89,7 @@ static int virNetDevVethGetFreeNum(int startDev)
>> >  * @veth2: pointer to return name for container end of veth pair
>> >  *
>> >  * Creates a veth device pair using the ip command:
>> >- * ip link add veth1 type veth peer name veth2
>> >+ * ip link add name veth1 type veth peer name veth2
>> >  * If veth1 points to NULL on entry, it will be a valid interface on
>> >  * return.  veth2 should point to NULL on entry.
>> >  *
>> >@@ -146,7 +146,7 @@ int virNetDevVethCreate(char** veth1, char** veth2)
>> > }
>> >
>> > cmd = virCommandNew("ip");
>> >-virCommandAddArgList(cmd, "link", "add",
>> >+virCommandAddArgList(cmd, "link", "add", "name",
>> >  *veth1 ? *veth1 : veth1auto,
>> >  "type", "veth", "peer", "name",
>> >  *veth2 ? *veth2 : veth2auto,
>> >--
>> >2.1.2
>> >
>>
>> I agree, the 'name' was always there, just optional.  But what version
>> of iproute2 do you have that requires it?  I checked the current HEAD
>> and it's still optional.  This must be a bug in that particular
>> implementation.
>>
>> ACK if you can argue with the version or platform this is required
>> on.
>
> At least the 3.16 shipped on openSUSE 13.2 has that problem... though I
> think it's just a side effect of another change in iproute2. It worked
> fine with version 3.12.

Instead of papering over the issue in libvirt better ship a non-broken iproute2
in openSUSE 13.2.
real fix: 
https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=f1b66ff

-- 
Thanks,
//richard

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

Re: [libvirt] [PATCH] Re-add use of locking with iptables/ip6tables/ebtables

[Boris Fiuczynski]

On 11/25/2014 03:20 PM, Boris Fiuczynski wrote:

On 11/11/2014 01:42 PM, Daniel P. Berrange wrote:

A previous commit introduced use of locking with invocation
of iptables in the viriptables.c module

   commit ba95426d6f39aec1da6e069dd7222f7a8c6a5862
   Author: Serge Hallyn 
   Date:   Fri Nov 1 12:36:59 2013 -0500

 util: use -w flag when calling iptables

This only ever had effect with the virtual network driver,
as it was not wired up into the nwfilter driver. Unfortunately
in the firewall refactoring the use of the -w flag was
accidentally lost.

This patch introduces it to the virfirewall.c module so that
both the virtual network and nwfilter drivers will be using
it. It also ensures that the equivalent --concurrent flag
to ebtables is used.
---
  src/util/virfirewall.c | 67
+++---
  src/util/viriptables.c |  2 --
  2 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index bab1634..c83fdc6 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -104,6 +104,44 @@ virFirewallOnceInit(void)

  VIR_ONCE_GLOBAL_INIT(virFirewall)

+static bool iptablesUseLock;
+static bool ip6tablesUseLock;
+static bool ebtablesUseLock;
+
+static void
+virFirewallCheckUpdateLock(bool *lockflag,
+   const char *const*args)
+{
+virCommandPtr cmd = virCommandNewArgs(args);
+if (virCommandRun(cmd, NULL) < 0) {
+VIR_INFO("locking not supported by %s", args[0]);
+} else {
+VIR_INFO("using locking for %s", args[0]);
+*lockflag = true;
+}
+virCommandFree(cmd);
+}
+
+static void
+virFirewallCheckUpdateLocking(void)
+{
+const char *iptablesArgs[] = {
+IPTABLES_PATH, "-w", "-L", "-n", NULL,
+};
+const char *ip6tablesArgs[] = {
+IP6TABLES_PATH, "-w", "-L", "-n", NULL,
+};
+const char *ebtablesArgs[] = {
+EBTABLES_PATH, "--concurrent", "-L", NULL,
+};
+virFirewallCheckUpdateLock(&iptablesUseLock,
+   iptablesArgs);
+virFirewallCheckUpdateLock(&ip6tablesUseLock,
+   ip6tablesArgs);
+virFirewallCheckUpdateLock(&ebtablesUseLock,
+   ebtablesArgs);
+}
+
  static int
  virFirewallValidateBackend(virFirewallBackend backend)
  {
@@ -161,6 +199,9 @@ virFirewallValidateBackend(virFirewallBackend
backend)
  }

  currentBackend = backend;
+
+virFirewallCheckUpdateLocking();
+
  return 0;
  }

@@ -201,6 +242,9 @@ virFirewallPtr virFirewallNew(void)
  {
  virFirewallPtr firewall;

+if (virFirewallInitialize() < 0)
+return NULL;
+
  if (VIR_ALLOC(firewall) < 0)
  return NULL;

@@ -321,6 +365,23 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
  rule->queryOpaque = opaque;
  rule->ignoreErrors = ignoreErrors;

+switch (rule->layer) {
+case VIR_FIREWALL_LAYER_ETHERNET:
+if (ebtablesUseLock)
+ADD_ARG(rule, "--concurrent");
+break;
+case VIR_FIREWALL_LAYER_IPV4:
+if (iptablesUseLock)
+ADD_ARG(rule, "-w");
+break;
+case VIR_FIREWALL_LAYER_IPV6:
+if (ip6tablesUseLock)
+ADD_ARG(rule, "-w");
+break;
+case VIR_FIREWALL_LAYER_LAST:
+break;
+}
+

By adding these parameters dynamically based on the above added support
checking logic will the network filter tests still work without any code
change?


OK, just saw that a fix was posted today.


--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

Re: [libvirt] [PATCH] Re-add use of locking with iptables/ip6tables/ebtables

[Boris Fiuczynski]

On 11/11/2014 01:42 PM, Daniel P. Berrange wrote:

A previous commit introduced use of locking with invocation
of iptables in the viriptables.c module

   commit ba95426d6f39aec1da6e069dd7222f7a8c6a5862
   Author: Serge Hallyn 
   Date:   Fri Nov 1 12:36:59 2013 -0500

 util: use -w flag when calling iptables

This only ever had effect with the virtual network driver,
as it was not wired up into the nwfilter driver. Unfortunately
in the firewall refactoring the use of the -w flag was
accidentally lost.

This patch introduces it to the virfirewall.c module so that
both the virtual network and nwfilter drivers will be using
it. It also ensures that the equivalent --concurrent flag
to ebtables is used.
---
  src/util/virfirewall.c | 67 +++---
  src/util/viriptables.c |  2 --
  2 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index bab1634..c83fdc6 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -104,6 +104,44 @@ virFirewallOnceInit(void)

  VIR_ONCE_GLOBAL_INIT(virFirewall)

+static bool iptablesUseLock;
+static bool ip6tablesUseLock;
+static bool ebtablesUseLock;
+
+static void
+virFirewallCheckUpdateLock(bool *lockflag,
+   const char *const*args)
+{
+virCommandPtr cmd = virCommandNewArgs(args);
+if (virCommandRun(cmd, NULL) < 0) {
+VIR_INFO("locking not supported by %s", args[0]);
+} else {
+VIR_INFO("using locking for %s", args[0]);
+*lockflag = true;
+}
+virCommandFree(cmd);
+}
+
+static void
+virFirewallCheckUpdateLocking(void)
+{
+const char *iptablesArgs[] = {
+IPTABLES_PATH, "-w", "-L", "-n", NULL,
+};
+const char *ip6tablesArgs[] = {
+IP6TABLES_PATH, "-w", "-L", "-n", NULL,
+};
+const char *ebtablesArgs[] = {
+EBTABLES_PATH, "--concurrent", "-L", NULL,
+};
+virFirewallCheckUpdateLock(&iptablesUseLock,
+   iptablesArgs);
+virFirewallCheckUpdateLock(&ip6tablesUseLock,
+   ip6tablesArgs);
+virFirewallCheckUpdateLock(&ebtablesUseLock,
+   ebtablesArgs);
+}
+
  static int
  virFirewallValidateBackend(virFirewallBackend backend)
  {
@@ -161,6 +199,9 @@ virFirewallValidateBackend(virFirewallBackend backend)
  }

  currentBackend = backend;
+
+virFirewallCheckUpdateLocking();
+
  return 0;
  }

@@ -201,6 +242,9 @@ virFirewallPtr virFirewallNew(void)
  {
  virFirewallPtr firewall;

+if (virFirewallInitialize() < 0)
+return NULL;
+
  if (VIR_ALLOC(firewall) < 0)
  return NULL;

@@ -321,6 +365,23 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
  rule->queryOpaque = opaque;
  rule->ignoreErrors = ignoreErrors;

+switch (rule->layer) {
+case VIR_FIREWALL_LAYER_ETHERNET:
+if (ebtablesUseLock)
+ADD_ARG(rule, "--concurrent");
+break;
+case VIR_FIREWALL_LAYER_IPV4:
+if (iptablesUseLock)
+ADD_ARG(rule, "-w");
+break;
+case VIR_FIREWALL_LAYER_IPV6:
+if (ip6tablesUseLock)
+ADD_ARG(rule, "-w");
+break;
+case VIR_FIREWALL_LAYER_LAST:
+break;
+}
+
By adding these parameters dynamically based on the above added support 
checking logic will the network filter tests still work without any code 
change?




--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

[libvirt] [sandbox][PATCH 1/2] AppArmor support

[Cédric Bosdonnat]

Implement construction of apparmor security labels. The choice between
selinux and apparmor model isn't exposed to the user, but guessed
depending on what the host supports.
---
 bin/virt-sandbox-service  | 15 ---
 libvirt-sandbox/libvirt-sandbox-builder.c | 32 +++
 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 5a3f6ab..7f72107 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -315,24 +315,25 @@ class Container:
 context.undefine()
 
 def get_security_model(self):
-# XXX selinux is the default for the while, needs to be configurable 
someday
-model = "selinux"
-supported = False
+model = None
 
 # Make sure we have a connection
 self.connect()
 
 # Loop over the security models from the host capabilities
+# The first in "selinux" and "apparmor" will be the returned model
+# Those two models can't coexist on a machine
 configCaps = self.conn.get_capabilities()
 hostCaps = configCaps.get_host()
 secmodels = hostCaps.get_secmodels()
 for secmodel in secmodels:
-if secmodel.get_model() == model:
-supported = True
+if secmodel.get_model() == "selinux":
+model = "selinux"
+break
+elif secmodel.get_model() == "apparmor":
+model = "apparmor"
 break
 
-if not supported:
-model = None
 return model
 
 
diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c 
b/libvirt-sandbox/libvirt-sandbox-builder.c
index 48fc9bc..bcad652 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder.c
@@ -358,6 +358,31 @@ static gboolean 
gvir_sandbox_builder_construct_security_selinux (GVirSandboxBuil
 return TRUE;
 }
 
+static gboolean 
gvir_sandbox_builder_construct_security_apparmor(GVirSandboxBuilder *builder,
+ 
GVirSandboxConfig *config,
+ 
GVirConfigDomain *domain,
+ GError 
**error)
+{
+GVirConfigDomainSeclabel *sec = gvir_config_domain_seclabel_new();
+const char *label = gvir_sandbox_config_get_security_label(config);
+
+gvir_config_domain_seclabel_set_model(sec, "apparmor");
+if (gvir_sandbox_config_get_security_dynamic(config)) {
+gvir_config_domain_seclabel_set_type(sec,
+ 
GVIR_CONFIG_DOMAIN_SECLABEL_DYNAMIC);
+} else {
+gvir_config_domain_seclabel_set_type(sec,
+ 
GVIR_CONFIG_DOMAIN_SECLABEL_STATIC);
+if (label)
+gvir_config_domain_seclabel_set_label(sec, label);
+}
+
+gvir_config_domain_set_seclabel(domain, sec);
+g_object_unref(sec);
+
+return TRUE;
+}
+
 static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder 
*builder,
 GVirSandboxConfig 
*config,
 const gchar *statedir 
G_GNUC_UNUSED,
@@ -369,6 +394,7 @@ static gboolean 
gvir_sandbox_builder_construct_security(GVirSandboxBuilder *buil
 GVirConfigCapabilitiesHost *hostCapabilities;
 GList *secmodels, *iter;
 gboolean supportsSelinux = FALSE;
+gboolean supportsAppArmor = FALSE;
 
 /* What security models are available on the host? */
 if (!(configCapabilities = gvir_connection_get_capabilities(connection, 
error))) {
@@ -383,6 +409,9 @@ static gboolean 
gvir_sandbox_builder_construct_security(GVirSandboxBuilder *buil
 if (g_str_equal(gvir_config_capabilities_host_secmodel_get_model(
 GVIR_CONFIG_CAPABILITIES_HOST_SECMODEL(iter->data)), 
"selinux"))
 supportsSelinux = TRUE;
+if (g_str_equal(gvir_config_capabilities_host_secmodel_get_model(
+GVIR_CONFIG_CAPABILITIES_HOST_SECMODEL(iter->data)), 
"apparmor"))
+supportsAppArmor = TRUE;
 g_object_unref(iter->data);
 }
 
@@ -394,6 +423,9 @@ static gboolean 
gvir_sandbox_builder_construct_security(GVirSandboxBuilder *buil
 if (supportsSelinux)
 return gvir_sandbox_builder_construct_security_selinux(builder, config,
domain, error);
+else if (supportsAppArmor)
+return gvir_sandbox_builder_construct_security_apparmor(builder, 
config,
+domain, error);
 
 return TRUE;
 }
-- 
2.1.2

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

[libvirt] [sandbox][PATCH 2/2] virt-sandbox-service: mount /var after all other file systems

[Cédric Bosdonnat]

When creating a sandbox with an image file, the /var folder contains
the mounted image. If we mount it before other file systems, how
could we possibly mount them? The new /var won't contain the mounted
image.
---
 bin/virt-sandbox-service | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service
index 7f72107..701bd6e 100755
--- a/bin/virt-sandbox-service
+++ b/bin/virt-sandbox-service
@@ -658,7 +658,7 @@ WantedBy=multi-user.target
 self.config.add_mount(mount)
 
 for d in self.BIND_SYSTEM_DIRS:
-if os.path.exists(d):
+if d != "/var" and os.path.exists(d):
 source = "%s%s" % ( self.dest, d)
 self.add_bind_mount(source, d)
 
@@ -677,6 +677,10 @@ WantedBy=multi-user.target
 if not found:
 source = "%s%s" % ( self.dest, d)
 self.add_bind_mount(source, d)
+
+# /var contains the mounted image if there is an image: should be the
+# last thing to mount
+self.add_bind_mount(source, "/var")
 self.add_mounts()
 
 def get_expanded_unit_template(self, unit):
-- 
2.1.2

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

Re: [libvirt] [PATCH 7/9] qemu: setup tap devices for promiscLinks='no'

[Laine Stump]

On 11/24/2014 06:41 PM, John Ferlan wrote:
>
> On 11/24/2014 12:48 PM, Laine Stump wrote:
>> In order for the kernel to be able to promiscuous mode on as many
>> ports of a bridge as possible, at most one attached device can have
>> learning and unicast_flood enabled (in practice, this one device is
>> always the physical device that connects the bridge to the real
>> world). If more than one device has those settings enabled, the kernel
>> cannot enable promiscuous mode. Since both settings default to
>> enabled, we need to turn them both off (using
>> virNetDevBridgeSetLearning() and virNetDevBridgeSetUnicastFlood()) for
>> every tap device plugged into a bridge that has promiscLinks='no'.
>>
>> If there is only one device with learning/unicast_flood enabled, then
>> that device has promiscuous mode disabled. If there are *no* devices
>> with learning/unicast_flood enabled (e.g. for a libvirt "route",
>> "nat", or isolated network that has no physical device attached), then
>> all devices will have promiscuous mode disabled.
>>
>> Once we have disabled learning and flooding, any packet that has a
>> destination MAC address not present in the forwarding database (fdb)
>> will be dropped by the bridge, and libvirt is responsible for adding
>> entries to the bridge fdb for the MAC address of each guest
>> interface. That is done with virNetDevBridgeFDBAdd().
>>
>> None of this has any effect for kernels prior to 3.15 (upstream kernel
>> commit 2796d0c648c940b4796f84384fbcfb0a2399db84 "bridge: Automatically
>> manage port promiscuous mode"). Even after that, until kernel 3.17
>> (upstream commit 5be5a2df40f005ea7fb7e280e87bbbcfcf1c2fc0 "bridge: Add
>> filtering support for default_pvid") the following workaround is
>> required in order for untagged traffic to pass (vlan tagged traffic
>> will in any case currently not pass with promiscLinks='no')
>> ---
>>  src/qemu/qemu_command.c | 16 
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index cbdef9c..d3d129a 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -35,6 +35,7 @@
>>  #include "virerror.h"
>>  #include "virfile.h"
>>  #include "virnetdev.h"
>> +#include "virnetdevbridge.h"
>>  #include "virstring.h"
>>  #include "virtime.h"
>>  #include "viruuid.h"
>> @@ -350,6 +351,21 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
>>  virDomainAuditNetDevice(def, net, tunpath, false);
>>  goto cleanup;
>>  }
>> +if (virDomainNetGetActualPromiscLinks(net) == VIR_TRISTATE_BOOL_NO) 
>> {
>> +/* In order to make as many as possible of the links to a
>> + * bridge promiscuous/no-flood, we need to turn off
>> + * learning and unicast_flood, and add an fdb entry to the
>> + * bridge for this new device.
>> + */
> In patch 6 we explicitly check vlan filtering being enabled - that's not
> done here, does it need to be?

No. vlan_filtering is a property of the bridge, while "learning" and
"unicast_flood" are properties of the ports of a bridge.

>
>> +if (virNetDevBridgePortSetLearning(brname, net->ifname, false) 
>> < 0)
>> +goto cleanup;
>> +if (virNetDevBridgePortSetUnicastFlood(brname, net->ifname, 
>> false) < 0)
>> +goto cleanup;
>> +if (virNetDevBridgeFDBAdd(&net->mac, net->ifname,
>> +  VIR_NETDEVBRIDGE_FDB_FLAG_MASTER |
>> +  VIR_NETDEVBRIDGE_FDB_FLAG_TEMP) < 0)
>> +goto cleanup;
> I'd probably have similar "concerns" as patch 6 with issues that could
> occur if the default for promiscLinks changed and the error path logic.
> Although I see now that restart logic doesn't mean much (hey, it's late
> in the day).
>
> ACK to what's here though. Obviously if you changed the name from my
> patch 3 suggestion, then the BOOL_NO above would change as well.
>

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

Re: [libvirt] [PATCH 6/9] network: setup bridge devices for promiscLinks='no'

[Laine Stump]

On 11/24/2014 06:22 PM, John Ferlan wrote:
>
> On 11/24/2014 12:48 PM, Laine Stump wrote:
>> When the bridge device for a network has promiscLinks='no', the intent
>> is to setup the bridge to opportunistically turn off promiscuous mode
>> and/or flood mode for as many ports/links on the bridge as possible,
>> thus improving performance and security. The setup for the bridge
>> itself is:
>>
>> 1) set the "vlan_filtering" property of the bridge device to 1.  2) If
>> the bridge has a "Dummy" tap device used to set a fixed MAC address on
>> the bridge, turn off learning and unicast_flood on this tap (this is
>> needed even though this tap is never IFF_UP, because the kernel
>> ignores the IFF_UP flag of other devices when using their settings to
>> automatically decide whether or not to turn off promiscuous mode for
>> any attached device).
>>
>> This is done both for libvirt-created/managed bridges, and for bridges
>> that are created by the host system config.
>>
>> There is no attempt to turn vlan_filtering off when destroying the
>> network because in the case of a libvirt-created bridge, the bridge is
>> about to be destroyed anyway, and in the case of a system bridge, if
>> the other devices attached to the bridge could operate properly before
>> destroying libvirt's network object, they will continue to operate
>> properly (this is similar to the way that libvirt will enable
>> ip_forwarding whenever a routed/natted network is started, but will
>> never attempt to disable it if they are stopped).
>> ---
>>  src/network/bridge_driver.c | 53 
>> +
>>  1 file changed, 53 insertions(+)
>>
>> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>> index bc8da79..f2564c3 100644
>> --- a/src/network/bridge_driver.c
>> +++ b/src/network/bridge_driver.c
>> @@ -1893,6 +1893,28 @@ networkAddAddrToBridge(virNetworkObjPtr network,
>>  return 0;
>>  }
>>  
>> +
>> +static int
>> +networkStartHandlePromiscLinks(virNetworkObjPtr network, const char 
>> *macTapIfName)
>> +{
>> +const char *brname = network->def->bridge;
>> +
>> +/* promiscuous mode won't be turned off unless vlan filtering is 
>> enabled. */
>> +if (brname &&
>> +network->def->promiscLinks == VIR_TRISTATE_BOOL_NO) {
>> +if (virNetDevBridgeSetVlanFiltering(brname, true) < 0)
> Could this return failure if we don't have the right kernel? Then do we
> really want to kill the whole startup processing?

Yes, and yes. If they've requested it and we can't comply, then we need
to fail.

> It almost seems as if
> the failure case here should be some kind of VIR_WARN or VIR_INFO,
> return 0 and do nothing.
>
> I guess what I'm most worried about is the "future" if the decision is
> to quietly change the default of 'promiscLinks' (or whatever name is
> used) and we get here from networkStartNetworkBridge which in a libvirtd
> restart has what effect on something that was running before?

If we ever do that, it will be done in a "quietly fall back" way. I
don't want to build in too much "intelligence" that ends up never
getting used (and in the meantime makes the code (more) confusing).

>
> You would be correct in pointing out that for the current design and
> assumptions this is the right course of action.  Someone set promisc=no
> and there was a failure, so we need to fail too.
>
>
>> +return -1;
>> +if (macTapIfName) {
>> +if (virNetDevBridgePortSetLearning(brname, macTapIfName, false) 
>> < 0)
>> +return -1;
>> +if (virNetDevBridgePortSetUnicastFlood(brname, macTapIfName, 
>> false) < 0)
>> +return -1;
> Ah - although networkStartNetworkBridge says to restore things on
> failure, since we'll be deleting the bridge if either of these fail so
> it doesn't matter.

Correct.

>
> The BOOL_NO logic could change if you took my suggestion from patch3
>
> ACK, but this is ultimately where changing the default would be affected
> and perhaps we need to somehow note that.
>

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

Re: [libvirt] [PATCH] qemu: add the missing jobinfo type in qemuDomainGetJobInfo

[Jiri Denemark]

On Tue, Nov 25, 2014 at 19:51:45 +0800, Wang Rui wrote:
> Commit 6fcddfcd refactored job statistics but missed the jobinfo type updated
> in qemuDomainGetJobInfo. After this patch, we can use virDomainGetJobInfo to
> get jobinfo type again.
> 
> Signed-off-by: Wang Rui 
> ---
>  src/qemu/qemu_domain.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 334bd40..6513c78 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -258,6 +258,7 @@ int
>  qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
>  virDomainJobInfoPtr info)
>  {
> +info->type = jobInfo->type;
>  info->timeElapsed = jobInfo->timeElapsed;
>  info->timeRemaining = jobInfo->timeRemaining;
>  

Oops. ACK and I will push the patch soon.

Jirka

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

[libvirt] [sandbox][PATCH 0/2] AppArmor support

[Cédric Bosdonnat]

Hi all,

Here are 2 patches: one to get apparmor support for virt-sandbox and 
virt-sandbox-service
and 1 to get services with disk image actually start.

The AppArmor support doesn't add any parameter to the user, virt-sandbox, just 
uses either
apparmor or selinux depending on the one available... as those can't be running 
at the same
time.

Cédric Bosdonnat (2):
  AppArmor support
  virt-sandbox-service: mount /var after all other file systems

 bin/virt-sandbox-service  | 21 
 libvirt-sandbox/libvirt-sandbox-builder.c | 32 +++
 2 files changed, 45 insertions(+), 8 deletions(-)

-- 
2.1.2

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

Re: [libvirt] [PATCH] automatic create tap device with network type ethernet

[Vasiliy Tolstov]

2014-11-25 0:31 GMT+03:00 Vasiliy Tolstov :
> If use not specify script in network type ethernet, assume that user
> needs simple tap device created with libvirt.
> This patch does not need to run external script to create tap device.
>
> Signed-off-by: Vasiliy Tolstov 
> ---
>  src/qemu/qemu_command.c | 88 
> -
>  src/qemu/qemu_hotplug.c | 10 ++
>  src/qemu/qemu_process.c |  4 +++
>  3 files changed, 64 insertions(+), 38 deletions(-)


Hello. Does i need to do something or i need to wait?

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

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

Re: [libvirt] [PATCH v3 06/12] parallels: rewrite parallelsApplyConfig with SDK

[Maxim Nestratov]

18.11.2014 16:17, Dmitry Guryanov пишет:

Rewrite code, which applies domain configuration given
to virDomainDefineXML function to the VM of container
registered in PCS.

This code first check if there are unsupported parameters
in domain XML and if yes - reports error. Some of such
parameters are not supported by PCS, for some - it's not
obvious, how to convert them into PCS's corresponding params,
so let's put off it, and implement only basic params in
this patch.

Signed-off-by: Dmitry Guryanov 
---
  src/parallels/parallels_driver.c | 737 +--
  src/parallels/parallels_sdk.c| 930 +++
  src/parallels/parallels_sdk.h|   4 +
  3 files changed, 935 insertions(+), 736 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 658969f..55ee003 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -65,19 +65,6 @@ VIR_LOG_INIT("parallels.parallels_driver");
  
  static int parallelsConnectClose(virConnectPtr conn);
  
-static const char * parallelsGetDiskBusName(int bus) {

-switch (bus) {
-case VIR_DOMAIN_DISK_BUS_IDE:
-return "ide";
-case VIR_DOMAIN_DISK_BUS_SATA:
-return "sata";
-case VIR_DOMAIN_DISK_BUS_SCSI:
-return "scsi";
-default:
-return NULL;
-}
-}
-
  void
  parallelsDriverLock(parallelsConnPtr driver)
  {
@@ -671,728 +658,6 @@ parallelsDomainGetAutostart(virDomainPtr domain, int 
*autostart)
  }
  
  static int

-parallelsApplyGraphicsParams(virDomainGraphicsDefPtr *oldgraphics, int nold,
- virDomainGraphicsDefPtr *newgraphics, int nnew)
-{
-virDomainGraphicsDefPtr new, old;
-
-/* parallels server supports only 1 VNC display per VM */
-if (nold != nnew || nnew > 1)
-goto error;
-
-if (nnew == 0)
-return 0;
-
-if (newgraphics[0]->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC)
-goto error;
-
-old = oldgraphics[0];
-new = newgraphics[0];
-
-if (old->data.vnc.port != new->data.vnc.port &&
-(old->data.vnc.port != 0 && new->data.vnc.port != 0)) {
-
-goto error;
-} else if (old->data.vnc.autoport != new->data.vnc.autoport ||
-new->data.vnc.keymap != NULL ||
-new->data.vnc.socket != NULL ||
-!STREQ_NULLABLE(old->data.vnc.auth.passwd, new->data.vnc.auth.passwd) 
||
-old->data.vnc.auth.expires != new->data.vnc.auth.expires ||
-old->data.vnc.auth.validTo != new->data.vnc.auth.validTo ||
-old->data.vnc.auth.connected != new->data.vnc.auth.connected) {
-
-goto error;
-} else if (old->nListens != new->nListens ||
-   new->nListens > 1 ||
-   old->listens[0].type != new->listens[0].type ||
- !STREQ_NULLABLE(old->listens[0].address, 
new->listens[0].address) ||
- !STREQ_NULLABLE(old->listens[0].network, 
new->listens[0].network)) {
-
-goto error;
-}
-
-return 0;
- error:
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("changing display parameters is not supported "
- "by parallels driver"));
-return -1;
-}
-
-static int
-parallelsApplySerialParams(virDomainChrDefPtr *oldserials, int nold,
-   virDomainChrDefPtr *newserials, int nnew)
-{
-size_t i, j;
-
-if (nold != nnew)
-goto error;
-
-for (i = 0; i < nold; i++) {
-virDomainChrDefPtr oldserial = oldserials[i];
-virDomainChrDefPtr newserial = NULL;
-
-for (j = 0; j < nnew; j++) {
-if (newserials[j]->target.port == oldserial->target.port) {
-newserial = newserials[j];
-break;
-}
-}
-
-if (!newserial)
-goto error;
-
-if (oldserial->source.type != newserial->source.type)
-goto error;
-
-if ((newserial->source.type == VIR_DOMAIN_CHR_TYPE_DEV ||
-newserial->source.type == VIR_DOMAIN_CHR_TYPE_FILE) &&
-!STREQ_NULLABLE(oldserial->source.data.file.path,
-newserial->source.data.file.path))
-goto error;
-if (newserial->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
-   (!STREQ_NULLABLE(oldserial->source.data.nix.path,
-newserial->source.data.nix.path) ||
-oldserial->source.data.nix.listen == 
newserial->source.data.nix.listen)) {
-
-goto error;
-}
-}
-
-return 0;
- error:
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("changing serial device parameters is "
- "not supported by parallels driver"));
-return -1;
-}
-
-static int
-parallelsApplyVideoParams(parallelsDomObjPtr pdom,
-  virDomainVideoDefPtr *oldvideos, int nold,
-   virDomainVideoDefPtr *newvideos, int nnew)
-{
-virDoma

Re: [libvirt] [PATCH v3 05/12] parallels: reimplement functions, which change domain state

[Maxim Nestratov]

18.11.2014 16:17, Dmitry Guryanov пишет:

Change domain state using parallels SDK functions instead of
prlctl command.

We don't need to send events from these functions now, becase
events handler will send them. But we still need to update
virDomainObj in privconn->domains.

Signed-off-by: Dmitry Guryanov 
---
  src/parallels/parallels_driver.c | 139 +++
  src/parallels/parallels_sdk.c|  70 
  src/parallels/parallels_sdk.h|  10 +++
  3 files changed, 105 insertions(+), 114 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index e145766..658969f 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -670,120 +670,6 @@ parallelsDomainGetAutostart(virDomainPtr domain, int 
*autostart)
  return ret;
  }
  
-typedef int (*parallelsChangeStateFunc)(virDomainObjPtr privdom);

-#define PARALLELS_UUID(x) (((parallelsDomObjPtr)(x->privateData))->uuid)
-
-static int
-parallelsDomainChangeState(virDomainPtr domain,
-   virDomainState req_state, const char 
*req_state_name,
-   parallelsChangeStateFunc chstate,
-   virDomainState new_state, int reason)
-{
-parallelsConnPtr privconn = domain->conn->privateData;
-virDomainObjPtr privdom;
-int state;
-int ret = -1;
-
-parallelsDriverLock(privconn);
-privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
-parallelsDriverUnlock(privconn);
-
-if (privdom == NULL) {
-parallelsDomNotFoundError(domain);
-goto cleanup;
-}
-
-state = virDomainObjGetState(privdom, NULL);
-if (state != req_state) {
-virReportError(VIR_ERR_INTERNAL_ERROR, _("domain '%s' not %s"),
-   privdom->def->name, req_state_name);
-goto cleanup;
-}
-
-if (chstate(privdom))
-goto cleanup;
-
-virDomainObjSetState(privdom, new_state, reason);
-
-ret = 0;
-
- cleanup:
-if (privdom)
-virObjectUnlock(privdom);
-
-return ret;
-}
-
-static int parallelsPause(virDomainObjPtr privdom)
-{
-return parallelsCmdRun(PRLCTL, "pause", PARALLELS_UUID(privdom), NULL);
-}
-
-static int
-parallelsDomainSuspend(virDomainPtr domain)
-{
-return parallelsDomainChangeState(domain,
-  VIR_DOMAIN_RUNNING, "running",
-  parallelsPause,
-  VIR_DOMAIN_PAUSED, 
VIR_DOMAIN_PAUSED_USER);
-}
-
-static int parallelsResume(virDomainObjPtr privdom)
-{
-return parallelsCmdRun(PRLCTL, "resume", PARALLELS_UUID(privdom), NULL);
-}
-
-static int
-parallelsDomainResume(virDomainPtr domain)
-{
-return parallelsDomainChangeState(domain,
-  VIR_DOMAIN_PAUSED, "paused",
-  parallelsResume,
-  VIR_DOMAIN_RUNNING, 
VIR_DOMAIN_RUNNING_UNPAUSED);
-}
-
-static int parallelsStart(virDomainObjPtr privdom)
-{
-return parallelsCmdRun(PRLCTL, "start", PARALLELS_UUID(privdom), NULL);
-}
-
-static int
-parallelsDomainCreate(virDomainPtr domain)
-{
-return parallelsDomainChangeState(domain,
-  VIR_DOMAIN_SHUTOFF, "stopped",
-  parallelsStart,
-  VIR_DOMAIN_RUNNING, 
VIR_DOMAIN_EVENT_STARTED_BOOTED);
-}
-
-static int parallelsKill(virDomainObjPtr privdom)
-{
-return parallelsCmdRun(PRLCTL, "stop", PARALLELS_UUID(privdom), "--kill", 
NULL);
-}
-
-static int
-parallelsDomainDestroy(virDomainPtr domain)
-{
-return parallelsDomainChangeState(domain,
-  VIR_DOMAIN_RUNNING, "running",
-  parallelsKill,
-  VIR_DOMAIN_SHUTOFF, 
VIR_DOMAIN_SHUTOFF_DESTROYED);
-}
-
-static int parallelsStop(virDomainObjPtr privdom)
-{
-return parallelsCmdRun(PRLCTL, "stop", PARALLELS_UUID(privdom), NULL);
-}
-
-static int
-parallelsDomainShutdown(virDomainPtr domain)
-{
-return parallelsDomainChangeState(domain,
-  VIR_DOMAIN_RUNNING, "running",
-  parallelsStop,
-  VIR_DOMAIN_SHUTOFF, 
VIR_DOMAIN_SHUTOFF_SHUTDOWN);
-}
-
  static int
  parallelsApplyGraphicsParams(virDomainGraphicsDefPtr *oldgraphics, int nold,
   virDomainGraphicsDefPtr *newgraphics, int nnew)
@@ -1762,6 +1648,31 @@ parallelsConnectDomainEventDeregisterAny(virConnectPtr 
conn,
  return ret;
  }
  
+static int parallelsDomainSuspend(virDomainPtr domain)

+{
+return prlsdkDomainChangeState(domain, prlsdkPause);
+}
+
+static int parallelsDomainResume(virDomainPtr domain)
+{
+return prlsdkDomainChangeState(domain, prlsdkResume);
+}
+
+static int parallelsDo

Re: [libvirt] [PATCH v3 12/12] parallels: implement domainUndefine and domainUndefineFlags

[Maxim Nestratov]

18.11.2014 16:17, Dmitry Guryanov пишет:

Signed-off-by: Dmitry Guryanov 
---
  src/parallels/parallels_driver.c | 26 ++
  src/parallels/parallels_sdk.c| 18 ++
  src/parallels/parallels_sdk.h|  2 ++
  3 files changed, 46 insertions(+)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 522c39f..08d2e30 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -917,6 +917,30 @@ parallelsDomainCreateWithFlags(virDomainPtr domain, 
unsigned int flags)
  return parallelsDomainCreate(domain);
  }
  
+static int

+parallelsDomainUndefineFlags(virDomainPtr domain,
+ unsigned int flags)
+{
+parallelsConnPtr privconn = domain->conn->privateData;
+virDomainObjPtr dom = NULL;
+
+virCheckFlags(0, -1);
+
+dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
+if (dom == NULL) {
+parallelsDomNotFoundError(domain);
+return -1;
+}
+
+return prlsdkUnregisterDomain(privconn, dom);
+}
+
+static int
+parallelsDomainUndefine(virDomainPtr domain)
+{
+return parallelsDomainUndefineFlags(domain, 0);
+}
+
  static virHypervisorDriver parallelsDriver = {
  .no = VIR_DRV_PARALLELS,
  .name = "Parallels",
@@ -949,6 +973,8 @@ static virHypervisorDriver parallelsDriver = {
  .domainCreate = parallelsDomainCreate,/* 0.10.0 */
  .domainCreateWithFlags = parallelsDomainCreateWithFlags, /* 1.2.10 */
  .domainDefineXML = parallelsDomainDefineXML,  /* 0.10.0 */
+.domainUndefine = parallelsDomainUndefine, /* 1.2.10 */
+.domainUndefineFlags = parallelsDomainUndefineFlags, /* 1.2.10 */
  .domainIsActive = parallelsDomainIsActive, /* 1.2.10 */
  .connectDomainEventRegisterAny = parallelsConnectDomainEventRegisterAny, 
/* 1.2.10 */
  .connectDomainEventDeregisterAny = 
parallelsConnectDomainEventDeregisterAny, /* 1.2.10 */
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 85193f1..e06b6bf 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2675,3 +2675,21 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def)
  PrlHandle_Free(sdkdom);
  return ret;
  }
+
+int
+prlsdkUnregisterDomain(parallelsConnPtr privconn, virDomainObjPtr dom)
+{
+parallelsDomObjPtr privdom = dom->privateData;
+PRL_HANDLE job;
+
+job = PrlVm_Unreg(privdom->sdkdom);
+if (waitJob(job, privconn->jobTimeout))
+return -1;
+
+if (prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_UNDEFINED,
+VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) < 0)
+return -1;
+
+virDomainObjListRemove(privconn->domains, dom);
+return 0;
+}
diff --git a/src/parallels/parallels_sdk.h b/src/parallels/parallels_sdk.h
index 1fdef1a..dee9359 100644
--- a/src/parallels/parallels_sdk.h
+++ b/src/parallels/parallels_sdk.h
@@ -51,3 +51,5 @@ prlsdkApplyConfig(virConnectPtr conn,
virDomainDefPtr new);
  int prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def);
  int prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def);
+int
+prlsdkUnregisterDomain(parallelsConnPtr privconn, virDomainObjPtr dom);

ack

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

Re: [libvirt] [PATCH v3 11/12] parallels: add cdroms support

[Maxim Nestratov]

18.11.2014 16:17, Dmitry Guryanov пишет:

Get cdrom devices list from parallels server in
prlsdkLoadDomains and add ability to define a domain
with cdroms.

Signed-off-by: Dmitry Guryanov 
---
  src/parallels/parallels_sdk.c | 70 ++-
  1 file changed, 63 insertions(+), 7 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index d1a8ebf..85193f1 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -446,7 +446,8 @@ prlsdkAddDomainVideoInfo(PRL_HANDLE sdkdom, virDomainDefPtr 
def)
  
  static int

  prlsdkGetDiskInfo(PRL_HANDLE prldisk,
-  virDomainDiskDefPtr disk)
+  virDomainDiskDefPtr disk,
+  bool isCdrom)
  {
  char *buf = NULL;
  PRL_UINT32 buflen = 0;
@@ -461,11 +462,19 @@ prlsdkGetDiskInfo(PRL_HANDLE prldisk,
  prlsdkCheckRetGoto(pret, cleanup);
  if (emulatedType == PDT_USE_IMAGE_FILE) {
  virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
-virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_PLOOP);
+if (isCdrom)
+virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_AUTO);
+else
+virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_PLOOP);
  } else {
  virDomainDiskSetType(disk, VIR_STORAGE_TYPE_BLOCK);
  }
  
+if (isCdrom)

+disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
+else
+disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
+
  pret = PrlVmDev_GetFriendlyName(prldisk, NULL, &buflen);
  prlsdkCheckRetGoto(pret, cleanup);
  
@@ -543,7 +552,7 @@ prlsdkAddDomainHardDisksInfo(PRL_HANDLE sdkdom, virDomainDefPtr def)

  if (!(disk = virDomainDiskDefNew()))
  goto error;
  
-if (prlsdkGetDiskInfo(hdd, disk) < 0)

+if (prlsdkGetDiskInfo(hdd, disk, false) < 0)
  goto error;
  
  PrlHandle_Free(hdd);

@@ -563,6 +572,43 @@ prlsdkAddDomainHardDisksInfo(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
  }
  
  static int

+prlsdkAddDomainOpticalDisksInfo(PRL_HANDLE sdkdom, virDomainDefPtr def)
+{
+PRL_RESULT pret;
+PRL_UINT32 cdromsCount;
+PRL_UINT32 i;
+PRL_HANDLE cdrom = PRL_INVALID_HANDLE;
+virDomainDiskDefPtr disk = NULL;
+
+pret = PrlVmCfg_GetOpticalDisksCount(sdkdom, &cdromsCount);
+prlsdkCheckRetGoto(pret, error);
+
+for (i = 0; i < cdromsCount; ++i) {
+pret = PrlVmCfg_GetOpticalDisk(sdkdom, i, &cdrom);
+prlsdkCheckRetGoto(pret, error);
+
+if (!(disk = virDomainDiskDefNew()))
+goto error;
+
+if (prlsdkGetDiskInfo(cdrom, disk, true) < 0)
+goto error;
+
+PrlHandle_Free(cdrom);
+cdrom = PRL_INVALID_HANDLE;
+
+if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
+goto error;
+}
+
+return 0;
+
+ error:
+PrlHandle_Free(cdrom);
+virDomainDiskDefFree(disk);
+return -1;
+}
+
+static int
  prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt)
  {
  char macstr[VIR_MAC_STRING_BUFLEN];
@@ -781,6 +827,9 @@ prlsdkAddDomainHardware(PRL_HANDLE sdkdom, virDomainDefPtr 
def)
  if (prlsdkAddDomainHardDisksInfo(sdkdom, def) < 0)
  goto error;
  
+if (prlsdkAddDomainOpticalDisksInfo(sdkdom, def) < 0)

+goto error;
+
  if (prlsdkAddDomainNetInfo(sdkdom, def) < 0)
  goto error;
  
@@ -2081,9 +2130,11 @@ static int prlsdkCheckNetUnsupportedParams(virDomainNetDefPtr net)
  
  static int prlsdkCheckDiskUnsupportedParams(virDomainDiskDefPtr disk)

  {
-if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) {
+if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK &&
+disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) {
+
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("Only hard disks are supported "
+   _("Only hard disks and cdroms are supported "
   "supported by parallels driver."));
  return -1;
  }
@@ -2393,7 +2444,10 @@ static int prlsdkAddDisk(PRL_HANDLE sdkdom, 
virDomainDiskDefPtr disk)
  if (prlsdkCheckDiskUnsupportedParams(disk) < 0)
  return -1;
  
-pret = PrlVmCfg_CreateVmDev(sdkdom, PDE_HARD_DISK, &sdkdisk);

+if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)
+pret = PrlVmCfg_CreateVmDev(sdkdom, PDE_HARD_DISK, &sdkdisk);
+else
+pret = PrlVmCfg_CreateVmDev(sdkdom, PDE_OPTICAL_DISK, &sdkdisk);
  prlsdkCheckRetGoto(pret, cleanup);
  
  pret = PrlVmDev_SetEnabled(sdkdisk, 1);

@@ -2403,7 +2457,9 @@ static int prlsdkAddDisk(PRL_HANDLE sdkdom, 
virDomainDiskDefPtr disk)
  prlsdkCheckRetGoto(pret, cleanup);
  
  if (disk->src->type == VIR_STORAGE_TYPE_FILE) {

-if (virDomainDiskGetFormat(disk) != VIR_STORAGE_FILE_PLOOP) {
+if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
+virDomainDiskGetFormat(disk) != VIR_STORAGE_FILE_PLOOP) {

Re: [libvirt] [PATCH v3 10/12] parallels: Add domainCreateWithFlags() function.

[Maxim Nestratov]

18.11.2014 16:17, Dmitry Guryanov пишет:

From: Alexander Burluka 

domainCreateWithFlags function is used by OpenStack/Nova to boot
an instance.

Signed-off-by: Dmitry Guryanov 
---
  src/parallels/parallels_driver.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 998f9ae..522c39f 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -908,6 +908,15 @@ static int parallelsDomainIsActive(virDomainPtr domain)
  return ret;
  }
  
+static int

+parallelsDomainCreateWithFlags(virDomainPtr domain, unsigned int flags)
+{
+/* we don't support any create flags */
+virCheckFlags(0, -1);
+
+return parallelsDomainCreate(domain);
+}
+
  static virHypervisorDriver parallelsDriver = {
  .no = VIR_DRV_PARALLELS,
  .name = "Parallels",
@@ -938,6 +947,7 @@ static virHypervisorDriver parallelsDriver = {
  .domainDestroy = parallelsDomainDestroy,  /* 0.10.0 */
  .domainShutdown = parallelsDomainShutdown, /* 0.10.0 */
  .domainCreate = parallelsDomainCreate,/* 0.10.0 */
+.domainCreateWithFlags = parallelsDomainCreateWithFlags, /* 1.2.10 */
  .domainDefineXML = parallelsDomainDefineXML,  /* 0.10.0 */
  .domainIsActive = parallelsDomainIsActive, /* 1.2.10 */
  .connectDomainEventRegisterAny = parallelsConnectDomainEventRegisterAny, 
/* 1.2.10 */

ack

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

Re: [libvirt] [PATCH v3 08/12] parallels: refactor parallelsDomainDefineXML

[Maxim Nestratov]

18.11.2014 16:17, Dmitry Guryanov пишет:

First, we don't need to call prlsdkApplyConfig after
creating new VM or containers, because it's done in
functions prlsdkCreateVm and prlsdkCreateCt.

No need to check, if domain exists in the list after
prlsdkAddDomain.

Also organize code, so that we can call virObjectUnlock
in one place.

Signed-off-by: Dmitry Guryanov 
---
  src/parallels/parallels_driver.c | 35 ++-
  src/parallels/parallels_sdk.c|  2 +-
  src/parallels/parallels_sdk.h|  1 +
  3 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 582ffdb..955516a 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -661,10 +661,9 @@ static virDomainPtr
  parallelsDomainDefineXML(virConnectPtr conn, const char *xml)
  {
  parallelsConnPtr privconn = conn->privateData;
-virDomainPtr ret = NULL;
+virDomainPtr retdom = NULL;
  virDomainDefPtr def;
  virDomainObjPtr olddom = NULL;
-virDomainObjPtr dom = NULL;
  
  parallelsDriverLock(privconn);

  if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt,
@@ -689,34 +688,28 @@ parallelsDomainDefineXML(virConnectPtr conn, const char 
*xml)
 _("Unsupported OS type: %s"), def->os.type);
  goto cleanup;
  }
-dom = prlsdkAddDomain(privconn, def->uuid);
-if (dom)
-virObjectUnlock(dom);
-else
+
+olddom = prlsdkAddDomain(privconn, def->uuid);
+if (!olddom)
  goto cleanup;
-olddom = virDomainObjListFindByName(privconn->domains, def->name);
-if (!olddom) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Domain for '%s' is not defined after creation"),
-   def->name ? def->name : _("(unnamed)"));
+} else {
+if (prlsdkApplyConfig(conn, olddom, def))
  goto cleanup;
-}
-}
  
-if (prlsdkApplyConfig(conn, olddom, def) < 0) {

-virObjectUnlock(olddom);
-goto cleanup;
+if (prlsdkUpdateDomain(privconn, olddom))
+goto cleanup;
  }
-virObjectUnlock(olddom);
  
-ret = virGetDomain(conn, def->name, def->uuid);

-if (ret)
-ret->id = def->id;
+retdom = virGetDomain(conn, def->name, def->uuid);
+if (retdom)
+retdom->id = def->id;
  
   cleanup:

+if (olddom)
+virObjectUnlock(olddom);
  virDomainDefFree(def);
  parallelsDriverUnlock(privconn);
-return ret;
+return retdom;
  }
  
  static int

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index a943f4b..d1a8ebf 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1248,7 +1248,7 @@ prlsdkAddDomain(parallelsConnPtr privconn, const unsigned 
char *uuid)
  return dom;
  }
  
-static int

+int
  prlsdkUpdateDomain(parallelsConnPtr privconn, virDomainObjPtr dom)
  {
  PRL_HANDLE job;
diff --git a/src/parallels/parallels_sdk.h b/src/parallels/parallels_sdk.h
index b654c2a..1fdef1a 100644
--- a/src/parallels/parallels_sdk.h
+++ b/src/parallels/parallels_sdk.h
@@ -32,6 +32,7 @@ int
  prlsdkLoadDomains(parallelsConnPtr privconn);
  virDomainObjPtr
  prlsdkAddDomain(parallelsConnPtr privconn, const unsigned char *uuid);
+int prlsdkUpdateDomain(parallelsConnPtr privconn, virDomainObjPtr dom);
  int prlsdkSubscribeToPCSEvents(parallelsConnPtr privconn);
  void prlsdkUnsubscribeFromPCSEvents(parallelsConnPtr privconn);
  int prlsdkStart(parallelsConnPtr privconn, PRL_HANDLE sdkdom);

ack

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

Re: [libvirt] [PATCH v3 09/12] parallels: added function virDomainIsActive()

[Maxim Nestratov]

18.11.2014 16:17, Dmitry Guryanov пишет:

From: Alexander Burluka 

That function is necessary for proper domain removal
in openstack/nova.

Signed-off-by: Dmitry Guryanov 
---
  src/parallels/parallels_driver.c | 19 +++
  1 file changed, 19 insertions(+)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 955516a..998f9ae 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -890,6 +890,24 @@ static int parallelsDomainShutdown(virDomainPtr domain)
  return prlsdkDomainChangeState(domain, prlsdkStop);
  }
  
+static int parallelsDomainIsActive(virDomainPtr domain)

+{
+parallelsConnPtr privconn = domain->conn->privateData;
+virDomainObjPtr dom = NULL;
+int ret = -1;
+
+dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
+if (dom == NULL) {
+parallelsDomNotFoundError(domain);
+return -1;
+}
+
+ret = virDomainObjIsActive(dom);
+virObjectUnlock(dom);
+
+return ret;
+}
+
  static virHypervisorDriver parallelsDriver = {
  .no = VIR_DRV_PARALLELS,
  .name = "Parallels",
@@ -921,6 +939,7 @@ static virHypervisorDriver parallelsDriver = {
  .domainShutdown = parallelsDomainShutdown, /* 0.10.0 */
  .domainCreate = parallelsDomainCreate,/* 0.10.0 */
  .domainDefineXML = parallelsDomainDefineXML,  /* 0.10.0 */
+.domainIsActive = parallelsDomainIsActive, /* 1.2.10 */
  .connectDomainEventRegisterAny = parallelsConnectDomainEventRegisterAny, 
/* 1.2.10 */
  .connectDomainEventDeregisterAny = 
parallelsConnectDomainEventDeregisterAny, /* 1.2.10 */
  .nodeGetCPUMap = parallelsNodeGetCPUMap, /* 1.2.8 */

ack

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

Re: [libvirt] [PATCH v3 07/12] parallels: create VMs and containers with sdk

[Maxim Nestratov]

18.11.2014 16:17, Dmitry Guryanov пишет:

This patch replaces code, which creates domains by
running prlctl command.

prlsdkCreateVm/Ct will do prlsdkApplyConfig, because
we send request to the server only once in this case.

But prlsdkApplyConfig will be called also from
parallelsDomainDefineXML function. There is no problem with
it, parallelsDomainDefineXML will be refactored later.

Signed-off-by: Dmitry Guryanov 
---
  src/parallels/parallels_driver.c | 45 +-
  src/parallels/parallels_sdk.c| 83 +++-
  src/parallels/parallels_sdk.h|  2 +
  3 files changed, 86 insertions(+), 44 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 55ee003..582ffdb 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -657,47 +657,6 @@ parallelsDomainGetAutostart(virDomainPtr domain, int 
*autostart)
  return ret;
  }
  
-static int

-parallelsCreateVm(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDefPtr def)
-{
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-
-virUUIDFormat(def->uuid, uuidstr);
-
-if (parallelsCmdRun(PRLCTL, "create", def->name, "--no-hdd",
-"--uuid", uuidstr, NULL) < 0)
-return -1;
-
-return 0;
-}
-
-static int
-parallelsCreateCt(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDefPtr def)
-{
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-
-virUUIDFormat(def->uuid, uuidstr);
-
-if (def->nfss != 1 ||
-def->fss[0]->type != VIR_DOMAIN_FS_TYPE_TEMPLATE) {
-
-virReportError(VIR_ERR_INVALID_ARG, "%s",
-   _("There must be only 1 template FS for "
- "container creation"));
-goto error;
-}
-
-if (parallelsCmdRun(PRLCTL, "create", def->name, "--vmtype", "ct",
-"--uuid", uuidstr,
-"--ostemplate", def->fss[0]->src, NULL) < 0)
-goto error;
-
-return 0;
-
- error:
-return -1;
-}
-
  static virDomainPtr
  parallelsDomainDefineXML(virConnectPtr conn, const char *xml)
  {
@@ -720,10 +679,10 @@ parallelsDomainDefineXML(virConnectPtr conn, const char 
*xml)
  if (olddom == NULL) {
  virResetLastError();
  if (STREQ(def->os.type, "hvm")) {
-if (parallelsCreateVm(conn, def))
+if (prlsdkCreateVm(conn, def))
  goto cleanup;
  } else if (STREQ(def->os.type, "exe")) {
-if (parallelsCreateCt(conn, def))
+if (prlsdkCreateCt(conn, def))
  goto cleanup;
  } else {
  virReportError(VIR_ERR_INVALID_ARG,
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index bccb2d7..a943f4b 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2506,7 +2506,6 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
  
   error:

  return -1;
-
  }
  
  int

@@ -2538,3 +2537,85 @@ prlsdkApplyConfig(virConnectPtr conn,
  
  return ret;

  }
+
+int
+prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def)
+{
+parallelsConnPtr privconn = conn->privateData;
+PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
+PRL_HANDLE job = PRL_INVALID_HANDLE;
+PRL_HANDLE result = PRL_INVALID_HANDLE;
+PRL_HANDLE srvconf = PRL_INVALID_HANDLE;
+PRL_RESULT pret;
+int ret = -1;
+
+pret = PrlSrv_CreateVm(privconn->server, &sdkdom);
+prlsdkCheckRetGoto(pret, cleanup);
+
+job = PrlSrv_GetSrvConfig(privconn->server);
+if (!(result = getJobResult(job, privconn->jobTimeout)))
+goto cleanup;
+
+pret = PrlResult_GetParamByIndex(result, 0, &srvconf);
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlVmCfg_SetDefaultConfig(sdkdom, srvconf, 
PVS_GUEST_VER_LIN_REDHAT, 0);
+prlsdkCheckRetGoto(pret, cleanup);
+
+ret = prlsdkDoApplyConfig(sdkdom, def);
+if (ret)
+goto cleanup;
+
+job = PrlVm_Reg(sdkdom, "", 1);
+ret = waitJob(job, privconn->jobTimeout);
+
+ cleanup:
+PrlHandle_Free(sdkdom);
+return ret;
+}
+
+int
+prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def)
+{
+parallelsConnPtr privconn = conn->privateData;
+PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
+PRL_GET_VM_CONFIG_PARAM_DATA confParam;
+PRL_HANDLE job = PRL_INVALID_HANDLE;
+PRL_HANDLE result = PRL_INVALID_HANDLE;
+PRL_RESULT pret;
+int ret = -1;
+
+if (def->nfss != 1 ||
+def->fss[0]->type != VIR_DOMAIN_FS_TYPE_TEMPLATE) {
+
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("There must be only 1 template FS for "
+ "container creation"));
+return -1;
+}
I'm not sure this is a mandatory parameter in case container is created 
with loop device.

+
+confParam.nVmType = PVT_CT;
+confParam.sConfigSample = "vswap.1024MB";
+confParam.nOsVersion = 0;
+

PrlVmCfg_SetVmType(sdkdom, PVT_CT) should be called here

+job = PrlSrv_GetDefaul

Re: [libvirt] [PATCH v3 04/12] parallels: handle events from parallels server

[Maxim Nestratov]

18.11.2014 16:16, Dmitry Guryanov пишет:

From: Alexander Burluka 

Subscribe to events from parallels server. It's
needed for 2 things: to update cached domains list
and to send corresponding libvirt events.

Parallels server sends a lot of different events, in
this patch we handle only some of them. In the future
we can handle for example, changes in a host network
configuration or devices states.

Signed-off-by: Dmitry Guryanov 
---
  src/parallels/parallels_driver.c |  46 ++
  src/parallels/parallels_sdk.c| 295 ++-
  src/parallels/parallels_sdk.h|   2 +
  3 files changed, 341 insertions(+), 2 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 83995d5..e145766 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -223,6 +223,12 @@ parallelsOpenDefault(virConnectPtr conn)
  if (!(privconn->domains = virDomainObjListNew()))
  goto error;
  
+if (!(privconn->domainEventState = virObjectEventStateNew()))

+goto error;
+
+if (prlsdkSubscribeToPCSEvents(privconn))
+goto error;
+
  conn->privateData = privconn;
  
  if (prlsdkLoadDomains(privconn))

@@ -234,6 +240,7 @@ parallelsOpenDefault(virConnectPtr conn)
  virObjectUnref(privconn->domains);
  virObjectUnref(privconn->caps);
  virStoragePoolObjListFree(&privconn->pools);
+virObjectEventStateFree(privconn->domainEventState);
  prlsdkDisconnect(privconn);
  prlsdkDeinit();
   err_free:
@@ -280,9 +287,11 @@ parallelsConnectClose(virConnectPtr conn)
  parallelsConnPtr privconn = conn->privateData;
  
  parallelsDriverLock(privconn);

+prlsdkUnsubscribeFromPCSEvents(privconn);
  virObjectUnref(privconn->caps);
  virObjectUnref(privconn->xmlopt);
  virObjectUnref(privconn->domains);
+virObjectEventStateFree(privconn->domainEventState);
  prlsdkDisconnect(privconn);
  conn->privateData = NULL;
  prlsdkDeinit();
@@ -1717,6 +1726,41 @@ parallelsNodeGetCPUMap(virConnectPtr conn 
ATTRIBUTE_UNUSED,
  return nodeGetCPUMap(cpumap, online, flags);
  }
  
+static int

+parallelsConnectDomainEventRegisterAny(virConnectPtr conn,
+   virDomainPtr domain,
+   int eventID,
+   virConnectDomainEventGenericCallback 
callback,
+   void *opaque,
+   virFreeCallback freecb)
+{
+int ret = -1;
+parallelsConnPtr privconn = conn->privateData;
+if (virDomainEventStateRegisterID(conn,
+  privconn->domainEventState,
+  domain, eventID,
+  callback, opaque, freecb, &ret) < 0)
+ret = -1;
+return ret;
+}
+
+static int
+parallelsConnectDomainEventDeregisterAny(virConnectPtr conn,
+ int callbackID)
+{
+parallelsConnPtr privconn = conn->privateData;
+int ret = -1;
+
+if (virObjectEventStateDeregisterID(conn,
+privconn->domainEventState,
+callbackID) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+return ret;
+}
  
  static virHypervisorDriver parallelsDriver = {

  .no = VIR_DRV_PARALLELS,
@@ -1749,6 +1793,8 @@ static virHypervisorDriver parallelsDriver = {
  .domainShutdown = parallelsDomainShutdown, /* 0.10.0 */
  .domainCreate = parallelsDomainCreate,/* 0.10.0 */
  .domainDefineXML = parallelsDomainDefineXML,  /* 0.10.0 */
+.connectDomainEventRegisterAny = parallelsConnectDomainEventRegisterAny, 
/* 1.2.10 */
+.connectDomainEventDeregisterAny = 
parallelsConnectDomainEventDeregisterAny, /* 1.2.10 */
  .nodeGetCPUMap = parallelsNodeGetCPUMap, /* 1.2.8 */
  .connectIsEncrypted = parallelsConnectIsEncrypted, /* 1.2.5 */
  .connectIsSecure = parallelsConnectIsSecure, /* 1.2.5 */
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index c6cf78a..01efc22 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -27,6 +27,7 @@
  #include "virstring.h"
  #include "nodeinfo.h"
  #include "virlog.h"
+#include "datatypes.h"
  
  #include "parallels_sdk.h"
  
@@ -1130,9 +1131,7 @@ prlsdkLoadDomain(parallelsConnPtr privconn,

   * for state and domain name */
  dom = olddom;
  virDomainDefFree(dom->def);
-virDomainDefFree(dom->newDef);
  dom->def = def;
-dom->newDef = def;
  } else {
  if (!(dom = virDomainObjListAdd(privconn->domains, def,
  privconn->xmlopt,
@@ -1247,3 +1246,295 @@ prlsdkAddDomain(parallelsConnPtr privconn, const 
unsigned char *uuid)
  PrlHandle_Free(sdkdom);
  return dom;
  }
+
+static int
+prlsdkUp

[libvirt] Jenkins build is back to normal : libvirt-syntax-check #2943

[Jenkins CI]

See 

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

Re: [libvirt] [PATCH] conf: replace call to virNetworkFree() with virOjectUnref()

[Michal Privoznik]

On 25.11.2014 11:15, Laine Stump wrote:

The function virNetworkObjListExport() in network_conf.c had a call to
the public API virNetworkFree() which was causing a link error:

CCLD libvirt_driver_vbox_network_impl.la
  ./.libs/libvirt_conf.a(libvirt_conf_la-network_conf.o): In function 
`virNetworkObjListExport':
/home/laine/devel/libvirt/src/conf/network_conf.c:4496: undefined reference to 
`virNetworkFree'

This would happen when I added

   #include "network_conf.h"

into domain_conf.c, then attempted to call a new function from that
file (and enum converter, similar to virNetworkForwardTypeToString())

In the end, virNetworkFree() ends up just calling virObjectUnref(obj)
anyway (after clearing all pending errors, which we probably *don't*
want to do in the cleanup of a utility function), so this is likely
more correct than the original code as well.
---

A quick look showed that there may be other places where we are
calling public APIs such as virNetworkFree and virDomainFree when we
really don't want to be clearning out the pending error - this would
result in the good old "an error was encountered but the cause is
unknown" type of log messages. It may warrant an audit...

  src/conf/network_conf.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 067334e..a249e32 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -4463,10 +4463,8 @@ virNetworkObjListExport(virConnectPtr conn,

   cleanup:
  if (tmp_nets) {
-for (i = 0; i < nnets; i++) {
-if (tmp_nets[i])
-virNetworkFree(tmp_nets[i]);
-}
+for (i = 0; i < nnets; i++)
+virObjectUnref(tmp_nets[i]);
  }

  VIR_FREE(tmp_nets);



ACK

Michal

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

Re: [libvirt] [PATCH 2/2] Tests : Make nwfilter testcases robust against optional locking flags.

[Martin Kletzander]

On Tue, Nov 25, 2014 at 05:12:48PM +0530, Prerna Saxena wrote:

Commit dc33e6e4a5a5d42 introduces iptables/ebtables to adding locking
flags if/when these are available. However, the nwfilter testcases
list outputs without taking into account whether locking flags have been passed.

This shows up false testcase failures such as :
2) ebiptablesTearOldRules...
Offset 1035
Expect [t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0
ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0
ebtables -t nat -L libvirt-I-vnet0
ebtables -t nat -L libvirt-O-vnet0
...snip...]
Actual [-concurrent -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0
ebtables --concurrent -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0
ebtables --concurrent -t nat -L libvirt-I-vnet0
ebtables --concurrent -t nat -L libvirt-O-vnet0
...snip...]

This scrubs all reference to locking flags from test results buffer,
so that achieved output matches the expected results.



Instead of parsing and re-creating the string (which also doesn't
check whether we use the locking flag properly), it would be way
better if we could unify the result.

From the top of my head, we can either expose the
virFirewallCheckUpdateLock() as non-static and mock it in tests to
always set the lock flags to true or we can create new functions that
will override setting of the flags.

Martin


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

Re: [libvirt] [PATCH 1/2] tests : Fix failure reporting for tests/nwfilterebiptablestest.c

[Martin Kletzander]

On Tue, Nov 25, 2014 at 05:10:04PM +0530, Prerna Saxena wrote:

Tests run with 'make check' generally report their results as :

Expected:
...

Actual:
...

'nwfilterebiptablestest' reports its outcome in opposite sequence, which
is confusing for an end-user.
This changes 'nwfilterebpitablestest' to report results in a consistent
fashion.

Signed-off-by: Prerna Saxena 
---
tests/nwfilterebiptablestest.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c
index e04bc21..f62e046 100644
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -114,8 +114,8 @@ testNWFilterEBIPTablesAllTeardown(const void *opaque 
ATTRIBUTE_UNUSED)
actual = virBufferContentAndReset(&buf);
virtTestClearCommandPath(actual);

-if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+if (STRNEQ_NULLABLE(expected, actual)) {
+virtTestDifference(stderr, expected, actual);


No need to change the condition, but it doesn't hurt and looks better.

ACK, will push in a while.

Martin


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

[libvirt] [PATCH] Resolve build breaker

[John Ferlan]

Commit 'c264eeaa' didn't do the prerequisite 'make syntax-check' before
pushing. There was a  in the whitespace for the comment.  Replaced
with spaces and aligned.

pushed as build breaker since Jenkins complained loudly

Signed-off-by: John Ferlan 
---
 src/security/virt-aa-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index f273e09..869bf18 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -571,7 +571,7 @@ valid_path(const char *path, const bool readonly)
 };
 /* override the above with these */
 const char * const override[] = {
-"/sys/devices/pci",/* for hostdev pci devices */
+"/sys/devices/pci",  /* for hostdev pci devices */
 "/etc/libvirt-sandbox/services/" /* for virt-sandbox service config */
 };
 
-- 
1.9.3

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

Re: [libvirt] [PATCH v3 02/12] parallels: get domain info with SDK

[Maxim Nestratov]

18.11.2014 16:16, Dmitry Guryanov пишет:

From: Alexander Burluka 

Obtain information about domains using parallels sdk
instead of prlctl. prlsdkLoadDomains functions behaves
as former parallelsLoadDomains with NULL as second
parameter (name) - it fills parallelsConn.domains list.

prlsdkLoadDomain is now able to update specified domain
by given virDomainObjPtr.

Signed-off-by: Dmitry Guryanov 
---
  src/parallels/parallels_driver.c |  747 +---
  src/parallels/parallels_sdk.c| 1008 ++
  src/parallels/parallels_sdk.h|4 +
  src/parallels/parallels_utils.h  |1 +
  4 files changed, 1019 insertions(+), 741 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 0085c8f..8db4997 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -49,7 +49,6 @@
  #include "virfile.h"
  #include "virstoragefile.h"
  #include "nodeinfo.h"
-#include "c-ctype.h"
  #include "virstring.h"
  #include "cpu/cpu.h"
  
@@ -99,21 +98,6 @@ parallelsDriverUnlock(parallelsConnPtr driver)

  virMutexUnlock(&driver->lock);
  }
  
-

-static void
-parallelsDomObjFreePrivate(void *p)
-{
-parallelsDomObjPtr pdom = p;
-
-if (!pdom)
-return;
-
-virBitmapFree(pdom->cpumask);
-VIR_FREE(pdom->uuid);
-VIR_FREE(pdom->home);
-VIR_FREE(p);
-};
-
  static virCapsPtr
  parallelsBuildCapabilities(void)
  {
@@ -191,729 +175,6 @@ parallelsConnectGetCapabilities(virConnectPtr conn)
  }
  
  static int

-parallelsGetSerialInfo(virDomainChrDefPtr chr,
-   const char *name, virJSONValuePtr value)
-{
-const char *tmp;
-
-chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL;
-chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
-if (virStrToLong_i(name + strlen("serial"),
-   NULL, 10, &chr->target.port) < 0) {
-parallelsParseError();
-return -1;
-}
-
-if (virJSONValueObjectHasKey(value, "output")) {
-chr->source.type = VIR_DOMAIN_CHR_TYPE_FILE;
-
-tmp = virJSONValueObjectGetString(value, "output");
-if (!tmp) {
-parallelsParseError();
-return -1;
-}
-
-if (VIR_STRDUP(chr->source.data.file.path, tmp) < 0)
-return -1;
-} else if (virJSONValueObjectHasKey(value, "socket")) {
-chr->source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
-
-tmp = virJSONValueObjectGetString(value, "socket");
-if (!tmp) {
-parallelsParseError();
-return -1;
-}
-
-if (VIR_STRDUP(chr->source.data.nix.path, tmp) < 0)
-return -1;
-chr->source.data.nix.listen = false;
-} else if (virJSONValueObjectHasKey(value, "real")) {
-chr->source.type = VIR_DOMAIN_CHR_TYPE_DEV;
-
-tmp = virJSONValueObjectGetString(value, "real");
-if (!tmp) {
-parallelsParseError();
-return -1;
-}
-
-if (VIR_STRDUP(chr->source.data.file.path, tmp) < 0)
-return -1;
-} else {
-parallelsParseError();
-return -1;
-}
-
-return 0;
-}
-
-static int
-parallelsAddSerialInfo(virDomainChrDefPtr **serials, size_t *nserials,
-   const char *key, virJSONValuePtr value)
-{
-virDomainChrDefPtr chr = NULL;
-
-if (!(chr = virDomainChrDefNew()))
-goto cleanup;
-
-if (parallelsGetSerialInfo(chr, key, value))
-goto cleanup;
-
-if (VIR_APPEND_ELEMENT(*serials, *nserials, chr) < 0)
-goto cleanup;
-
-return 0;
-
- cleanup:
-virDomainChrDefFree(chr);
-return -1;
-}
-
-static int
-parallelsAddVideoInfo(virDomainDefPtr def, virJSONValuePtr value)
-{
-virDomainVideoDefPtr video = NULL;
-virDomainVideoAccelDefPtr accel = NULL;
-const char *tmp;
-char *endptr;
-unsigned long mem;
-
-if (!(tmp = virJSONValueObjectGetString(value, "size"))) {
-parallelsParseError();
-goto error;
-}
-
-if (virStrToLong_ul(tmp, &endptr, 10, &mem) < 0) {
-parallelsParseError();
-goto error;
-}
-
-if (!STREQ(endptr, "Mb")) {
-parallelsParseError();
-goto error;
-}
-
-if (VIR_ALLOC(video) < 0)
-goto error;
-
-if (VIR_ALLOC(accel) < 0)
-goto error;
-
-if (VIR_APPEND_ELEMENT_COPY(def->videos, def->nvideos, video) < 0)
-goto error;
-
-video->type = VIR_DOMAIN_VIDEO_TYPE_VGA;
-video->vram = mem << 20;
-video->heads = 1;
-video->accel = accel;
-
-return 0;
-
- error:
-VIR_FREE(accel);
-virDomainVideoDefFree(video);
-return -1;
-}
-
-static int
-parallelsGetHddInfo(virDomainDefPtr def,
-virDomainDiskDefPtr disk,
-const char *key,
-virJSONValuePtr value)
-{
-const char *tmp;
-unsigned int idx;
-
-disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
-
-if (virJSO

[libvirt] [PATCH] qemu: add the missing jobinfo type in qemuDomainGetJobInfo

[Wang Rui]

Commit 6fcddfcd refactored job statistics but missed the jobinfo type updated
in qemuDomainGetJobInfo. After this patch, we can use virDomainGetJobInfo to
get jobinfo type again.

Signed-off-by: Wang Rui 
---
 src/qemu/qemu_domain.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 334bd40..6513c78 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -258,6 +258,7 @@ int
 qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
 virDomainJobInfoPtr info)
 {
+info->type = jobInfo->type;
 info->timeElapsed = jobInfo->timeElapsed;
 info->timeRemaining = jobInfo->timeRemaining;
 
-- 
1.7.12.4


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

[libvirt] [PATCH 1/2] tests : Fix failure reporting for tests/nwfilterebiptablestest.c

[Prerna Saxena]

Tests run with 'make check' generally report their results as :

Expected:
...

Actual:
...

'nwfilterebiptablestest' reports its outcome in opposite sequence, which
is confusing for an end-user.
This changes 'nwfilterebpitablestest' to report results in a consistent
fashion.

Signed-off-by: Prerna Saxena 
---
 tests/nwfilterebiptablestest.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c
index e04bc21..f62e046 100644
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -114,8 +114,8 @@ testNWFilterEBIPTablesAllTeardown(const void *opaque 
ATTRIBUTE_UNUSED)
 actual = virBufferContentAndReset(&buf);
 virtTestClearCommandPath(actual);
 
-if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+if (STRNEQ_NULLABLE(expected, actual)) {
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -185,8 +185,8 @@ testNWFilterEBIPTablesTearOldRules(const void *opaque 
ATTRIBUTE_UNUSED)
 actual = virBufferContentAndReset(&buf);
 virtTestClearCommandPath(actual);
 
-if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+if (STRNEQ_NULLABLE(expected, actual)) {
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -234,8 +234,8 @@ testNWFilterEBIPTablesRemoveBasicRules(const void *opaque 
ATTRIBUTE_UNUSED)
 actual = virBufferContentAndReset(&buf);
 virtTestClearCommandPath(actual);
 
-if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+if (STRNEQ_NULLABLE(expected, actual)) {
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -268,8 +268,8 @@ testNWFilterEBIPTablesTearNewRules(const void *opaque 
ATTRIBUTE_UNUSED)
 actual = virBufferContentAndReset(&buf);
 virtTestClearCommandPath(actual);
 
-if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+if (STRNEQ_NULLABLE(expected, actual)) {
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -340,8 +340,8 @@ testNWFilterEBIPTablesApplyBasicRules(const void *opaque 
ATTRIBUTE_UNUSED)
 actual = virBufferContentAndReset(&buf);
 virtTestClearCommandPath(actual);
 
-if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+if (STRNEQ_NULLABLE(expected, actual)) {
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -430,8 +430,8 @@ testNWFilterEBIPTablesApplyDHCPOnlyRules(const void *opaque 
ATTRIBUTE_UNUSED)
 actual = virBufferContentAndReset(&buf);
 virtTestClearCommandPath(actual);
 
-if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+if (STRNEQ_NULLABLE(expected, actual)) {
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -503,8 +503,8 @@ testNWFilterEBIPTablesApplyDropAllRules(const void *opaque 
ATTRIBUTE_UNUSED)
 actual = virBufferContentAndReset(&buf);
 virtTestClearCommandPath(actual);
 
-if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+if (STRNEQ_NULLABLE(expected, actual)) {
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
-- 
1.9.3

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

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

[libvirt] [PATCH 2/2] Tests : Make nwfilter testcases robust against optional locking flags.

[Prerna Saxena]

Commit dc33e6e4a5a5d42 introduces iptables/ebtables to adding locking
flags if/when these are available. However, the nwfilter testcases
list outputs without taking into account whether locking flags have been passed.

This shows up false testcase failures such as :
 2) ebiptablesTearOldRules...
Offset 1035
Expect [t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0
ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0
ebtables -t nat -L libvirt-I-vnet0
ebtables -t nat -L libvirt-O-vnet0
...snip...]
Actual [-concurrent -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0
ebtables --concurrent -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0
ebtables --concurrent -t nat -L libvirt-I-vnet0
ebtables --concurrent -t nat -L libvirt-O-vnet0
...snip...]

This scrubs all reference to locking flags from test results buffer,
so that achieved output matches the expected results.

Signed-off-by: Prerna Saxena 
---
 tests/nwfilterebiptablestest.c   |  7 +
 tests/nwfilterxml2firewalltest.c |  3 ++
 tests/testutils.c| 62 
 tests/testutils.h|  6 
 4 files changed, 78 insertions(+)

diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c
index f62e046..8bf7d53 100644
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -112,6 +112,7 @@ testNWFilterEBIPTablesAllTeardown(const void *opaque 
ATTRIBUTE_UNUSED)
 goto cleanup;
 
 actual = virBufferContentAndReset(&buf);
+virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES);
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(expected, actual)) {
@@ -183,6 +184,7 @@ testNWFilterEBIPTablesTearOldRules(const void *opaque 
ATTRIBUTE_UNUSED)
 goto cleanup;
 
 actual = virBufferContentAndReset(&buf);
+virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES);
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(expected, actual)) {
@@ -232,6 +234,7 @@ testNWFilterEBIPTablesRemoveBasicRules(const void *opaque 
ATTRIBUTE_UNUSED)
 goto cleanup;
 
 actual = virBufferContentAndReset(&buf);
+virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES);
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(expected, actual)) {
@@ -266,6 +269,7 @@ testNWFilterEBIPTablesTearNewRules(const void *opaque 
ATTRIBUTE_UNUSED)
 goto cleanup;
 
 actual = virBufferContentAndReset(&buf);
+virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES);
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(expected, actual)) {
@@ -338,6 +342,7 @@ testNWFilterEBIPTablesApplyBasicRules(const void *opaque 
ATTRIBUTE_UNUSED)
 goto cleanup;
 
 actual = virBufferContentAndReset(&buf);
+virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES);
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(expected, actual)) {
@@ -428,6 +433,7 @@ testNWFilterEBIPTablesApplyDHCPOnlyRules(const void *opaque 
ATTRIBUTE_UNUSED)
 goto cleanup;
 
 actual = virBufferContentAndReset(&buf);
+virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES);
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(expected, actual)) {
@@ -501,6 +507,7 @@ testNWFilterEBIPTablesApplyDropAllRules(const void *opaque 
ATTRIBUTE_UNUSED)
 goto cleanup;
 
 actual = virBufferContentAndReset(&buf);
+virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES);
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(expected, actual)) {
diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c
index 01527f4..b8c895c 100644
--- a/tests/nwfilterxml2firewalltest.c
+++ b/tests/nwfilterxml2firewalltest.c
@@ -402,6 +402,9 @@ static int testCompareXMLToArgvFiles(const char *xml,
 goto cleanup;
 
 actualargv = virBufferContentAndReset(&buf);
+virtTestClearLockingArgs(actualargv, VIR_TEST_IPTABLES);
+virtTestClearLockingArgs(actualargv, VIR_TEST_IP6TABLES);
+virtTestClearLockingArgs(actualargv, VIR_TEST_EBTABLES);
 virtTestClearCommandPath(actualargv);
 virCommandSetDryRun(NULL, NULL, NULL);
 
diff --git a/tests/testutils.c b/tests/testutils.c
index 9a79f98..fbb41b6 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -930,6 +930,68 @@ void virtTestClearCommandPath(char *cmdset)
 cmdset[offset] = '\0';
 }
 
+/*
+ * Scrub all reference to arguments that allow iptables to be locked.
+ * This is a newer iptables change that is unavailable with older distros.
+ * For seamless comparison of test results between 'expected' &'actual' values,
+ * it makes sense to *not* compare :
+ * EXPECTED : /path/to/ebtables -ARG1 -ARG2 -ARG3
+ * vs
+ * ACTUAL : /path/to/iptables --LOCKFLAG -ARG1 -ARG2 -ARG3
+ */
+void virtTestClearLockingArgs(char *cmdset, virTestNwFilter filter)
+{
+const char *iptables_str = "iptables -w";
+const char *ip6tables_str = "ip6tables -w";
+const char *ebtables_str =

[libvirt] [PATCH 0/2] Testcase fixes for nwfilter.

[Prerna Saxena]

This series adds fixes for nwfilter testcases.

Patch 1/2 : Flips result reporting for tests/nwfilterebiptablestest.c in line 
with other tests.
Patch 2/2 : Makes nwfilter testcases more resilient to the presence of locking 
flags, introduced by Commit dc33e6e4a5a5d42

Prerna Saxena (2):
  tests : Fix failure reporting for tests/nwfilterebiptablestest.c
  Tests : Make nwfilter testcases robust against optional locking flags.

 tests/nwfilterebiptablestest.c   | 35 ++-
 tests/nwfilterxml2firewalltest.c |  3 ++
 tests/testutils.c| 62 
 tests/testutils.h|  6 
 4 files changed, 92 insertions(+), 14 deletions(-)

Regards,
-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

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

[libvirt] Build failed in Jenkins: libvirt-syntax-check #2942

[Jenkins CI]

See 

--
Started by upstream project "libvirt-build" build number 3319
Building on master in workspace 

[workspace] $ /bin/sh -xe /tmp/hudson1736521433734130077.sh
+ make syntax-check
  GENbracket-spacing-check
Invalid character after comma:
src/security/virt-aa-helper.c:574: "/sys/devices/pci",  /* for hostdev 
pci devices */
maint.mk: incorrect formatting, see HACKING for rules
make: *** [bracket-spacing-check] Error 1
Build step 'Execute shell' marked build as failure

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

Re: [libvirt] [libvirt-glib PATCHv2] Add gvir_domain_open_graphics_fd()

[Christophe Fergeau]

Hey,

On Fri, Nov 21, 2014 at 03:11:36PM +, Zeeshan Ali (Khattak) wrote:
> Add binding for virDomainOpenGraphicsFD. If virDomainOpenGraphicsFD is
> not available, it means we are dealing with older libvirt so we create
> the socket pair ourselves if that is the case.
> ---
>  configure.ac |  4 ++
>  libvirt-gobject/libvirt-gobject-domain.c | 72 
> 
>  libvirt-gobject/libvirt-gobject-domain.h |  4 ++
>  libvirt-gobject/libvirt-gobject.sym  |  5 +++
>  4 files changed, 85 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 8cc3fca..bcb5cda 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -93,6 +93,10 @@ m4_if(m4_version_compare([2.61a.100],
>  LIBVIRT_GLIB_COMPILE_WARNINGS
>  
>  PKG_CHECK_MODULES(LIBVIRT, libvirt >= $LIBVIRT_REQUIRED)
> +# virDomainOpenGraphicsFD was introduced in libvirt 1.2.8
> +AC_CHECK_LIB([virt],
> + [virDomainOpenGraphicsFD],
> + [AC_DEFINE([HAVE_VIR_DOMAIN_OPEN_GRAPHICS_FD], 1, [Have 
> virDomainOpenGraphicsFD?])])
>  enable_tests=no
>  PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_TEST_REQUIRED,
>[enable_tests=yes],
> diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
> b/libvirt-gobject/libvirt-gobject-domain.c
> index 8df30d7..d41dadd 100644
> --- a/libvirt-gobject/libvirt-gobject-domain.c
> +++ b/libvirt-gobject/libvirt-gobject-domain.c
> @@ -25,6 +25,7 @@
>  
>  #include 
>  #include 

> +#include 

This could be enclosed in

#ifndef HAVE_VIR_DOMAIN_OPEN_GRAPHICS_FD

but that's not very important

>  
>  #include "libvirt-glib/libvirt-glib.h"
>  #include "libvirt-gobject/libvirt-gobject.h"
> @@ -1222,6 +1223,77 @@ cleanup:
>  }
>  
>  /**
> + * gvir_domain_open_graphics_fd:
> + * @dom: the domain
> + * @idx: the graphics index
> + * @flags: extra flags, currently unused
> + *
> + * This will create a socket pair connected to the graphics backend of @dom. 
> One
> + * end of the socket will be returned on success, and the other end is 
> handed to
> + * the hypervisor. If @dom has multiple graphics backends configured, then 
> @idx
> + * will determine which one is opened, starting from @idx 0.
> + *
> + * Returns: An fd on success, -1 on failure.
> + *
> + * Since: 0.2.0
> + */
> +int gvir_domain_open_graphics_fd(GVirDomain *dom,
> + guint idx,
> + unsigned int flags,
> + GError **err)
> +{
> +GVirDomainPrivate *priv;
> +int ret = -1;
> +#ifndef HAVE_VIR_DOMAIN_OPEN_GRAPHICS_FD
> +int pair[2];
> +#endif
> +
> +g_return_val_if_fail(GVIR_IS_DOMAIN(dom), -1);
> +g_return_val_if_fail(err == NULL || *err == NULL, -1);
> +
> +priv = dom->priv;
> +
> +#ifdef HAVE_VIR_DOMAIN_OPEN_GRAPHICS_FD
> +ret = virDomainOpenGraphicsFD(priv->handle, idx, flags);
> +if (ret <= 0) {
> +gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +   0,
> +   "Unable to open graphics");
> +goto end;
> +}
> +#else
> +

I'd either get rid of that blank line, or add one before the #else as
well.

> +if (socketpair(PF_UNIX, SOCK_STREAM, 0, pair) < 0) {
> +gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +   0,
> +   "Failed to create socket pair");

gvir_set_error_literal should only be used to report errors from libvirt
API calls as this will virGetLastError() to generate the error message.

> +goto end;
> +}
> +
> +if (virDomainOpenGraphics(priv->handle, idx, pair[0], flags) < 0) {
> +virErrorPtr vir_err;
> +
> +vir_err = virGetLastError();
> +gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +   0,
> +   vir_err && vir_err->message ?
> +   vir_err->message :
> +   "Unable to open graphics");

You don't need to look at vir_err->message here as
gvir_set_error_literal will already append it to the end of the
GError::message.

Christophe


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

Re: [libvirt] [PATCH 0/5] Getting virt-sandbox-service to work

[Cedric Bosdonnat]

Pushed them all with changes to patch #2 as requested by Martin.

Thanks Martin for your review.

--
Cedric

On Mon, 2014-11-24 at 21:54 +0100, Cédric Bosdonnat wrote:
> Hi all,
> 
> this patch series fixes a few problems I encountered when getting 
> virt-sandbox-service 
> containers with images to work. The very first patch fixes a crasher in 
> virt-aa-helper
> tests... but we were pretty lucky it didn't explode before: virErrors weren't 
> initialized.
> 
> Cédric Bosdonnat (5):
>   virt-aa-helper wasn't running virErrorInitialize
>   virt-aa-helper: /etc/libvirt-sandbox/services isn't restricted
>   ip link needs 'name' in 3.16 to create the veth pair
>   lxc: be more patient while resolving symlinks
>   lxc: don't unmount subtree if it contains the source of the mount
> 
>  src/conf/domain_conf.h|  1 +
>  src/lxc/lxc_container.c   | 85 
> ++-
>  src/security/virt-aa-helper.c | 14 ++-
>  src/util/virnetdevveth.c  |  4 +-
>  4 files changed, 74 insertions(+), 30 deletions(-)
> 


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

Re: [libvirt] [PATCH RFC] storage: perform btrfs clone if possible

[Chen, Hanxiao]

> -Original Message-
> From: Eric Blake [mailto:ebl...@redhat.com]
> Sent: Tuesday, November 25, 2014 12:25 AM
> To: Martin Kletzander; Chen, Hanxiao/陈 晗霄
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH RFC] storage: perform btrfs clone if possible
> 
> On 11/24/2014 12:09 AM, Martin Kletzander wrote:
> > On Mon, Nov 24, 2014 at 02:11:47PM +0800, Chen Hanxiao wrote:
> >> We already had nocow flags in virStorageSource.
> >> But when creating RAW file, we don't take advantage
> >> of clone of btrfs.
> >> This file introduce btrfs_clone_file function,
> >> and try to use it when !nocow.
> >>
> >
> > I'm not sure we want to do this, but I have nothing against that
> > either.  So I'll just review the code without any other comments.
> >
> 
> >
> > As I said, I'm not commenting on whether we want this in or not, so
> > for that you should wait for someone's response.  I bet there's a
> > (good) reason behind libvirt not using some lvm/zfs/btrfs features,
> > but I am too lazy to search for it since it'd be inaccurate anyway.
> 
> I think it makes sense to expose this functionality; although I suspect
> it is better if we do so by having the user pass an explicit new flag
> value to existing API instead of doing it automatically.
> 
Thanks for your clarification.

But we've already had nocow in virStorageSource and  tags.
So I think if we do not specify  tags in XML,
we should try it according to 'nocow' in codes.

Or do we need a new flags such as --reflink
for tools like virt-clone?

Thanks,
- Chen

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

[libvirt] [PATCH] conf: replace call to virNetworkFree() with virOjectUnref()

[Laine Stump]

The function virNetworkObjListExport() in network_conf.c had a call to
the public API virNetworkFree() which was causing a link error:

CCLD libvirt_driver_vbox_network_impl.la
 ./.libs/libvirt_conf.a(libvirt_conf_la-network_conf.o): In function 
`virNetworkObjListExport':
/home/laine/devel/libvirt/src/conf/network_conf.c:4496: undefined reference to 
`virNetworkFree'

This would happen when I added

  #include "network_conf.h"

into domain_conf.c, then attempted to call a new function from that
file (and enum converter, similar to virNetworkForwardTypeToString())

In the end, virNetworkFree() ends up just calling virObjectUnref(obj)
anyway (after clearing all pending errors, which we probably *don't*
want to do in the cleanup of a utility function), so this is likely
more correct than the original code as well.
---

A quick look showed that there may be other places where we are
calling public APIs such as virNetworkFree and virDomainFree when we
really don't want to be clearning out the pending error - this would
result in the good old "an error was encountered but the cause is
unknown" type of log messages. It may warrant an audit...

 src/conf/network_conf.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 067334e..a249e32 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -4463,10 +4463,8 @@ virNetworkObjListExport(virConnectPtr conn,
 
  cleanup:
 if (tmp_nets) {
-for (i = 0; i < nnets; i++) {
-if (tmp_nets[i])
-virNetworkFree(tmp_nets[i]);
-}
+for (i = 0; i < nnets; i++)
+virObjectUnref(tmp_nets[i]);
 }
 
 VIR_FREE(tmp_nets);
-- 
1.9.3

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

Re: [libvirt] [Xen-devel] [PATCH 1/2] virdbus: don't force users to pass int for bool values

[Ian Campbell]

On Mon, 2014-11-24 at 09:21 -0700, Eric Blake wrote:
> On 11/24/2014 02:43 AM, Ian Campbell wrote:
> 
> 
>  I think this change breaks build on FreeBSD:
> 
>    CC   util/libvirt_util_la-virdbus.lo
>  util/virdbus.c:956:13: error: cast from 'bool *' to 'dbus_bool_t *' (aka 
>  'unsigned int *') increases required alignment from 1 to 4 
>  [-Werror,-Wcast-align]
>  GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
>  ^~~
>  util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
>  x = (dbustype *)(*xptrptr + (*narrayptr - 1));  \
>  ^ 1 error 
>  generated.
> >>>
> >>> Valid complaint, so I'll have to figure something out to avoid it.  :(
> 
> > I'm, guessing that this is the same underlying issue as:
> > util/virdbus.c: In function 'virDBusMessageIterDecode':
> > util/virdbus.c:956:346: error: cast increases required alignment of 
> > target type [-Werror=cast-align]
> 
> Yes.
> 
> > 
> > which we are seeing in the Xen automated tests [0, 1] (on armhf only,
> > probably compiler dependent?).
> 
> So, do I have an ACK on my proposed fix yet? :)
> https://www.redhat.com/archives/libvir-list/2014-November/msg00838.html

Well, FWIW it looks good to me:

Acked-by: Ian Campbell 

Ian.

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

Re: [libvirt] [PATCH 5/5] lxc: don't unmount subtree if it contains the source of the mount

[Martin Kletzander]

On Tue, Nov 25, 2014 at 09:27:28AM +0100, Cedric Bosdonnat wrote:

On Tue, 2014-11-25 at 08:48 +0100, Martin Kletzander wrote:

On Mon, Nov 24, 2014 at 09:54:46PM +0100, Cédric Bosdonnat wrote:
>The typical case where we had a problem is with such a filesystem
>definition as created by virt-sandbox-service:
>
>
>  
>  
>
>
>In this case, we don't want to unmount the /var subtree or we may
>loose the access to the source folder.

I probably didn't quite get this.  This is only true when host root is
the root of the container, isn't it?  And in that case it doesn't make
much sense to do this.


Indeed that happens when the host root is mounted as the container
root... and that's what virt-sandbox-service does. We have this
situation when the libvirt-sandbox service has a disk image:
 * The disk image is mounted to /var/lib/libvirt/filesystems/
 * Quite a few items from /var/lib/libvirt/filesystems/ are
   bind mounted to their equivalent in the container root, and /var is
   one of them.



OK, and the mount below is done in the namespace of the container and
since we drop CAP_SYS_ADMIN, we're safe from the guest unmounting the
filesystem.

We can't handle this in case of circular dependencies and I guess it
doesn't make sense to even check for it.  It will just fail anyway.

So ACK, thanks for the explanation.

Martin


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

Re: [libvirt] [PATCH 2/2] getstats: add block.n.source stat

[Peter Krempa]

On 11/25/14 06:21, Eric Blake wrote:
> I noticed that for an offline domain, 'virsh domstats --block $dom'
> was producing just the domain name, with no stats.  But the older
> 'virsh domblkinfo' works just fine on offline domains.  Furthermore,
> I'm about to make block stats optionally more complex to cover
> backing chains, where block.count will no longer equal the number
> of  for a domain.  For these reasons, it is nicer if the
> statistics output always includes minimal information on every
> resource being described, with enough to correlate back to host
> resources, and even when some statistics are available only on a
> running domain.
> 
> With this patch, I now see the following for an offline domain
> with one disk and an empty cdrom drive:
> $ virsh domstats --block foo
> Domain: 'foo'
>   block.count=2
>   block.0.name=hda
>   block.0.source=/dev/sda4
>   block.1.name=hdc
> 
> * src/libvirt-domain.c (virConnectGetAllDomainStats)
> (virDomainListGetStats): Document new field; clarify that not all
> fields must be present for a group to be supported.
> * tools/virsh.pod (domstats): Document new field.
> * src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Return the new
> stat always, even for offline domains.
> (QEMU_ADD_NAME_PARAM): Add parameter.
> (qemuDomainGetStatsInterface): Update caller.
> 
> Signed-off-by: Eric Blake 
> ---
>  src/libvirt-domain.c   | 16 +---
>  src/qemu/qemu_driver.c | 36 
>  tools/virsh.pod|  1 +
>  3 files changed, 34 insertions(+), 19 deletions(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 2b0defc..8cd21ae 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -10907,6 +10907,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
>   * "block..name" - name of the block device  as string.
>   *  matches the target name (vda/sda/hda) of the
>   *  block device.
> + * "block..source" - string describing the source of block device ,
> + *such as the host path of a file or device.

Fair enough as long as we document that we only return it for
non-network sources. We had quite a few problems with network sources so
I'd rather not expose it for those due to possible ambiguity.

>   * "block..rd.reqs" - number of read requests as unsigned long long.
>   * "block..rd.bytes" - number of read bytes as unsigned long long.
>   * "block..rd.times" - total time (ns) spent on reads as
> @@ -10937,7 +10939,11 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
>   *
>   * Specifying VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS as @flags makes
>   * the function return error in case some of the stat types in @stats were
> - * not recognized by the daemon.
> + * not recognized by the daemon.  However, even with this flag, a hypervisor
> + * may omit individual fields within a group if the information is not
> + * available; as an extreme example, a supported group may produce zero
> + * fields for offline domains if the statistics are meaningful only for a
> + * running domain.
>   *
>   * Similarly to virConnectListAllDomains, @flags can contain various flags to
>   * filter the list of domains to provide stats for.
> @@ -11017,9 +11023,13 @@ virConnectGetAllDomainStats(virConnectPtr conn,
>   *
>   * Specifying VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS as @flags makes
>   * the function return error in case some of the stat types in @stats were
> - * not recognized by the daemon.
> + * not recognized by the daemon.  However, even with this flag, a hypervisor
> + * may omit individual fields within a group if the information is not
> + * available; as an extreme example, a supported group may produce zero
> + * fields for offline domains if the statistics are meaningful only for a
> + * running domain.
>   *

The code changes are not entirely related to this doc change.

> - * Note that any of the domain list filtering flags in @flags will be 
> rejected
> + * Note that any of the domain list filtering flags in @flags may be rejected
>   * by this function.
>   *
>   * Returns the count of returned statistics structures on success, -1 on 
> error.
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 07da3e3..9295a05 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18408,11 +18408,11 @@ do { \
>  return -1; \
>  } while (0)
> 
> -#define QEMU_ADD_NAME_PARAM(record, maxparams, type, num, name) \
> +#define QEMU_ADD_NAME_PARAM(record, maxparams, type, subtype, num, name) \
>  do { \
>  char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
>  snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
> - "%s.%zu.name", type, num); \
> + "%s.%zu.%s", type, num, subtype); \
>  if (virTypedParamsAddString(&(record)->params, \
>  &(record)->nparams, \
>  maxparams, \
> @@ -18457

[libvirt] Waiting for review of [PATCH v2 2/21] hyperv: implementation of virConnectGetVersion

[Yves Vinter]

Hi All,

As I described the 27th of October in the following thread:
https://www.redhat.com/archives/libvir-list/2014-October/msg00840.html
[libvirt] [PATCH v2 0/21] hyperv: hyperv: set of new functionalities


new functionalities has been implemented in the libvirt hyperv driver as a set 
of 21 patches.

PATCH 01/21: hyperv: avoid query memleaks on failure
PATCH 02/21: hyperv: implementation of virConnectGetVersion
PATCH 03/21: hyperv: implementation of virConnectGetCapabilities
PATCH 04/21: hyperv: implementation of virDomainGetVcpus and 
virConnectGetMaxVcpus
PATCH 05/21: hyperv: implementation of virNodeGetFreeMemory
PATCH 06/21: hyperv: implementation of virDomainShutdown and 
virDomainShutdownFlags
PATCH 07/21: hyperv: implementation of virDomainGetSchedulerType and 
virDomainGetSchedulerParameters
PATCH 08/21: hyperv: implementation of virNetworkLookupByName
PATCH 09/21: hyperv: implementation of virNetworkGetXMLDesc
PATCH 10/21: hyperv: implementation of virConnectNumOfNetworks and 
virConnectListNetworks
PATCH 11/21: hyperv: implementation of virConnectNumOfDefinedNetworks and 
virConnectListDefinedNetworks
PATCH 12/21: hyperv: implementation of hypervInvokeMethod to handle complex 
parameters
PATCH 13/21: hyperv: implementation of virDomainSetAutostart and 
virDomainGetAutostart
PATCH 14/21: hyperv: implementation of virDomainSetMaxMemory
PATCH 15/21: hyperv: implementation of virDomainSetMemory and 
virDomainSetMemoryFlags
PATCH 16/21: hyperv: implementation of virDomainSetVcpus and 
virDomainSetVcpusFlags
PATCH 17/21: hyperv: implementation of virDomainUndefine and 
virDomainUndefineFlags
PATCH 18/21: hyperv: implementation of internal function hypervDomainAttachDisk
PATCH 19/21: hyperv: implementation of internal function 
hypervDomainAttachNetwork
PATCH 20/21: hyperv: implementation of virDomainAttachDevice and 
virDomainAttachDeviceFlags
PATCH 21/21: hyperv: implementation of virDomainDefineXML and virDomainCreateXML

A first version of this set has been submitted the 8th of October.
The first 3 patches have been already reviewed by Eric and the first patch has 
been pushed in the main stream.

Based on the remarks after this 3 reviews, I will produce a V2 for the whole 
set of patches.
It will be more convenient for me to submit V2 versions patch after patch, only 
after the previous one has been approved and pushed in the main branch.

The V2 of patch 02/21 has been submitted the 27th of October here:
https://www.redhat.com/archives/libvir-list/2014-October/msg00824.html
[libvirt] [PATCH v2 2/21] hyperv: implementation of virConnectGetVersion

I've not received any feedback since, and it has not been pushed in the main 
stream.
Is there any issue?
I'm still waiting for its approval before submitting the next patch...

Thanks for you answer,
Yves.

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

Re: [libvirt] [PATCH 2/9] util: functions to manage bridge fdb (forwarding database)

[Laine Stump]

On 11/24/2014 04:23 PM, John Ferlan wrote:
>
> On 11/24/2014 12:48 PM, Laine Stump wrote:
>> +if (nlmsg_append(nl_msg, &ndm, sizeof(ndm), NLMSG_ALIGNTO) < 0)
>> +goto buffer_too_small;
>> +if (nla_put(nl_msg, NDA_LLADDR, VIR_MAC_BUFLEN, mac) < 0)
>> +goto buffer_too_small;
> So if someone adds the same thing twice, what happens?  Does it matter?
>  IOW: Is there any need for us to check a return status here that
> indicates "duplicate entry" and ignore?  Or is there any need for a
> *FDBGet() function to determine whether what we're about to add already
> exists?

If you try to add something that's already there, or remove something
that isn't there, the kernel returns an error. We're only adding these
entries immediately after creating a tap device, so the entry will never
exist, and we actually never delete anything - it's automatically
deleted when the tap device is deleted.

In the future when we allow changing MAC address, then we'll need to
think about those things, but for now we're safe.

The "FDBGet()" type function is considerably more complicated (you
basically get a dump of the entire db into a list), and not needed for
what I'm implementing now.

>
> Similar argument of course for delete, but removing something that
> doesn't exist - what happens?
>
> Then course there's the "FDBModify()" type function.  We have something,
> but want to change a setting.

That really never happens. You add them and take them away.

>
>> +return -1;
>> +}
>> +
>> +
>> +#endif
>> +
>> +int
>> +virNetDevBridgeFDBAdd(const virMacAddr *mac, const char *ifname,
>> +  unsigned int flags)
>> +{
>> +return virNetDevBridgeFDBAddDel(mac, ifname, flags, true);
>> +}
>> +
>> +
> Thinking out loud for the future patches. The 'flags' here - must they
> match how they were added?  

I think they do matter, as it's possible to have two entries with the
same MAC address and interface name, but different flags.

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

Re: [libvirt] [PATCH 1/2] virsh: document block.n.allocation stat

[Peter Krempa]

On 11/25/14 06:21, Eric Blake wrote:
> Commit 7557ddf added some additional block.* stats to
> virDomainListGetStats, but failed to document them in 'man
> virsh'.  Also, I noticed some inconsistent use of commas.
> 
> * tools/virsh.pod (domstats): Tweak commas, add missing stats.
> 
> Signed-off-by: Eric Blake 
> ---
>  tools/virsh.pod | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 

ACK,

Peter




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

Re: [libvirt] [PATCH 3/5] ip link needs 'name' in 3.16 to create the veth pair

[Martin Kletzander]

On Tue, Nov 25, 2014 at 09:21:10AM +0100, Cedric Bosdonnat wrote:

On Tue, 2014-11-25 at 08:42 +0100, Martin Kletzander wrote:

On Mon, Nov 24, 2014 at 09:54:44PM +0100, Cédric Bosdonnat wrote:
>Due to a change (or bug?) in ip link implementation, the command
>'ip link add vnet0...'
>is forced into
>'ip link add name vnet0...'
>The changed command also works on older versions of iproute2, just the
>'name' parameter has been made mandatory.
>---
> src/util/virnetdevveth.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
>index e9d6f9c..ad30e1d 100644
>--- a/src/util/virnetdevveth.c
>+++ b/src/util/virnetdevveth.c
>@@ -89,7 +89,7 @@ static int virNetDevVethGetFreeNum(int startDev)
>  * @veth2: pointer to return name for container end of veth pair
>  *
>  * Creates a veth device pair using the ip command:
>- * ip link add veth1 type veth peer name veth2
>+ * ip link add name veth1 type veth peer name veth2
>  * If veth1 points to NULL on entry, it will be a valid interface on
>  * return.  veth2 should point to NULL on entry.
>  *
>@@ -146,7 +146,7 @@ int virNetDevVethCreate(char** veth1, char** veth2)
> }
>
> cmd = virCommandNew("ip");
>-virCommandAddArgList(cmd, "link", "add",
>+virCommandAddArgList(cmd, "link", "add", "name",
>  *veth1 ? *veth1 : veth1auto,
>  "type", "veth", "peer", "name",
>  *veth2 ? *veth2 : veth2auto,
>--
>2.1.2
>

I agree, the 'name' was always there, just optional.  But what version
of iproute2 do you have that requires it?  I checked the current HEAD
and it's still optional.  This must be a bug in that particular
implementation.

ACK if you can argue with the version or platform this is required
on.


At least the 3.16 shipped on openSUSE 13.2 has that problem... though I
think it's just a side effect of another change in iproute2. It worked
fine with version 3.12.



Must be OpenSUSE specific.  Anyway, we need to work with that as
well.  ACK then ;)

Martin


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

Re: [libvirt] [PATCH 5/5] lxc: don't unmount subtree if it contains the source of the mount

[Cedric Bosdonnat]

On Tue, 2014-11-25 at 08:48 +0100, Martin Kletzander wrote:
> On Mon, Nov 24, 2014 at 09:54:46PM +0100, Cédric Bosdonnat wrote:
> >The typical case where we had a problem is with such a filesystem
> >definition as created by virt-sandbox-service:
> >
> >
> >  
> >  
> >
> >
> >In this case, we don't want to unmount the /var subtree or we may
> >loose the access to the source folder.
> 
> I probably didn't quite get this.  This is only true when host root is
> the root of the container, isn't it?  And in that case it doesn't make
> much sense to do this.

Indeed that happens when the host root is mounted as the container
root... and that's what virt-sandbox-service does. We have this
situation when the libvirt-sandbox service has a disk image:
  * The disk image is mounted to /var/lib/libvirt/filesystems/
  * Quite a few items from /var/lib/libvirt/filesystems/ are
bind mounted to their equivalent in the container root, and /var is
one of them.

--
Cedric

> >---
> > src/lxc/lxc_container.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> >diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> >index 12f3a41..334a1df 100644
> >--- a/src/lxc/lxc_container.c
> >+++ b/src/lxc/lxc_container.c
> >@@ -1597,11 +1597,15 @@ static int lxcContainerMountAllFS(virDomainDefPtr 
> >vmDef,
> > if (STREQ(vmDef->fss[i]->dst, "/"))
> > continue;
> >
> >+VIR_DEBUG("Mounting '%s' -> '%s'", vmDef->fss[i]->src, 
> >vmDef->fss[i]->dst);
> >+
> > if (lxcContainerResolveSymlinks(vmDef->fss[i], false) < 0)
> > return -1;
> >
> >-if (lxcContainerUnmountSubtree(vmDef->fss[i]->dst,
> >-   false) < 0)
> >+
> >+if (!(vmDef->fss[i]->src &&
> >+  STRPREFIX(vmDef->fss[i]->src, vmDef->fss[i]->dst)) &&
> >+lxcContainerUnmountSubtree(vmDef->fss[i]->dst, false) < 0)
> > return -1;
> >
> > if (lxcContainerMountFS(vmDef->fss[i], sec_mount_options) < 0)
> >--
> >2.1.2
> >
> >--
> >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 3/5] ip link needs 'name' in 3.16 to create the veth pair

[Cedric Bosdonnat]

On Tue, 2014-11-25 at 08:42 +0100, Martin Kletzander wrote:
> On Mon, Nov 24, 2014 at 09:54:44PM +0100, Cédric Bosdonnat wrote:
> >Due to a change (or bug?) in ip link implementation, the command
> >'ip link add vnet0...'
> >is forced into
> >'ip link add name vnet0...'
> >The changed command also works on older versions of iproute2, just the
> >'name' parameter has been made mandatory.
> >---
> > src/util/virnetdevveth.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
> >index e9d6f9c..ad30e1d 100644
> >--- a/src/util/virnetdevveth.c
> >+++ b/src/util/virnetdevveth.c
> >@@ -89,7 +89,7 @@ static int virNetDevVethGetFreeNum(int startDev)
> >  * @veth2: pointer to return name for container end of veth pair
> >  *
> >  * Creates a veth device pair using the ip command:
> >- * ip link add veth1 type veth peer name veth2
> >+ * ip link add name veth1 type veth peer name veth2
> >  * If veth1 points to NULL on entry, it will be a valid interface on
> >  * return.  veth2 should point to NULL on entry.
> >  *
> >@@ -146,7 +146,7 @@ int virNetDevVethCreate(char** veth1, char** veth2)
> > }
> >
> > cmd = virCommandNew("ip");
> >-virCommandAddArgList(cmd, "link", "add",
> >+virCommandAddArgList(cmd, "link", "add", "name",
> >  *veth1 ? *veth1 : veth1auto,
> >  "type", "veth", "peer", "name",
> >  *veth2 ? *veth2 : veth2auto,
> >--
> >2.1.2
> >
> 
> I agree, the 'name' was always there, just optional.  But what version
> of iproute2 do you have that requires it?  I checked the current HEAD
> and it's still optional.  This must be a bug in that particular
> implementation.
> 
> ACK if you can argue with the version or platform this is required
> on.

At least the 3.16 shipped on openSUSE 13.2 has that problem... though I
think it's just a side effect of another change in iproute2. It worked
fine with version 3.12.

--
Cedric

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

Re: [libvirt] [PATCH 2/5] virt-aa-helper: /etc/libvirt-sandbox/services isn't restricted

[Cedric Bosdonnat]

On Tue, 2014-11-25 at 08:05 +0100, Martin Kletzander wrote:
> On Mon, Nov 24, 2014 at 09:54:43PM +0100, Cédric Bosdonnat wrote:
> >To get virt-sandbox-service working with AppArmor, virt-aa-helper
> >needs not to choke on path in /etc/libvirt-sandbox/services.
> >---
> > src/security/virt-aa-helper.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> >diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> >index 81f9f40..9c9e570 100644
> >--- a/src/security/virt-aa-helper.c
> >+++ b/src/security/virt-aa-helper.c
> >@@ -608,8 +608,12 @@ valid_path(const char *path, const bool readonly)
> >
> > npaths = sizeof(restricted)/sizeof(*(restricted));
> > if (array_starts_with(path, restricted, npaths) == 0 &&
> >-array_starts_with(path, override, opaths) != 0)
> >-return 1;
> >+array_starts_with(path, override, opaths) != 0) {
> >+
> >+/* We want to have virt-sandbox services config allowed */
> >+if (!STRPREFIX(path, "/etc/libvirt-sandbox/services/"))
> >+return 1;
> >+}
> >
> 
> Isn't this what override[] is there for?  It'd be way easier to read
> if it's just added there.

Oh indeed! I overlooked that one. I'll change that.

--
Cedric

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