[libvirt] [libvirt-test-API][PATCH 1/2] Add connection_getAllDomainStats test case to linux_domain.conf
--- cases/linux_domain.conf | 6 ++ 1 file changed, 6 insertions(+) diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf index a5ada35..d3d6aa5 100644 --- a/cases/linux_domain.conf +++ b/cases/linux_domain.conf @@ -34,6 +34,12 @@ domain:start guestname $defaultname +virconn:connection_getAllDomainStats +stats +state|cpu|balloon|vcpu|interface|block +flags + active|inactive|persistent|transient|running|paused|shutoff|other|backing|enforce + domain:destroy guestname $defaultname -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API][PATCH 0/2] Add connection_getAllDomainStats test case
The testing case will validate the getAllDomainStats API in class virConnect jiahu (2): Add connection_getAllDomainStats test case to linux_domain.conf Add connection_getAllDomainStats test case cases/linux_domain.conf | 6 + repos/virconn/connection_getAllDomainStats.py | 528 ++ 2 files changed, 534 insertions(+) create mode 100644 repos/virconn/connection_getAllDomainStats.py -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API][PATCH 2/2] Add connection_getAllDomainStats test case
The case will validate the getAllDomainStats API in class virConnect
---
repos/virconn/connection_getAllDomainStats.py | 528 ++
1 file changed, 528 insertions(+)
create mode 100644 repos/virconn/connection_getAllDomainStats.py
diff --git a/repos/virconn/connection_getAllDomainStats.py
b/repos/virconn/connection_getAllDomainStats.py
new file mode 100644
index 000..023564a
--- /dev/null
+++ b/repos/virconn/connection_getAllDomainStats.py
@@ -0,0 +1,528 @@
+#!/usr/bin/env python
+# test getAllDomainStats() API for libvirt
+
+import libvirt
+
+from xml.dom import minidom
+from libvirt import libvirtError
+from src import sharedmod
+from utils import utils
+
+required_params = ()
+optional_params = {'stats': '','flags': ''}
+
+ds = {"state": libvirt.VIR_DOMAIN_STATS_STATE,
+ "cpu": libvirt.VIR_DOMAIN_STATS_CPU_TOTAL,
+ "balloon": libvirt.VIR_DOMAIN_STATS_BALLOON,
+ "vcpu": libvirt.VIR_DOMAIN_STATS_VCPU,
+ "interface": libvirt.VIR_DOMAIN_STATS_INTERFACE,
+ "block": libvirt.VIR_DOMAIN_STATS_BLOCK}
+
+fg = {"active": libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE,
+ "inactive": libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE,
+ "persistent": libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT,
+ "transient": libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT,
+ "running": libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_RUNNING,
+ "paused": libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PAUSED,
+ "shutoff": libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF,
+ "other": libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER,
+ "backing": libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING,
+ "enforce": libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS}
+
+def filer_domains(logger, flags):
+"""
+ return a filtered domains set
+"""
+a = set(active_domains(logger))
+d = set(defined_domains(logger))
+if flags & libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT and \
+ flags & libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT:
+domains = a | d
+elif flags & libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT:
+domains = d
+elif flags & libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT:
+domains = a - d
+else:
+domains = a | d
+if flags & libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE and \
+ flags & libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE:
+domains &= (a | d)
+elif flags & libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE:
+domains &= a
+elif flags & libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE:
+domains &= (d - a)
+else:
+domains &= a | d
+return domains
+
+def active_domains(logger):
+"""
+ return active domains on current uri
+"""
+NUM = "ls /run/libvirt/qemu|grep \".xml\""
+status, output = utils.exec_cmd(NUM, shell=True)
+output = [item.replace(".xml","") for item in output]
+if status == 0:
+logger.debug("Got active domains: %s" % output)
+return output
+else:
+logger.debug("Got active domains: %s" % output)
+return output
+
+def defined_domains(logger):
+"""
+ return defined domains on current uri
+"""
+NUM = "ls /etc/libvirt/qemu|grep \".xml\""
+status, output = utils.exec_cmd(NUM, shell=True)
+output = [item.replace(".xml","") for item in output]
+if status == 0:
+logger.debug("Got defined domains: %s" % output)
+return output
+else:
+logger.debug("Got defined domains: %s" % output)
+return output
+
+def compare_value(logger,op1,op2):
+"""
+ compare 2 variables value
+"""
+if op1 != op2:
+logger.debug("Check %s: Fail" % op2)
+return False
+else:
+logger.debug("Check %s: Pass" % op2)
+return True
+
+def check_vcpu(logger,dom_name,dom_active,dom_eles):
+"""
+ check vcpu info of given domain
+"""
+iDOM_XML = "/etc/libvirt/qemu/" + dom_name +".xml"
+aDOM_XML = "/run/libvirt/qemu/" + dom_name +".xml"
+if dom_active:
+xml = minidom.parse(aDOM_XML)
+dom = xml.getElementsByTagName('domain')[0]
+vcpu = dom.getElementsByTagName('vcpu')[0]
+vcpu_max = int(vcpu.childNodes[0].data)
+if not vcpu.getAttribute('current'):
+vcpu_cur = vcpu_max
+else:
+vcpu_cur = int(vcpu.getAttribute('current'))
+
+logger.debug("Checking vcpu.current: %d" \
+% dom_eles.get("vcpu.current"))
+if not compare_value(logger,vcpu_cur, \
+dom_eles.get("vcpu.current")):
+return False
+logger.debug("Checking vcpu.maximum: %d" \
+% dom_eles.get("vcpu.maximum"))
+if not compare_value(logger,vcpu_max, \
+dom_eles.get("vcpu.maximum")):
+return False
+else:
+xml = minidom.parse(iDOM_XML)
+vcpu
Re: [libvirt] [PATCH 1/3] virsh: fix memtune to also accept 0 as valid value
On Wed, Mar 04, 2015 at 17:17:05 +0100, Pavel Hrdina wrote:
Commit message is too sparse.
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146539
Also I doubt that this on itself resolves this bug.
>
> Signed-off-by: Pavel Hrdina
> ---
> tools/virsh-domain.c | 62
> +---
> 1 file changed, 20 insertions(+), 42 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 55c269c..773f9f1 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -8276,7 +8276,7 @@ vshMemtuneGetSize(const vshCmd *cmd, const char *name,
> long long *value)
> if (virScaleInteger(&tmp, end, 1024, LLONG_MAX) < 0)
> return -1;
> *value = VIR_DIV_UP(tmp, 1024);
> -return 0;
> +return 1;
Now that the return values from this function actually make sense it
would be worth adding a comment what they mean.
> }
>
> static bool
> @@ -8294,6 +8294,7 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd)
> bool current = vshCommandOptBool(cmd, "current");
> bool config = vshCommandOptBool(cmd, "config");
> bool live = vshCommandOptBool(cmd, "live");
> +int rc = 0;
rc doesn't need to be initialized as the macro below sets and tests it.
>
> VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> @@ -8306,50 +8307,26 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd)
> if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> return false;
>
> -if (vshMemtuneGetSize(cmd, "hard-limit", &hard_limit) < 0 ||
> -vshMemtuneGetSize(cmd, "soft-limit", &soft_limit) < 0 ||
> -vshMemtuneGetSize(cmd, "swap-hard-limit", &swap_hard_limit) < 0 ||
> -vshMemtuneGetSize(cmd, "min-guarantee", &min_guarantee) < 0) {
> -vshError(ctl, "%s",
> - _("Unable to parse integer parameter"));
> -goto cleanup;
> -}
> +#define PARSE_MEMTUNE_PARAM(NAME, VALUE, FIELD) \
> +if ((rc = vshMemtuneGetSize(cmd, NAME, &VALUE)) < 0) { \
> +vshError(ctl, "%s", _("Unable to parse integer parameter 'NAME'")); \
> +goto cleanup; \
> +} \
> +if (rc == 1) { \
> +if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, \
> +FIELD, VALUE) < 0) \
> +goto save_error;\
> +} \
>
> -if (hard_limit) {
> -if (hard_limit == -1)
> -hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> -if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> -VIR_DOMAIN_MEMORY_HARD_LIMIT,
> -hard_limit) < 0)
> -goto save_error;
> -}
>
> -if (soft_limit) {
> -if (soft_limit == -1)
> -soft_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> -if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> -VIR_DOMAIN_MEMORY_SOFT_LIMIT,
> -soft_limit) < 0)
> -goto save_error;
> -}
> -
> -if (swap_hard_limit) {
> -if (swap_hard_limit == -1)
> -swap_hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> -if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> -VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT,
> -swap_hard_limit) < 0)
> -goto save_error;
> -}
> +PARSE_MEMTUNE_PARAM("hard-limit", hard_limit,
> VIR_DOMAIN_MEMORY_HARD_LIMIT);
> +PARSE_MEMTUNE_PARAM("soft-limit", soft_limit,
> VIR_DOMAIN_MEMORY_SOFT_LIMIT);
> +PARSE_MEMTUNE_PARAM("swap-hard-limit", swap_hard_limit,
> +VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT);
> +PARSE_MEMTUNE_PARAM("min-guarantee", min_guarantee,
> +VIR_DOMAIN_MEMORY_MIN_GUARANTEE);
>
> -if (min_guarantee) {
> -if (min_guarantee == -1)
> -min_guarantee = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> -if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> -VIR_DOMAIN_MEMORY_MIN_GUARANTEE,
> -min_guarantee) < 0)
> -goto save_error;
> -}
> +#undef PARSE_MEMTUNE_PARAM
Um ...
>
> if (nparams == 0) {
> /* get the number of memory parameters */
> @@ -8390,6 +8367,7 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd)
> ret = true;
>
> cleanup:
> +#undef PARSE_MEMTUNE_PARAM
... why are you undefing it twice?
> virTypedParamsFree(params, nparams);
> virDomainFree(d
Re: [libvirt] [PATCH 2/3] virutil: introduce helper to set memory limits
On Wed, Mar 04, 2015 at 17:17:06 +0100, Pavel Hrdina wrote: > Using this macro will ensure that the value stored in domain def will > never be greater than VIR_DOMAIN_MEMORY_PARAM_UNLIMITED. > > Signed-off-by: Pavel Hrdina > --- > src/util/virutil.h | 4 > 1 file changed, 4 insertions(+) > > diff --git a/src/util/virutil.h b/src/util/virutil.h > index d1173c1..003eee6 100644 > --- a/src/util/virutil.h > +++ b/src/util/virutil.h > @@ -98,6 +98,10 @@ const char *virEnumToString(const char *const*types, > const char *name ## TypeToString(int type); \ > int name ## TypeFromString(const char*type); > > +# define VIR_SET_MEMORY_LIMIT(limit, value) \ > +limit = (value) < VIR_DOMAIN_MEMORY_PARAM_UNLIMITED ? (value) : \ > +VIR_DOMAIN_MEMORY_PARAM_UNLIMITED You should document that @value cannot have side effects as it's evaluated twice. Also why don't you make an inline function that returns the truncated value? Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] memtune: change the way how we store unlimited value
On Wed, Mar 04, 2015 at 17:17:07 +0100, Pavel Hrdina wrote:
> There was a mess in the way how we store unlimited value for memory
> limits and how we handled values provided by user. Internally there
> were two possible ways how to store unlimited value: as 0 value or as
> VIR_DOMAIN_MEMORY_PARAM_UNLIMITED. Because we chose to store memory
> limits as unsigned long long, we cannot use -1 to represent unlimited.
> It's much easier for us to say that everything greater than
> VIR_DOMAIN_MEMORY_PARAM_UNLIMITED means unlimited and leave 0 as valid
> value despite that it makes no sense to set limit to 0.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146539
>
> Signed-off-by: Pavel Hrdina
> ---
> docs/formatdomain.html.in | 4 +-
> src/conf/domain_conf.c | 75
> +-
> src/libvirt-domain.c | 3 +
> src/lxc/lxc_cgroup.c | 18 +++---
> src/lxc/lxc_driver.c | 3 -
> src/lxc/lxc_fuse.c | 12 ++--
> src/lxc/lxc_native.c | 6 +-
> src/openvz/openvz_conf.c | 4 +-
> src/openvz/openvz_driver.c | 4 +-
> src/qemu/qemu_cgroup.c | 24 +++
> src/qemu/qemu_command.c| 8 ++-
> src/qemu/qemu_driver.c | 6 +-
> src/qemu/qemu_hotplug.c| 2 +-
> src/qemu/qemu_migration.c | 5 +-
> src/util/virutil.c | 8 +--
> tests/qemuxml2argvdata/qemuxml2argv-memtune.xml| 2 +-
> .../qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml | 2 +-
> 17 files changed, 116 insertions(+), 70 deletions(-)
>
...
>
> +/**
> + * virDomainParseMemoryLimit:
> + *
> + * @xpath: XPath to memory amount
> + * @units_xpath: XPath to units attribute
> + * @ctxt: XPath context
> + * @mem: scaled memory amount is stored here
> + *
> + * Parse a memory element or attribute located at @xpath within @ctxt, and
> + * store the result into @mem, in blocks of 1024. The value is scaled by
> + * units located at @units_xpath (or the 'unit' attribute under @xpath if
> + * @units_xpath is NULL). If units are not present, he default scale of 1024
> + * is used. The value must not exceed VIR_DOMAIN_MEMORY_PARAM_UNLIMITED
> + * once scaled.
> + *
> + * This helper should be used only on *_limit memory elements.
> + *
> + * Return 0 on success, -1 on failure after issuing error.
> + */
> +static int
> +virDomainParseMemoryLimit(const char *xpath,
> + const char *units_xpath,
> + xmlXPathContextPtr ctxt,
> + unsigned long long *mem)
> +{
> +int ret;
> +unsigned long long bytes;
> +
> +ret = virDomainParseScaledValue(xpath, units_xpath, ctxt, &bytes, 1024,
> +VIR_DOMAIN_MEMORY_PARAM_UNLIMITED << 10,
> +false);
> +
> +if (ret < 0)
> +return -1;
> +
> +if (ret == 0)
> +*mem = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> +else
> +VIR_SET_MEMORY_LIMIT(*mem, VIR_DIV_UP(bytes, 1024));
As said in previous patch, this looks very opaque. Having it as
*mem = virMemoryTruncate(VIR_DIV_UP(bytes, 1024);
would be better. (the name is subject to change).
> +
> +return 0;
> +}
> +
> +
>
> -if (def->mem.hard_limit &&
> -virCgroupSetMemoryHardLimit(cgroup, def->mem.hard_limit) < 0)
> -goto cleanup;
> +if (def->mem.hard_limit < VIR_DOMAIN_MEMORY_PARAM_UNLIMITED)
Perhaps adding a function virMemoryLimitIsSet(def->mem.hard_limit) would
also make things more clear too.
> +if (virCgroupSetMemoryHardLimit(cgroup, def->mem.hard_limit) < 0)
> +goto cleanup;
>
> -if (def->mem.soft_limit &&
> -virCgroupSetMemorySoftLimit(cgroup, def->mem.soft_limit) < 0)
> -goto cleanup;
> +if (def->mem.soft_limit < VIR_DOMAIN_MEMORY_PARAM_UNLIMITED)
> +if (virCgroupSetMemorySoftLimit(cgroup, def->mem.soft_limit) < 0)
> +goto cleanup;
>
> -if (def->mem.swap_hard_limit &&
> -virCgroupSetMemSwapHardLimit(cgroup, def->mem.swap_hard_limit) < 0)
> -goto cleanup;
> +if (def->mem.swap_hard_limit < VIR_DOMAIN_MEMORY_PARAM_UNLIMITED)
> +if (virCgroupSetMemSwapHardLimit(cgroup, def->mem.swap_hard_limit) <
> 0)
> +goto cleanup;
>
> ret = 0;
> cleanup:
...
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 4a95292..02ad626 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -2367,8 +2367,7 @@ virFindFCHostCapableVport(const char *sysfs_prefix
> ATTRIBUTE_UNUSED)
> /**
> * virCompareLimitUlong:
> *
> - * Compare two unsigned long long numbers. Value '0' of the arguments h
Re: [libvirt] [PATCH v1 09/31] parallels: s/virNetworkObjList/virNetworkObjListPtr/
26.02.2015 18:56, Peter Krempa пишет:
On Thu, Feb 26, 2015 at 15:17:18 +0100, Michal Privoznik wrote:
In order to hide the object internals (and use just accessors
everywhere), lets store a pointer to the object, instead of object
itself.
Signed-off-by: Michal Privoznik
---
src/parallels/parallels_driver.c | 5 +++-
src/parallels/parallels_network.c | 60 +++
src/parallels/parallels_utils.h | 2 +-
3 files changed, 35 insertions(+), 32 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index c9338b5..32f2ede 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -207,7 +207,8 @@ parallelsOpenDefault(virConnectPtr conn)
NULL, NULL)))
goto error;
-if (!(privconn->domains = virDomainObjListNew()))
+if (!(privconn->domains = virDomainObjListNew()) ||
+VIR_ALLOC(privconn->networks) < 0)
This is a bit confusing. The network object is allocated in the VM
driver open function ...
goto error;
if (!(privconn->domainEventState = virObjectEventStateNew()))
@@ -225,6 +226,7 @@ parallelsOpenDefault(virConnectPtr conn)
error:
virObjectUnref(privconn->domains);
+VIR_FREE(privconn->networks);
virObjectUnref(privconn->caps);
virStoragePoolObjListFree(&privconn->pools);
virObjectEventStateFree(privconn->domainEventState);
@@ -283,6 +285,7 @@ parallelsConnectClose(virConnectPtr conn)
virObjectUnref(privconn->caps);
virObjectUnref(privconn->xmlopt);
virObjectUnref(privconn->domains);
+VIR_FREE(privconn->networks);
And deleted in the VM driver ...
virObjectEventStateFree(privconn->domainEventState);
prlsdkDisconnect(privconn);
conn->privateData = NULL;
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index 960bd50..bfa7432 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -226,7 +226,7 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
goto cleanup;
}
-if (!(net = virNetworkAssignDef(&privconn->networks, def, false)))
+if (!(net = virNetworkAssignDef(privconn->networks, def, false)))
goto cleanup;
net->active = 1;
net->autostart = 1;
@@ -259,7 +259,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
}
def->uuid_specified = 1;
-if (!(net = virNetworkAssignDef(&privconn->networks, def, false))) {
+if (!(net = virNetworkAssignDef(privconn->networks, def, false))) {
virNetworkDefFree(def);
goto cleanup;
}
@@ -337,7 +337,7 @@ int parallelsNetworkClose(virConnectPtr conn)
{
parallelsConnPtr privconn = conn->privateData;
parallelsDriverLock(privconn);
-virNetworkObjListFree(&privconn->networks);
+virNetworkObjListFree(privconn->networks);
But is cleared in the network subdriver clenup function.
parallelsDriverUnlock(privconn);
return 0;
}
I think it should be put into parallelsNetworkOpen.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
I think Peter's notes are reasonable. It is worth moving
allocation/freeing of privconn->networks to parallelsNetworkOpen.
Maxim Nestratov
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-tck][PATCH] Add two APIs testing for metadata
$dom->set_metadata
$dom->get_metadata
only have title and destription
element not support in libvirt now
Signed-off-by: Zhe Peng
---
scripts/domain/500-metadata.t | 72 +++
1 file changed, 72 insertions(+)
create mode 100644 scripts/domain/500-metadata.t
diff --git a/scripts/domain/500-metadata.t b/scripts/domain/500-metadata.t
new file mode 100644
index 000..11f2f6d
--- /dev/null
+++ b/scripts/domain/500-metadata.t
@@ -0,0 +1,72 @@
+# -*- perl -*-
+#
+# Copyright (C) 2014 Red Hat, Inc.
+# Copyright (C) 2014 Zhe Peng
+#
+# This program is free software; You can redistribute it and/or modify
+# it under the GNU General Public License as published by the Free
+# Software Foundation; either version 2, or (at your option) any
+# later version
+#
+# The file "LICENSE" distributed along with this file provides full
+# details of the terms and conditions
+#
+
+=pod
+
+=head1 NAME
+
+domain/500-metadata.t -- set/get metadata from guest.
+
+=head1 DESCRIPTION
+
+The test case validates that libvirt can set/get guest metadata
+Sys::Virt::Domain::METADATA_TITLE
+Sys::Virt::Domain::METADATA_DESCRIPTION
+not support Sys::Virt::Domain::METADATA_ELEMENT
+
+=cut
+
+use strict;
+use warnings;
+
+use Test::More tests => 8;
+
+use Sys::Virt::TCK;
+use Test::Exception;
+
+my $tck = Sys::Virt::TCK->new();
+my $conn = eval { $tck->setup(); };
+BAIL_OUT "failed to setup test harness: $@" if $@;
+END {
+$tck->cleanup if $tck;
+}
+
+my $xml = $tck->generic_domain(name => "tck")->as_xml;
+
+diag "Creating a new transient domain";
+my $dom;
+ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain
object");
+
+my $title = "libvirt tck testing title";
+my $des = "perl-Sys-Virt description";
+
+lives_ok(sub {$dom->set_metadata(Sys::Virt::Domain::METADATA_TITLE, $title,
undef, undef, 0)}, "Set title to $title" );
+lives_ok(sub {$dom->set_metadata(Sys::Virt::Domain::METADATA_DESCRIPTION,
$des, undef, undef, 0)}, "Set description to $des" );
+
+my $hasMetadata = $dom->get_xml_description;
+
+ok($hasMetadata =~ m|$title|, "title has added in guest");
+ok($hasMetadata =~ m|$des|, "description has added in guest");
+
+my $mTitle = $dom->get_metadata(Sys::Virt::Domain::METADATA_TITLE,undef,0);
+my $mDes = $dom->get_metadata(Sys::Virt::Domain::METADATA_DESCRIPTION,undef,0);
+
+is($mTitle, $title, "Get title from guest");
+is($mDes, $des, "Get description from guest");
+
+diag "Destroy domain";
+$dom->destroy;
+
+ok_error(sub { $conn->get_domain_by_name("tck") }, "NO_DOMAIN error raised
from missing domain", 42);
+
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-tck][PATCH] This testcase tests if the open_graphics_fd function works properly.
Signed-off-by: Hao Liu
---
scripts/domain/275-open-graphics-fd.t | 76 +++
1 file changed, 76 insertions(+)
create mode 100644 scripts/domain/275-open-graphics-fd.t
diff --git a/scripts/domain/275-open-graphics-fd.t
b/scripts/domain/275-open-graphics-fd.t
new file mode 100644
index 000..9377f50
--- /dev/null
+++ b/scripts/domain/275-open-graphics-fd.t
@@ -0,0 +1,76 @@
+# -*- perl -*-
+#
+# Copyright (C) 2012-2014 Red Hat, Inc.
+# Copyright (C) 2014 Hao Liu
+#
+# This program is free software; You can redistribute it and/or modify
+# it under the GNU General Public License as published by the Free
+# Software Foundation; either version 2, or (at your option) any
+# later version
+#
+# The file "LICENSE" distributed along with this file provides full
+# details of the terms and conditions
+#
+
+=pod
+
+=head1 NAME
+
+domain/275-open-graphics-fd.t - Test open_graphics_fd function
+
+=head1 DESCRIPTION
+
+The test case validates that open_graphics_fd function works well
+
+=cut
+
+use strict;
+use warnings;
+
+use Test::More tests => 9;
+
+use Sys::Virt::TCK;
+use Test::Exception;
+use IO::Handle;
+
+my $tck = Sys::Virt::TCK->new();
+my $conn = eval { $tck->setup(); };
+BAIL_OUT "failed to setup test harness: $@" if $@;
+END {
+$tck->cleanup if $tck;
+}
+
+
+# looking up domain
+my $dom_name ="tck";
+my $xml = $tck->generic_domain(name => $dom_name)->as_xml;
+diag "domain xml is $xml";
+my $dom = $conn->define_domain($xml);
+$dom->create;
+ok($dom->get_id() > 0, "running domain has an ID > 0");
+my $graphics_cnt = xpath($dom, "count(/domain/devices/graphics)")->value();
+my $valid_idx = $graphics_cnt - 1;
+my $invalid_idx = $graphics_cnt;
+
+my $fd = 0;
+my $fh = IO::Handle->new();
+
+diag "open fd for valid graphics device";
+lives_ok(sub {$fd = $dom->open_graphics_fd($valid_idx, 0)}, "open graphics
fd");
+ok($fd > 0, "graphic fd is $fd");
+$fh->fdopen($fd, "r");
+ok($fh->opened, "fd is accessible");
+$fh->close();
+
+diag "open fd for valid graphics device skip auth";
+$fd = 0;
+lives_ok(sub {$fd = $dom->open_graphics_fd($valid_idx,
Sys::Virt::Domain::OPEN_GRAPHICS_SKIPAUTH)}, "open graphics fd");
+ok($fd > 0, "graphic fd is $fd");
+$fh->fdopen($fd, "r");
+ok($fh->opened, "fd is accessible");
+$fh->close();
+
+diag "open fd for non-exist graphics device";
+$fd = 0;
+ok_error(sub {$fd = $dom->open_graphics_fd($invalid_idx, 0)}, "open graphics
fd");
+ok($fd eq 0, "graphic fd is not returned");
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-tck][PATCH] Fix disk interface hot plug unplug test scripts
This patch fixed following issues:
1) Hot unplug won't work for a VM without OS. Create a working VM
instead.
2) Avoid using multicast MAC address.
Signed-off-by: Zhe Peng
---
scripts/domain/200-disk-hotplug.t | 5 +++--
scripts/domain/205-disk-hotplug-ordering.t | 3 ++-
scripts/domain/210-nic-hotplug.t | 5 +++--
scripts/domain/215-nic-hotplug-many.t | 9 +
4 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/scripts/domain/200-disk-hotplug.t
b/scripts/domain/200-disk-hotplug.t
index 4c54b6b..7ed5b27 100644
--- a/scripts/domain/200-disk-hotplug.t
+++ b/scripts/domain/200-disk-hotplug.t
@@ -41,13 +41,14 @@ END {
}
-my $xml = $tck->generic_domain(name => "tck")->as_xml;
+my $xml = $tck->generic_domain(name => "tck", fullos => 1)->as_xml;
diag "Creating a new transient domain";
my $dom;
ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain
object");
+sleep(30);
my $path = $tck->create_sparse_disk("200-disk-hotplug", "extra.img", 100);
my $dev = "vdb";
@@ -76,4 +77,4 @@ lives_ok(sub { $dom->detach_device($diskxml); }, "disk has
been detached");
my $finalxml = $dom->get_xml_description;
-is($initialxml, $finalxml, "final XML has removed the disk")
+is($finalxml, $initialxml, "final XML has removed the disk")
diff --git a/scripts/domain/205-disk-hotplug-ordering.t
b/scripts/domain/205-disk-hotplug-ordering.t
index bc4990f..c9a300c 100644
--- a/scripts/domain/205-disk-hotplug-ordering.t
+++ b/scripts/domain/205-disk-hotplug-ordering.t
@@ -41,12 +41,13 @@ END {
}
-my $xml = $tck->generic_domain(name => "tck")->as_xml;
+my $xml = $tck->generic_domain(name => "tck", fullos => 1)->as_xml;
diag "Creating a new transient domain";
my $dom;
ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain
object");
+sleep(30);
my $supported = 1;
foreach my $dev (qw/vdb sdb/) {
my $path = $tck->create_sparse_disk("200-disk-hotplug", "extra-$dev.img",
100);
diff --git a/scripts/domain/210-nic-hotplug.t b/scripts/domain/210-nic-hotplug.t
index ac9048e..4a2763f 100644
--- a/scripts/domain/210-nic-hotplug.t
+++ b/scripts/domain/210-nic-hotplug.t
@@ -41,11 +41,12 @@ END {
}
-my $xml = $tck->generic_domain(name => "tck")->as_xml;
+my $xml = $tck->generic_domain(name => "tck", fullos => 1)->as_xml;
diag "Creating a new transient domain";
my $dom;
ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain
object");
+sleep(30);
my $mac = "00:11:22:33:44:55";
my $model = "virtio";
@@ -72,4 +73,4 @@ lives_ok(sub { $dom->detach_device($netxml); }, "interface
has been detached");
my $finalxml = $dom->get_xml_description;
-is($initialxml, $finalxml, "final XML has removed the disk")
+is($finalxml, $initialxml, "final XML has removed the interface")
diff --git a/scripts/domain/215-nic-hotplug-many.t
b/scripts/domain/215-nic-hotplug-many.t
index 0270054..d4fa23e 100644
--- a/scripts/domain/215-nic-hotplug-many.t
+++ b/scripts/domain/215-nic-hotplug-many.t
@@ -41,15 +41,16 @@ END {
}
-my $xml = $tck->generic_domain(name => "tck")->as_xml;
+my $xml = $tck->generic_domain(name => "tck", fullos => 1)->as_xml;
diag "Creating a new transient domain";
my $dom;
ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain
object");
+sleep(30);
-my $mac1 = "01:11:22:33:44:55";
-my $mac2 = "02:11:22:33:44:55";
-my $mac3 = "03:11:22:33:44:55";
+my $mac1 = "02:11:22:33:44:55";
+my $mac2 = "04:11:22:33:44:55";
+my $mac3 = "06:11:22:33:44:55";
my $model = "virtio";
my $netxml1 =
[libvirt] [libvirt-tck][PATCH] Add new api get_all_domain_stats testing
$vmm->get_all_domain_stats
Sys::Virt::GET_ALL_STATS_ACTIVE
Sys::Virt::GET_ALL_STATS_INACTIVE
Sys::Virt::GET_ALL_STATS_OTHER
Sys::Virt::GET_ALL_STATS_PAUSED
Sys::Virt::GET_ALL_STATS_PERSISTENT
Sys::Virt::GET_ALL_STATS_RUNNING
Sys::Virt::GET_ALL_STATS_SHUTOFF
Sys::Virt::GET_ALL_STATS_TRANSIENT
Sys::Virt::GET_ALL_STATS_ENFORCE_STATS
Signed-off-by: Zhe Peng
---
scripts/domain/800-get-all-stats.t | 137 +
1 file changed, 137 insertions(+)
create mode 100644 scripts/domain/800-get-all-stats.t
diff --git a/scripts/domain/800-get-all-stats.t
b/scripts/domain/800-get-all-stats.t
new file mode 100644
index 000..4ff4bf3
--- /dev/null
+++ b/scripts/domain/800-get-all-stats.t
@@ -0,0 +1,137 @@
+# -*- perl -*-
+#
+# Copyright (C) 2014 Red Hat, Inc.
+# Copyright (C) 2014 Zhe Peng
+#
+# This program is free software; You can redistribute it and/or modify
+# it under the GNU General Public License as published by the Free
+# Software Foundation; either version 2, or (at your option) any
+# later version
+#
+# The file "LICENSE" distributed along with this file provides full
+# details of the terms and conditions
+#
+
+=pod
+
+=head1 NAME
+
+domain/800-get-all-stats.t
+
+=head1 DESCRIPTION
+
+The test case validates that:
+$vmm->get_all_domain_stats
+Sys::Virt::GET_ALL_STATS_ACTIVE
+Sys::Virt::GET_ALL_STATS_INACTIVE
+Sys::Virt::GET_ALL_STATS_OTHER
+Sys::Virt::GET_ALL_STATS_PAUSED
+Sys::Virt::GET_ALL_STATS_PERSISTENT
+Sys::Virt::GET_ALL_STATS_RUNNING
+Sys::Virt::GET_ALL_STATS_SHUTOFF
+Sys::Virt::GET_ALL_STATS_TRANSIENT
+Sys::Virt::GET_ALL_STATS_ENFORCE_STATS
+
+=cut
+
+use strict;
+use warnings;
+
+use Test::More tests => 11;
+
+use Sys::Virt::TCK;
+use Test::Exception;
+
+my $tck = Sys::Virt::TCK->new();
+my $conn = eval { $tck->setup(); };
+BAIL_OUT "failed to setup test harness: $@" if $@;
+END {
+$tck->cleanup if $tck;
+}
+
+my @doms = ("tck1", "tck2", "tck3", "tck4");
+
+#create two persistent domain, tck1 and tck2
+
+my $xml = $tck->generic_domain(name => "tck1")->as_xml;
+
+diag "Defining an inactive domain config";
+
+my $dom1;
+
+ok_domain(sub { $dom1 = $conn->define_domain($xml) }, "defined persistent
domain config");
+
+my $xml2 = $tck->generic_domain(name => "tck2")->as_xml;
+
+diag "Defining an inactive domain config";
+
+my $dom2;
+
+ok_domain(sub { $dom2 = $conn->define_domain($xml2) }, "defined persistent
domain config");
+
+my $stats_persistent =
Sys::Virt::Domain::GET_ALL_STATS_PERSISTENT|Sys::Virt::Domain::GET_ALL_STATS_INACTIVE;
+my @dom_persistent =
$conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE, undef,
$stats_persistent);
+
+is($#dom_persistent+1, 2, "list all persistent domain");
+
+#create two transient domain, tck3 and tck4
+
+my $xml3 = $tck->generic_domain(name => "tck3")->as_xml;
+
+diag "Creating a new transient domain";
+my $dom3;
+ok_domain(sub { $dom3 = $conn->create_domain($xml3) }, "created transient
domain object");
+
+my $xml4 = $tck->generic_domain(name => "tck4")->as_xml;
+
+diag "Creating a new transient domain";
+my $dom4;
+ok_domain(sub { $dom4 = $conn->create_domain($xml4) }, "created transient
domain object");
+
+my $stats_transient =
Sys::Virt::Domain::GET_ALL_STATS_TRANSIENT|Sys::Virt::Domain::GET_ALL_STATS_RUNNING;
+
+my @dom_transient =
$conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE, undef,
$stats_transient);
+
+is($#dom_transient+1, 2, "list all transient domain");
+
+#start & pause persistent domain 2 and one transient dom
+$dom2->create();
+$dom2->suspend();
+$dom3->suspend();
+
+my $stats_pause = Sys::Virt::Domain::GET_ALL_STATS_PAUSED;
+
+my @dom_pause = $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE,
undef, $stats_pause);
+
+is($#dom_pause+1, 2, "list all pause domain");
+
+#resume dom2 and dom3
+$dom2->resume();
+$dom3->resume();
+
+my $stats_active = Sys::Virt::Domain::GET_ALL_STATS_ACTIVE;
+
+my@dom_active = $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE,
undef, $stats_active);
+
+is($#dom_active+1, 3, "list all active domain");
+
+my $stats_shutoff = Sys::Virt::Domain::GET_ALL_STATS_SHUTOFF;
+
+my @dom_shutoff = $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE,
undef, $stats_shutoff);
+
+is($#dom_shutoff+1, 1, "list all shutoff domain");
+
+my $stats_other = Sys::Virt::Domain::GET_ALL_STATS_OTHER;
+
+my @dom_other = $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE,
undef, $stats_other);
+
+is($#dom_other+1, 0, "list all other domain");
+
+my $stats_enforce = Sys::Virt::Domain::GET_ALL_STATS_ENFORCE_STATS;
+
+my @dom_enforce = $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE,
undef, $stats_enforce);
+
+is($#dom_enforce+1, 4, "list all enforce domain");
+
+
+
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] SRIOV NIC offload feature discovery
On Mon, Feb 23, 2015 at 03:38:29PM +, James Chapman wrote:
> Adding functionality to libvirt that will allow it
> query the ethtool interface for the availability
> of certain NIC HW offload features
>
> Here is an example of the feature XML definition:
>
>
> net_eth4_90_e2_ba_5e_a5_45
> /sys/devices/pci:00/:00:03.0/:08:00.1/net/eth4
> pci__08_00_1
>
> eth4
> 90:e2:ba:5e:a5:45
>
>
>
>
>
>
>
>
>
>
>
>
>
> ---
> docs/formatnode.html.in | 18 +++
> docs/schemas/nodedev.rng | 14 +++
> src/conf/device_conf.h| 6 +
> src/conf/node_device_conf.c | 45 ++-
> src/conf/node_device_conf.h | 2 +
> src/libvirt_private.syms | 1 +
> src/node_device/node_device_udev.c| 4 +
> src/util/virnetdev.c | 143
> ++
> src/util/virnetdev.h | 7 ++
> tests/nodedevschemadata/net_00_13_02_b9_f9_d3.xml | 9 ++
> tests/nodedevschemadata/net_00_15_58_2f_e9_55.xml | 9 ++
> 11 files changed, 257 insertions(+), 1 deletion(-)
>
> diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
> index 7256cdc..091f2f0 100644
> --- a/src/conf/device_conf.h
> +++ b/src/conf/device_conf.h
> @@ -62,6 +62,12 @@ struct _virInterfaceLink {
> unsigned int speed; /* link speed in Mbits per second */
> };
>
> +typedef struct _virDevFeature virDevFeature;
> +typedef virDevFeature *virDevFeaturePtr;
> +struct _virDevFeature {
> + char *name; /* device feature */
> +};
> +
This would be much nicer stored in a bitmap.
I added an enum type for all the features, fixed the nits below and
pushed the patch.
> +/**
> + * virNetDevGetFeatures:
> + * This function gets the nic offloads features available for ifname
> + *
> + * @ifname: name of the interface
> + * @features: network device feature structures
> + * @nfeatures: number of features available
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +int
> +virNetDevGetFeatures(const char *ifname,
> + virDevFeaturePtr *features,
> + size_t *nfeatures)
> +{
> +int ret = -1;
> +size_t i = -1;
> +size_t j = -1;
> +struct ethtool_value cmd;
> +virDevFeaturePtr tmpfeature = NULL;
> +
> +struct elem{
> +const char *name;
> +const int cmd;
> +};
> +/* legacy ethtool getters */
> +struct elem cmds[] = {
> +{"rx", ETHTOOL_GRXCSUM},
> +{"tx", ETHTOOL_GTXCSUM },
> +{"sg", ETHTOOL_GSG},
> +{"tso",ETHTOOL_GTSO},
> +{"gso",ETHTOOL_GGSO},
> +{"gro",ETHTOOL_GGRO},
> +};
> +/* ethtool masks */
> +struct elem flags[] = {
> +{"lro",ETH_FLAG_LRO},
> +{"rxvlan", ETH_FLAG_RXVLAN},
> +{"txvlan", ETH_FLAG_TXVLAN},
> +{"ntuple", ETH_FLAG_NTUPLE},
> +{"rxhash", ETH_FLAG_RXHASH},
> +};
> +
> +for (i = 0; i < ARRAY_CARDINALITY(cmds); i++) {
> +cmd.cmd = cmds[i].cmd;
> +if (virNetDevFeatureAvailable(ifname, &cmd)) {
> +if (VIR_ALLOC(tmpfeature) < 0)
> +goto cleanup;
> +if ((ret = VIR_STRDUP(tmpfeature->name, cmds[i].name)) != 1)
> +goto cleanup;
> +if (VIR_APPEND_ELEMENT(*features, *nfeatures, *tmpfeature) < 0)
> +goto cleanup;
> +}
> +}
> +
> +cmd.cmd = ETHTOOL_GFLAGS;
> +for (j = 0; j < ARRAY_CARDINALITY(flags); j++) {
> +if (virNetDevFeatureAvailable(ifname, &cmd)) {
These two lines can be exchanged to reduce number of ioctl calls.
> +if (cmd.data & (flags[j].cmd)) {
> +if (VIR_ALLOC(tmpfeature) < 0)
> +goto cleanup;
> +if ((ret = VIR_STRDUP(tmpfeature->name, flags[j].name)) != 1)
> +goto cleanup;
> +if (VIR_APPEND_ELEMENT(*features, *nfeatures, *tmpfeature) <
> 0)
> +goto cleanup;
> +}
> +}
> +}
> +
> +ret = 0;
> diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
> index de8b480..22ef1a2 100644
> --- a/src/util/virnetdev.h
> +++ b/src/util/virnetdev.h
> @@ -31,6 +31,8 @@
> # include "virpci.h"
> # include "device_conf.h"
>
> +# include
> +typedef struct ethtool_cmd virEthCmd;
Unused type.
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] spec: Enable RBD storage driver in RHEL-7
Use correct package names too as they differ.
---
libvirt.spec.in | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 12bd17c..d85bd4e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -108,7 +108,7 @@
%define with_storage_iscsi0%{!?_without_storage_iscsi:%{server_drivers}}
%define with_storage_disk 0%{!?_without_storage_disk:%{server_drivers}}
%define with_storage_mpath0%{!?_without_storage_mpath:%{server_drivers}}
-%if 0%{?fedora} >= 16
+%if 0%{?fedora} >= 16 || 0%{?rhel} >= 7
%define with_storage_rbd 0%{!?_without_storage_rbd:%{server_drivers}}
%else
%define with_storage_rbd 0
@@ -566,7 +566,12 @@ BuildRequires: device-mapper-devel
%endif
%endif
%if %{with_storage_rbd}
+%if %if 0%{?rhel} >= 7
+BuildRequires: librados2-devel
+BuildRequires: librbd1-devel
+%else
BuildRequires: ceph-devel
+%endif
%endif
%if %{with_storage_gluster}
%if 0%{?rhel} >= 6
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Allow spaces in disk serial
On Tue, Feb 24, 2015 at 04:34:25PM +0100, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1195660 > > There's been a bug report appearing on the qemu-devel list, that > libvirt is unable to pass spaces in disk serial number [1]. Not only > our RNG schema forbids that, the code is not prepared either. However, > with a bit of escaping (if needed) we can allow spaces there. > > 1: https://lists.gnu.org/archive/html/qemu-devel/2015-02/msg04041.html > > Signed-off-by: Michal Privoznik > --- > > diff to v1: > - Switch from virBufferEscapeShell to virBufferEscape > > docs/schemas/domaincommon.rng | 2 +- > src/qemu/qemu_command.c| 5 ++-- > .../qemuxml2argvdata/qemuxml2argv-disk-serial.args | 7 ++ > .../qemuxml2argvdata/qemuxml2argv-disk-serial.xml | 27 > ++ > tests/qemuxml2argvtest.c | 5 > 5 files changed, 43 insertions(+), 3 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml > ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 13/24] bridge_driver: Use virNetworkObjEndAPI
So far, this is pure code replacement. But once we introduce
reference counting to virNetworkObj this will be more handy as
there'll be only one function to change: virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik
---
src/network/bridge_driver.c | 57 +++--
1 file changed, 19 insertions(+), 38 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 51df46d..8501603 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2496,8 +2496,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr
conn,
ret = virGetNetwork(conn, network->def->name, network->def->uuid);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return ret;
}
@@ -2522,8 +2521,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr
conn,
ret = virGetNetwork(conn, network->def->name, network->def->uuid);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return ret;
}
@@ -2670,8 +2668,7 @@ static int networkIsActive(virNetworkPtr net)
ret = virNetworkObjIsActive(obj);
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(&obj);
return ret;
}
@@ -2689,8 +2686,7 @@ static int networkIsPersistent(virNetworkPtr net)
ret = obj->persistent;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(&obj);
return ret;
}
@@ -2959,8 +2955,7 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn,
const char *xml)
virNetworkDefFree(def);
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3016,8 +3011,7 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn,
const char *xml)
virObjectEventStateQueue(driver->networkEventState, event);
if (freeDef)
virNetworkDefFree(def);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3077,8 +3071,7 @@ networkUndefine(virNetworkPtr net)
cleanup:
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3249,8 +3242,7 @@ networkUpdate(virNetworkPtr net,
}
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3284,8 +3276,7 @@ static int networkCreate(virNetworkPtr net)
cleanup:
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3335,8 +3326,7 @@ static int networkDestroy(virNetworkPtr net)
cleanup:
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3364,8 +3354,7 @@ static char *networkGetXMLDesc(virNetworkPtr net,
ret = virNetworkDefFormat(def, flags);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return ret;
}
@@ -3389,8 +3378,7 @@ static char *networkGetBridgeName(virNetworkPtr net) {
ignore_value(VIR_STRDUP(bridge, network->def->bridge));
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return bridge;
}
@@ -3410,8 +3398,7 @@ static int networkGetAutostart(virNetworkPtr net,
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return ret;
}
@@ -3478,8 +3465,7 @@ static int networkSetAutostart(virNetworkPtr net,
cleanup:
VIR_FREE(configFile);
VIR_FREE(autostartLink);
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3651,8 +3637,7 @@ networkGetDHCPLeases(virNetworkPtr network,
VIR_FREE(custom_lease_file);
virJSONValueFree(leases_array);
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(&obj);
return rv;
@@ -4124,8 +4109,7 @@ networkAllocateActualDevice(virDomainDefPtr dom,
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return ret;
error:
@@ -4327,8 +4311,7 @@ networkNotifyActualDevice(virDomainDefPtr dom,
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return ret;
error:
@@ -4477
[libvirt] [PATCH v2 11/24] virNetworkObjList: Derive from virObjectLockableClass
Later we can turn APIs to lock the object if needed instead of
relying on caller to mutually exclude itself (probably done by
locking a big lock anyway).
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index c1dc694..88f1689 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -52,7 +52,7 @@
#define CLASS_ID_BITMAP_SIZE (1<<16)
struct _virNetworkObjList {
-virObject parent;
+virObjectLockable parent;
size_t count;
virNetworkObjPtr *objs;
@@ -93,7 +93,7 @@ static int virNetworkObjOnceInit(void)
virNetworkObjDispose)))
return -1;
-if (!(virNetworkObjListClass = virClassNew(virClassForObject(),
+if (!(virNetworkObjListClass = virClassNew(virClassForObjectLockable(),
"virNetworkObjList",
sizeof(virNetworkObjList),
virNetworkObjListDispose)))
@@ -137,7 +137,7 @@ virNetworkObjListPtr virNetworkObjListNew(void)
if (virNetworkObjInitialize() < 0)
return NULL;
-if (!(nets = virObjectNew(virNetworkObjListClass)))
+if (!(nets = virObjectLockableNew(virNetworkObjListClass)))
return NULL;
return nets;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 16/24] network_conf: Introduce locked versions of lookup functions
This is going to be needed later, when some functions needs to avoid
calling multiple times at once. It will work like this:
1) gain the object list mutex
2) find the object to work on
3) do the work
4) release the mutex
As an example of such function is virNetworkAssignDef(). The other
use case might be in virNetworkObjListForEach() callback.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 45 ++---
src/conf/network_conf.h | 4
src/libvirt_private.syms | 2 ++
3 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index a821f6c..8cf9ffd 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -153,34 +153,57 @@ virNetworkObjListPtr virNetworkObjListNew(void)
return nets;
}
+virNetworkObjPtr
+virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
+ const unsigned char *uuid)
+{
+size_t i;
+
+for (i = 0; i < nets->count; i++) {
+virObjectLock(nets->objs[i]);
+if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
+return nets->objs[i];
+virObjectUnlock(nets->objs[i]);
+}
+
+return NULL;
+}
+
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
const unsigned char *uuid)
{
+virNetworkObjPtr ret;
+
+virObjectLock(nets);
+ret = virNetworkObjFindByUUIDLocked(nets, uuid);
+virObjectUnlock(nets);
+return ret;
+}
+
+virNetworkObjPtr
+virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
+ const char *name)
+{
size_t i;
for (i = 0; i < nets->count; i++) {
virObjectLock(nets->objs[i]);
-if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
+if (STREQ(nets->objs[i]->def->name, name))
return nets->objs[i];
virObjectUnlock(nets->objs[i]);
}
return NULL;
}
-
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name)
{
-size_t i;
+virNetworkObjPtr ret;
-for (i = 0; i < nets->count; i++) {
-virObjectLock(nets->objs[i]);
-if (STREQ(nets->objs[i]->def->name, name))
-return nets->objs[i];
-virObjectUnlock(nets->objs[i]);
-}
-
-return NULL;
+virObjectLock(nets);
+ret = virNetworkObjFindByNameLocked(nets, name);
+virObjectUnlock(nets);
+return ret;
}
bool
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index e0ed714..3e926f7 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -300,8 +300,12 @@ virNetworkObjIsActive(const virNetworkObj *net)
virNetworkObjListPtr virNetworkObjListNew(void);
+virNetworkObjPtr virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
+ const unsigned char *uuid);
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
const unsigned char *uuid);
+virNetworkObjPtr virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
+ const char *name);
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name);
bool virNetworkObjTaint(virNetworkObjPtr obj,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c770177..c678dd8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -565,7 +565,9 @@ virNetworkLoadAllState;
virNetworkObjAssignDef;
virNetworkObjEndAPI;
virNetworkObjFindByName;
+virNetworkObjFindByNameLocked;
virNetworkObjFindByUUID;
+virNetworkObjFindByUUIDLocked;
virNetworkObjGetPersistentDef;
virNetworkObjIsDuplicate;
virNetworkObjListExport;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 14/24] test_driver: Use virNetworkObjEndAPI
So far, this is pure code replacement. But once we introduce
reference counting to virNetworkObj this will be more handy as
there'll be only one function to change: virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik
---
src/test/test_driver.c | 42 ++
1 file changed, 14 insertions(+), 28 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 20c77de..72f40ed 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3507,8 +3507,7 @@ static virNetworkPtr
testNetworkLookupByUUID(virConnectPtr conn,
ret = virGetNetwork(conn, net->def->name, net->def->uuid);
cleanup:
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(&net);
return ret;
}
@@ -3531,8 +3530,7 @@ static virNetworkPtr
testNetworkLookupByName(virConnectPtr conn,
ret = virGetNetwork(conn, net->def->name, net->def->uuid);
cleanup:
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(&net);
return ret;
}
@@ -3620,8 +3618,7 @@ static int testNetworkIsActive(virNetworkPtr net)
ret = virNetworkObjIsActive(obj);
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(&obj);
return ret;
}
@@ -3641,8 +3638,7 @@ static int testNetworkIsPersistent(virNetworkPtr net)
ret = obj->persistent;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(&obj);
return ret;
}
@@ -3674,8 +3670,7 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr
conn, const char *xml)
virNetworkDefFree(def);
if (event)
testObjectEventQueue(privconn, event);
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(&net);
testDriverUnlock(privconn);
return ret;
}
@@ -3707,8 +3702,7 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn,
const char *xml)
virNetworkDefFree(def);
if (event)
testObjectEventQueue(privconn, event);
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(&net);
testDriverUnlock(privconn);
return ret;
}
@@ -3745,8 +3739,7 @@ static int testNetworkUndefine(virNetworkPtr network)
cleanup:
if (event)
testObjectEventQueue(privconn, event);
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(&privnet);
testDriverUnlock(privconn);
return ret;
}
@@ -3795,8 +3788,7 @@ testNetworkUpdate(virNetworkPtr net,
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
testDriverUnlock(privconn);
return ret;
}
@@ -3832,8 +3824,7 @@ static int testNetworkCreate(virNetworkPtr network)
cleanup:
if (event)
testObjectEventQueue(privconn, event);
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -3865,8 +3856,7 @@ static int testNetworkDestroy(virNetworkPtr network)
cleanup:
if (event)
testObjectEventQueue(privconn, event);
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(&privnet);
testDriverUnlock(privconn);
return ret;
}
@@ -3892,8 +3882,7 @@ static char *testNetworkGetXMLDesc(virNetworkPtr network,
ret = virNetworkDefFormat(privnet->def, flags);
cleanup:
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -3921,8 +3910,7 @@ static char *testNetworkGetBridgeName(virNetworkPtr
network) {
ignore_value(VIR_STRDUP(bridge, privnet->def->bridge));
cleanup:
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(&privnet);
return bridge;
}
@@ -3946,8 +3934,7 @@ static int testNetworkGetAutostart(virNetworkPtr network,
ret = 0;
cleanup:
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -3971,8 +3958,7 @@ static int testNetworkSetAutostart(virNetworkPtr network,
ret = 0;
cleanup:
-if (privnet)
-virObjectUnlock(privnet);
+virNetworkObjEndAPI(&privnet);
return ret;
}
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 15/24] parallels_network: Use virNetworkObjEndAPI
So far, this is pure code replacement. But once we introduce
reference counting to virNetworkObj this will be more handy as
there'll be only one function to change: virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik
---
src/parallels/parallels_network.c | 29 -
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index 86038bf..1bcd2d3 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -230,7 +230,6 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
goto cleanup;
net->active = 1;
net->autostart = 1;
-virObjectUnlock(net);
return net;
cleanup:
@@ -241,7 +240,7 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
virJSONValuePtr jobj)
static virNetworkObjPtr
parallelsAddRoutedNetwork(parallelsConnPtr privconn)
{
-virNetworkObjPtr net;
+virNetworkObjPtr net = NULL;
virNetworkDefPtr def;
if (VIR_ALLOC(def) < 0)
@@ -265,7 +264,6 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
}
net->active = 1;
net->autostart = 1;
-virObjectUnlock(net);
return net;
@@ -277,7 +275,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
static int parallelsLoadNetworks(parallelsConnPtr privconn)
{
virJSONValuePtr jobj, jobj2;
-virNetworkObjPtr net;
+virNetworkObjPtr net = NULL;
int ret = -1;
int count;
size_t i;
@@ -305,16 +303,19 @@ static int parallelsLoadNetworks(parallelsConnPtr
privconn)
net = parallelsLoadNetwork(privconn, jobj2);
if (!net)
goto cleanup;
+else
+virNetworkObjEndAPI(&net);
}
-if (!parallelsAddRoutedNetwork(privconn))
+if (!(net = parallelsAddRoutedNetwork(privconn)))
goto cleanup;
ret = 0;
cleanup:
virJSONValueFree(jobj);
+virNetworkObjEndAPI(&net);
return ret;
}
@@ -444,8 +445,7 @@ static virNetworkPtr
parallelsNetworkLookupByUUID(virConnectPtr conn,
ret = virGetNetwork(conn, network->def->name, network->def->uuid);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return ret;
}
@@ -468,8 +468,7 @@ static virNetworkPtr
parallelsNetworkLookupByName(virConnectPtr conn,
ret = virGetNetwork(conn, network->def->name, network->def->uuid);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return ret;
}
@@ -495,8 +494,7 @@ static char *parallelsNetworkGetXMLDesc(virNetworkPtr net,
ret = virNetworkDefFormat(network->def, flags);
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return ret;
}
@@ -516,8 +514,7 @@ static int parallelsNetworkIsActive(virNetworkPtr net)
ret = virNetworkObjIsActive(obj);
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(&obj);
return ret;
}
@@ -537,8 +534,7 @@ static int parallelsNetworkIsPersistent(virNetworkPtr net)
ret = obj->persistent;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virNetworkObjEndAPI(&obj);
return ret;
}
@@ -562,8 +558,7 @@ static int parallelsNetworkGetAutostart(virNetworkPtr net,
ret = 0;
cleanup:
-if (network)
-virObjectUnlock(network);
+virNetworkObjEndAPI(&network);
return ret;
}
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 22/24] bridge_driver: Use more of networkObjFromNetwork
Now that the network driver lock is ash heap of history,
we can use more of networkObjFromNetwork().
Signed-off-by: Michal Privoznik
---
src/network/bridge_driver.c | 33 +
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 529ba2b..2eb225f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -130,7 +130,6 @@ networkObjFromNetwork(virNetworkPtr net)
char uuidstr[VIR_UUID_STRING_BUFLEN];
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-
if (!network) {
virUUIDFormat(net->uuid, uuidstr);
virReportError(VIR_ERR_NO_NETWORK,
@@ -3005,12 +3004,8 @@ networkUndefine(virNetworkPtr net)
bool active = false;
virObjectEventPtr event = NULL;
-network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-if (!network) {
-virReportError(VIR_ERR_NO_NETWORK,
- "%s", _("no network with matching uuid"));
+if (!(network = networkObjFromNetwork(net)))
goto cleanup;
-}
if (virNetworkUndefineEnsureACL(net->conn, network->def) < 0)
goto cleanup;
@@ -3071,12 +3066,8 @@ networkUpdate(virNetworkPtr net,
VIR_NETWORK_UPDATE_AFFECT_CONFIG,
-1);
-network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-if (!network) {
-virReportError(VIR_ERR_NO_NETWORK,
- "%s", _("no network with matching uuid"));
+if (!(network = networkObjFromNetwork(net)))
goto cleanup;
-}
if (virNetworkUpdateEnsureACL(net->conn, network->def, flags) < 0)
goto cleanup;
@@ -3225,13 +3216,8 @@ static int networkCreate(virNetworkPtr net)
int ret = -1;
virObjectEventPtr event = NULL;
-network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-
-if (!network) {
-virReportError(VIR_ERR_NO_NETWORK,
- "%s", _("no network with matching uuid"));
+if (!(network = networkObjFromNetwork(net)))
goto cleanup;
-}
if (virNetworkCreateEnsureACL(net->conn, network->def) < 0)
goto cleanup;
@@ -3257,13 +3243,8 @@ static int networkDestroy(virNetworkPtr net)
int ret = -1;
virObjectEventPtr event = NULL;
-network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-
-if (!network) {
-virReportError(VIR_ERR_NO_NETWORK,
- "%s", _("no network with matching uuid"));
+if (!(network = networkObjFromNetwork(net)))
goto cleanup;
-}
if (virNetworkDestroyEnsureACL(net->conn, network->def) < 0)
goto cleanup;
@@ -3375,13 +3356,9 @@ static int networkSetAutostart(virNetworkPtr net,
int ret = -1;
networkDriverLock();
-network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-if (!network) {
-virReportError(VIR_ERR_NO_NETWORK,
- "%s", _("no network with matching uuid"));
+if (!(network = networkObjFromNetwork(net)))
goto cleanup;
-}
if (virNetworkSetAutostartEnsureACL(net->conn, network->def) < 0)
goto cleanup;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 08/24] network_conf: Turn virNetworkObjList into virObject
Well, one day this will be self-locking object, but not today.
But lets prepare the code for that! Moreover,
virNetworkObjListFree() is no longer needed, so turn it into
virNetworkObjListDispose().
Signed-off-by: Michal Privoznik
---
cfg.mk| 1 -
src/conf/network_conf.c | 53 +--
src/conf/network_conf.h | 11
src/libvirt_private.syms | 2 +-
src/network/bridge_driver.c | 5 ++--
src/parallels/parallels_network.c | 7 +++---
src/test/test_driver.c| 13 --
7 files changed, 57 insertions(+), 35 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index d72b039..07a794a 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -250,7 +250,6 @@ useless_free_options = \
# n virNetworkFree (returns int)
# n virNetworkFreeName (returns int)
# y virNetworkObjFree
-# n virNetworkObjListFree FIXME
# n virNodeDevCapsDefFree FIXME
# y virNodeDeviceDefFree
# n virNodeDeviceFree (returns int)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ea9a9d4..f843e3c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -73,17 +73,33 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName,
VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST,
"hook-script");
-bool
-virNetworkObjTaint(virNetworkObjPtr obj,
- virNetworkTaintFlags taint)
+static virClassPtr virNetworkObjListClass;
+static void virNetworkObjListDispose(void *obj);
+
+static int virNetworkObjOnceInit(void)
+{
+if (!(virNetworkObjListClass = virClassNew(virClassForObject(),
+ "virNetworkObjList",
+ sizeof(virNetworkObjList),
+ virNetworkObjListDispose)))
+return -1;
+return 0;
+}
+
+
+VIR_ONCE_GLOBAL_INIT(virNetworkObj)
+
+virNetworkObjListPtr virNetworkObjListNew(void)
{
-unsigned int flag = (1 << taint);
+virNetworkObjListPtr nets;
+
+if (virNetworkObjInitialize() < 0)
+return NULL;
-if (obj->taint & flag)
-return false;
+if (!(nets = virObjectNew(virNetworkObjListClass)))
+return NULL;
-obj->taint |= flag;
-return true;
+return nets;
}
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
@@ -116,6 +132,19 @@ virNetworkObjPtr
virNetworkObjFindByName(virNetworkObjListPtr nets,
return NULL;
}
+bool
+virNetworkObjTaint(virNetworkObjPtr obj,
+ virNetworkTaintFlags taint)
+{
+unsigned int flag = (1 << taint);
+
+if (obj->taint & flag)
+return false;
+
+obj->taint |= flag;
+return true;
+}
+
static void
virPortGroupDefClear(virPortGroupDefPtr def)
@@ -275,18 +304,16 @@ void virNetworkObjFree(virNetworkObjPtr net)
VIR_FREE(net);
}
-void virNetworkObjListFree(virNetworkObjListPtr nets)
+static void
+virNetworkObjListDispose(void *obj)
{
+virNetworkObjListPtr nets = obj;
size_t i;
-if (!nets)
-return;
-
for (i = 0; i < nets->count; i++)
virNetworkObjFree(nets->objs[i]);
VIR_FREE(nets->objs);
-nets->count = 0;
}
/*
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 3fbd609..0c2609a 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -40,6 +40,7 @@
# include "device_conf.h"
# include "virbitmap.h"
# include "networkcommon_conf.h"
+# include "virobject.h"
typedef enum {
VIR_NETWORK_FORWARD_NONE = 0,
@@ -277,6 +278,8 @@ struct _virNetworkObj {
typedef struct _virNetworkObjList virNetworkObjList;
typedef virNetworkObjList *virNetworkObjListPtr;
struct _virNetworkObjList {
+virObject parent;
+
size_t count;
virNetworkObjPtr *objs;
};
@@ -298,19 +301,17 @@ virNetworkObjIsActive(const virNetworkObj *net)
return net->active;
}
-bool virNetworkObjTaint(virNetworkObjPtr obj,
-virNetworkTaintFlags taint);
+virNetworkObjListPtr virNetworkObjListNew(void);
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
const unsigned char *uuid);
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name);
-
+bool virNetworkObjTaint(virNetworkObjPtr obj,
+virNetworkTaintFlags taint);
void virNetworkDefFree(virNetworkDefPtr def);
void virNetworkObjFree(virNetworkObjPtr net);
-void virNetworkObjListFree(virNetworkObjListPtr nets);
-
typedef bool (*virNetworkObjListFilter)(virConnectPtr conn,
virNetworkDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4ce5e3a..e2b558f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -570,8 +570,8 @@ virNetworkObjGetPersistentDef;
virNetworkObjIsDuplicate;
virNetworkObjListExport;
[libvirt] [PATCH v2 10/24] network_conf: Make virNetworkObj actually virObject
So far it's just a structure which happens to have 'Obj' in its
name, but otherwise it not related to virObject at all. No
reference counting, not virObjectLock(), nothing.
Signed-off-by: Michal Privoznik
---
cfg.mk| 2 -
src/conf/network_conf.c | 135 --
src/conf/network_conf.h | 8 +--
src/libvirt_private.syms | 4 +-
src/network/bridge_driver.c | 56
src/parallels/parallels_network.c | 16 ++---
src/test/test_driver.c| 32 -
tests/networkxml2conftest.c | 4 +-
tests/objectlocking.ml| 2 -
9 files changed, 129 insertions(+), 130 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 07a794a..6885f9e 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -160,7 +160,6 @@ useless_free_options = \
--name=virNWFilterRuleDefFree\
--name=virNWFilterRuleInstFree \
--name=virNetworkDefFree \
- --name=virNetworkObjFree \
--name=virNodeDeviceDefFree \
--name=virNodeDeviceObjFree \
--name=virObjectUnref \
@@ -249,7 +248,6 @@ useless_free_options = \
# y virNetworkDefFree
# n virNetworkFree (returns int)
# n virNetworkFreeName (returns int)
-# y virNetworkObjFree
# n virNodeDevCapsDefFree FIXME
# y virNodeDeviceDefFree
# n virNodeDeviceFree (returns int)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 9c1d578..c1dc694 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -80,11 +80,19 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName,
VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST,
"hook-script");
+static virClassPtr virNetworkObjClass;
static virClassPtr virNetworkObjListClass;
+static void virNetworkObjDispose(void *obj);
static void virNetworkObjListDispose(void *obj);
static int virNetworkObjOnceInit(void)
{
+if (!(virNetworkObjClass = virClassNew(virClassForObjectLockable(),
+ "virNetworkObj",
+ sizeof(virNetworkObj),
+ virNetworkObjDispose)))
+return -1;
+
if (!(virNetworkObjListClass = virClassNew(virClassForObject(),
"virNetworkObjList",
sizeof(virNetworkObjList),
@@ -96,6 +104,32 @@ static int virNetworkObjOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virNetworkObj)
+virNetworkObjPtr
+virNetworkObjNew(void)
+{
+virNetworkObjPtr net;
+
+if (virNetworkObjInitialize() < 0)
+return NULL;
+
+if (!(net = virObjectLockableNew(virNetworkObjClass)))
+return NULL;
+
+if (!(net->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
+goto error;
+
+/* The first three class IDs are already taken */
+ignore_value(virBitmapSetBit(net->class_id, 0));
+ignore_value(virBitmapSetBit(net->class_id, 1));
+ignore_value(virBitmapSetBit(net->class_id, 2));
+
+return net;
+
+ error:
+virObjectUnref(net);
+return NULL;
+}
+
virNetworkObjListPtr virNetworkObjListNew(void)
{
virNetworkObjListPtr nets;
@@ -115,10 +149,10 @@ virNetworkObjPtr
virNetworkObjFindByUUID(virNetworkObjListPtr nets,
size_t i;
for (i = 0; i < nets->count; i++) {
-virNetworkObjLock(nets->objs[i]);
+virObjectLock(nets->objs[i]);
if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
return nets->objs[i];
-virNetworkObjUnlock(nets->objs[i]);
+virObjectUnlock(nets->objs[i]);
}
return NULL;
@@ -130,10 +164,10 @@ virNetworkObjPtr
virNetworkObjFindByName(virNetworkObjListPtr nets,
size_t i;
for (i = 0; i < nets->count; i++) {
-virNetworkObjLock(nets->objs[i]);
+virObjectLock(nets->objs[i]);
if (STREQ(nets->objs[i]->def->name, name))
return nets->objs[i];
-virNetworkObjUnlock(nets->objs[i]);
+virObjectUnlock(nets->objs[i]);
}
return NULL;
@@ -297,18 +331,14 @@ virNetworkDefFree(virNetworkDefPtr def)
VIR_FREE(def);
}
-void virNetworkObjFree(virNetworkObjPtr net)
+static void
+virNetworkObjDispose(void *obj)
{
-if (!net)
-return;
+virNetworkObjPtr net = obj;
virNetworkDefFree(net->def);
virNetworkDefFree(net->newDef);
virBitmapFree(net->class_id);
-
-virMutexDestroy(&net->lock);
-
-VIR_FREE(net);
}
static void
@@ -318,7 +348,7 @@ virNetworkObjListDispose(void *obj)
size_t i;
for (i = 0; i < nets->count; i++)
-virNetworkObjFree(nets->objs[i]);
+virObjectUnref(nets->objs[i]);
VIR_FREE(nets->objs);
}
@@ -409,32 +439,20 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
return network;
}
-if
[libvirt] [PATCH v2 18/24] virNetworkObjFindBy*: Return an reference to found object
This patch turns both virNetworkObjFindByUUID() and
virNetworkObjFindByName() to return an referenced object so that
even if caller unlocks it, it's for sure that object won't
disappear meanwhile. Especially if the object (in general) is
locked and unlocked during the caller run.
Moreover, this commit is nicely small, since the object unrefing
can be done in virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 7 +--
src/network/bridge_driver.c | 18 +-
src/test/test_driver.c | 10 --
3 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 7af303e..3d318ce 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -137,6 +137,7 @@ virNetworkObjEndAPI(virNetworkObjPtr *net)
return;
virObjectUnlock(*net);
+virObjectUnref(*net);
*net = NULL;
}
@@ -164,6 +165,7 @@ virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
virObjectLock(nets->objs[i]);
if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) {
ret = nets->objs[i];
+virObjectRef(ret);
break;
}
virObjectUnlock(nets->objs[i]);
@@ -194,6 +196,7 @@ virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
virObjectLock(nets->objs[i]);
if (STREQ(nets->objs[i]->def->name, name)) {
ret = nets->objs[i];
+virObjectRef(ret);
break;
}
virObjectUnlock(nets->objs[i]);
@@ -491,13 +494,13 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
network->def = def;
network->persistent = !live;
+virObjectRef(network);
virObjectUnlock(nets);
return network;
error:
virObjectUnlock(nets);
-virObjectUnlock(network);
-virObjectUnref(network);
+virNetworkObjEndAPI(&network);
return NULL;
}
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8501603..d3f3f4a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2939,7 +2939,6 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn,
const char *xml)
if (networkStartNetwork(network) < 0) {
virNetworkRemoveInactive(driver->networks,
network);
-network = NULL;
goto cleanup;
}
@@ -2988,7 +2987,6 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn,
const char *xml)
if (virNetworkSaveConfig(driver->networkConfigDir, def) < 0) {
if (!virNetworkObjIsActive(network)) {
virNetworkRemoveInactive(driver->networks, network);
-network = NULL;
goto cleanup;
}
/* if network was active already, just undo new persistent
@@ -3053,11 +3051,8 @@ networkUndefine(virNetworkPtr net)
VIR_INFO("Undefining network '%s'", network->def->name);
if (!active) {
-if (networkRemoveInactive(network) < 0) {
-network = NULL;
+if (networkRemoveInactive(network) < 0)
goto cleanup;
-}
-network = NULL;
} else {
/* if the network still exists, it was active, and we need to make
@@ -3314,13 +3309,10 @@ static int networkDestroy(virNetworkPtr net)
VIR_NETWORK_EVENT_STOPPED,
0);
-if (!network->persistent) {
-if (networkRemoveInactive(network) < 0) {
-network = NULL;
-ret = -1;
-goto cleanup;
-}
-network = NULL;
+if (!network->persistent &&
+networkRemoveInactive(network) < 0) {
+ret = -1;
+goto cleanup;
}
cleanup:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 72f40ed..90af80a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -787,7 +787,7 @@ testOpenDefault(virConnectPtr conn)
goto error;
}
netobj->active = 1;
-virObjectUnlock(netobj);
+virNetworkObjEndAPI(&netobj);
if (!(interfacedef = virInterfaceDefParseString(defaultInterfaceXML)))
goto error;
@@ -1155,7 +1155,7 @@ testParseNetworks(testConnPtr privconn,
}
obj->active = 1;
-virObjectUnlock(obj);
+virNetworkObjEndAPI(&obj);
}
ret = 0;
@@ -3733,7 +3733,6 @@ static int testNetworkUndefine(virNetworkPtr network)
0);
virNetworkRemoveInactive(privconn->networks, privnet);
-privnet = NULL;
ret = 0;
cleanup:
@@ -3847,10 +3846,9 @@ static int testNetworkDestroy(virNetworkPtr network)
event = virNetworkEventLifecycleNew(privnet->def->name, privnet->def->uuid,
VIR_NETWORK_EVENT_STOPPED,
0);
-if (!privnet->persistent) {
+if (!privnet->persistent)
virNetworkRemoveInactive(privconn->networks, privnet
[libvirt] [PATCH v2 21/24] parallels_network: Drop parallelsDriverLock() from everywhere.
While in previous commits there were some places that relied on
the big lock, in this file there's no such place and the big
driver lock can be dropped completely. Yay!
Signed-off-by: Michal Privoznik
---
src/parallels/parallels_network.c | 33 +
1 file changed, 1 insertion(+), 32 deletions(-)
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index 1bcd2d3..1d3b694 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -349,9 +349,7 @@ int parallelsNetworkClose(virConnectPtr conn)
if (!privconn)
return 0;
-parallelsDriverLock(privconn);
virObjectUnref(privconn->networks);
-parallelsDriverUnlock(privconn);
return 0;
}
@@ -360,11 +358,8 @@ static int parallelsConnectNumOfNetworks(virConnectPtr
conn)
int nactive;
parallelsConnPtr privconn = conn->privateData;
-parallelsDriverLock(privconn);
nactive = virNetworkObjListNumOfNetworks(privconn->networks,
true, NULL, conn);
-parallelsDriverUnlock(privconn);
-
return nactive;
}
@@ -375,11 +370,8 @@ static int parallelsConnectListNetworks(virConnectPtr conn,
parallelsConnPtr privconn = conn->privateData;
int got;
-parallelsDriverLock(privconn);
got = virNetworkObjListGetNames(privconn->networks,
true, names, nnames, NULL, conn);
-parallelsDriverUnlock(privconn);
-
return got;
}
@@ -388,11 +380,8 @@ static int
parallelsConnectNumOfDefinedNetworks(virConnectPtr conn)
int ninactive;
parallelsConnPtr privconn = conn->privateData;
-parallelsDriverLock(privconn);
ninactive = virNetworkObjListNumOfNetworks(privconn->networks,
false, NULL, conn);
-parallelsDriverUnlock(privconn);
-
return ninactive;
}
@@ -403,10 +392,8 @@ static int
parallelsConnectListDefinedNetworks(virConnectPtr conn,
parallelsConnPtr privconn = conn->privateData;
int got;
-parallelsDriverLock(privconn);
got = virNetworkObjListGetNames(privconn->networks,
false, names, nnames, NULL, conn);
-parallelsDriverUnlock(privconn);
return got;
}
@@ -415,15 +402,10 @@ static int parallelsConnectListAllNetworks(virConnectPtr
conn,
unsigned int flags)
{
parallelsConnPtr privconn = conn->privateData;
-int ret = -1;
virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
-parallelsDriverLock(privconn);
-ret = virNetworkObjListExport(conn, privconn->networks, nets, NULL, flags);
-parallelsDriverUnlock(privconn);
-
-return ret;
+return virNetworkObjListExport(conn, privconn->networks, nets, NULL,
flags);
}
static virNetworkPtr parallelsNetworkLookupByUUID(virConnectPtr conn,
@@ -433,9 +415,7 @@ static virNetworkPtr
parallelsNetworkLookupByUUID(virConnectPtr conn,
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
-parallelsDriverLock(privconn);
network = virNetworkObjFindByUUID(privconn->networks, uuid);
-parallelsDriverUnlock(privconn);
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
"%s", _("no network with matching uuid"));
@@ -456,9 +436,7 @@ static virNetworkPtr
parallelsNetworkLookupByName(virConnectPtr conn,
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
-parallelsDriverLock(privconn);
network = virNetworkObjFindByName(privconn->networks, name);
-parallelsDriverUnlock(privconn);
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
_("no network with matching name '%s'"), name);
@@ -481,10 +459,7 @@ static char *parallelsNetworkGetXMLDesc(virNetworkPtr net,
virCheckFlags(VIR_NETWORK_XML_INACTIVE, NULL);
-parallelsDriverLock(privconn);
network = virNetworkObjFindByUUID(privconn->networks, net->uuid);
-parallelsDriverUnlock(privconn);
-
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
"%s", _("no network with matching uuid"));
@@ -504,9 +479,7 @@ static int parallelsNetworkIsActive(virNetworkPtr net)
virNetworkObjPtr obj;
int ret = -1;
-parallelsDriverLock(privconn);
obj = virNetworkObjFindByUUID(privconn->networks, net->uuid);
-parallelsDriverUnlock(privconn);
if (!obj) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -524,9 +497,7 @@ static int parallelsNetworkIsPersistent(virNetworkPtr net)
virNetworkObjPtr obj;
int ret = -1;
-parallelsDriverLock(privconn);
obj = virNetworkObjFindByUUID(privconn->networks, net->uuid);
-parallelsDriverUnlock(privconn);
if (!obj) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -545,9 +516,7 @@ static int parallelsNetworkGetAutostart(virNetworkP
[libvirt] [PATCH v2 07/24] parallels_network: Adapt to new virNetworkObjList accessors
Signed-off-by: Michal Privoznik
---
src/parallels/parallels_network.c | 66 +++
1 file changed, 12 insertions(+), 54 deletions(-)
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index e1c6040..6b53518 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -357,17 +357,12 @@ int parallelsNetworkClose(virConnectPtr conn)
static int parallelsConnectNumOfNetworks(virConnectPtr conn)
{
-int nactive = 0;
-size_t i;
+int nactive;
parallelsConnPtr privconn = conn->privateData;
parallelsDriverLock(privconn);
-for (i = 0; i < privconn->networks->count; i++) {
-virNetworkObjLock(privconn->networks->objs[i]);
-if (virNetworkObjIsActive(privconn->networks->objs[i]))
-nactive++;
-virNetworkObjUnlock(privconn->networks->objs[i]);
-}
+nactive = virNetworkObjListNumOfNetworks(privconn->networks,
+ true, NULL, conn);
parallelsDriverUnlock(privconn);
return nactive;
@@ -378,45 +373,24 @@ static int parallelsConnectListNetworks(virConnectPtr
conn,
int nnames)
{
parallelsConnPtr privconn = conn->privateData;
-int got = 0;
-size_t i;
+int got;
parallelsDriverLock(privconn);
-for (i = 0; i < privconn->networks->count && got < nnames; i++) {
-virNetworkObjLock(privconn->networks->objs[i]);
-if (virNetworkObjIsActive(privconn->networks->objs[i])) {
-if (VIR_STRDUP(names[got], privconn->networks->objs[i]->def->name)
< 0) {
-virNetworkObjUnlock(privconn->networks->objs[i]);
-goto cleanup;
-}
-got++;
-}
-virNetworkObjUnlock(privconn->networks->objs[i]);
-}
+got = virNetworkObjListGetNames(privconn->networks,
+true, names, nnames, NULL, conn);
parallelsDriverUnlock(privconn);
return got;
-
- cleanup:
-parallelsDriverUnlock(privconn);
-for (i = 0; i < got; i++)
-VIR_FREE(names[i]);
-return -1;
}
static int parallelsConnectNumOfDefinedNetworks(virConnectPtr conn)
{
-int ninactive = 0;
-size_t i;
+int ninactive;
parallelsConnPtr privconn = conn->privateData;
parallelsDriverLock(privconn);
-for (i = 0; i < privconn->networks->count; i++) {
-virNetworkObjLock(privconn->networks->objs[i]);
-if (!virNetworkObjIsActive(privconn->networks->objs[i]))
-ninactive++;
-virNetworkObjUnlock(privconn->networks->objs[i]);
-}
+ninactive = virNetworkObjListNumOfNetworks(privconn->networks,
+ false, NULL, conn);
parallelsDriverUnlock(privconn);
return ninactive;
@@ -427,29 +401,13 @@ static int
parallelsConnectListDefinedNetworks(virConnectPtr conn,
int nnames)
{
parallelsConnPtr privconn = conn->privateData;
-int got = 0;
-size_t i;
+int got;
parallelsDriverLock(privconn);
-for (i = 0; i < privconn->networks->count && got < nnames; i++) {
-virNetworkObjLock(privconn->networks->objs[i]);
-if (!virNetworkObjIsActive(privconn->networks->objs[i])) {
-if (VIR_STRDUP(names[got], privconn->networks->objs[i]->def->name)
< 0) {
-virNetworkObjUnlock(privconn->networks->objs[i]);
-goto cleanup;
-}
-got++;
-}
-virNetworkObjUnlock(privconn->networks->objs[i]);
-}
+got = virNetworkObjListGetNames(privconn->networks,
+false, names, nnames, NULL, conn);
parallelsDriverUnlock(privconn);
return got;
-
- cleanup:
-parallelsDriverUnlock(privconn);
-for (i = 0; i < got; i++)
-VIR_FREE(names[i]);
-return -1;
}
static int parallelsConnectListAllNetworks(virConnectPtr conn,
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 09/24] network_conf: Turn struct _virNetworkObjList private
Now that all the code uses accessors, don't expose the structure
anyway.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 7 +++
src/conf/network_conf.h | 6 --
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index f843e3c..9c1d578 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -51,6 +51,13 @@
/* currently, /sbin/tc implementation allows up to 16 bits for minor class
size */
#define CLASS_ID_BITMAP_SIZE (1<<16)
+struct _virNetworkObjList {
+virObject parent;
+
+size_t count;
+virNetworkObjPtr *objs;
+};
+
VIR_ENUM_IMPL(virNetworkForward,
VIR_NETWORK_FORWARD_LAST,
"none", "nat", "route", "bridge", "private", "vepa",
"passthrough", "hostdev")
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 0c2609a..3575659 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -277,12 +277,6 @@ struct _virNetworkObj {
typedef struct _virNetworkObjList virNetworkObjList;
typedef virNetworkObjList *virNetworkObjListPtr;
-struct _virNetworkObjList {
-virObject parent;
-
-size_t count;
-virNetworkObjPtr *objs;
-};
typedef enum {
VIR_NETWORK_TAINT_HOOK, /* Hook script was executed over
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 19/24] bridge_driver: Drop networkDriverLock() from almost everywhere
Now that we have fine grained locks, there's no need to lock the
whole driver. We can rely on self-locking APIs.
Signed-off-by: Michal Privoznik
---
src/network/bridge_driver.c | 49 +++--
1 file changed, 7 insertions(+), 42 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d3f3f4a..529ba2b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -129,9 +129,7 @@ networkObjFromNetwork(virNetworkPtr net)
virNetworkObjPtr network;
char uuidstr[VIR_UUID_STRING_BUFLEN];
-networkDriverLock();
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-networkDriverUnlock();
if (!network) {
virUUIDFormat(net->uuid, uuidstr);
@@ -264,6 +262,11 @@ networkRemoveInactive(virNetworkObjPtr net)
int ret = -1;
+virObjectRef(net);
+virObjectUnlock(net);
+networkDriverLock();
+virObjectLock(net);
+
/* remove the (possibly) existing dnsmasq and radvd files */
if (!(dctx = dnsmasqContextNew(def->name,
driver->dnsmasqStateDir))) {
@@ -315,6 +318,8 @@ networkRemoveInactive(virNetworkObjPtr net)
VIR_FREE(radvdpidbase);
VIR_FREE(statusfile);
dnsmasqContextFree(dctx);
+networkDriverUnlock();
+virObjectUnref(net);
return ret;
}
@@ -700,11 +705,9 @@ networkStateAutoStart(void)
if (!driver)
return;
-networkDriverLock();
virNetworkObjListForEach(driver->networks,
networkAutostartConfig,
NULL);
-networkDriverUnlock();
}
/**
@@ -2478,9 +2481,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr
conn,
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
-networkDriverLock();
network = virNetworkObjFindByUUID(driver->networks, uuid);
-networkDriverUnlock();
if (!network) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(uuid, uuidstr);
@@ -2506,9 +2507,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr
conn,
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
-networkDriverLock();
network = virNetworkObjFindByName(driver->networks, name);
-networkDriverUnlock();
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
_("no network with matching name '%s'"), name);
@@ -2532,12 +2531,10 @@ static int networkConnectNumOfNetworks(virConnectPtr
conn)
if (virConnectNumOfNetworksEnsureACL(conn) < 0)
return -1;
-networkDriverLock();
nactive = virNetworkObjListNumOfNetworks(driver->networks,
true,
virConnectNumOfNetworksCheckACL,
conn);
-networkDriverUnlock();
return nactive;
}
@@ -2548,12 +2545,10 @@ static int networkConnectListNetworks(virConnectPtr
conn, char **const names, in
if (virConnectListNetworksEnsureACL(conn) < 0)
return -1;
-networkDriverLock();
got = virNetworkObjListGetNames(driver->networks,
true, names, nnames,
virConnectListNetworksCheckACL,
conn);
-networkDriverUnlock();
return got;
}
@@ -2565,12 +2560,10 @@ static int
networkConnectNumOfDefinedNetworks(virConnectPtr conn)
if (virConnectNumOfDefinedNetworksEnsureACL(conn) < 0)
return -1;
-networkDriverLock();
ninactive = virNetworkObjListNumOfNetworks(driver->networks,
false,
virConnectNumOfDefinedNetworksCheckACL,
conn);
-networkDriverUnlock();
return ninactive;
}
@@ -2581,12 +2574,10 @@ static int
networkConnectListDefinedNetworks(virConnectPtr conn, char **const na
if (virConnectListDefinedNetworksEnsureACL(conn) < 0)
return -1;
-networkDriverLock();
got = virNetworkObjListGetNames(driver->networks,
false, names, nnames,
virConnectListDefinedNetworksCheckACL,
conn);
-networkDriverUnlock();
return got;
}
@@ -2602,11 +2593,9 @@ networkConnectListAllNetworks(virConnectPtr conn,
if (virConnectListAllNetworksEnsureACL(conn) < 0)
goto cleanup;
-networkDriverLock();
ret = virNetworkObjListExport(conn, driver->networks, nets,
virConnectListAllNetworksCheckACL,
flags);
-networkDriverUnlock();
cleanup:
return ret;
@@ -2917,8 +2906,6 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn,
const char *xml)
virNetworkPtr ret = NULL;
virObjectEventPtr event
[libvirt] [PATCH v2 17/24] virNetworkObjListPtr: Make APIs self-locking
Every API that touches internal structure of the object must lock
the object first. Not every API that has the object as an
argument needs to do that though. Some APIs just pass the object
to lower layers which, however, must lock the object then. Look
at the code, you'll get my meaning soon.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 46 +-
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 8cf9ffd..7af303e 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -157,16 +157,19 @@ virNetworkObjPtr
virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
const unsigned char *uuid)
{
+virNetworkObjPtr ret = NULL;
size_t i;
for (i = 0; i < nets->count; i++) {
virObjectLock(nets->objs[i]);
-if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
-return nets->objs[i];
+if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) {
+ret = nets->objs[i];
+break;
+}
virObjectUnlock(nets->objs[i]);
}
-return NULL;
+return ret;
}
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
@@ -184,16 +187,19 @@ virNetworkObjPtr
virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
const char *name)
{
+virNetworkObjPtr ret = NULL;
size_t i;
for (i = 0; i < nets->count; i++) {
virObjectLock(nets->objs[i]);
-if (STREQ(nets->objs[i]->def->name, name))
-return nets->objs[i];
+if (STREQ(nets->objs[i]->def->name, name)) {
+ret = nets->objs[i];
+break;
+}
virObjectUnlock(nets->objs[i]);
}
-return NULL;
+return ret;
}
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name)
@@ -467,13 +473,17 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
{
virNetworkObjPtr network;
-if ((network = virNetworkObjFindByName(nets, def->name))) {
+virObjectLock(nets);
+if ((network = virNetworkObjFindByNameLocked(nets, def->name))) {
+virObjectUnlock(nets);
virNetworkObjAssignDef(network, def, live);
return network;
}
-if (!(network = virNetworkObjNew()))
+if (!(network = virNetworkObjNew())) {
+virObjectUnlock(nets);
return NULL;
+}
virObjectLock(network);
if (VIR_APPEND_ELEMENT_COPY(nets->objs, nets->count, network) < 0)
@@ -481,9 +491,11 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
network->def = def;
network->persistent = !live;
+virObjectUnlock(nets);
return network;
error:
+virObjectUnlock(nets);
virObjectUnlock(network);
virObjectUnref(network);
return NULL;
@@ -654,6 +666,7 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
size_t i;
virObjectUnlock(net);
+virObjectLock(nets);
for (i = 0; i < nets->count; i++) {
virObjectLock(nets->objs[i]);
if (nets->objs[i] == net) {
@@ -664,6 +677,7 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
}
virObjectUnlock(nets->objs[i]);
}
+virObjectUnlock(nets);
}
/* return ips[index], or NULL if there aren't enough ips */
@@ -3158,6 +3172,7 @@ int virNetworkBridgeInUse(virNetworkObjListPtr nets,
size_t i;
unsigned int ret = 0;
+virObjectLock(nets);
for (i = 0; i < nets->count; i++) {
virObjectLock(nets->objs[i]);
if (nets->objs[i]->def->bridge &&
@@ -3166,6 +3181,7 @@ int virNetworkBridgeInUse(virNetworkObjListPtr nets,
ret = 1;
virObjectUnlock(nets->objs[i]);
}
+virObjectUnlock(nets);
return ret;
}
@@ -4325,6 +4341,7 @@ virNetworkObjListExport(virConnectPtr conn,
if (nets && VIR_ALLOC_N(tmp_nets, netobjs->count + 1) < 0)
goto cleanup;
+virObjectLock(netobjs);
for (i = 0; i < netobjs->count; i++) {
virNetworkObjPtr netobj = netobjs->objs[i];
virObjectLock(netobj);
@@ -4335,6 +4352,7 @@ virNetworkObjListExport(virConnectPtr conn,
netobj->def->name,
netobj->def->uuid))) {
virObjectUnlock(netobj);
+virObjectUnlock(netobjs);
goto cleanup;
}
tmp_nets[nnets] = net;
@@ -4343,6 +4361,7 @@ virNetworkObjListExport(virConnectPtr conn,
}
virObjectUnlock(netobj);
}
+virObjectUnlock(netobjs);
if (tmp_nets) {
/* trim the array to the final size */
@@ -4370,7 +4389,9 @@ virNetworkObjListExport(virConnectPtr conn,
* @opaque: pointer to pass to the @callback
*
* Function iterates over the list of network objects and calls
- *
[libvirt] [PATCH v2 12/24] network_conf: Introduce virNetworkObjEndAPI
This is practically copy of qemuDomObjEndAPI. The reason why is
it so widely available is to avoid code duplication, since the
function is going to be called from our bridge driver, test
driver and parallels driver too.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 20 ++--
src/conf/network_conf.h | 1 +
src/libvirt_private.syms | 1 +
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 88f1689..a821f6c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -130,6 +130,16 @@ virNetworkObjNew(void)
return NULL;
}
+void
+virNetworkObjEndAPI(virNetworkObjPtr *net)
+{
+if (!*net)
+return;
+
+virObjectUnlock(*net);
+*net = NULL;
+}
+
virNetworkObjListPtr virNetworkObjListNew(void)
{
virNetworkObjListPtr nets;
@@ -3030,8 +3040,8 @@ virNetworkLoadAllState(virNetworkObjListPtr nets,
if (!virFileStripSuffix(entry->d_name, ".xml"))
continue;
-if ((net = virNetworkLoadState(nets, stateDir, entry->d_name)))
-virObjectUnlock(net);
+net = virNetworkLoadState(nets, stateDir, entry->d_name);
+virNetworkObjEndAPI(&net);
}
closedir(dir);
@@ -3071,8 +3081,7 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets,
configDir,
autostartDir,
entry->d_name);
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(&net);
}
closedir(dir);
@@ -4239,8 +4248,7 @@ virNetworkObjIsDuplicate(virNetworkObjListPtr nets,
}
cleanup:
-if (net)
-virObjectUnlock(net);
+virNetworkObjEndAPI(&net);
return ret;
}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 1423676..e0ed714 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -276,6 +276,7 @@ struct _virNetworkObj {
};
virNetworkObjPtr virNetworkObjNew(void);
+void virNetworkObjEndAPI(virNetworkObjPtr *net);
typedef struct _virNetworkObjList virNetworkObjList;
typedef virNetworkObjList *virNetworkObjListPtr;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5dae05d..c770177 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -563,6 +563,7 @@ virNetworkIpDefPrefix;
virNetworkLoadAllConfigs;
virNetworkLoadAllState;
virNetworkObjAssignDef;
+virNetworkObjEndAPI;
virNetworkObjFindByName;
virNetworkObjFindByUUID;
virNetworkObjGetPersistentDef;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 20/24] test_driver: Drop testDriverLock() from almost everywhere
Well, if 'everywhere' is defined as that part of the driver code
that serves virNetwork* APIs. Again, we lower layers already have
their locks, so there's no point doing big lock.
Signed-off-by: Michal Privoznik
---
src/test/test_driver.c | 56 +-
1 file changed, 1 insertion(+), 55 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 90af80a..187bd3d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3495,10 +3495,7 @@ static virNetworkPtr
testNetworkLookupByUUID(virConnectPtr conn,
virNetworkObjPtr net;
virNetworkPtr ret = NULL;
-testDriverLock(privconn);
net = virNetworkObjFindByUUID(privconn->networks, uuid);
-testDriverUnlock(privconn);
-
if (net == NULL) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -3518,10 +3515,7 @@ static virNetworkPtr
testNetworkLookupByName(virConnectPtr conn,
virNetworkObjPtr net;
virNetworkPtr ret = NULL;
-testDriverLock(privconn);
net = virNetworkObjFindByName(privconn->networks, name);
-testDriverUnlock(privconn);
-
if (net == NULL) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -3540,11 +3534,8 @@ static int testConnectNumOfNetworks(virConnectPtr conn)
testConnPtr privconn = conn->privateData;
int numActive;
-testDriverLock(privconn);
numActive = virNetworkObjListNumOfNetworks(privconn->networks,
true, NULL, conn);
-testDriverUnlock(privconn);
-
return numActive;
}
@@ -3552,11 +3543,8 @@ static int testConnectListNetworks(virConnectPtr conn,
char **const names, int n
testConnPtr privconn = conn->privateData;
int n;
-testDriverLock(privconn);
n = virNetworkObjListGetNames(privconn->networks,
true, names, nnames, NULL, conn);
-testDriverUnlock(privconn);
-
return n;
}
@@ -3565,11 +3553,8 @@ static int testConnectNumOfDefinedNetworks(virConnectPtr
conn)
testConnPtr privconn = conn->privateData;
int numInactive;
-testDriverLock(privconn);
numInactive = virNetworkObjListNumOfNetworks(privconn->networks,
false, NULL, conn);
-testDriverUnlock(privconn);
-
return numInactive;
}
@@ -3577,11 +3562,8 @@ static int testConnectListDefinedNetworks(virConnectPtr
conn, char **const names
testConnPtr privconn = conn->privateData;
int n;
-testDriverLock(privconn);
n = virNetworkObjListGetNames(privconn->networks,
false, names, nnames, NULL, conn);
-testDriverUnlock(privconn);
-
return n;
}
@@ -3591,15 +3573,10 @@ testConnectListAllNetworks(virConnectPtr conn,
unsigned int flags)
{
testConnPtr privconn = conn->privateData;
-int ret = -1;
virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
-testDriverLock(privconn);
-ret = virNetworkObjListExport(conn, privconn->networks, nets, NULL, flags);
-testDriverUnlock(privconn);
-
-return ret;
+return virNetworkObjListExport(conn, privconn->networks, nets, NULL,
flags);
}
static int testNetworkIsActive(virNetworkPtr net)
@@ -3608,9 +3585,7 @@ static int testNetworkIsActive(virNetworkPtr net)
virNetworkObjPtr obj;
int ret = -1;
-testDriverLock(privconn);
obj = virNetworkObjFindByUUID(privconn->networks, net->uuid);
-testDriverUnlock(privconn);
if (!obj) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -3628,9 +3603,7 @@ static int testNetworkIsPersistent(virNetworkPtr net)
virNetworkObjPtr obj;
int ret = -1;
-testDriverLock(privconn);
obj = virNetworkObjFindByUUID(privconn->networks, net->uuid);
-testDriverUnlock(privconn);
if (!obj) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -3651,7 +3624,6 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr
conn, const char *xml)
virNetworkPtr ret = NULL;
virObjectEventPtr event = NULL;
-testDriverLock(privconn);
if ((def = virNetworkDefParseString(xml)) == NULL)
goto cleanup;
@@ -3671,7 +3643,6 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr
conn, const char *xml)
if (event)
testObjectEventQueue(privconn, event);
virNetworkObjEndAPI(&net);
-testDriverUnlock(privconn);
return ret;
}
@@ -3684,7 +3655,6 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn,
const char *xml)
virNetworkPtr ret = NULL;
virObjectEventPtr event = NULL;
-testDriverLock(privconn);
if ((def = virNetworkDefParseString(xml)) == NULL)
goto cleanup;
@@ -3703,7 +3673,6 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn,
const char *xml)
if (event)
testObjectEventQueue(privconn, event);
virN
[libvirt] [PATCH v2 05/24] bridge_driver: Adapt to new virNetworkObjList accessors
Signed-off-by: Michal Privoznik
---
src/network/bridge_driver.c | 334
1 file changed, 148 insertions(+), 186 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 2a61991..1878833 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -342,105 +342,87 @@ networkBridgeDummyNicName(const char *brname)
return nicname;
}
-/* Update the internal status of all allegedly active networks
- * according to external conditions on the host (i.e. anything that
- * isn't stored directly in each network's state file). */
-static void
-networkUpdateAllState(void)
+static int
+networkUpdateState(virNetworkObjPtr obj,
+ void *opaque ATTRIBUTE_UNUSED)
{
-size_t i;
+int ret = -1;
-for (i = 0; i < driver->networks->count; i++) {
-virNetworkObjPtr obj = driver->networks->objs[i];
+virNetworkObjLock(obj);
+if (!virNetworkObjIsActive(obj)) {
+virNetworkObjUnlock(obj);
+return 0;
+}
-virNetworkObjLock(obj);
-if (!virNetworkObjIsActive(obj)) {
-virNetworkObjUnlock(obj);
-continue;
-}
+switch (obj->def->forward.type) {
+case VIR_NETWORK_FORWARD_NONE:
+case VIR_NETWORK_FORWARD_NAT:
+case VIR_NETWORK_FORWARD_ROUTE:
+/* If bridge doesn't exist, then mark it inactive */
+if (!(obj->def->bridge && virNetDevExists(obj->def->bridge) == 1))
+obj->active = 0;
+break;
-switch (obj->def->forward.type) {
-case VIR_NETWORK_FORWARD_NONE:
-case VIR_NETWORK_FORWARD_NAT:
-case VIR_NETWORK_FORWARD_ROUTE:
-/* If bridge doesn't exist, then mark it inactive */
-if (!(obj->def->bridge && virNetDevExists(obj->def->bridge) == 1))
+case VIR_NETWORK_FORWARD_BRIDGE:
+if (!(obj->def->bridge && virNetDevExists(obj->def->bridge) == 1)) {
obj->active = 0;
break;
-
-case VIR_NETWORK_FORWARD_BRIDGE:
-if (obj->def->bridge) {
-if (virNetDevExists(obj->def->bridge) != 1)
-obj->active = 0;
-break;
-}
-/* intentionally drop through to common case for all
- * macvtap networks (forward='bridge' with no bridge
- * device defined is macvtap using its 'bridge' mode)
- */
-case VIR_NETWORK_FORWARD_PRIVATE:
-case VIR_NETWORK_FORWARD_VEPA:
-case VIR_NETWORK_FORWARD_PASSTHROUGH:
-/* so far no extra checks */
-break;
-
-case VIR_NETWORK_FORWARD_HOSTDEV:
-/* so far no extra checks */
-break;
-}
-
-/* Try and read dnsmasq/radvd pids of active networks */
-if (obj->active && obj->def->ips && (obj->def->nips > 0)) {
-char *radvdpidbase;
-
-ignore_value(virPidFileReadIfAlive(driver->pidDir,
- obj->def->name,
- &obj->dnsmasqPid,
-
dnsmasqCapsGetBinaryPath(driver->dnsmasqCaps)));
-radvdpidbase = networkRadvdPidfileBasename(obj->def->name);
-if (!radvdpidbase)
-break;
-ignore_value(virPidFileReadIfAlive(driver->pidDir,
- radvdpidbase,
- &obj->radvdPid, RADVD));
-VIR_FREE(radvdpidbase);
}
-
-virNetworkObjUnlock(obj);
+/* intentionally drop through to common case for all
+ * macvtap networks (forward='bridge' with no bridge
+ * device defined is macvtap using its 'bridge' mode)
+ */
+case VIR_NETWORK_FORWARD_PRIVATE:
+case VIR_NETWORK_FORWARD_VEPA:
+case VIR_NETWORK_FORWARD_PASSTHROUGH:
+/* so far no extra checks */
+break;
+
+case VIR_NETWORK_FORWARD_HOSTDEV:
+/* so far no extra checks */
+break;
}
-/* remove inactive transient networks */
-i = 0;
-while (i < driver->networks->count) {
-virNetworkObjPtr obj = driver->networks->objs[i];
-virNetworkObjLock(obj);
-
-if (!obj->persistent && !obj->active) {
-networkRemoveInactive(obj);
-continue;
-}
-
-virNetworkObjUnlock(obj);
-i++;
+/* Try and read dnsmasq/radvd pids of active networks */
+if (obj->active && obj->def->ips && (obj->def->nips > 0)) {
+char *radvdpidbase;
+
+ignore_value(virPidFileReadIfAlive(driver->pidDir,
+ obj->def->name,
+ &obj->dnsmasqPid,
+
dnsmasqCapsGetBinaryPath(driver->dnsmasqCaps)));
+radvdpidbase = networkRadvdPidfileBasename(obj->def->name);
+
[libvirt] [PATCH v2 06/24] test_driver: Adapt to new virNetworkObjList accessors
Signed-off-by: Michal Privoznik
---
src/test/test_driver.c | 64 ++
1 file changed, 12 insertions(+), 52 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 9591b7c..2bfe0ad 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3543,16 +3543,11 @@ static virNetworkPtr
testNetworkLookupByName(virConnectPtr conn,
static int testConnectNumOfNetworks(virConnectPtr conn)
{
testConnPtr privconn = conn->privateData;
-int numActive = 0;
-size_t i;
+int numActive;
testDriverLock(privconn);
-for (i = 0; i < privconn->networks->count; i++) {
-virNetworkObjLock(privconn->networks->objs[i]);
-if (virNetworkObjIsActive(privconn->networks->objs[i]))
-numActive++;
-virNetworkObjUnlock(privconn->networks->objs[i]);
-}
+numActive = virNetworkObjListNumOfNetworks(privconn->networks,
+ true, NULL, conn);
testDriverUnlock(privconn);
return numActive;
@@ -3560,44 +3555,24 @@ static int testConnectNumOfNetworks(virConnectPtr conn)
static int testConnectListNetworks(virConnectPtr conn, char **const names, int
nnames) {
testConnPtr privconn = conn->privateData;
-int n = 0;
-size_t i;
+int n;
testDriverLock(privconn);
-memset(names, 0, sizeof(*names)*nnames);
-for (i = 0; i < privconn->networks->count && n < nnames; i++) {
-virNetworkObjLock(privconn->networks->objs[i]);
-if (virNetworkObjIsActive(privconn->networks->objs[i]) &&
-VIR_STRDUP(names[n++], privconn->networks->objs[i]->def->name) <
0) {
-virNetworkObjUnlock(privconn->networks->objs[i]);
-goto error;
-}
-virNetworkObjUnlock(privconn->networks->objs[i]);
-}
+n = virNetworkObjListGetNames(privconn->networks,
+ true, names, nnames, NULL, conn);
testDriverUnlock(privconn);
return n;
-
- error:
-for (n = 0; n < nnames; n++)
-VIR_FREE(names[n]);
-testDriverUnlock(privconn);
-return -1;
}
static int testConnectNumOfDefinedNetworks(virConnectPtr conn)
{
testConnPtr privconn = conn->privateData;
-int numInactive = 0;
-size_t i;
+int numInactive;
testDriverLock(privconn);
-for (i = 0; i < privconn->networks->count; i++) {
-virNetworkObjLock(privconn->networks->objs[i]);
-if (!virNetworkObjIsActive(privconn->networks->objs[i]))
-numInactive++;
-virNetworkObjUnlock(privconn->networks->objs[i]);
-}
+numInactive = virNetworkObjListNumOfNetworks(privconn->networks,
+ false, NULL, conn);
testDriverUnlock(privconn);
return numInactive;
@@ -3605,29 +3580,14 @@ static int
testConnectNumOfDefinedNetworks(virConnectPtr conn)
static int testConnectListDefinedNetworks(virConnectPtr conn, char **const
names, int nnames) {
testConnPtr privconn = conn->privateData;
-int n = 0;
-size_t i;
+int n;
testDriverLock(privconn);
-memset(names, 0, sizeof(*names)*nnames);
-for (i = 0; i < privconn->networks->count && n < nnames; i++) {
-virNetworkObjLock(privconn->networks->objs[i]);
-if (!virNetworkObjIsActive(privconn->networks->objs[i]) &&
-VIR_STRDUP(names[n++], privconn->networks->objs[i]->def->name) <
0) {
-virNetworkObjUnlock(privconn->networks->objs[i]);
-goto error;
-}
-virNetworkObjUnlock(privconn->networks->objs[i]);
-}
+n = virNetworkObjListGetNames(privconn->networks,
+ false, names, nnames, NULL, conn);
testDriverUnlock(privconn);
return n;
-
- error:
-for (n = 0; n < nnames; n++)
-VIR_FREE(names[n]);
-testDriverUnlock(privconn);
-return -1;
}
static int
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Do not inline virNumaNodeIsAvailable
Explicitly request that virNumaNodeIsAvailable not be inlined. This fixes the test suite when building with clang (3.5.1). --- This only leaves the mysterious check-protocol failure. And too large stack frame size when building the tests with -O0. src/internal.h | 10 ++ src/util/virnuma.h | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/internal.h b/src/internal.h index 4d473af..84aa330 100644 --- a/src/internal.h +++ b/src/internal.h @@ -139,6 +139,16 @@ # endif /** + * ATTRIBUTE_NOINLINE: + * + * Macro to indicate a function that cannot be inlined + * (e.g. a function that is mocked in the test suite) + */ +# ifndef ATTRIBUTE_NOINLINE +# define ATTRIBUTE_NOINLINE __attribute__((__noinline__)) +# endif + +/** * ATTRIBUTE_SENTINEL: * * Macro to check for NULL-terminated varargs lists diff --git a/src/util/virnuma.h b/src/util/virnuma.h index 1f3c0ad..4ddcc5a 100644 --- a/src/util/virnuma.h +++ b/src/util/virnuma.h @@ -37,7 +37,8 @@ virBitmapPtr virNumaGetHostNodeset(void); bool virNumaNodesetIsAvailable(virBitmapPtr nodeset); bool virNumaIsAvailable(void); int virNumaGetMaxNode(void); -bool virNumaNodeIsAvailable(int node); +bool virNumaNodeIsAvailable(int node) +ATTRIBUTE_NOINLINE; int virNumaGetDistances(int node, int **distances, int *ndistances); -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 00/24] Drop network driver lock
Yet another version. This time with: 1) Peter's review worked in 2) Even more patches, that allow even more parallelism. With my test program [1], I've been able to go from 56s to 23s. 1: https://www.redhat.com/archives/libvir-list/2015-February/msg01214.html Michal Privoznik (24): network_conf: Introduce virNetworkObjListForEach network_conf: Introduce virNetworkObjListGetNames network_conf: Introduce virNetworkObjListNumOfNetworks network_conf: Introduce virNetworkObjListPrune bridge_driver: Adapt to new virNetworkObjList accessors test_driver: Adapt to new virNetworkObjList accessors parallels_network: Adapt to new virNetworkObjList accessors network_conf: Turn virNetworkObjList into virObject network_conf: Turn struct _virNetworkObjList private network_conf: Make virNetworkObj actually virObject virNetworkObjList: Derive from virObjectLockableClass network_conf: Introduce virNetworkObjEndAPI bridge_driver: Use virNetworkObjEndAPI test_driver: Use virNetworkObjEndAPI parallels_network: Use virNetworkObjEndAPI network_conf: Introduce locked versions of lookup functions virNetworkObjListPtr: Make APIs self-locking virNetworkObjFindBy*: Return an reference to found object bridge_driver: Drop networkDriverLock() from almost everywhere test_driver: Drop testDriverLock() from almost everywhere parallels_network: Drop parallelsDriverLock() from everywhere. bridge_driver: Use more of networkObjFromNetwork virNetworkObjUnsetDefTransient: Lock object list if needed virNetworkObjFindBy*: Don't lock all networks in the list cfg.mk| 3 - src/conf/network_conf.c | 422 +--- src/conf/network_conf.h | 52 ++-- src/libvirt_private.syms | 13 +- src/network/bridge_driver.c | 500 ++ src/parallels/parallels_network.c | 135 +++--- src/test/test_driver.c| 185 +++--- tests/networkxml2conftest.c | 4 +- tests/objectlocking.ml| 2 - 9 files changed, 634 insertions(+), 682 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 04/24] network_conf: Introduce virNetworkObjListPrune
The API will iterate over the list of network object and remove
desired ones from it.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 31 +++
src/conf/network_conf.h | 3 +++
src/libvirt_private.syms | 1 +
3 files changed, 35 insertions(+)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index dea180a..ea9a9d4 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -4381,3 +4381,34 @@ virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
return count;
}
+
+/**
+ * virNetworkObjListPrune:
+ * @nets: a list of network objects
+ * @flags: bitwise-OR of virConnectListAllNetworksFlags
+ *
+ * Iterate over list of network objects and remove the desired
+ * ones from it.
+ */
+void
+virNetworkObjListPrune(virNetworkObjListPtr nets,
+ unsigned int flags)
+{
+size_t i = 0;
+
+while (i < nets->count) {
+virNetworkObjPtr obj = nets->objs[i];
+
+virNetworkObjLock(obj);
+
+if (virNetworkMatch(obj, flags)) {
+virNetworkObjUnlock(obj);
+virNetworkObjFree(obj);
+
+VIR_DELETE_ELEMENT(nets->objs, i, nets->count);
+} else {
+virNetworkObjUnlock(obj);
+i++;
+}
+}
+}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index bd9e3b4..3fbd609 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -464,6 +464,9 @@ int virNetworkObjListNumOfNetworks(virNetworkObjListPtr
nets,
virNetworkObjListFilter filter,
virConnectPtr conn);
+void virNetworkObjListPrune(virNetworkObjListPtr nets,
+unsigned int flags);
+
/* for testing */
int
virNetworkDefUpdateSection(virNetworkDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 57c1a8c..4ce5e3a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -573,6 +573,7 @@ virNetworkObjListForEach;
virNetworkObjListFree;
virNetworkObjListGetNames;
virNetworkObjListNumOfNetworks;
+virNetworkObjListPrune;
virNetworkObjLock;
virNetworkObjReplacePersistentDef;
virNetworkObjSetDefTransient;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 01/24] network_conf: Introduce virNetworkObjListForEach
This API will be used in the future to call passed callback over
each network object in the list. It's slightly different to its
virDomainObjListForEach counterpart, because virDomainObjList
uses a hash table to store domain object, while virNetworkObjList
uses an array.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 27 +++
src/conf/network_conf.h | 6 ++
src/libvirt_private.syms | 1 +
3 files changed, 34 insertions(+)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index d0e7e1b..cb54e56 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -4290,3 +4290,30 @@ virNetworkObjListExport(virConnectPtr conn,
VIR_FREE(tmp_nets);
return ret;
}
+
+/**
+ * virNetworkObjListForEach:
+ * @nets: a list of network objects
+ * @callback: function to call over each of object in the list
+ * @opaque: pointer to pass to the @callback
+ *
+ * Function iterates over the list of network objects and calls
+ * passed callback over each one of them.
+ *
+ * Returns: 0 on success, -1 otherwise.
+ */
+int
+virNetworkObjListForEach(virNetworkObjListPtr nets,
+ virNetworkObjListIterator callback,
+ void *opaque)
+{
+int ret = 0;
+size_t i = 0;
+
+for (i = 0; i < nets->count; i++) {
+if (callback(nets->objs[i], opaque) < 0)
+ret = -1;
+}
+
+return ret;
+}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 16aea99..749c7fb 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -445,6 +445,12 @@ int virNetworkObjListExport(virConnectPtr conn,
virNetworkObjListFilter filter,
unsigned int flags);
+typedef int (*virNetworkObjListIterator)(virNetworkObjPtr net,
+ void *opaque);
+
+int virNetworkObjListForEach(virNetworkObjListPtr nets,
+ virNetworkObjListIterator callback,
+ void *opaque);
/* for testing */
int
virNetworkDefUpdateSection(virNetworkDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 13e0931..a7285d8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -569,6 +569,7 @@ virNetworkObjFree;
virNetworkObjGetPersistentDef;
virNetworkObjIsDuplicate;
virNetworkObjListExport;
+virNetworkObjListForEach;
virNetworkObjListFree;
virNetworkObjLock;
virNetworkObjReplacePersistentDef;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 23/24] virNetworkObjUnsetDefTransient: Lock object list if needed
This patch alone does not make much sense, I know. But it
prepares ground for next patch which when looking up a network in
the object list will not lock each network separately when
accessing its definition. Therefore we must have all the places
changing network definition lock the list.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 9 -
src/conf/network_conf.h | 3 ++-
src/network/bridge_driver.c | 4 ++--
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 3d318ce..007cebb 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -537,12 +537,19 @@ virNetworkObjSetDefTransient(virNetworkObjPtr network,
bool live)
* This *undoes* what virNetworkObjSetDefTransient did.
*/
void
-virNetworkObjUnsetDefTransient(virNetworkObjPtr network)
+virNetworkObjUnsetDefTransient(virNetworkObjListPtr nets,
+ virNetworkObjPtr network)
{
if (network->newDef) {
+virObjectRef(network);
+virObjectUnlock(network);
+virObjectLock(nets);
+virObjectLock(network);
+virObjectUnref(network);
virNetworkDefFree(network->def);
network->def = network->newDef;
network->newDef = NULL;
+virObjectUnlock(nets);
}
}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 3e926f7..c2e1885 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -323,7 +323,8 @@ void virNetworkObjAssignDef(virNetworkObjPtr network,
virNetworkDefPtr def,
bool live);
int virNetworkObjSetDefTransient(virNetworkObjPtr network, bool live);
-void virNetworkObjUnsetDefTransient(virNetworkObjPtr network);
+void virNetworkObjUnsetDefTransient(virNetworkObjListPtr nets,
+virNetworkObjPtr network);
virNetworkDefPtr virNetworkObjGetPersistentDef(virNetworkObjPtr network);
int virNetworkObjReplacePersistentDef(virNetworkObjPtr network,
virNetworkDefPtr def);
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 2eb225f..c112d50 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2415,7 +2415,7 @@ networkStartNetwork(virNetworkObjPtr network)
cleanup:
if (ret < 0) {
-virNetworkObjUnsetDefTransient(network);
+virNetworkObjUnsetDefTransient(driver->networks, network);
virErrorPtr save_err = virSaveLastError();
int save_errno = errno;
networkShutdownNetwork(network);
@@ -2469,7 +2469,7 @@ static int networkShutdownNetwork(virNetworkObjPtr
network)
VIR_HOOK_SUBOP_END);
network->active = 0;
-virNetworkObjUnsetDefTransient(network);
+virNetworkObjUnsetDefTransient(driver->networks, network);
return ret;
}
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 02/24] network_conf: Introduce virNetworkObjListGetNames
An accessor following pattern laid out by virDomainObjList* APIs.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 38 ++
src/conf/network_conf.h | 8
src/libvirt_private.syms | 1 +
3 files changed, 47 insertions(+)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index cb54e56..fdf5907 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -4317,3 +4317,41 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
return ret;
}
+
+int
+virNetworkObjListGetNames(virNetworkObjListPtr nets,
+ bool active,
+ char **names,
+ int nnames,
+ virNetworkObjListFilter filter,
+ virConnectPtr conn)
+{
+int got = 0;
+size_t i;
+
+for (i = 0; i < nets->count && got < nnames; i++) {
+virNetworkObjPtr obj = nets->objs[i];
+virNetworkObjLock(obj);
+if (filter && !filter(conn, obj->def)) {
+virNetworkObjUnlock(obj);
+continue;
+}
+
+if ((active && virNetworkObjIsActive(obj)) ||
+(!active && !virNetworkObjIsActive(obj))) {
+if (VIR_STRDUP(names[got], obj->def->name) < 0) {
+virNetworkObjUnlock(obj);
+goto error;
+}
+got++;
+}
+virNetworkObjUnlock(obj);
+}
+
+return got;
+
+ error:
+for (i = 0; i < got; i++)
+VIR_FREE(names[i]);
+return -1;
+}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 749c7fb..598ddc2 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -451,6 +451,14 @@ typedef int (*virNetworkObjListIterator)(virNetworkObjPtr
net,
int virNetworkObjListForEach(virNetworkObjListPtr nets,
virNetworkObjListIterator callback,
void *opaque);
+
+int virNetworkObjListGetNames(virNetworkObjListPtr nets,
+ bool active,
+ char **names,
+ int nnames,
+ virNetworkObjListFilter filter,
+ virConnectPtr conn);
+
/* for testing */
int
virNetworkDefUpdateSection(virNetworkDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a7285d8..f572592 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -571,6 +571,7 @@ virNetworkObjIsDuplicate;
virNetworkObjListExport;
virNetworkObjListForEach;
virNetworkObjListFree;
+virNetworkObjListGetNames;
virNetworkObjLock;
virNetworkObjReplacePersistentDef;
virNetworkObjSetDefTransient;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 24/24] virNetworkObjFindBy*: Don't lock all networks in the list
Ta-Da! Finally the commit you've been waiting for. This is yet
one of the bottlenecks. Each API has to go through the list of
network objects to find the correct one to work on. But currently
it's done in suboptimal way: every single network object is
locked, and then compared. If found, it's returned, if not it's
unlocked and the loop continues with its sibling. This is not
optimal as locking every network object can make us waiting for
other APIs finish their job. Therefore we serialize here
effectively. Fortunately, with previous patches we are sure that
network definition will not change as we traverse the list.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 26 --
src/conf/network_conf.h | 1 +
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 007cebb..a1bdaf5 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -162,13 +162,20 @@ virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
size_t i;
for (i = 0; i < nets->count; i++) {
-virObjectLock(nets->objs[i]);
if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) {
ret = nets->objs[i];
virObjectRef(ret);
break;
}
-virObjectUnlock(nets->objs[i]);
+}
+
+if (ret) {
+virObjectLock(ret);
+if (ret->removing) {
+virObjectUnref(ret);
+virObjectUnlock(ret);
+ret = NULL;
+}
}
return ret;
@@ -193,13 +200,20 @@ virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
size_t i;
for (i = 0; i < nets->count; i++) {
-virObjectLock(nets->objs[i]);
if (STREQ(nets->objs[i]->def->name, name)) {
ret = nets->objs[i];
virObjectRef(ret);
break;
}
-virObjectUnlock(nets->objs[i]);
+}
+
+if (ret) {
+virObjectLock(ret);
+if (ret->removing) {
+virObjectUnref(ret);
+virObjectUnlock(ret);
+ret = NULL;
+}
}
return ret;
@@ -675,17 +689,17 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
{
size_t i;
+net->removing = true;
virObjectUnlock(net);
virObjectLock(nets);
+virObjectLock(net);
for (i = 0; i < nets->count; i++) {
-virObjectLock(nets->objs[i]);
if (nets->objs[i] == net) {
VIR_DELETE_ELEMENT(nets->objs, i, nets->count);
virObjectUnlock(net);
virObjectUnref(net);
break;
}
-virObjectUnlock(nets->objs[i]);
}
virObjectUnlock(nets);
}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index c2e1885..29d13c9 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -273,6 +273,7 @@ struct _virNetworkObj {
unsigned long long floor_sum; /* sum of all 'floor'-s of attached NICs */
unsigned int taint;
+bool removing;
};
virNetworkObjPtr virNetworkObjNew(void);
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 03/24] network_conf: Introduce virNetworkObjListNumOfNetworks
An accessor following pattern laid out by virDomainObjList* APIs.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 26 ++
src/conf/network_conf.h | 5 +
src/libvirt_private.syms | 1 +
3 files changed, 32 insertions(+)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index fdf5907..dea180a 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -4355,3 +4355,29 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
VIR_FREE(names[i]);
return -1;
}
+
+int
+virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
+ bool active,
+ virNetworkObjListFilter filter,
+ virConnectPtr conn)
+{
+int count = 0;
+size_t i;
+
+for (i = 0; i < nets->count; i++) {
+virNetworkObjPtr obj = nets->objs[i];
+virNetworkObjLock(obj);
+if (filter && !filter(conn, obj->def)) {
+virNetworkObjUnlock(obj);
+continue;
+}
+
+if ((active && virNetworkObjIsActive(obj)) ||
+(!active && !virNetworkObjIsActive(obj)))
+count++;
+virNetworkObjUnlock(obj);
+}
+
+return count;
+}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 598ddc2..bd9e3b4 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -459,6 +459,11 @@ int virNetworkObjListGetNames(virNetworkObjListPtr nets,
virNetworkObjListFilter filter,
virConnectPtr conn);
+int virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
+ bool active,
+ virNetworkObjListFilter filter,
+ virConnectPtr conn);
+
/* for testing */
int
virNetworkDefUpdateSection(virNetworkDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f572592..57c1a8c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -572,6 +572,7 @@ virNetworkObjListExport;
virNetworkObjListForEach;
virNetworkObjListFree;
virNetworkObjListGetNames;
+virNetworkObjListNumOfNetworks;
virNetworkObjLock;
virNetworkObjReplacePersistentDef;
virNetworkObjSetDefTransient;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not inline virNumaNodeIsAvailable
On Thu, Mar 05, 2015 at 12:05:52PM +0100, Ján Tomko wrote: > Explicitly request that virNumaNodeIsAvailable not be inlined. > This fixes the test suite when building with clang (3.5.1). Huh, so clang will inline functions, even if they are exported in the .so library ? Is there some clang compiler flag we can use to stop that ? I'd only expect it to inline stuff which was declared static, or whose impl body was in the header file > --- > This only leaves the mysterious check-protocol failure. > And too large stack frame size when building the tests with -O0. > > src/internal.h | 10 ++ > src/util/virnuma.h | 3 ++- > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/src/internal.h b/src/internal.h > index 4d473af..84aa330 100644 > --- a/src/internal.h > +++ b/src/internal.h > @@ -139,6 +139,16 @@ > # endif > > /** > + * ATTRIBUTE_NOINLINE: > + * > + * Macro to indicate a function that cannot be inlined > + * (e.g. a function that is mocked in the test suite) > + */ > +# ifndef ATTRIBUTE_NOINLINE > +# define ATTRIBUTE_NOINLINE __attribute__((__noinline__)) > +# endif > + > +/** > * ATTRIBUTE_SENTINEL: > * > * Macro to check for NULL-terminated varargs lists > diff --git a/src/util/virnuma.h b/src/util/virnuma.h > index 1f3c0ad..4ddcc5a 100644 > --- a/src/util/virnuma.h > +++ b/src/util/virnuma.h > @@ -37,7 +37,8 @@ virBitmapPtr virNumaGetHostNodeset(void); > bool virNumaNodesetIsAvailable(virBitmapPtr nodeset); > bool virNumaIsAvailable(void); > int virNumaGetMaxNode(void); > -bool virNumaNodeIsAvailable(int node); > +bool virNumaNodeIsAvailable(int node) > +ATTRIBUTE_NOINLINE; > int virNumaGetDistances(int node, > int **distances, > int *ndistances); Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix build on mingw
Last commit unconditionally included a linux-specific header. Do not do that. --- Pushed as trivial. Forgot to send the mail. src/util/virnetdev.c | 4 src/util/virnetdev.h | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 36e69a3..971db43 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -47,6 +47,10 @@ # undef HAVE_STRUCT_IFREQ #endif +#if defined(SIOCETHTOOL) && defined(HAVE_STRUCT_IFREQ) +# include +#endif + #if HAVE_DECL_LINK_ADDR # include # include diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 643479d..856127b 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -32,7 +32,6 @@ # include "virpci.h" # include "device_conf.h" -# include # ifdef HAVE_STRUCT_IFREQ typedef struct ifreq virIfreq; # else -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Enable RBD storage driver in RHEL-7
On Thu, Mar 05, 2015 at 11:44:07 +0100, Peter Krempa wrote: > Use correct package names too as they differ. > --- > libvirt.spec.in | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) Self NACK. The two libraries are available only on x86_64. v2 comming soon. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] spec: Enable RBD storage driver in RHEL-7
Use correct package names too as they differ.
---
libvirt.spec.in | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 12bd17c..ce792e6 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -108,7 +108,7 @@
%define with_storage_iscsi0%{!?_without_storage_iscsi:%{server_drivers}}
%define with_storage_disk 0%{!?_without_storage_disk:%{server_drivers}}
%define with_storage_mpath0%{!?_without_storage_mpath:%{server_drivers}}
-%if 0%{?fedora} >= 16
+%if 0%{?fedora} >= 16 || 0%{?rhel} >= 7
%define with_storage_rbd 0%{!?_without_storage_rbd:%{server_drivers}}
%else
%define with_storage_rbd 0
@@ -182,6 +182,13 @@
%endif
%endif
+# librados and librbd are built only on x86_64 on rhel
+%ifnarch x86_64
+%if 0%{?rhel} >= 7
+%define with_storage_rbd 0
+%endif
+%endif
+
# RHEL doesn't ship OpenVZ, VBox, UML, PowerHypervisor,
# VMWare, libxenserver (xenapi), libxenlight (Xen 4.1 and newer),
# or HyperV.
@@ -566,7 +573,12 @@ BuildRequires: device-mapper-devel
%endif
%endif
%if %{with_storage_rbd}
+%if 0%{?rhel} >= 7
+BuildRequires: librados2-devel
+BuildRequires: librbd1-devel
+%else
BuildRequires: ceph-devel
+%endif
%endif
%if %{with_storage_gluster}
%if 0%{?rhel} >= 6
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] virsh: fix memtune to also accept 0 as valid value
On Thu, Mar 05, 2015 at 10:08:45AM +0100, Peter Krempa wrote:
> On Wed, Mar 04, 2015 at 17:17:05 +0100, Pavel Hrdina wrote:
>
> Commit message is too sparse.
>
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146539
>
> Also I doubt that this on itself resolves this bug.
Well, not directly so I'll remove it from commit message.
>
> >
> > Signed-off-by: Pavel Hrdina
> > ---
> > tools/virsh-domain.c | 62
> > +---
> > 1 file changed, 20 insertions(+), 42 deletions(-)
> >
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 55c269c..773f9f1 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -8276,7 +8276,7 @@ vshMemtuneGetSize(const vshCmd *cmd, const char
> > *name, long long *value)
> > if (virScaleInteger(&tmp, end, 1024, LLONG_MAX) < 0)
> > return -1;
> > *value = VIR_DIV_UP(tmp, 1024);
> > -return 0;
> > +return 1;
>
> Now that the return values from this function actually make sense it
> would be worth adding a comment what they mean.
Good point, I'll add a comment to this function.
>
> > }
> >
> > static bool
> > @@ -8294,6 +8294,7 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd)
> > bool current = vshCommandOptBool(cmd, "current");
> > bool config = vshCommandOptBool(cmd, "config");
> > bool live = vshCommandOptBool(cmd, "live");
> > +int rc = 0;
>
> rc doesn't need to be initialized as the macro below sets and tests it.
>
> >
> > VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> > VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> > @@ -8306,50 +8307,26 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd)
> > if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> > return false;
> >
> > -if (vshMemtuneGetSize(cmd, "hard-limit", &hard_limit) < 0 ||
> > -vshMemtuneGetSize(cmd, "soft-limit", &soft_limit) < 0 ||
> > -vshMemtuneGetSize(cmd, "swap-hard-limit", &swap_hard_limit) < 0 ||
> > -vshMemtuneGetSize(cmd, "min-guarantee", &min_guarantee) < 0) {
> > -vshError(ctl, "%s",
> > - _("Unable to parse integer parameter"));
> > -goto cleanup;
> > -}
> > +#define PARSE_MEMTUNE_PARAM(NAME, VALUE, FIELD)
> > \
> > +if ((rc = vshMemtuneGetSize(cmd, NAME, &VALUE)) < 0) {
> > \
> > +vshError(ctl, "%s", _("Unable to parse integer parameter
> > 'NAME'")); \
> > +goto cleanup;
> > \
> > +}
> > \
> > +if (rc == 1) {
> > \
> > +if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> > \
> > +FIELD, VALUE) < 0)
> > \
> > +goto save_error;
> > \
> > +}
> > \
> >
> > -if (hard_limit) {
> > -if (hard_limit == -1)
> > -hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> > -if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> > -VIR_DOMAIN_MEMORY_HARD_LIMIT,
> > -hard_limit) < 0)
> > -goto save_error;
> > -}
> >
> > -if (soft_limit) {
> > -if (soft_limit == -1)
> > -soft_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> > -if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> > -VIR_DOMAIN_MEMORY_SOFT_LIMIT,
> > -soft_limit) < 0)
> > -goto save_error;
> > -}
> > -
> > -if (swap_hard_limit) {
> > -if (swap_hard_limit == -1)
> > -swap_hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> > -if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> > -VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT,
> > -swap_hard_limit) < 0)
> > -goto save_error;
> > -}
> > +PARSE_MEMTUNE_PARAM("hard-limit", hard_limit,
> > VIR_DOMAIN_MEMORY_HARD_LIMIT);
> > +PARSE_MEMTUNE_PARAM("soft-limit", soft_limit,
> > VIR_DOMAIN_MEMORY_SOFT_LIMIT);
> > +PARSE_MEMTUNE_PARAM("swap-hard-limit", swap_hard_limit,
> > +VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT);
> > +PARSE_MEMTUNE_PARAM("min-guarantee", min_guarantee,
> > +VIR_DOMAIN_MEMORY_MIN_GUARANTEE);
> >
> > -if (min_guarantee) {
> > -if (min_guarantee == -1)
> > -min_guarantee = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
> > -if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> > -VIR_DOMAIN_MEMORY_MIN_GUARANTEE,
> > -
Re: [libvirt] [PATCH v3 1/9] Implement public API for virDomainGetIOThreadsInfo
On 03/04/2015 12:24 PM, Ján Tomko wrote:
> On Tue, Feb 17, 2015 at 04:03:50PM -0500, John Ferlan wrote:
>> Add virDomainGetIOThreadsInfo in order to return a list of
>> virDomainIOThreadsInfoPtr structures which list the IOThread ID,
>> the CPU Affinity map, and associated resources for each IOThread
>> for the domain. For an active domain, the live data will be
>> returned, while for an inactive domain, the config data will be
>> returned. The resources data is expected to be the src path for
>> the disks that have an assigned iothread.
>>
>> The API supports either the --live or --config flag, but not both.
>>
>> Also added virDomainIOThreadsInfoFree in order to free the cpumap,
>> list of resources, and the array entry structure.
>>
>> Signed-off-by: John Ferlan
>> ---
>> include/libvirt/libvirt-domain.h | 23 +++-
>> src/driver-hypervisor.h | 8 +++-
>> src/libvirt-domain.c | 80
>> +++-
>> src/libvirt_public.syms | 6 +++
>> 4 files changed, 114 insertions(+), 3 deletions(-)
>>
>>
>> /**
>> + * virIOThreadsInfo:
>
> s/Threads/Thread/ as it only contains info about one thread.
>
Since IOThreads was the "techology name" - I just followed suit and used
it, but will change it.
>> + *
>> + * The data structure for information about IOThreads in a domain
>> + */
>> +typedef struct _virDomainIOThreadsInfo virDomainIOThreadsInfo;
>> +typedef virDomainIOThreadsInfo *virDomainIOThreadsInfoPtr;
>> +struct _virDomainIOThreadsInfo {
>> +unsigned int iothread_id; /* IOThread ID */
>
> Since iothread ids are sequential, starting from 1, this value will
> always be the index in the array + 1.
>
Well - today they are, but once the "Add" and "Remove" functionality is
added we could have gaps since you'll be able to define/start with 3
threads and conceivably delete thread #2 (or the middle one).
>> +unsigned char *cpumap; /* CPU map for thread */
>> +int cpumaplen; /* cpumap size */
>
>> +size_t nresources; /* count of resources using IOThread
>> */
>> +char **resources; /* array of resources using IOThread
>> */
>
> "resources" is too vague.
Suggestion? Is "devices" better?
Today it's the disk source path, but I remember reading something where
an IOThread could be potentially used for something else (perhaps
network, but I cannot find the reference quickly).
> Moreover, storing the source path here does not seem that much useful to
> me, considering that the corresponding *Set API cannot change the
> resources used by the thread (also, some disks don't even have a path).
Adding a disk to a domain and assigning an iothread to it is
accomplished via the 'attach-disk'. There's a "finite" set of support
for now - a virtio-scsi local disk, which I thought had to have a path
defined as opposed to some remote disk where the "path" is generated.
>
> Considering that the XML format is copying , maybe this API
> could have a similar signature? I.e.:
>
> int
> virDomainGetIOThreadPin(virDomainPtr domain,
> unsigned int ncpumaps,
> unsigned char *cpumaps,
> unsigned int maplen,
> unsigned int flags)
>
> Or without the 'ncpumaps' parameter, so we don't need an API to get the
> number of iothreads.
>
> Another possibility is getting rid of the 'maplen' as well and return
> the bitmap as formatted by virBitmapFormat (but the client app still
> needs the node CPU count to interpret it correctly).
This is where I disagree. The "GetIOThreadsInfo" API is more like the
vcpuinfo API insomuch as it gets multiple aspects of IOThread objects.
As a way of understanding my thought process - I went with the one API
rather than an API to get IOThread data and a separate API to get
IOThread pin data because I felt a one round trip operation was 'better'
than 1..n round trips. If a separate API is requested or required, I'll
add it, but I'd prefer to not call it during the IOThreadsInfo fetch
operation.
Thanks for the review -
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/9] qemu: Implement the qemu driver fetch for IOThreads
On 03/04/2015 12:45 PM, Ján Tomko wrote:
> On Tue, Feb 17, 2015 at 04:03:52PM -0500, John Ferlan wrote:
>> Depending on the flags passed, either attempt to return the active/live
>> IOThread data for the domain or the config data.
>>
>> The active/live path will call into the Monitor in order to get the
>> IOThread data and then correlate the thread_id's returned from the
>> monitor to the currently running system/threads in order to ascertain
>> the affinity for each iothread_id.
>>
>> The config path will map each of the configured IOThreads and return
>> any configured iothreadspin data
>>
>> Both paths will peruse the 'targetDef' domain list looking for 'disks'
>> that have been assigned to a specific IOThread. An IOThread may have
>> no resources associated
>>
>> Signed-off-by: John Ferlan
>> ---
>> src/qemu/qemu_driver.c | 281
>> +
>> 1 file changed, 281 insertions(+)
>>
>
> Just a few nits, until the API gets final...
>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 1bbbe9b..2c9d08c 100644
>
>> +static int
>> +
>> +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
>> +goto cleanup;
>> +
>> +if (!virDomainObjIsActive(vm)) {
>> +virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> + _("cannot list IOThreads for an inactive domain"));
>> +goto endjob;
>> +}
>> +
>> +priv = vm->privateData;
>> +if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("IOThreads not supported with this binary"));
>> +goto endjob;
>> +}
>> +
>> +if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
>> +goto endjob;
>
> EnterMonitorAsync with ASYNC_JOB_NONE is essentially EnterMonitor
>
>> +for (i = 0; i < targetDef->iothreads; i++) {
>> +if (VIR_ALLOC(info_ret[i]) < 0)
>> +goto cleanup;
>> +
>> +/* IOThreads being counting at 1 */
>> +info_ret[i]->iothread_id = i + 1;
>> +
>> +if (VIR_ALLOC_N(info_ret[i]->cpumap, maplen) < 0)
>> +goto cleanup;
>> +
>> +/* Initialize the cpumap */
>> +info_ret[i]->cpumaplen = maplen;
>> +memset(info_ret[i]->cpumap, 0xff, maplen);
>> +if (maxcpu % 8)
>> +info_ret[i]->cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1;
>
> virBitmapToData could make this more readable.
>
This is where I'm not sure varying from the existing API's model w/r/t
cpumap & maplen, etc. should be done..
>> +}
>> +
>> +/* If iothreadspin setting exists, there are unused physical cpus */
>> +iothreadspin_list = targetDef->cputune.iothreadspin;
>
> A temporary variable pointing to iothreadspin[i] is more common.
>
straight copy of qemuDomainGetVcpuPinInfo
>> +for (i = 0; i < targetDef->cputune.niothreadspin; i++) {
>> +/* vcpuid is the iothread_id...
>> + * iothread_id is the index into info_ret + 1, so we can
>> + * assume that the info_ret index we want is vcpuid - 1
>> + */
>> +cpumap = info_ret[iothreadspin_list[i]->vcpuid - 1]->cpumap;
>> +cpumask = iothreadspin_list[i]->cpumask;
>> +
>> +for (pcpu = 0; pcpu < maxcpu; pcpu++) {
>> +if (virBitmapGetBit(cpumask, pcpu, &pinned) < 0)
>> +goto cleanup;
>> +if (!pinned)
>> +VIR_UNUSE_CPU(cpumap, pcpu);
>> +}
>> +}
>> +
tks -
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/9] virsh: Add 'iothreads' command
On 03/04/2015 12:48 PM, Ján Tomko wrote:
> On Tue, Feb 17, 2015 at 04:03:53PM -0500, John Ferlan wrote:
>> Add the 'iothreads' command to display IOThread Info data. Allow for
>> [--live] or [--config] options in order to display live or config data
>> for an active domain.
>>
>> An active domain may return:
>>
>> $ virsh iothreads $dom
>> IOThread ID CPU AffinityResource(s)
>> -
>> 1 2 /home/vm-images/f18
>> 2 3 /home/vm-images/iothr-vol1
>> 3 0
>>
>> $ echo $?
>> 0
>>
>> For domains which don't have IOThreads the following is returned:
>>
>> $ virsh iothreads $dom
>> error: No IOThreads found for the domain
>>
>> $ echo $?
>> 0
>
> Printing an error, but not returning one seems strange. Do we do this
> somewhere else too?
>
Hmm.. Cannot remember my reference on this... Searched on "No " in
virsh-domain.c and found "No current block job"..
I could change to using vshPrintExtra for the same text if that seems
better.
>> +
>> +vshPrintExtra(ctl, " %-15s %-15s %-15s\n",
>> + _("IOThread ID"), _("CPU Affinity"), _("Resource(s)"));
>> +vshPrintExtra(ctl,
>> "-\n");
>> +for (i = 0; i < niothreads; i++) {
>> +
>> +vshPrintExtra(ctl, " %-15u ", info[i]->iothread_id);
>
> vshPrint should be used for the actual data, to make the command work
> with --quiet.
>
Over aggressive with copy-n-paste...
tks,
John
>> +ignore_value(vshPrintPinInfo(info[i]->cpumap, info[i]->cpumaplen,
>> + maxcpu, 0));
>> +for (j = 0; j < info[i]->nresources; j++) {
>> +if (j == 0)
>> +vshPrintExtra(ctl, "\t\t %s", info[i]->resources[j]);
>> +else
>> +vshPrintExtra(ctl, "\n %-15s %s\t\t %s",
>> + " ", " ", info[i]->resources[j]);
>> +}
>
> Jan
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 5/9] Implement public API for virDomainSetIOThreads
On 03/04/2015 01:00 PM, Ján Tomko wrote:
> On Tue, Feb 17, 2015 at 04:03:54PM -0500, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1135491
>>
>> Add the libvirt API infrastructure to support setting IOThread data.
>> For now this is the pinned CPU information, but eventually will also
>> support hot plug add/del
>>
>> The API will support the LIVE, CONFIG, or CURRENT flags. If the
>> VIR_DOMAIN_IOTHREADS_PIN flag is not provided, it's left up to the
>> hypervisor to handle, but when not provided the cpumap/maplen arguments
>> are not checked.
>>
>> Signed-off-by: John Ferlan
>> ---
>> include/libvirt/libvirt-domain.h | 16 ++
>> src/driver-hypervisor.h | 8 +
>> src/libvirt-domain.c | 69
>>
>> src/libvirt_public.syms | 1 +
>> 4 files changed, 94 insertions(+)
>>
>> diff --git a/include/libvirt/libvirt-domain.h
>> b/include/libvirt/libvirt-domain.h
>> index 9dcc424..e07db16 100644
>> --- a/include/libvirt/libvirt-domain.h
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -1582,11 +1582,27 @@ struct _virDomainIOThreadsInfo {
>> char **resources; /* array of resources using IOThread
>> */
>> };
>>
>> +/* Flags for controlling virtual IOThread pinning. */
>> +typedef enum {
>> +/* See virDomainModificationImpact for these flags. */
>> +VIR_DOMAIN_IOTHREADS_CURRENT = VIR_DOMAIN_AFFECT_CURRENT,
>> +VIR_DOMAIN_IOTHREADS_LIVE= VIR_DOMAIN_AFFECT_LIVE,
>> +VIR_DOMAIN_IOTHREADS_CONFIG = VIR_DOMAIN_AFFECT_CONFIG,
>> +
>> +/* Additionally, these flags may be bitwise-OR'd in. */
>> +VIR_DOMAIN_IOTHREADS_PIN = (1 << 2), /* thread_id to pin using cpumap */
>> +} virDomainIOThreadsFlags;
>> +
>> void virDomainIOThreadsInfoFree(virDomainIOThreadsInfoPtr
>> info);
>>
>> int virDomainGetIOThreadsInfo(virDomainPtr domain,
>> virDomainIOThreadsInfoPtr
>> **info,
>> unsigned int flags);
>> +int virDomainSetIOThreads(virDomainPtr domain,
>> + unsigned int iothread_val,
>> + unsigned char *cpumap,
>> + int maplen,
>> + unsigned int flags);
>>
>> /**
>> * VIR_USE_CPU:
>> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
>> index 2ce1a51..120d761 100644
>> --- a/src/driver-hypervisor.h
>> +++ b/src/driver-hypervisor.h
>> @@ -386,6 +386,13 @@ typedef int
>> unsigned int flags);
>>
>> typedef int
>> +(*virDrvDomainSetIOThreads)(virDomainPtr domain,
>> +unsigned int iothread_val,
>> +unsigned char *cpumap,
>> +int maplen,
>> +unsigned int flags);
>> +
>> +typedef int
>> (*virDrvDomainGetSecurityLabel)(virDomainPtr domain,
>> virSecurityLabelPtr seclabel);
>>
>> @@ -1260,6 +1267,7 @@ struct _virHypervisorDriver {
>> virDrvDomainGetVcpus domainGetVcpus;
>> virDrvDomainGetMaxVcpus domainGetMaxVcpus;
>> virDrvDomainGetIOThreadsInfo domainGetIOThreadsInfo;
>> +virDrvDomainSetIOThreads domainSetIOThreads;
>> virDrvDomainGetSecurityLabel domainGetSecurityLabel;
>> virDrvDomainGetSecurityLabelList domainGetSecurityLabelList;
>> virDrvNodeGetSecurityModel nodeGetSecurityModel;
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index f893b35..bf73773 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -7969,6 +7969,75 @@ virDomainIOThreadsInfoFree(virDomainIOThreadsInfoPtr
>> info)
>>
>>
>> /**
>> + * virDomainSetIOThreads:
>> + * @domain: a domain object
>> + * @iothread_val: either the thread_id to modify or a count of IOThreads
>> + * to be added or removed from the domain depending on the @flags
>> setting
>
> IMO this would look nicer as two separate APIs:
> virDomainSetIOThreadPin
> virDomainSetIOThreadCount
>
OK - fair enough. I was hoping to be able to "reuse" code, but changing
this to SetIOThreadPin is fine.
The other API will end up being the Add/Remove an IOThread, but will
require a bit of internal plumbing changes to allow for a removal of a
thread in the middle of the array.
Tks -
John
>> + * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN)
>> + * Each bit set to 1 means that corresponding CPU is usable.
>> + * Bytes are stored in little-endian order: CPU0-7, 8-15...
>> + * In each byte, lowest CPU number is least significant bit.
>> + * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in
>> + * underlying virtualization system (Xen...).
>> + * If maplen < size, missing bytes are set to zero.
>> + * If maple
Re: [libvirt] [PATCH 2/3] virutil: introduce helper to set memory limits
On Thu, Mar 05, 2015 at 10:11:50AM +0100, Peter Krempa wrote: > On Wed, Mar 04, 2015 at 17:17:06 +0100, Pavel Hrdina wrote: > > Using this macro will ensure that the value stored in domain def will > > never be greater than VIR_DOMAIN_MEMORY_PARAM_UNLIMITED. > > > > Signed-off-by: Pavel Hrdina > > --- > > src/util/virutil.h | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/src/util/virutil.h b/src/util/virutil.h > > index d1173c1..003eee6 100644 > > --- a/src/util/virutil.h > > +++ b/src/util/virutil.h > > @@ -98,6 +98,10 @@ const char *virEnumToString(const char *const*types, > > const char *name ## TypeToString(int type); \ > > int name ## TypeFromString(const char*type); > > > > +# define VIR_SET_MEMORY_LIMIT(limit, value) \ > > +limit = (value) < VIR_DOMAIN_MEMORY_PARAM_UNLIMITED ? (value) : \ > > +VIR_DOMAIN_MEMORY_PARAM_UNLIMITED > > You should document that @value cannot have side effects as it's > evaluated twice. > > Also why don't you make an inline function that returns the truncated > value? I was not sure whether to use MACRO or function, but as there is a one usage where the value would be evaluated twice, I'll change it to function. Additionally as suggested in next patch, I'll also create a function called virMemoryLimitIsSet because that code is repeated a lot. Pavel > > Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 01/24] network_conf: Introduce virNetworkObjListForEach
On Thu, Mar 05, 2015 at 12:05:02 +0100, Michal Privoznik wrote: > This API will be used in the future to call passed callback over > each network object in the list. It's slightly different to its > virDomainObjListForEach counterpart, because virDomainObjList > uses a hash table to store domain object, while virNetworkObjList > uses an array. > > Signed-off-by: Michal Privoznik > --- > src/conf/network_conf.c | 27 +++ > src/conf/network_conf.h | 6 ++ > src/libvirt_private.syms | 1 + > 3 files changed, 34 insertions(+) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] domain_conf: Format without address correctly
We have something like pvpanic device. However, in some cases it does
not have any address assigned, in which case we produce this ugly XML
(still valid though):
/usr/bin/qemu
...
Lets format "" instead.
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 20 ++-
.../qemuxml2argv-panic-no-address.args | 5
.../qemuxml2argv-panic-no-address.xml | 29 ++
tests/qemuxml2argvtest.c | 3 +++
tests/qemuxml2xmltest.c| 1 +
5 files changed, 52 insertions(+), 6 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6da02b0..a02d656 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18750,13 +18750,21 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
static int virDomainPanicDefFormat(virBufferPtr buf,
virDomainPanicDefPtr def)
{
-virBufferAddLit(buf, "\n");
-virBufferAdjustIndent(buf, 2);
-if (virDomainDeviceInfoFormat(buf, &def->info, 0) < 0)
-return -1;
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
+virBuffer childrenBuf = VIR_BUFFER_INITIALIZER;
+int indent = virBufferGetIndent(buf, false);
+virBufferAddLit(buf, "info, 0) < 0)
+return -1;
+if (virBufferUse(&childrenBuf)) {
+virBufferAddLit(buf, ">\n");
+virBufferAddBuffer(buf, &childrenBuf);
+virBufferAddLit(buf, "\n");
+} else {
+virBufferAddLit(buf, "/>\n");
+}
+virBufferFreeAndReset(&childrenBuf);
return 0;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.args
b/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.args
new file mode 100644
index 000..3cbd688
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.args
@@ -0,0 +1,5 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
+-hda /dev/HostVG/QEMUGuest1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 -device pvpanic
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml
b/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml
new file mode 100644
index 000..79f8a1e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml
@@ -0,0 +1,29 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 00fd044..e2ad713 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1522,6 +1522,9 @@ mymain(void)
DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC,
QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+DO_TEST("panic-no-address", QEMU_CAPS_DEVICE_PANIC,
+QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+
DO_TEST("fips-enabled", QEMU_CAPS_ENABLE_FIPS);
DO_TEST("shmem", QEMU_CAPS_PCIDEVICE,
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 58917c3..9e85d4e 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -400,6 +400,7 @@ mymain(void)
DO_TEST("pcihole64-q35");
DO_TEST("panic");
+DO_TEST("panic-no-address");
DO_TEST_DIFFERENT("disk-backing-chains");
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Remove duplicate entries in by namespace
On 04.03.2015 10:06, Peter Krempa wrote: > Since the APIs support just one element per namespace and while > modifying an element all duplicates would be removed, let's do this > right away in the post parse callback. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1190590 > --- > src/conf/domain_conf.c | 42 > ++ > .../qemuxml2argv-metadata-duplicate.xml| 34 ++ > .../qemuxml2xmlout-metadata-duplicate.xml | 31 > tests/qemuxml2xmltest.c| 1 + > 4 files changed, 108 insertions(+) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-metadata-duplicate.xml > create mode 100644 > tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata-duplicate.xml ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 03/24] network_conf: Introduce virNetworkObjListNumOfNetworks
On Thu, Mar 05, 2015 at 12:05:04 +0100, Michal Privoznik wrote: > An accessor following pattern laid out by virDomainObjList* APIs. > > Signed-off-by: Michal Privoznik > --- ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 02/24] network_conf: Introduce virNetworkObjListGetNames
On Thu, Mar 05, 2015 at 12:05:03 +0100, Michal Privoznik wrote: > An accessor following pattern laid out by virDomainObjList* APIs. > > Signed-off-by: Michal Privoznik > --- > src/conf/network_conf.c | 38 ++ > src/conf/network_conf.h | 8 > src/libvirt_private.syms | 1 + > 3 files changed, 47 insertions(+) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] libxl: a few minor patches
On 04.03.2015 01:09, Jim Fehlig wrote: > This series contains a few simple changes to the libxl driver. > Patches 1 and 2 were included in > > https://www.redhat.com/archives/libvir-list/2015-February/msg00611.html > > To make an upcoming V2 of that series easier to review, I removed the > mostly unrelated patches and include them here, along with one other > small fix. > > Jim Fehlig (3): > libxl: remove redundant calls to libxl_evdisable_domain_death > libxl: use libxl_ctx passed to libxlConsoleCallback > libxl: remove unneeded cleanup_unlock label > > src/libxl/libxl_domain.c | 13 ++--- > src/libxl/libxl_driver.c | 10 +++--- > 2 files changed, 5 insertions(+), 18 deletions(-) > ACK to all, but see my comment to 3/3. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/24] network_conf: Introduce virNetworkObjListPrune
On Thu, Mar 05, 2015 at 12:05:05 +0100, Michal Privoznik wrote: > The API will iterate over the list of network object and remove > desired ones from it. > > Signed-off-by: Michal Privoznik > --- > src/conf/network_conf.c | 31 +++ > src/conf/network_conf.h | 3 +++ > src/libvirt_private.syms | 1 + > 3 files changed, 35 insertions(+) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] libxl: remove unneeded cleanup_unlock label
On 04.03.2015 01:09, Jim Fehlig wrote: > In the old days of a global driver lock, it was necessary to unlock > the driver after a domain restore operation. When the global lock > was removed from the driver, some remnants were left behind in > libxlDomainRestoreFlags. Remove this unneeded (and incorrect) code. > > Signed-off-by: Jim Fehlig > --- > src/libxl/libxl_driver.c | 10 +++--- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index ce3a99b..55fa64f 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -1464,17 +1464,17 @@ libxlDomainRestoreFlags(virConnectPtr conn, const > char *from, > > fd = libxlDomainSaveImageOpen(driver, cfg, from, &def, &hdr); > if (fd < 0) > -goto cleanup_unlock; > +return -1; While this one is okay, ... > > if (virDomainRestoreFlagsEnsureACL(conn, def) < 0) > -goto cleanup_unlock; > +return -1; .. this one's not. If EnsureACL fails, the @fd opened just above is leaked. ACKed with s/return -1/goto cleanup/. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 05/24] bridge_driver: Adapt to new virNetworkObjList accessors
On Thu, Mar 05, 2015 at 12:05:06 +0100, Michal Privoznik wrote: > Signed-off-by: Michal Privoznik > --- > src/network/bridge_driver.c | 334 > > 1 file changed, 148 insertions(+), 186 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v8 1/4] domifaddr: Implement the public APIs
On Mon, Jan 26, 2015 at 12:08:46AM +0530, Nehal J Wani wrote:
> Define helper function virDomainInterfaceFree, which allows
> the upper layer application to free the domain interface object
> conveniently.
>
> The API is going to provide multiple methods by flags, e.g.
> * Query guest agent
> * Parse DHCP lease file
>
> include/libvirt/libvirt-domain.h
> * Define virDomainInterfaceAddresses, virDomainInterfaceFree
> * Define structs virDomainInterface, virDomainIPAddress
>
> src/driver-hypervisor.h:
> * Define domainInterfaceAddresses
>
> src/libvirt-domain.c:
> * Implement virDomainInterfaceAddresses
> * Implement virDomainInterfaceFree
>
> src/libvirt_public.syms:
> * Export the new symbols
>
> Signed-off-by: Nehal J Wani
> ---
> include/libvirt/libvirt-domain.h | 27
> src/driver-hypervisor.h | 5 ++
> src/libvirt-domain.c | 129
> +++
> src/libvirt_public.syms | 6 ++
> 4 files changed, 167 insertions(+)
>
> diff --git a/include/libvirt/libvirt-domain.h
> b/include/libvirt/libvirt-domain.h
> index 4dbd7f5..1f832d0 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -3682,5 +3682,32 @@ typedef struct _virTypedParameter virMemoryParameter;
> */
> typedef virMemoryParameter *virMemoryParameterPtr;
>
> +typedef enum {
> +VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE = (1 << 0), /* Parse DHCP lease
> file */
> +VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT = (1 << 1), /* Query qemu guest
> agent */
> +} virDomainInterfaceAddressesFlags;
> +
> +typedef struct _virDomainInterfaceIPAddress virDomainIPAddress;
> +typedef virDomainIPAddress *virDomainIPAddressPtr;
> +struct _virDomainInterfaceIPAddress {
> +int type;/* virIPAddrType */
> +char *addr; /* IP address */
> +unsigned int prefix; /* IP address prefix */
> +};
> +
> +typedef struct _virDomainInterface virDomainInterface;
> +typedef virDomainInterface *virDomainInterfacePtr;
> +struct _virDomainInterface {
> +char *name; /* interface name */
> +char *hwaddr; /* hardware address */
> +unsigned int naddrs;/* number of items in @addrs */
> +virDomainIPAddressPtr addrs;/* array of IP addresses */
> +};
> +
> +int virDomainInterfaceAddresses(virDomainPtr dom,
> +virDomainInterfacePtr **ifaces,
> +unsigned int flags);
> +
> +void virDomainInterfaceFree(virDomainInterfacePtr iface);
>
> #endif /* __VIR_LIBVIRT_DOMAIN_H__ */
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index a1d2a0a..174c7bd 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -1174,6 +1174,10 @@ typedef int
> unsigned int cellCount,
> unsigned int flags);
>
> +typedef int
> +(*virDrvDomainInterfaceAddresses)(virDomainPtr dom,
> + virDomainInterfacePtr **ifaces,
> + unsigned int flags);
>
> typedef struct _virHypervisorDriver virHypervisorDriver;
> typedef virHypervisorDriver *virHypervisorDriverPtr;
> @@ -1401,6 +1405,7 @@ struct _virHypervisorDriver {
> virDrvConnectGetAllDomainStats connectGetAllDomainStats;
> virDrvNodeAllocPages nodeAllocPages;
> virDrvDomainGetFSInfo domainGetFSInfo;
> +virDrvDomainInterfaceAddresses domainInterfaceAddresses;
> };
>
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 492e90a..4149332 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11249,3 +11249,132 @@ virDomainFSInfoFree(virDomainFSInfoPtr info)
> VIR_FREE(info->devAlias[i]);
> VIR_FREE(info->devAlias);
> }
> +
> +/**
> + * virDomainInterfaceAddresses:
> + * @dom: domain object
> + * @ifaces: pointer to an array of pointers pointing to interface objects
> + * @flags: bitwise-OR of virDomainInterfaceAddressesFlags
> + *
> + * Return a pointer to the allocated array of pointers pointing to interfaces
> + * present in given domain along with their IP and MAC addresses. Note that
> + * single interface can have multiple or even 0 IP address.
> + *
> + * This API dynamically allocates the virDomainInterfacePtr struct based on
> + * how many interfaces domain @dom has, usually there's 1:1 correlation. The
> + * count of the interfaces is returned as the return value.
> + *
> + * In case @flags includes VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT, a configured
> + * guest agent is needed for successful return from this API. Moreover, if
> + * guest agent is used then the interface name is the one seen by guest OS.
> + * To match such interface with the one from @dom XML use MAC address or IP
> + * range.
> + *
> + * The lease-file parsing method returns the interface name of the form
> "vnetN",
> + * which is different from what guest agent returns (like
Re: [libvirt] [PATCH v2 07/24] parallels_network: Adapt to new virNetworkObjList accessors
On Thu, Mar 05, 2015 at 12:05:08 +0100, Michal Privoznik wrote: > Signed-off-by: Michal Privoznik > --- > src/parallels/parallels_network.c | 66 > +++ > 1 file changed, 12 insertions(+), 54 deletions(-) ACK Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/24] test_driver: Adapt to new virNetworkObjList accessors
On Thu, Mar 05, 2015 at 12:05:07 +0100, Michal Privoznik wrote: > Signed-off-by: Michal Privoznik > --- > src/test/test_driver.c | 64 > ++ > 1 file changed, 12 insertions(+), 52 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 09/24] network_conf: Turn struct _virNetworkObjList private
On Thu, Mar 05, 2015 at 12:05:10 +0100, Michal Privoznik wrote: > Now that all the code uses accessors, don't expose the structure > anyway. > > Signed-off-by: Michal Privoznik > --- > src/conf/network_conf.c | 7 +++ > src/conf/network_conf.h | 6 -- > 2 files changed, 7 insertions(+), 6 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 08/24] network_conf: Turn virNetworkObjList into virObject
On Thu, Mar 05, 2015 at 12:05:09 +0100, Michal Privoznik wrote: > Well, one day this will be self-locking object, but not today. > But lets prepare the code for that! Moreover, > virNetworkObjListFree() is no longer needed, so turn it into > virNetworkObjListDispose(). > > Signed-off-by: Michal Privoznik > --- > cfg.mk| 1 - > src/conf/network_conf.c | 53 > +-- > src/conf/network_conf.h | 11 > src/libvirt_private.syms | 2 +- > src/network/bridge_driver.c | 5 ++-- > src/parallels/parallels_network.c | 7 +++--- > src/test/test_driver.c| 13 -- > 7 files changed, 57 insertions(+), 35 deletions(-) I still dislike the fact that you move the virNetworkObjTaint() function but I don't want to waste more time on it than writing this rant. ACK Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] virsh: fix memtune to also accept 0 as valid value
Signed-off-by: Pavel Hrdina
---
tools/virsh-domain.c | 83
1 file changed, 38 insertions(+), 45 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 55c269c..3836a1b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8255,6 +8255,22 @@ static const vshCmdOptDef opts_memtune[] = {
{.name = NULL}
};
+/**
+ * vshMemtuneGetSize
+ *
+ * @cmd: pointer to vshCmd
+ * @name: name of a parameter for which we would like to get a value
+ * @value: pointer to variable where the value will be stored
+ *
+ * This function will parse virsh command line in order to load a value of
+ * specified parameter. If the value is -1 we will handle it as unlimited and
+ * use VIR_DOMAIN_MEMORY_PARAM_UNLIMITED instead.
+ *
+ * Returns:
+ * >0 if option found and valid (@value updated)
+ * 0 if option not found and not required (@value untouched)
+ * <0 in all other cases (@value untouched)
+ */
static int
vshMemtuneGetSize(const vshCmd *cmd, const char *name, long long *value)
{
@@ -8276,7 +8292,7 @@ vshMemtuneGetSize(const vshCmd *cmd, const char *name,
long long *value)
if (virScaleInteger(&tmp, end, 1024, LLONG_MAX) < 0)
return -1;
*value = VIR_DIV_UP(tmp, 1024);
-return 0;
+return 1;
}
static bool
@@ -8294,6 +8310,7 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd)
bool current = vshCommandOptBool(cmd, "current");
bool config = vshCommandOptBool(cmd, "config");
bool live = vshCommandOptBool(cmd, "live");
+int rc;
VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
@@ -8306,50 +8323,26 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
-if (vshMemtuneGetSize(cmd, "hard-limit", &hard_limit) < 0 ||
-vshMemtuneGetSize(cmd, "soft-limit", &soft_limit) < 0 ||
-vshMemtuneGetSize(cmd, "swap-hard-limit", &swap_hard_limit) < 0 ||
-vshMemtuneGetSize(cmd, "min-guarantee", &min_guarantee) < 0) {
-vshError(ctl, "%s",
- _("Unable to parse integer parameter"));
-goto cleanup;
-}
-
-if (hard_limit) {
-if (hard_limit == -1)
-hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
-if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
-VIR_DOMAIN_MEMORY_HARD_LIMIT,
-hard_limit) < 0)
-goto save_error;
-}
-
-if (soft_limit) {
-if (soft_limit == -1)
-soft_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
-if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
-VIR_DOMAIN_MEMORY_SOFT_LIMIT,
-soft_limit) < 0)
-goto save_error;
-}
-
-if (swap_hard_limit) {
-if (swap_hard_limit == -1)
-swap_hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
-if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
-VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT,
-swap_hard_limit) < 0)
-goto save_error;
-}
-
-if (min_guarantee) {
-if (min_guarantee == -1)
-min_guarantee = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
-if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
-VIR_DOMAIN_MEMORY_MIN_GUARANTEE,
-min_guarantee) < 0)
-goto save_error;
-}
+#define PARSE_MEMTUNE_PARAM(NAME, VALUE, FIELD) \
+if ((rc = vshMemtuneGetSize(cmd, NAME, &VALUE)) < 0) { \
+vshError(ctl, "%s", _("Unable to parse integer parameter 'NAME'")); \
+goto cleanup; \
+} \
+if (rc == 1) { \
+if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, \
+FIELD, VALUE) < 0) \
+goto save_error;\
+} \
+
+
+PARSE_MEMTUNE_PARAM("hard-limit", hard_limit,
VIR_DOMAIN_MEMORY_HARD_LIMIT);
+PARSE_MEMTUNE_PARAM("soft-limit", soft_limit,
VIR_DOMAIN_MEMORY_SOFT_LIMIT);
+PARSE_MEMTUNE_PARAM("swap-hard-limit", swap_hard_limit,
+VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT);
+PARSE_MEMTUNE_PARAM("min-guarantee", min_guarantee,
+VIR_DOMAIN_MEMORY_MIN_GUARANTEE);
+
+#undef PARSE_MEMTUNE_PARAM
if (nparams == 0) {
/* get the number of memory parameters */
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.co
Re: [libvirt] [PATCH v2 10/24] network_conf: Make virNetworkObj actually virObject
On Thu, Mar 05, 2015 at 12:05:11 +0100, Michal Privoznik wrote: > So far it's just a structure which happens to have 'Obj' in its > name, but otherwise it not related to virObject at all. No > reference counting, not virObjectLock(), nothing. > > Signed-off-by: Michal Privoznik > --- > cfg.mk| 2 - > src/conf/network_conf.c | 135 > -- > src/conf/network_conf.h | 8 +-- > src/libvirt_private.syms | 4 +- > src/network/bridge_driver.c | 56 > src/parallels/parallels_network.c | 16 ++--- > src/test/test_driver.c| 32 - > tests/networkxml2conftest.c | 4 +- > tests/objectlocking.ml| 2 - > 9 files changed, 129 insertions(+), 130 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/3] memtune: change the way how we store unlimited value
There was a mess in the way how we store unlimited value for memory limits and how we handled values provided by user. Internally there were two possible ways how to store unlimited value: as 0 value or as VIR_DOMAIN_MEMORY_PARAM_UNLIMITED. Because we chose to store memory limits as unsigned long long, we cannot use -1 to represent unlimited. It's much easier for us to say that everything greater than VIR_DOMAIN_MEMORY_PARAM_UNLIMITED means unlimited and leave 0 as valid value despite that it makes no sense to set limit to 0. Remove unnecessary function virCompareLimitUlong. The update of test is to prevent the 0 to be miss-used as unlimited in future. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146539 Signed-off-by: Pavel Hrdina --- docs/formatdomain.html.in | 4 +- src/conf/domain_conf.c | 75 +- src/libvirt-domain.c | 3 + src/libvirt_private.syms | 1 - src/lxc/lxc_cgroup.c | 18 +++--- src/lxc/lxc_driver.c | 7 +- src/lxc/lxc_fuse.c | 12 ++-- src/lxc/lxc_native.c | 6 +- src/openvz/openvz_conf.c | 4 +- src/openvz/openvz_driver.c | 4 +- src/qemu/qemu_cgroup.c | 24 +++ src/qemu/qemu_command.c| 8 ++- src/qemu/qemu_driver.c | 10 +-- src/qemu/qemu_hotplug.c| 2 +- src/qemu/qemu_migration.c | 5 +- src/util/virutil.c | 23 --- src/util/virutil.h | 2 - tests/qemuxml2argvdata/qemuxml2argv-memtune.xml| 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml | 2 +- 19 files changed, 118 insertions(+), 94 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6276a61..335763f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -798,7 +798,9 @@ for. For backwards compatibility, output is always in KiB. unit -since 0.9.11 +since 0.9.11 +Possible values for all *_limit parameters are in range from 0 to +VIR_DOMAIN_MEMORY_PARAM_UNLIMITED. hard_limit The optional hard_limit element is the maximum memory the guest can use. The units for this value are kibibytes (i.e. blocks diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6da02b0..efa142c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2320,6 +2320,10 @@ virDomainDefNew(void) if (!(ret->numa = virDomainNumaNew())) goto error; +ret->mem.hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; +ret->mem.soft_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; +ret->mem.swap_hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + return ret; error: @@ -6884,6 +6888,50 @@ virDomainParseMemory(const char *xpath, } +/** + * virDomainParseMemoryLimit: + * + * @xpath: XPath to memory amount + * @units_xpath: XPath to units attribute + * @ctxt: XPath context + * @mem: scaled memory amount is stored here + * + * Parse a memory element or attribute located at @xpath within @ctxt, and + * store the result into @mem, in blocks of 1024. The value is scaled by + * units located at @units_xpath (or the 'unit' attribute under @xpath if + * @units_xpath is NULL). If units are not present, he default scale of 1024 + * is used. The value must not exceed VIR_DOMAIN_MEMORY_PARAM_UNLIMITED + * once scaled. + * + * This helper should be used only on *_limit memory elements. + * + * Return 0 on success, -1 on failure after issuing error. + */ +static int +virDomainParseMemoryLimit(const char *xpath, + const char *units_xpath, + xmlXPathContextPtr ctxt, + unsigned long long *mem) +{ +int ret; +unsigned long long bytes; + +ret = virDomainParseScaledValue(xpath, units_xpath, ctxt, &bytes, 1024, +VIR_DOMAIN_MEMORY_PARAM_UNLIMITED << 10, +false); + +if (ret < 0) +return -1; + +if (ret == 0) +*mem = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; +else +*mem = virMemoryLimitTruncate(VIR_DIV_UP(bytes, 1024)); + +return 0; +} + + static int virDomainControllerModelTypeFromString(const virDomainControllerDef *def, const char *model) @@ -13185,20 +13233,20 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(nodes); /* Extract other memory tunables */ -if (virDomainParseMemory("./memtune/hard_limit[1]", NULL, ctxt, - &def->mem.hard_limit, false, false) < 0) +if (virDomainParseMemoryLimit
[libvirt] [PATCH v2 0/3] unify memtune value representation in libvirt
The first patch rewrites virsh memtune command to accept 0 as valid value instead of ignoring it. In the second patch I'll introduce a simple helper to crop the *_limit values to VIR_DOMAIN_MEMORY_PARAM_UNLIMITED as its used several times while reading values from cgroups and also a helper to check whether the memory limits are set or not. The last patch actually changes the internal representation of unlimited memtune settings. More detailed description is in commit message. My motivation to change it completely is to prevent future bugs with those limits. Pavel Hrdina (3): virsh: fix memtune to also accept 0 as valid value virutil: introduce helper functions for memory limits memtune: change the way how we store unlimited value docs/formatdomain.html.in | 4 +- src/conf/domain_conf.c | 75 +++ src/libvirt-domain.c | 3 + src/libvirt_private.syms | 3 +- src/lxc/lxc_cgroup.c | 18 ++--- src/lxc/lxc_driver.c | 7 +- src/lxc/lxc_fuse.c | 12 ++-- src/lxc/lxc_native.c | 6 +- src/openvz/openvz_conf.c | 4 +- src/openvz/openvz_driver.c | 4 +- src/qemu/qemu_cgroup.c | 24 +++ src/qemu/qemu_command.c| 8 ++- src/qemu/qemu_driver.c | 10 +-- src/qemu/qemu_hotplug.c| 2 +- src/qemu/qemu_migration.c | 5 +- src/util/virutil.c | 47 ++-- src/util/virutil.h | 5 +- tests/qemuxml2argvdata/qemuxml2argv-memtune.xml| 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml | 2 +- tools/virsh-domain.c | 83 ++ 20 files changed, 185 insertions(+), 139 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] virutil: introduce helper functions for memory limits
The first one is to truncate the memory limit to
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED if the value is greater and the second
one is to decide whether the memory limit is set or not, unlimited means
that it's not set.
Signed-off-by: Pavel Hrdina
---
src/libvirt_private.syms | 2 ++
src/util/virutil.c | 24
src/util/virutil.h | 3 +++
3 files changed, 29 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c810cf7..66d2333 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2269,6 +2269,8 @@ virIsCapableVport;
virIsDevMapperDevice;
virIsSUID;
virManageVport;
+virMemoryLimitIsSet;
+virMemoryLimitTruncate;
virParseNumber;
virParseOwnershipIds;
virParseVersionString;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 4a95292..599d59c 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2598,3 +2598,27 @@ long virGetSystemPageSizeKB(void)
return val;
return val / 1024;
}
+
+/**
+ * virMemoryLimitTruncate
+ *
+ * Return truncated memory limit to VIR_DOMAIN_MEMORY_PARAM_UNLIMITED as
maximum
+ * which means that the limit is not set => unlimited.
+ */
+unsigned long long
+virMemoryLimitTruncate(unsigned long long value)
+{
+return value < VIR_DOMAIN_MEMORY_PARAM_UNLIMITED ? value :
+VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
+}
+
+/**
+ * virMemoryLimitIsSet
+ *
+ * Returns true if the limit is set and false for unlimited value.
+ */
+bool
+virMemoryLimitIsSet(unsigned long long value)
+{
+return value < VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
+}
diff --git a/src/util/virutil.h b/src/util/virutil.h
index d1173c1..b8f5036 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -247,4 +247,7 @@ unsigned int virGetListenFDs(void);
long virGetSystemPageSize(void);
long virGetSystemPageSizeKB(void);
+unsigned long long virMemoryLimitTruncate(unsigned long long value);
+bool virMemoryLimitIsSet(unsigned long long value);
+
#endif /* __VIR_UTIL_H__ */
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-python PATCH] Clarify description for virNodeGetSecurityModel
s/host/hypervisor/ to match the wording used by the C binding. https://bugzilla.redhat.com/show_bug.cgi?id=1198518 --- libvirt-override-api.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Pushed as trivial. diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml index 439cb40..144479c 100644 --- a/libvirt-override-api.xml +++ b/libvirt-override-api.xml @@ -105,7 +105,7 @@ - Extract information about the host security model + Extract information about the hypervisor security model -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 12/24] network_conf: Introduce virNetworkObjEndAPI
On Thu, Mar 05, 2015 at 12:05:13 +0100, Michal Privoznik wrote: > This is practically copy of qemuDomObjEndAPI. The reason why is > it so widely available is to avoid code duplication, since the > function is going to be called from our bridge driver, test > driver and parallels driver too. > > Signed-off-by: Michal Privoznik > --- > src/conf/network_conf.c | 20 ++-- > src/conf/network_conf.h | 1 + > src/libvirt_private.syms | 1 + > 3 files changed, 16 insertions(+), 6 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 11/24] virNetworkObjList: Derive from virObjectLockableClass
On Thu, Mar 05, 2015 at 12:05:12 +0100, Michal Privoznik wrote: > Later we can turn APIs to lock the object if needed instead of > relying on caller to mutually exclude itself (probably done by > locking a big lock anyway). > > Signed-off-by: Michal Privoznik > --- > src/conf/network_conf.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) ACK if you shuffle this one after the EndAPI addition. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] qemu: Pass file descriptor when using TPM passthrough
On 03.03.2015 15:40, Stefan Berger wrote:
> Pass the TPM file descriptor to QEMU via command line.
> Instead of passing /dev/tpm0 we now pass /dev/fdset/10 and the additional
> parameters -add-fd set=10,fd=20.
>
> This addresses the use case when QEMU is started with non-root privileges
> and QEMU cannot open /dev/tpm0 for example.
>
> Signed-off-by: Stefan Berger
> ---
> src/qemu/qemu_command.c | 121
> ++--
> 1 file changed, 117 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index d6bc294..4c6b76d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -161,6 +161,58 @@ VIR_ENUM_IMPL(qemuNumaPolicy,
> VIR_DOMAIN_NUMATUNE_MEM_LAST,
>"interleave");
>
> /**
> + * qemuVirCommandGetFDSet:
> + * @cmd: the command to modify
> + * @fd: fd to reassign to the child
> + *
> + * Get the parameters for the QEMU -add-fd command line option
> + * for the given file descriptor. The file descriptor must previously
> + * have been 'transferred' in a virCommandPassFD() call.
> + * This function for example returns "set=10,fd=20".
> + */
> +static char *
> +qemuVirCommandGetFDSet(virCommandPtr cmd, int fd)
> +{
> +char *result = NULL;
> +int idx = virCommandPassFDGetFDIndex(cmd, fd);
> +
> +if (idx >= 0) {
> +ignore_value(virAsprintf(&result, "set=%d,fd=%d", idx, fd));
> +} else {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("file descriptor %d has not been transferred"), fd);
> +}
> +
> +return result;
> +}
> +
> +/**
> + * qemuVirCommandGetDevSet:
> + * @cmd: the command to modify
> + * @fd: fd to reassign to the child
> + *
> + * Get the parameters for the QEMU path= parameter where a file
> + * descriptor is accessed via a file descriptor set, for example
> + * /dev/fdset/10. The file descriptor must previously have been
> + * 'transferred' in a virCommandPassFD() call.
> + */
> +static char *
> +qemuVirCommandGetDevSet(virCommandPtr cmd, int fd)
> +{
> +char *result = NULL;
> +int idx = virCommandPassFDGetFDIndex(cmd, fd);
> +
> +if (idx >= 0) {
> +ignore_value(virAsprintf(&result, "/dev/fdset/%d", idx));
> +} else {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("file descriptor %d has not been transferred"), fd);
> +}
> +return result;
> +}
> +
> +
> +/**
> * qemuPhysIfaceConnect:
> * @def: the definition of the VM (needed by 802.1Qbh and audit)
> * @driver: pointer to the driver instance
> @@ -6321,15 +6373,20 @@ qemuBuildRNGDevStr(virDomainDefPtr def,
>
>
> static char *qemuBuildTPMBackendStr(const virDomainDef *def,
> +virCommandPtr cmd,
> virQEMUCapsPtr qemuCaps,
> -const char *emulator)
> +const char *emulator,
> +int *tpmfd, int *cancelfd)
> {
> const virDomainTPMDef *tpm = def->tpm;
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> const char *type = virDomainTPMBackendTypeToString(tpm->type);
> -char *cancel_path;
> +char *cancel_path = NULL, *devset = NULL;
> const char *tpmdev;
>
> +*tpmfd = -1;
> +*cancelfd = -1;
> +
> virBufferAsprintf(&buf, "%s,id=tpm-%s", type, tpm->info.alias);
>
> switch (tpm->type) {
> @@ -6341,11 +6398,42 @@ static char *qemuBuildTPMBackendStr(const
> virDomainDef *def,
> if (!(cancel_path = virTPMCreateCancelPath(tpmdev)))
> goto error;
>
> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ADD_FD)) {
> +*tpmfd = open(tpmdev, O_RDWR);
> +if (*tpmfd < 0) {
> +virReportSystemError(errno, _("Could not open TPM device
> %s"),
> + tpmdev);
> +goto error;
> +}
> +
> +virCommandPassFD(cmd, *tpmfd,
> + VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +devset = qemuVirCommandGetDevSet(cmd, *tpmfd);
> +if (devset == NULL)
> +goto error;
> +
> +*cancelfd = open(cancel_path, O_WRONLY);
> +if (*cancelfd < 0) {
> +virReportSystemError(errno,
> + _("Could not open TPM device's cancel "
> + "path %s"), cancel_path);
> +goto error;
> +}
> +VIR_FREE(cancel_path);
> +
> +virCommandPassFD(cmd, *cancelfd,
> + VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +cancel_path = qemuVirCommandGetDevSet(cmd, *cancelfd);
> +if (cancel_path == NULL)
> +goto error;
> +}
I'm thinking of how to test this code, but those open() complicate the
things. Unless we want to mock them in qemuxml2argvmock.c w
Re: [libvirt] [PATCHv2] spec: Enable RBD storage driver in RHEL-7
On Thu, Mar 05, 2015 at 13:26:09 +0100, Peter Krempa wrote: > Use correct package names too as they differ. > --- > libvirt.spec.in | 14 +- > 1 file changed, 13 insertions(+), 1 deletion(-) ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Remove duplicate entries in by namespace
On Thu, Mar 05, 2015 at 15:03:22 +0100, Michal Privoznik wrote: > On 04.03.2015 10:06, Peter Krempa wrote: > > Since the APIs support just one element per namespace and while > > modifying an element all duplicates would be removed, let's do this > > right away in the post parse callback. > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1190590 > > --- > > src/conf/domain_conf.c | 42 > > ++ > > .../qemuxml2argv-metadata-duplicate.xml| 34 ++ > > .../qemuxml2xmlout-metadata-duplicate.xml | 31 > > tests/qemuxml2xmltest.c| 1 + > > 4 files changed, 108 insertions(+) > > create mode 100644 > > tests/qemuxml2argvdata/qemuxml2argv-metadata-duplicate.xml > > create mode 100644 > > tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata-duplicate.xml > > ACK Pushed; Thanks Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] spec: Enable RBD storage driver in RHEL-7
On Thu, Mar 05, 2015 at 16:20:55 +0100, Jiri Denemark wrote: > On Thu, Mar 05, 2015 at 13:26:09 +0100, Peter Krempa wrote: > > Use correct package names too as they differ. > > --- > > libvirt.spec.in | 14 +- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > ACK Pushed; Thanks. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 13/24] bridge_driver: Use virNetworkObjEndAPI
On Thu, Mar 05, 2015 at 12:05:14 +0100, Michal Privoznik wrote: > So far, this is pure code replacement. But once we introduce > reference counting to virNetworkObj this will be more handy as > there'll be only one function to change: virNetworkObjEndAPI(). > > Signed-off-by: Michal Privoznik > --- > src/network/bridge_driver.c | 57 > +++-- > 1 file changed, 19 insertions(+), 38 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain_conf: Format without address correctly
On Thu, Mar 05, 2015 at 02:59:52PM +0100, Michal Privoznik wrote: > We have something like pvpanic device. However, in some cases it does > not have any address assigned, in which case we produce this ugly XML > (still valid though): > > > /usr/bin/qemu > ... > > > > > Lets format "" instead. s/pvpanic/panic/ > > Signed-off-by: Michal Privoznik > --- > src/conf/domain_conf.c | 20 ++- > .../qemuxml2argv-panic-no-address.args | 5 > .../qemuxml2argv-panic-no-address.xml | 29 > ++ > tests/qemuxml2argvtest.c | 3 +++ > tests/qemuxml2xmltest.c| 1 + > 5 files changed, 52 insertions(+), 6 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-no-address.xml > ACK Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 14/24] test_driver: Use virNetworkObjEndAPI
On Thu, Mar 05, 2015 at 12:05:15 +0100, Michal Privoznik wrote: > So far, this is pure code replacement. But once we introduce > reference counting to virNetworkObj this will be more handy as > there'll be only one function to change: virNetworkObjEndAPI(). > > Signed-off-by: Michal Privoznik > --- > src/test/test_driver.c | 42 ++ > 1 file changed, 14 insertions(+), 28 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 16/24] network_conf: Introduce locked versions of lookup functions
On Thu, Mar 05, 2015 at 12:05:17 +0100, Michal Privoznik wrote:
> This is going to be needed later, when some functions needs to avoid
> calling multiple times at once. It will work like this:
>
> 1) gain the object list mutex
> 2) find the object to work on
> 3) do the work
> 4) release the mutex
>
> As an example of such function is virNetworkAssignDef(). The other
> use case might be in virNetworkObjListForEach() callback.
>
> Signed-off-by: Michal Privoznik
> ---
> src/conf/network_conf.c | 45 ++---
> src/conf/network_conf.h | 4
> src/libvirt_private.syms | 2 ++
> 3 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index a821f6c..8cf9ffd 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -153,34 +153,57 @@ virNetworkObjListPtr virNetworkObjListNew(void)
> return nets;
> }
>
> +virNetworkObjPtr
> +virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
> + const unsigned char *uuid)
> +{
> +size_t i;
> +
> +for (i = 0; i < nets->count; i++) {
> +virObjectLock(nets->objs[i]);
> +if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
> +return nets->objs[i];
This also creates a deadlock that you fix in the next one. Looks like a
rebase artifact.
> +virObjectUnlock(nets->objs[i]);
> +}
> +
> +return NULL;
> +}
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/9] Implement public API for virDomainGetIOThreadsInfo
On Thu, Mar 05, 2015 at 07:45:51AM -0500, John Ferlan wrote: > >> +unsigned char *cpumap; /* CPU map for thread */ > >> +int cpumaplen; /* cpumap size */ > > > >> +size_t nresources; /* count of resources using > >> IOThread */ > >> +char **resources; /* array of resources using > >> IOThread */ > > > > "resources" is too vague. > > Suggestion? Is "devices" better? > > Today it's the disk source path, but I remember reading something where > an IOThread could be potentially used for something else (perhaps > network, but I cannot find the reference quickly). > I just worry that putting the path here could be a problem sometime in the future if the attribute gets extended (I think some of the Block* APIs had that problem). Perhaps Peter or Eric will voice their opinion. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Document the effect of empty string as a rom file
This can be used to disable the ROM. --- docs/formatdomain.html.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6276a61..050476f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4265,7 +4265,8 @@ qemu-kvm -net nic,model=? /dev/null The optional file attribute is used to point to a binary file to be presented to the guest as the device's ROM BIOS. This can be useful to provide an alternative boot ROM for a - network device. + network device. If the file attribute is present, but empty, + no ROM file will be loaded. Since 0.9.10 (QEMU and KVM only). Setting up a network backend in a driver domain -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 15/24] parallels_network: Use virNetworkObjEndAPI
On Thu, Mar 05, 2015 at 12:05:16 +0100, Michal Privoznik wrote:
> So far, this is pure code replacement. But once we introduce
> reference counting to virNetworkObj this will be more handy as
> there'll be only one function to change: virNetworkObjEndAPI().
For this patch this isn't entirely true, as ...
>
> Signed-off-by: Michal Privoznik
> ---
> src/parallels/parallels_network.c | 29 -
> 1 file changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/src/parallels/parallels_network.c
> b/src/parallels/parallels_network.c
> index 86038bf..1bcd2d3 100644
> --- a/src/parallels/parallels_network.c
> +++ b/src/parallels/parallels_network.c
> @@ -230,7 +230,6 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
> virJSONValuePtr jobj)
> goto cleanup;
> net->active = 1;
> net->autostart = 1;
> -virObjectUnlock(net);
... this ...
> return net;
>
> cleanup:
> @@ -241,7 +240,7 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
> virJSONValuePtr jobj)
> static virNetworkObjPtr
> parallelsAddRoutedNetwork(parallelsConnPtr privconn)
> {
> -virNetworkObjPtr net;
> +virNetworkObjPtr net = NULL;
> virNetworkDefPtr def;
>
> if (VIR_ALLOC(def) < 0)
> @@ -265,7 +264,6 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
> }
> net->active = 1;
> net->autostart = 1;
> -virObjectUnlock(net);
>
> return net;
>
> @@ -277,7 +275,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
> static int parallelsLoadNetworks(parallelsConnPtr privconn)
> {
> virJSONValuePtr jobj, jobj2;
> -virNetworkObjPtr net;
> +virNetworkObjPtr net = NULL;
> int ret = -1;
> int count;
> size_t i;
> @@ -305,16 +303,19 @@ static int parallelsLoadNetworks(parallelsConnPtr
> privconn)
> net = parallelsLoadNetwork(privconn, jobj2);
> if (!net)
> goto cleanup;
> +else
> +virNetworkObjEndAPI(&net);
... and this are semantic changes.
>
> }
>
> -if (!parallelsAddRoutedNetwork(privconn))
> +if (!(net = parallelsAddRoutedNetwork(privconn)))
> goto cleanup;
>
> ret = 0;
>
> cleanup:
> virJSONValueFree(jobj);
> +virNetworkObjEndAPI(&net);
> return ret;
> }
>
They make sense though. ACK.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/9] Implement public API for virDomainGetIOThreadsInfo
On Thu, Mar 05, 2015 at 05:21:41PM +0100, Ján Tomko wrote: > On Thu, Mar 05, 2015 at 07:45:51AM -0500, John Ferlan wrote: > > >> +unsigned char *cpumap; /* CPU map for thread */ > > >> +int cpumaplen; /* cpumap size */ > > > > > >> +size_t nresources; /* count of resources using > > >> IOThread */ > > >> +char **resources; /* array of resources using > > >> IOThread */ > > > > > > "resources" is too vague. > > > > Suggestion? Is "devices" better? > > > > Today it's the disk source path, but I remember reading something where > > an IOThread could be potentially used for something else (perhaps > > network, but I cannot find the reference quickly). > > > > I just worry that putting the path here could be a problem sometime in > the future if the attribute gets extended (I think some of the Block* > APIs had that problem). > > Perhaps Peter or Eric will voice their opinion. The XML config already provides information about what device is associated with what I/O thread. As such I don't think we should include the 'resources' field in the struct here at all. It is just duplicating information from the XML, in a format that is not at all extensible, which is just asking for trouble as you point out. These proposed APIs are all about reporting & updating the CPU pinning of the I/O threads, and info about the list of resource names is not relevant for that task, so again this just seems like extra pain for no gain. IOW, just drop the 'resources' and 'nresources' fields from the struct Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 16/24] network_conf: Introduce locked versions of lookup functions
On Thu, Mar 05, 2015 at 12:05:17 +0100, Michal Privoznik wrote:
> This is going to be needed later, when some functions needs to avoid
> calling multiple times at once. It will work like this:
>
> 1) gain the object list mutex
> 2) find the object to work on
> 3) do the work
> 4) release the mutex
>
> As an example of such function is virNetworkAssignDef(). The other
> use case might be in virNetworkObjListForEach() callback.
>
> Signed-off-by: Michal Privoznik
> ---
> src/conf/network_conf.c | 45 ++---
> src/conf/network_conf.h | 4
> src/libvirt_private.syms | 2 ++
> 3 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index a821f6c..8cf9ffd 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -153,34 +153,57 @@ virNetworkObjListPtr virNetworkObjListNew(void)
> return nets;
> }
>
> +virNetworkObjPtr
> +virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
> + const unsigned char *uuid)
> +{
> +size_t i;
> +
> +for (i = 0; i < nets->count; i++) {
> +virObjectLock(nets->objs[i]);
> +if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
> +return nets->objs[i];
> +virObjectUnlock(nets->objs[i]);
> +}
> +
> +return NULL;
> +}
> +
> virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
> const unsigned char *uuid)
> {
> +virNetworkObjPtr ret;
> +
> +virObjectLock(nets);
> +ret = virNetworkObjFindByUUIDLocked(nets, uuid);
> +virObjectUnlock(nets);
> +return ret;
> +}
> +
> +virNetworkObjPtr
> +virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
> + const char *name)
> +{
> size_t i;
>
> for (i = 0; i < nets->count; i++) {
> virObjectLock(nets->objs[i]);
> -if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
> +if (STREQ(nets->objs[i]->def->name, name))
> return nets->objs[i];
> virObjectUnlock(nets->objs[i]);
> }
>
> return NULL;
> }
> -
This deletes the whitespace between functions.
> virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
> const char *name)a
ACK without damaging the serenity of whitespace :D
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10] libxl: Stubdom emulator type
On Wed, Mar 04, 2015 at 08:34:26PM +, Daniel P. Berrange wrote: > On Thu, Feb 19, 2015 at 10:19:22PM +0100, Marek Marczykowski-Górecki wrote: > > On Thu, Feb 19, 2015 at 01:45:52PM -0700, Jim Fehlig wrote: > > > Marek Marczykowski-Górecki wrote: > > > > Xen have feature of having device model in separate domain (called stub > > > > domain). Add a 'type' attribute to 'emulator' element to allow selecting > > > > such a configuration. > > > > > > Or maybe 'mode', describing the mode in which the emulator runs: as a > > > process or as a VM. > > > > > > > Emulator path is still used for qemu running in dom0 (if > > > > any). Libxl currently do not allow to select stubdomain path. > > > > > > > > > > That seems to overload a single . Would it be better to have > > > multiple ? E.g. > > > > > > /usr/lib/xen/bin/qemu-system-i386 > > > /usr/lib/xen/bin/stubdom-dm > > > > So the existence of would enable this feature? > > What if someday the default will be to run stubdomain emulator - how > > the user can disable it in such case? > > This suggests to me we need to have two separate controls. > First an attribute / element somewhere saying what device > mode is used, and the second (optionally) saying what the > path to the stubdom emulator is. > > eg no stubdomain: > > > /usr/lib/xen/bin/qemu-system-i386 > ... > > > Stubdomain with default path > > > /usr/lib/xen/bin/qemu-system-i386 > > > > Stubdomain with custom path > > > > /usr/lib/xen/bin/qemu-system-i386 > >/usr/lib/xen/bin/stubdom-dm > > I like this approach :) > We can of course not worry about this 3rd option until libxl actually > supports this. So, just implement > for now. If is not listed in the XML from the application, > the XML post-parse callback can be used by the hypervisor to fill in > the default either or > as appropriate. So that copes with xen changing its default in the > future I see libvirt has cool virTristateBool type, which I will use here. So the lack of element will mean "use hypervisor default". As in many other places I think. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? pgpDoOfOPxVmO.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/9] qemu: Implement the qemu driver fetch for IOThreads
On Thu, Mar 05, 2015 at 07:51:33AM -0500, John Ferlan wrote:
> >> +for (i = 0; i < targetDef->iothreads; i++) {
> >> +if (VIR_ALLOC(info_ret[i]) < 0)
> >> +goto cleanup;
> >> +
> >> +/* IOThreads being counting at 1 */
> >> +info_ret[i]->iothread_id = i + 1;
> >> +
> >> +if (VIR_ALLOC_N(info_ret[i]->cpumap, maplen) < 0)
> >> +goto cleanup;
> >> +
> >> +/* Initialize the cpumap */
> >> +info_ret[i]->cpumaplen = maplen;
> >> +memset(info_ret[i]->cpumap, 0xff, maplen);
> >> +if (maxcpu % 8)
> >> +info_ret[i]->cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1;
> >
> > virBitmapToData could make this more readable.
> >
>
> This is where I'm not sure varying from the existing API's model w/r/t
> cpumap & maplen, etc. should be done..
>
Yes, virBitmapToData converts it from the virBitmap format used by
theiothreadspin info to the cpumap/maplen format matching the API you
proposed.
> >> +}
> >> +
> >> +/* If iothreadspin setting exists, there are unused physical cpus */
> >> +iothreadspin_list = targetDef->cputune.iothreadspin;
> >
> > A temporary variable pointing to iothreadspin[i] is more common.
> >
>
> straight copy of qemuDomainGetVcpuPinInfo
>
Oh, that explains it.
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 5/9] Implement public API for virDomainSetIOThreads
On Tue, Feb 17, 2015 at 04:03:54PM -0500, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1135491
>
> Add the libvirt API infrastructure to support setting IOThread data.
> For now this is the pinned CPU information, but eventually will also
> support hot plug add/del
>
> The API will support the LIVE, CONFIG, or CURRENT flags. If the
> VIR_DOMAIN_IOTHREADS_PIN flag is not provided, it's left up to the
> hypervisor to handle, but when not provided the cpumap/maplen arguments
> are not checked.
>
> Signed-off-by: John Ferlan
> ---
> include/libvirt/libvirt-domain.h | 16 ++
> src/driver-hypervisor.h | 8 +
> src/libvirt-domain.c | 69
>
> src/libvirt_public.syms | 1 +
> 4 files changed, 94 insertions(+)
>
> diff --git a/include/libvirt/libvirt-domain.h
> b/include/libvirt/libvirt-domain.h
> index 9dcc424..e07db16 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1582,11 +1582,27 @@ struct _virDomainIOThreadsInfo {
> char **resources; /* array of resources using IOThread
> */
> };
>
> +/* Flags for controlling virtual IOThread pinning. */
> +typedef enum {
> +/* See virDomainModificationImpact for these flags. */
> +VIR_DOMAIN_IOTHREADS_CURRENT = VIR_DOMAIN_AFFECT_CURRENT,
> +VIR_DOMAIN_IOTHREADS_LIVE= VIR_DOMAIN_AFFECT_LIVE,
> +VIR_DOMAIN_IOTHREADS_CONFIG = VIR_DOMAIN_AFFECT_CONFIG,
> +
> +/* Additionally, these flags may be bitwise-OR'd in. */
> +VIR_DOMAIN_IOTHREADS_PIN = (1 << 2), /* thread_id to pin using cpumap */
> +} virDomainIOThreadsFlags;
> +
> void virDomainIOThreadsInfoFree(virDomainIOThreadsInfoPtr
> info);
>
> int virDomainGetIOThreadsInfo(virDomainPtr domain,
> virDomainIOThreadsInfoPtr
> **info,
> unsigned int flags);
> +int virDomainSetIOThreads(virDomainPtr domain,
> + unsigned int iothread_val,
> + unsigned char *cpumap,
> + int maplen,
> + unsigned int flags);
>
> /**
> * VIR_USE_CPU:
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index 2ce1a51..120d761 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -386,6 +386,13 @@ typedef int
> unsigned int flags);
>
> typedef int
> +(*virDrvDomainSetIOThreads)(virDomainPtr domain,
> +unsigned int iothread_val,
> +unsigned char *cpumap,
> +int maplen,
> +unsigned int flags);
> +
> +typedef int
> (*virDrvDomainGetSecurityLabel)(virDomainPtr domain,
> virSecurityLabelPtr seclabel);
>
> @@ -1260,6 +1267,7 @@ struct _virHypervisorDriver {
> virDrvDomainGetVcpus domainGetVcpus;
> virDrvDomainGetMaxVcpus domainGetMaxVcpus;
> virDrvDomainGetIOThreadsInfo domainGetIOThreadsInfo;
> +virDrvDomainSetIOThreads domainSetIOThreads;
> virDrvDomainGetSecurityLabel domainGetSecurityLabel;
> virDrvDomainGetSecurityLabelList domainGetSecurityLabelList;
> virDrvNodeGetSecurityModel nodeGetSecurityModel;
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index f893b35..bf73773 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -7969,6 +7969,75 @@ virDomainIOThreadsInfoFree(virDomainIOThreadsInfoPtr
> info)
>
>
> /**
> + * virDomainSetIOThreads:
> + * @domain: a domain object
> + * @iothread_val: either the thread_id to modify or a count of IOThreads
> + * to be added or removed from the domain depending on the @flags
> setting
> + * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN)
> + * Each bit set to 1 means that corresponding CPU is usable.
> + * Bytes are stored in little-endian order: CPU0-7, 8-15...
> + * In each byte, lowest CPU number is least significant bit.
> + * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in
> + * underlying virtualization system (Xen...).
> + * If maplen < size, missing bytes are set to zero.
> + * If maplen > size, failure code is returned.
> + * @flags: bitwise-OR of supported virDomainIOThreadsFlags
> + *
> + * If the VIR_DOMAIN_IOTHREADS_PIN flag is set, the @iothread_val will be
> + * an existing IOThread to be pinned
I don't really like the sound of this flags & iothread_val usage.
Using flags as a way to multiple different logical operations into
one public API is not very nice design and ends up getting confusing
for developers & ourselves. IMHO we should have a virDomainSetIOThreadsPin()
API for pinning, and another API for adjusting t
Re: [libvirt] [libvirt-python PATCH 2/2] Support virDomainSetIOThreads
On Thu, Feb 19, 2015 at 07:59:39AM -0500, John Ferlan wrote:
> Support the libvirt_virDomainSetIOThreads method using code that mimics
> the existing libvirt_virDomainPinVcpuFlags method
>
> The following is a sample session assuming guest 'iothr-gst' has IOThreads
> configured (it's currently running, too)
>
> >>> import libvirt
> >>> con=libvirt.open("qemu:///system")
> >>> dom=con.lookupByName('iothr-gst')
> >>> dom.getIOThreadsInfo()
> [(1, [False, False, True, False], ['/home/vm-images/iothr-vol1']), (2,
> [False, False, False, True], ['/home/vm-images/iothr-vol2']), (3, [False,
> False, False, True], [])]
> >>> cpumap=(True,True,True,False)
> >>> dom.setIOThreads(3,cpumap)
> 0
> >>> print dom.getIOThreadsInfo()
> [(1, [False, False, True, False], ['/home/vm-images/iothr-vol1']), (2,
> [False, False, False, True], ['/home/vm-images/iothr-vol2']), (3, [True,
> True, True, False], [])]
> >>>
>
> Signed-off-by: John Ferlan
> ---
> generator.py | 1 +
> libvirt-override-api.xml | 8 ++
> libvirt-override.c | 67
>
> 3 files changed, 76 insertions(+)
>
> diff --git a/generator.py b/generator.py
> index 327c896..c79acf1 100755
> --- a/generator.py
> +++ b/generator.py
> @@ -436,6 +436,7 @@ skip_impl = (
> 'virDomainGetEmulatorPinInfo',
> 'virDomainPinEmulator',
> 'virDomainGetIOThreadsInfo',
> +'virDomainSetIOThreads',
> 'virSecretGetValue',
> 'virSecretSetValue',
> 'virSecretGetUUID',
> diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
> index e512311..19667b6 100644
> --- a/libvirt-override-api.xml
> +++ b/libvirt-override-api.xml
> @@ -284,6 +284,14 @@
>
>
>
> +
> + Dynamically change the real CPUs which can be allocated to an
> IOThread. This function requires privileged access to the hypervisor.
> +
> +
> +
> +
> +
> +
>
>Change the scheduler parameters
>
> diff --git a/libvirt-override.c b/libvirt-override.c
> index 17f3bf3..f9af5a9 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -2082,6 +2082,72 @@ cleanup:
> return VIR_PY_NONE;
> }
>
> +static PyObject *
> +libvirt_virDomainSetIOThreads(PyObject *self ATTRIBUTE_UNUSED,
> + PyObject *args)
> +{
> +virDomainPtr domain;
> +PyObject *pyobj_domain, *pycpumap;
> +PyObject *ret = NULL;
> +unsigned char *cpumap;
> +int cpumaplen, iothread_val, tuple_size, cpunum;
> +size_t i;
> +unsigned int flags;
> +int i_retval;
> +
> +if (!PyArg_ParseTuple(args, (char *)"OiOI:virDomainSetIOThread",
> + &pyobj_domain, &iothread_val, &pycpumap, &flags))
> +return NULL;
> +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
> +
> +if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0)
> +return VIR_PY_INT_FAIL;
> +
> +if (PyTuple_Check(pycpumap)) {
> +tuple_size = PyTuple_Size(pycpumap);
> +if (tuple_size == -1)
> +return ret;
> +} else {
> + PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is
> required");
> + return ret;
> +}
> +
> +cpumaplen = VIR_CPU_MAPLEN(cpunum);
> +if (VIR_ALLOC_N(cpumap, cpumaplen) < 0)
> +return PyErr_NoMemory();
> +
> +for (i = 0; i < tuple_size; i++) {
> +PyObject *flag = PyTuple_GetItem(pycpumap, i);
> +bool b;
> +
> +if (!flag || libvirt_boolUnwrap(flag, &b) < 0)
> +goto cleanup;
> +
> +if (b)
> +VIR_USE_CPU(cpumap, i);
> +else
> +VIR_UNUSE_CPU(cpumap, i);
> +}
> +
> +for (; i < cpunum; i++)
> +VIR_UNUSE_CPU(cpumap, i);
> +
> +flags |= VIR_DOMAIN_IOTHREADS_PIN;
This hardcoding of flags in the python API is a great example of why it
is unsatisfactory to overload multiplex different operations into one
API, instead of just defining separate APis for each.
> +LIBVIRT_BEGIN_ALLOW_THREADS;
> +i_retval = virDomainSetIOThreads(domain, iothread_val,
> + cpumap, cpumaplen, flags);
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] libxl: a few minor patches
Michal Privoznik wrote: > On 04.03.2015 01:09, Jim Fehlig wrote: > >> This series contains a few simple changes to the libxl driver. >> Patches 1 and 2 were included in >> >> https://www.redhat.com/archives/libvir-list/2015-February/msg00611.html >> >> To make an upcoming V2 of that series easier to review, I removed the >> mostly unrelated patches and include them here, along with one other >> small fix. >> >> Jim Fehlig (3): >> libxl: remove redundant calls to libxl_evdisable_domain_death >> libxl: use libxl_ctx passed to libxlConsoleCallback >> libxl: remove unneeded cleanup_unlock label >> >> src/libxl/libxl_domain.c | 13 ++--- >> src/libxl/libxl_driver.c | 10 +++--- >> 2 files changed, 5 insertions(+), 18 deletions(-) >> >> > > ACK to all, but see my comment to 3/3. > Thanks for catching that! I fixed it and pushed the series. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] memory ballooning and nova
On Wed, Feb 25, 2015 at 11:38:46AM +0100, Raymond Durand wrote: > Thanks. > > Is it possible to enable/disable which parameters are triggered by Nova on > Libvirt? ie. > -device virtio-balloon > -pci-device isa-serial No, these are a standard part of Nova Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] memory ballooning and nova
Ok thanks. Regards, 2015-03-05 20:45 GMT+01:00 Daniel P. Berrange : > On Wed, Feb 25, 2015 at 11:38:46AM +0100, Raymond Durand wrote: > > Thanks. > > > > Is it possible to enable/disable which parameters are triggered by Nova > on > > Libvirt? ie. > > -device virtio-balloon > > -pci-device isa-serial > > No, these are a standard part of Nova > > Regards, > Daniel > -- > |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ > :| > |: http://libvirt.org -o- http://virt-manager.org > :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ > :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc > :| > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-tck][PATCH] This testcase tests if the open_graphics_fd function works properly.
Zhe Peng wrote:
> Signed-off-by: Hao Liu
> ---
> scripts/domain/275-open-graphics-fd.t | 76
> +++
> 1 file changed, 76 insertions(+)
> create mode 100644 scripts/domain/275-open-graphics-fd.t
>
> diff --git a/scripts/domain/275-open-graphics-fd.t
> b/scripts/domain/275-open-graphics-fd.t
> new file mode 100644
> index 000..9377f50
> --- /dev/null
> +++ b/scripts/domain/275-open-graphics-fd.t
> @@ -0,0 +1,76 @@
> +# -*- perl -*-
> +#
> +# Copyright (C) 2012-2014 Red Hat, Inc.
> +# Copyright (C) 2014 Hao Liu
> +#
> +# This program is free software; You can redistribute it and/or modify
> +# it under the GNU General Public License as published by the Free
> +# Software Foundation; either version 2, or (at your option) any
> +# later version
> +#
> +# The file "LICENSE" distributed along with this file provides full
> +# details of the terms and conditions
> +#
> +
> +=pod
> +
> +=head1 NAME
> +
> +domain/275-open-graphics-fd.t - Test open_graphics_fd function
> +
> +=head1 DESCRIPTION
> +
> +The test case validates that open_graphics_fd function works well
> +
> +=cut
> +
> +use strict;
> +use warnings;
> +
> +use Test::More tests => 9;
> +
> +use Sys::Virt::TCK;
> +use Test::Exception;
> +use IO::Handle;
> +
> +my $tck = Sys::Virt::TCK->new();
> +my $conn = eval { $tck->setup(); };
> +BAIL_OUT "failed to setup test harness: $@" if $@;
> +END {
> +$tck->cleanup if $tck;
> +}
> +
> +
> +# looking up domain
> +my $dom_name ="tck";
> +my $xml = $tck->generic_domain(name => $dom_name)->as_xml;
> +diag "domain xml is $xml";
> +my $dom = $conn->define_domain($xml);
> +$dom->create;
> +ok($dom->get_id() > 0, "running domain has an ID > 0");
> +my $graphics_cnt = xpath($dom, "count(/domain/devices/graphics)")->value();
> +my $valid_idx = $graphics_cnt - 1;
> +my $invalid_idx = $graphics_cnt;
> +
> +my $fd = 0;
> +my $fh = IO::Handle->new();
> +
> +diag "open fd for valid graphics device";
> +lives_ok(sub {$fd = $dom->open_graphics_fd($valid_idx, 0)}, "open graphics
> fd");
> +ok($fd > 0, "graphic fd is $fd");
>
It looks like the test fails when virDomainOpenGraphicsFD is not
supported by the hypervisor. Can it be skipped if the error is "not
supported"?
Regards,
Jim
> +$fh->fdopen($fd, "r");
> +ok($fh->opened, "fd is accessible");
> +$fh->close();
> +
> +diag "open fd for valid graphics device skip auth";
> +$fd = 0;
> +lives_ok(sub {$fd = $dom->open_graphics_fd($valid_idx,
> Sys::Virt::Domain::OPEN_GRAPHICS_SKIPAUTH)}, "open graphics fd");
> +ok($fd > 0, "graphic fd is $fd");
> +$fh->fdopen($fd, "r");
> +ok($fh->opened, "fd is accessible");
> +$fh->close();
> +
> +diag "open fd for non-exist graphics device";
> +$fd = 0;
> +ok_error(sub {$fd = $dom->open_graphics_fd($invalid_idx, 0)}, "open graphics
> fd");
> +ok($fd eq 0, "graphic fd is not returned");
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] direct device assignment
Ok, thanks. So as soon as a device is directly assigned to a VM, setrlimit() or prlimit() are applied to the qemu-kvm process to avoid paging at the host level, right? and if there is not enough physical RAM, is the device simply not connected or is the VM halted? And the VM itself can page in/page out with its own page file, this is not a problem, right? Regards, 2015-03-04 16:57 GMT+01:00 Laine Stump : > On 03/04/2015 10:27 AM, Raymond Durand wrote: > > I read this in the context of direct device assignment: "All of the > > guest's memory must kept permanently in memory. This is because the > > guest may program the device with any address in its address space and > > the hypervisor has no way of handling a DMA page fault" > > > > is it still true? > > > > Yes. libvirt takes care of this automatically (by calling setrlimit() or > prlimit()) when you assign a device to a guest. It does mean that you'll > need enough physical RAM on the machine to account for it, though. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-tck][PATCH] Add new api get_all_domain_stats testing
Zhe Peng wrote:
> $vmm->get_all_domain_stats
> Sys::Virt::GET_ALL_STATS_ACTIVE
> Sys::Virt::GET_ALL_STATS_INACTIVE
> Sys::Virt::GET_ALL_STATS_OTHER
> Sys::Virt::GET_ALL_STATS_PAUSED
> Sys::Virt::GET_ALL_STATS_PERSISTENT
> Sys::Virt::GET_ALL_STATS_RUNNING
> Sys::Virt::GET_ALL_STATS_SHUTOFF
> Sys::Virt::GET_ALL_STATS_TRANSIENT
> Sys::Virt::GET_ALL_STATS_ENFORCE_STATS
>
> Signed-off-by: Zhe Peng
> ---
> scripts/domain/800-get-all-stats.t | 137
> +
> 1 file changed, 137 insertions(+)
> create mode 100644 scripts/domain/800-get-all-stats.t
>
Same comment for this patch. Can we ensure the test is skipped, rather
than failing, if virConnectGetAllDomainsStats returns "not supported"?
Regards,
Jim
> diff --git a/scripts/domain/800-get-all-stats.t
> b/scripts/domain/800-get-all-stats.t
> new file mode 100644
> index 000..4ff4bf3
> --- /dev/null
> +++ b/scripts/domain/800-get-all-stats.t
> @@ -0,0 +1,137 @@
> +# -*- perl -*-
> +#
> +# Copyright (C) 2014 Red Hat, Inc.
> +# Copyright (C) 2014 Zhe Peng
> +#
> +# This program is free software; You can redistribute it and/or modify
> +# it under the GNU General Public License as published by the Free
> +# Software Foundation; either version 2, or (at your option) any
> +# later version
> +#
> +# The file "LICENSE" distributed along with this file provides full
> +# details of the terms and conditions
> +#
> +
> +=pod
> +
> +=head1 NAME
> +
> +domain/800-get-all-stats.t
> +
> +=head1 DESCRIPTION
> +
> +The test case validates that:
> +$vmm->get_all_domain_stats
> +Sys::Virt::GET_ALL_STATS_ACTIVE
> +Sys::Virt::GET_ALL_STATS_INACTIVE
> +Sys::Virt::GET_ALL_STATS_OTHER
> +Sys::Virt::GET_ALL_STATS_PAUSED
> +Sys::Virt::GET_ALL_STATS_PERSISTENT
> +Sys::Virt::GET_ALL_STATS_RUNNING
> +Sys::Virt::GET_ALL_STATS_SHUTOFF
> +Sys::Virt::GET_ALL_STATS_TRANSIENT
> +Sys::Virt::GET_ALL_STATS_ENFORCE_STATS
> +
> +=cut
> +
> +use strict;
> +use warnings;
> +
> +use Test::More tests => 11;
> +
> +use Sys::Virt::TCK;
> +use Test::Exception;
> +
> +my $tck = Sys::Virt::TCK->new();
> +my $conn = eval { $tck->setup(); };
> +BAIL_OUT "failed to setup test harness: $@" if $@;
> +END {
> +$tck->cleanup if $tck;
> +}
> +
> +my @doms = ("tck1", "tck2", "tck3", "tck4");
> +
> +#create two persistent domain, tck1 and tck2
> +
> +my $xml = $tck->generic_domain(name => "tck1")->as_xml;
> +
> +diag "Defining an inactive domain config";
> +
> +my $dom1;
> +
> +ok_domain(sub { $dom1 = $conn->define_domain($xml) }, "defined persistent
> domain config");
> +
> +my $xml2 = $tck->generic_domain(name => "tck2")->as_xml;
> +
> +diag "Defining an inactive domain config";
> +
> +my $dom2;
> +
> +ok_domain(sub { $dom2 = $conn->define_domain($xml2) }, "defined persistent
> domain config");
> +
> +my $stats_persistent =
> Sys::Virt::Domain::GET_ALL_STATS_PERSISTENT|Sys::Virt::Domain::GET_ALL_STATS_INACTIVE;
> +my @dom_persistent =
> $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE, undef,
> $stats_persistent);
> +
> +is($#dom_persistent+1, 2, "list all persistent domain");
> +
> +#create two transient domain, tck3 and tck4
> +
> +my $xml3 = $tck->generic_domain(name => "tck3")->as_xml;
> +
> +diag "Creating a new transient domain";
> +my $dom3;
> +ok_domain(sub { $dom3 = $conn->create_domain($xml3) }, "created transient
> domain object");
> +
> +my $xml4 = $tck->generic_domain(name => "tck4")->as_xml;
> +
> +diag "Creating a new transient domain";
> +my $dom4;
> +ok_domain(sub { $dom4 = $conn->create_domain($xml4) }, "created transient
> domain object");
> +
> +my $stats_transient =
> Sys::Virt::Domain::GET_ALL_STATS_TRANSIENT|Sys::Virt::Domain::GET_ALL_STATS_RUNNING;
> +
> +my @dom_transient =
> $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE, undef,
> $stats_transient);
> +
> +is($#dom_transient+1, 2, "list all transient domain");
> +
> +#start & pause persistent domain 2 and one transient dom
> +$dom2->create();
> +$dom2->suspend();
> +$dom3->suspend();
> +
> +my $stats_pause = Sys::Virt::Domain::GET_ALL_STATS_PAUSED;
> +
> +my @dom_pause = $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE,
> undef, $stats_pause);
> +
> +is($#dom_pause+1, 2, "list all pause domain");
> +
> +#resume dom2 and dom3
> +$dom2->resume();
> +$dom3->resume();
> +
> +my $stats_active = Sys::Virt::Domain::GET_ALL_STATS_ACTIVE;
> +
> +my@dom_active = $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE,
> undef, $stats_active);
> +
> +is($#dom_active+1, 3, "list all active domain");
> +
> +my $stats_shutoff = Sys::Virt::Domain::GET_ALL_STATS_SHUTOFF;
> +
> +my @dom_shutoff =
> $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE, undef,
> $stats_shutoff);
> +
> +is($#dom_shutoff+1, 1, "list all shutoff domain");
> +
> +my $stats_other = Sys::Virt::Domain::GET_ALL_STATS_OTHER;
> +
> +my @dom_other = $conn->get_all_domain_stats(Sys::Virt::Domain::STATS_STATE,
> undef, $stats_other);
> +
> +is($#dom_other+1, 0, "list all other domain")
Re: [libvirt] [PATCH] Document the effect of empty string as a rom file
On 03/05/2015 09:01 AM, Ján Tomko wrote: > This can be used to disable the ROM. > --- > docs/formatdomain.html.in | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) ACK > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 6276a61..050476f 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -4265,7 +4265,8 @@ qemu-kvm -net nic,model=? /dev/null >The optional file attribute is used to point to a >binary file to be presented to the guest as the device's ROM >BIOS. This can be useful to provide an alternative boot ROM for a > - network device. > + network device. If the file attribute is present, but empty, > + no ROM file will be loaded. >Since 0.9.10 (QEMU and KVM only). > > Setting up a network backend in a driver > domain > -- 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 v8 1/4] domifaddr: Implement the public APIs
On 03/05/2015 07:45 AM, Daniel P. Berrange wrote:
> On Mon, Jan 26, 2015 at 12:08:46AM +0530, Nehal J Wani wrote:
>> Define helper function virDomainInterfaceFree, which allows
>> the upper layer application to free the domain interface object
>> conveniently.
>>
>> The API is going to provide multiple methods by flags, e.g.
>> * Query guest agent
>> * Parse DHCP lease file
>>
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -3682,5 +3682,32 @@ typedef struct _virTypedParameter virMemoryParameter;
>> */
>> typedef virMemoryParameter *virMemoryParameterPtr;
>>
>> +typedef enum {
>> +VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE = (1 << 0), /* Parse DHCP lease
>> file */
>> +VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT = (1 << 1), /* Query qemu guest
>> agent */
>> +} virDomainInterfaceAddressesFlags;
This feels like it should not be named 'Flags', because it seems like
you can never request multiple modes at once.
>
> I can't help but have a nagging feeling this ambiguos return data means our
> API design is wrong. Instead of having a bitwise OR of flags to select the
> data source, should we perhaps use a straight enum, so the application always
> specifies exactly which data source to use ?
>
> eg an API that is
>
> enum {
> VIR_DOMAIN_INTERFACE_ADDRESSES_SOURCE_LEASES,
> VIR_DOMAIN_INTERFACE_ADDRESSES_SOURCE_AGENT,
> VIR_DOMAIN_INTERFACE_ADDRESSES_SOURCE_SNOOP,
> };
>
Yep - that's exactly my concern, and the right solution (that is,
_LEASES is 0, _AGENT is 1, _SNOOP is 2, and for now we have a _LAST at 3
that can later bump to 4 if we add another method).
> virDomainInterfaceAddresses(virDomainPtr dom,
> int source,
> virDomainInterfacePtr **ifaces,
> unsigned int flags);
>
> so the caller always knows how to interpret the returned data for
> mac addr andname. Leave the flags field unused, for future help
> if needed.
Yes, that sounds like the right direction.
--
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 v2 3/3] qemu: Pass file descriptor when using TPM passthrough
On 03/05/2015 10:25 AM, Michal Privoznik wrote:
On 03.03.2015 15:40, Stefan Berger wrote:
Pass the TPM file descriptor to QEMU via command line.
Instead of passing /dev/tpm0 we now pass /dev/fdset/10 and the additional
parameters -add-fd set=10,fd=20.
This addresses the use case when QEMU is started with non-root privileges
and QEMU cannot open /dev/tpm0 for example.
Signed-off-by: Stefan Berger
---
src/qemu/qemu_command.c | 121 ++--
1 file changed, 117 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d6bc294..4c6b76d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -161,6 +161,58 @@ VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST,
"interleave");
/**
+ * qemuVirCommandGetFDSet:
+ * @cmd: the command to modify
+ * @fd: fd to reassign to the child
+ *
+ * Get the parameters for the QEMU -add-fd command line option
+ * for the given file descriptor. The file descriptor must previously
+ * have been 'transferred' in a virCommandPassFD() call.
+ * This function for example returns "set=10,fd=20".
+ */
+static char *
+qemuVirCommandGetFDSet(virCommandPtr cmd, int fd)
+{
+char *result = NULL;
+int idx = virCommandPassFDGetFDIndex(cmd, fd);
+
+if (idx >= 0) {
+ignore_value(virAsprintf(&result, "set=%d,fd=%d", idx, fd));
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("file descriptor %d has not been transferred"), fd);
+}
+
+return result;
+}
+
+/**
+ * qemuVirCommandGetDevSet:
+ * @cmd: the command to modify
+ * @fd: fd to reassign to the child
+ *
+ * Get the parameters for the QEMU path= parameter where a file
+ * descriptor is accessed via a file descriptor set, for example
+ * /dev/fdset/10. The file descriptor must previously have been
+ * 'transferred' in a virCommandPassFD() call.
+ */
+static char *
+qemuVirCommandGetDevSet(virCommandPtr cmd, int fd)
+{
+char *result = NULL;
+int idx = virCommandPassFDGetFDIndex(cmd, fd);
+
+if (idx >= 0) {
+ignore_value(virAsprintf(&result, "/dev/fdset/%d", idx));
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("file descriptor %d has not been transferred"), fd);
+}
+return result;
+}
+
+
+/**
* qemuPhysIfaceConnect:
* @def: the definition of the VM (needed by 802.1Qbh and audit)
* @driver: pointer to the driver instance
@@ -6321,15 +6373,20 @@ qemuBuildRNGDevStr(virDomainDefPtr def,
static char *qemuBuildTPMBackendStr(const virDomainDef *def,
+virCommandPtr cmd,
virQEMUCapsPtr qemuCaps,
-const char *emulator)
+const char *emulator,
+int *tpmfd, int *cancelfd)
{
const virDomainTPMDef *tpm = def->tpm;
virBuffer buf = VIR_BUFFER_INITIALIZER;
const char *type = virDomainTPMBackendTypeToString(tpm->type);
-char *cancel_path;
+char *cancel_path = NULL, *devset = NULL;
const char *tpmdev;
+*tpmfd = -1;
+*cancelfd = -1;
+
virBufferAsprintf(&buf, "%s,id=tpm-%s", type, tpm->info.alias);
switch (tpm->type) {
@@ -6341,11 +6398,42 @@ static char *qemuBuildTPMBackendStr(const virDomainDef
*def,
if (!(cancel_path = virTPMCreateCancelPath(tpmdev)))
goto error;
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ADD_FD)) {
+*tpmfd = open(tpmdev, O_RDWR);
+if (*tpmfd < 0) {
+virReportSystemError(errno, _("Could not open TPM device %s"),
+ tpmdev);
+goto error;
+}
+
+virCommandPassFD(cmd, *tpmfd,
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+devset = qemuVirCommandGetDevSet(cmd, *tpmfd);
+if (devset == NULL)
+goto error;
+
+*cancelfd = open(cancel_path, O_WRONLY);
+if (*cancelfd < 0) {
+virReportSystemError(errno,
+ _("Could not open TPM device's cancel "
+ "path %s"), cancel_path);
+goto error;
+}
+VIR_FREE(cancel_path);
+
+virCommandPassFD(cmd, *cancelfd,
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+cancel_path = qemuVirCommandGetDevSet(cmd, *cancelfd);
+if (cancel_path == NULL)
+goto error;
+}
I'm thinking of how to test this code, but those open() complicate the
things. Unless we want to mock them in qemuxml2argvmock.c which would
not be trivial after all.
ACK series.
Michal
Pushed
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 7/9] domain: Introduce virDomainIOThreadsPin{Add|Del}
https://bugzilla.redhat.com/show_bug.cgi?id=1135491
More or less a virtual copy of the existing virDomainVcpuPin{Add|Del} API's.
NB: The IOThreads implementation "reused" the virDomainVcpuPinDefPtr
since it provided everything necessary - an "id" and a "map" for each
thread id configured.
Signed-off-by: John Ferlan
---
src/conf/domain_conf.c | 64
src/conf/domain_conf.h | 10
src/libvirt_private.syms | 2 ++
3 files changed, 76 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6da02b0..fc4e8bd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16708,6 +16708,70 @@ virDomainEmulatorPinDel(virDomainDefPtr def)
return 0;
}
+int
+virDomainIOThreadsPinAdd(virDomainVcpuPinDefPtr **iothreadspin_list,
+ size_t *niothreadspin,
+ unsigned char *cpumap,
+ int maplen,
+ int iothread_val)
+{
+/* IOThreads share the virDomainVcpuPinDefPtr */
+virDomainVcpuPinDefPtr iothreadpin = NULL;
+
+if (!iothreadspin_list)
+return -1;
+
+iothreadpin = virDomainVcpuPinFindByVcpu(*iothreadspin_list,
+ *niothreadspin,
+ iothread_val);
+if (iothreadpin) {
+iothreadpin->vcpuid = iothread_val;
+virBitmapFree(iothreadpin->cpumask);
+iothreadpin->cpumask = virBitmapNewData(cpumap, maplen);
+if (!iothreadpin->cpumask)
+return -1;
+
+return 0;
+}
+
+/* No existing iothreadpin matches iothread_val, adding a new one */
+
+if (VIR_ALLOC(iothreadpin) < 0)
+goto error;
+
+iothreadpin->vcpuid = iothread_val;
+iothreadpin->cpumask = virBitmapNewData(cpumap, maplen);
+if (!iothreadpin->cpumask)
+goto error;
+
+if (VIR_APPEND_ELEMENT(*iothreadspin_list, *niothreadspin, iothreadpin) <
0)
+goto error;
+
+return 0;
+
+ error:
+virDomainVcpuPinDefFree(iothreadpin);
+return -1;
+}
+
+void
+virDomainIOThreadsPinDel(virDomainDefPtr def,
+ int iothread_val)
+{
+size_t i;
+/* IOThreads share the virDomainVcpuPinDefPtr */
+virDomainVcpuPinDefPtr *iothreadspin_list = def->cputune.iothreadspin;
+
+for (i = 0; i < def->cputune.niothreadspin; i++) {
+if (iothreadspin_list[i]->vcpuid == iothread_val) {
+virBitmapFree(iothreadspin_list[i]->cpumask);
+VIR_DELETE_ELEMENT(def->cputune.iothreadspin, i,
+ def->cputune.niothreadspin);
+return;
+}
+}
+}
+
static int
virDomainEventActionDefFormat(virBufferPtr buf,
int type,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 36bb418..17342a4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2563,6 +2563,16 @@ int virDomainEmulatorPinAdd(virDomainDefPtr def,
int virDomainEmulatorPinDel(virDomainDefPtr def);
+/* IOThreads share the virDomainVcpuPinDefPtr */
+int virDomainIOThreadsPinAdd(virDomainVcpuPinDefPtr **iothreadspin_list,
+ size_t *niothreads,
+ unsigned char *cpumap,
+ int maplen,
+ int iothread_val);
+
+void virDomainIOThreadsPinDel(virDomainDefPtr def,
+ int iothread_val);
+
void virDomainRNGDefFree(virDomainRNGDefPtr def);
bool virDomainDiskDefDstDuplicates(virDomainDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c810cf7..baacdc2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -308,6 +308,8 @@ virDomainHubTypeToString;
virDomainHypervTypeFromString;
virDomainHypervTypeToString;
virDomainInputDefFree;
+virDomainIOThreadsPinAdd;
+virDomainIOThreadsPinDel;
virDomainLeaseDefFree;
virDomainLeaseIndex;
virDomainLeaseInsert;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 8/9] qemu: Add support to get/set specific IOThread pinning data
Add qemuDomainGetIOThreadPin to handle getting the CPU Affinity for
one specific IOThread
Add qemuDomainPinIOThread to handle setting the cpuset for a specific
IOThread
Signed-off-by: John Ferlan
---
include/libvirt/libvirt-domain.h | 9 ++
src/qemu/qemu_driver.c | 302 +++
2 files changed, 311 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index fc35cd2..e81389a 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3223,6 +3223,15 @@ typedef void
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
# define VIR_DOMAIN_TUNABLE_CPU_EMULATORPIN "cputune.emulatorpin"
/**
+ * VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN:
+ *
+ * Macro represents formatted pinning for one IOThread specified by id which is
+ * appended to the parameter name, for example "cputune.iothreadpin1",
+ * as VIR_TYPED_PARAM_STRING.
+ */
+# define VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN "cputune.iothreadpin%u"
+
+/**
* VIR_DOMAIN_TUNABLE_CPU_CPU_SHARES:
*
* Macro represents proportional weight of the scheduler used on the
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d8a761d..5c8040e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5780,6 +5780,306 @@ qemuDomainGetIOThreadsInfo(virDomainPtr dom,
return ret;
}
+static int
+qemuDomainGetIOThreadPin(virDomainPtr dom,
+ unsigned int iothread_id,
+ unsigned char *cpumap,
+ int maplen,
+ unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+virDomainDefPtr targetDef = NULL;
+int ret = -1;
+virCapsPtr caps = NULL;
+int maxcpu, hostcpus, pcpu;
+virBitmapPtr cpumask = NULL;
+bool pinned;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetIOThreadPinEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+goto cleanup;
+
+if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
+vm, &flags, &targetDef) < 0)
+goto cleanup;
+
+if (flags & VIR_DOMAIN_AFFECT_LIVE)
+targetDef = vm->def;
+
+/* Coverity didn't realize that targetDef must be set if we got here. */
+sa_assert(targetDef);
+
+if ((hostcpus = nodeGetCPUCount()) < 0)
+goto cleanup;
+
+maxcpu = maplen * 8;
+if (maxcpu > hostcpus)
+maxcpu = hostcpus;
+
+/* initialize cpumap */
+memset(cpumap, 0xff, maplen);
+if (maxcpu % 8)
+cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1;
+
+if (targetDef->cputune.iothreadspin) {
+if (iothread_id > targetDef->iothreads) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("iothread value out of range %d > %d"),
+ iothread_id, targetDef->iothreads);
+goto cleanup;
+}
+
+cpumask = targetDef->cputune.iothreadspin[iothread_id - 1]->cpumask;
+} else if (targetDef->cpumask) {
+cpumask = targetDef->cpumask;
+} else {
+ret = 0;
+goto cleanup;
+}
+
+for (pcpu = 0; pcpu < maxcpu; pcpu++) {
+if (virBitmapGetBit(cpumask, pcpu, &pinned) < 0)
+goto cleanup;
+if (!pinned)
+VIR_UNUSE_CPU(cpumap, pcpu);
+}
+ret = 1;
+
+ cleanup:
+qemuDomObjEndAPI(&vm);
+virObjectUnref(caps);
+return ret;
+}
+
+static int
+qemuDomainPinIOThread(virDomainPtr dom,
+ unsigned int iothread_id,
+ unsigned char *cpumap,
+ int maplen,
+ unsigned int flags)
+{
+int ret = -1;
+virQEMUDriverPtr driver = dom->conn->privateData;
+virQEMUDriverConfigPtr cfg = NULL;
+virDomainObjPtr vm;
+virCapsPtr caps = NULL;
+virDomainDefPtr persistentDef = NULL;
+virBitmapPtr pcpumap = NULL;
+bool doReset = false;
+qemuDomainObjPrivatePtr priv;
+virDomainVcpuPinDefPtr *newIOThreadsPin = NULL;
+size_t newIOThreadsPinNum = 0;
+virCgroupPtr cgroup_iothread = NULL;
+virObjectEventPtr event = NULL;
+char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = "";
+char *str = NULL;
+virTypedParameterPtr eventParams = NULL;
+int eventNparams = 0;
+int eventMaxparams = 0;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+cfg = virQEMUDriverGetConfig(driver);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+goto cleanup;
+priv = vm->privateData;
+
+if (virDomainPinIOThreadEnsureACL(dom->conn, vm->def, flags) < 0)
+goto cleanup;
+
+if (!(caps = virQEMUDriverGetCapabilities(driver, fa
