Re: [libvirt] [PATCH v2 2/6] rpc: Initialize a worker pool for max_workers=0 as well
On Thu, Jul 19, 2018 at 04:52 PM +0200, John Ferlan wrote:
> On 07/03/2018 07:37 AM, Marc Hartmayer wrote:
>> Semantically, there is no difference between an uninitialized worker
>> pool and an initialized worker pool with zero workers. Let's allow the
>> worker pool to be initialized for max_workers=0 as well then which
>> makes the API more symmetric and simplifies code. Validity of the
>> worker pool is delegated to virThreadPoolGetMaxWorkersLocked instead.
>>
>> This patch fixes segmentation faults in
>> virNetServerGetThreadPoolParameters and
>> virNetServerSetThreadPoolParameters for the case when no worker pool
>> is actually initialized (max_workers=0).
>>
>> Signed-off-by: Marc Hartmayer
>> Reviewed-by: Boris Fiuczynski
>> Reviewed-by: Bjoern Walk
>> ---
>> src/libvirt_private.syms | 4 +++
>> src/rpc/virnetserver.c | 13 -
>> src/util/virthreadpool.c | 73
>>
>> src/util/virthreadpool.h | 8 ++
>> 4 files changed, 73 insertions(+), 25 deletions(-)
>>
>
> So it seems there's multiple things going on in this patch - maybe
> they're semantically tied and I'm missing it ;-)...
>
> The virNetServerNew to not inhibit the call to virThreadPoolNew if
> max_workers == 0 would seem to be easily separable with the other places
> that would check if srv->workers == NULL before continuing.
Is the code still safe if the worker count changes after getting the
worker count?
If yes, then I can split the patch if you want to.
>
> I don't understand why virNetServerDispatchNewMessage needs anything
> more than if (virThreadPoolGetMaxWorkers(srv->workers)
>
> If I think back to the previous patch, then why not:
>
> size_t workers = 0;
> ...
>
> if (srv->workers)
> workers = virThreadPoolGetMaxWorkers(srv->workers);
The reason is/was that my original code assumed it’s allowed to switch
between zero and non-zero worker counts. My intention was to hold the
lock for srv->workers (and therefore forbid any changes on the worker
count) as long as needed.
>
> /* we can unlock @srv since @prog can only become invalid in case
> * of disposing @srv, but let's grab a ref first to ensure nothing
> * else disposes of it before we use it. */
> virObjectRef(srv);
> virObjectUnlock(srv);
>
> if (workers > 0) {
> ...
>
> In the long run, I don't think it's been the desire to expose so many
> virthreadpool.c API's - especially the locks.
Yes, I don’t like it either.
>
> John
>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index ffe5dfd19b11..aa496ddf8012 100644
[…snip]
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/6] virThreadPool: Prevent switching between zero and non-zero maxWorkers
On Thu, Jul 19, 2018 at 04:53 PM +0200, John Ferlan wrote: > On 07/03/2018 07:37 AM, Marc Hartmayer wrote: >> ...since maxWorkers=0 is only intended for virtlockd or virlogd which >> must not be multithreaded. >> >> Signed-off-by: Marc Hartmayer >> Reviewed-by: Boris Fiuczynski >> Reviewed-by: Bjoern Walk >> --- >> src/util/virthreadpool.c | 8 >> 1 file changed, 8 insertions(+) >> > > I think this is separable - this could be pushed regardless of the > outcome of patch2, right? Let me know - I can reduce the load the next > update based on patch2 comments. Yep. Thanks for reviewing. > > Reviewed-by: John Ferlan > > John > -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 03/15] Switch from yajl to Jansson
On Thu, Jul 19, 2018 at 07:24:43PM -0400, John Ferlan wrote:
On 07/18/2018 10:44 AM, Ján Tomko wrote:
Yajl has not seen much activity upstream recently.
Switch to using Jansson >= 2.5.
All the platforms we target on https://libvirt.org/platforms.html
have a version >= 2.7 listed on the sites below:
https://repology.org/metapackage/jansson/versions
https://build.opensuse.org/package/show/devel:libraries:c_c++/libjansson
Additionally, Ubuntu 14.04 on Travis-CI has 2.5. Set the requirement
to 2.5 since we don't use anything from newer versions.
Implement virJSONValue{From,To}String using Jansson, delete the yajl
code (and the related virJSONParser structure) and report an error
if someone explicitly specifies --with-yajl.
Also adjust the test data to account for Jansson's different whitespace
usage for empty arrays and tune up the specfile to keep 'make rpm'
working when bisecting.
Signed-off-by: Ján Tomko
---
src/util/virjson.c | 211 +
1 file changed, 211 insertions(+)
Too late now, but git bisect and running the test suite seems to be
broken starting with this patch as I started getting "FAIL:
qemublocktest" here running a bisect chasing something else.
As I found out "later" I didn't have jansson-devel dnf installed either,
so that may have affected my results. Who knows at this point.
The failure goes away with patch 13, but that introduced other issues
for me. I'll respond there with what I found.
Until patch 15/15, qemublocktest was only guarded by WITH_QEMU, despite
requiring a working JSON implementation, so that failure does not
surprise me.
Jano
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 13/15] build: switch --with-qemu default from yes to check
On Fri, Jul 20, 2018 at 12:24:50PM +0200, Ján Tomko wrote: > On Thu, Jul 19, 2018 at 07:38:15PM -0400, John Ferlan wrote: > > > > > > On 07/18/2018 10:44 AM, Ján Tomko wrote: > > > Unless explicitly requested, enable the QEMU driver > > > only if the Jansson library is present. > > > > > > Signed-off-by: Ján Tomko > > > --- > > > m4/virt-driver-qemu.m4 | 6 +- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > Perhaps it's obvious for someone else, but I think there some sort of > > dependency missing. > > Starting with this patch I found that my with-qemu > > "went away". > > > > I have: > > > > jansson.x86_64 2.11-1.fc28 > > @fedora > > > > > > But not: > > > > jansson-devel x86_64 2.11-1.fc28 fedora > > 15 k > > > > If I explicitly add --with-jansson onto the command line, then I get: > > > > checking for JANSSON... no > > configure: error: You must install the jansson >= 2.5 pkg-config module > > to compile libvirt > > error: configure failed > > > > If I then install jansson-devel, the build succeeds. > > > > Honestly I think we need to be much more in your face in this instance - > > something isn't quite right and it eventually leads to some really > > strange results because nothing in/for qemu is built, but you are left > > with old build bits in your tree. Eventually something fails. > > > > There was not that much discussion about it: > https://www.redhat.com/archives/libvir-list/2018-May/msg01321.html > > I do not oppose reverting this bit and failing by default if we don't > have a JSON library (as Andrea mentioned, more drivers might possibly > require a working JSON implementation). We use virjson.h throughout src/util and src/rpc so that code is not even driver specific, however, there are not currently mingw pacakges for jansson in Fedora at least. Maybe it will work, but we would need someone todo the work to add that to Fedora before we can consider making this mandatory. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 13/15] build: switch --with-qemu default from yes to check
On Thu, Jul 19, 2018 at 07:38:15PM -0400, John Ferlan wrote: On 07/18/2018 10:44 AM, Ján Tomko wrote: Unless explicitly requested, enable the QEMU driver only if the Jansson library is present. Signed-off-by: Ján Tomko --- m4/virt-driver-qemu.m4 | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) Perhaps it's obvious for someone else, but I think there some sort of dependency missing. Starting with this patch I found that my with-qemu "went away". I have: jansson.x86_64 2.11-1.fc28 @fedora But not: jansson-devel x86_64 2.11-1.fc28 fedora 15 k If I explicitly add --with-jansson onto the command line, then I get: checking for JANSSON... no configure: error: You must install the jansson >= 2.5 pkg-config module to compile libvirt error: configure failed If I then install jansson-devel, the build succeeds. Honestly I think we need to be much more in your face in this instance - something isn't quite right and it eventually leads to some really strange results because nothing in/for qemu is built, but you are left with old build bits in your tree. Eventually something fails. There was not that much discussion about it: https://www.redhat.com/archives/libvir-list/2018-May/msg01321.html I do not oppose reverting this bit and failing by default if we don't have a JSON library (as Andrea mentioned, more drivers might possibly require a working JSON implementation). However after dropping old QEMU support, building QEMU driver without JSON makes no sense. (There was some logic in virt-yajl.m4 that tried to figure out whether lack of yajl should be fatal based on the version of the QEMU executable present on the system). This whole build/config system is a generally a mystery to me, so I have zero suggestions. Ironically if I had to build from source, I'd know from libvirt.spec that jansson-devel was required, although there's no = version check so that probably should be fixed. Right, the specfile still has: %define min_rhel 6 Suffice to say digging into the config.log trying to figure why one's QEMU is disabled is not an enjoyable or easy experience. Yeah, yeah, we're developers we're supposed to be smart, we get what we get... I'll bet some qemu devel will hit this some day and wonder how to actually build libvirt with qemu because it's not obvious and it "used to be" a default=yes. I expected that people surprised by the lack of QEMU driver would pass --with-qemu, which should have the proper error. Jano John Off to go drown the rest of my frustration ;-) 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 1/6] rpc: Fix deadlock if there is no worker pool available
On Thu, Jul 19, 2018 at 04:51 PM +0200, John Ferlan wrote:
> On 07/03/2018 07:37 AM, Marc Hartmayer wrote:
>> @srv must be unlocked for the call virNetServerProcessMsg otherwise a
>> deadlock can occur.
>>
>> Since the pointer 'srv->workers' will never be changed after
>> initialization and the thread pool has it's own locking we can release
>> the lock of 'srv' earlier. This also fixes the deadlock.
>>
>> Signed-off-by: Marc Hartmayer
>> Reviewed-by: Boris Fiuczynski
>> Reviewed-by: Bjoern Walk
>> ---
>> src/rpc/virnetserver.c | 15 ++-
>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
>> index 5c7f7dd08fe1..091e3ef23406 100644
>> --- a/src/rpc/virnetserver.c
>> +++ b/src/rpc/virnetserver.c
>> @@ -51,6 +51,7 @@ struct _virNetServer {
>>
>> char *name;
>>
>> +/* Immutable pointer, self-locking APIs */
>> virThreadPoolPtr workers;
>>
>> char *mdnsGroupName;
>> @@ -177,9 +178,11 @@ static void virNetServerHandleJob(void *jobOpaque, void
>> *opaque)
>> VIR_FREE(job);
>> }
>>
>> -static void virNetServerDispatchNewMessage(virNetServerClientPtr client,
>> - virNetMessagePtr msg,
>> - void *opaque)
>> +
>> +static void
>> +virNetServerDispatchNewMessage(virNetServerClientPtr client,
>> + virNetMessagePtr msg,
>> + void *opaque)
>> {
>> virNetServerPtr srv = opaque;
>> virNetServerProgramPtr prog = NULL;
>> @@ -196,6 +199,9 @@ static void
>> virNetServerDispatchNewMessage(virNetServerClientPtr client,
>> break;
>> }
>> }
>
> Should we grab an object reference then to avoid the chance that
> something disposes srv right after it's unlocked? And then Unref instead
> of Unlock later on?
>
> Following virThreadPoolSendJob finds that it'll grab the srv->workers
> pool mutex and check the quit flag before adding a job to and of course
> as you point out virNetServerProcessMsg would eventually assume that the
> server is unlocked in virNetServerProgramDispatchCall before calling
> dispatcher->func.
>
> With a Ref/Unref, I'd feel better and thus consider this,
Yep, I’m fine with that.
>
> Reviewed-by: John Ferlan
Thanks!
>
> John
>
>> +/* we can unlock @srv since @prog can only become invalid in case
>> + * of disposing @srv */
>> +virObjectUnlock(srv);
>>
>> if (srv->workers) {
>> virNetServerJobPtr job;
>> @@ -223,15 +229,14 @@ static void
>> virNetServerDispatchNewMessage(virNetServerClientPtr client,
>> goto error;
>> }
>>
>> -virObjectUnlock(srv);
>> return;
>>
>> error:
>> virNetMessageFree(msg);
>> virNetServerClientClose(client);
>> -virObjectUnlock(srv);
>> }
>>
>> +
>> /**
>> * virNetServerCheckLimits:
>> * @srv: server to check limits on
>>
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] replace 'if' type conditions with 'switch' for VIR_NETWORK_FORWARD_*
On Fri, July 20, 2018 at 4:52 PM, Erik wrote:
> On Fri, Jul 20, 2018 at 12:00:53PM +0800, Shi Lei wrote:
> > Hi, everyone!
> > For VIR_NETWORK_FORWARD_*, I try to replace 'if' type conditions
> > with typed 'switch()'.
> > It might be more clear.
> >
> > Signed-off-by: Shi Lei
> >
> > ---
>
> ...
>
> > +networkRemoveFirewallRules(def);
> > +if (networkAddFirewallRules(def) < 0) {
> > +/* failed to add but already logged */
> > +}
>
> ^This if should go away, just call networkAddFirewallRules(def);
OK.
>
> ...
>
> Conceptually, I agree with your changes, however, since you're already doing
> the convert, recently we've changed our practice on how we approach the
> switches. First, you should typecast def->forward.type to
> virNetworkForwardType
> explicitly, so that we can utilize compile time checks. Second, we use the
> default case only to report an out of range error with virReportEnumRangeError
> (mainly because the values you care about are all already enumerated), you can
> find plenty of examples throughout the code base if you look for the function
> I
> just mentioned. No other objections from my side.
>
> Regards,
> Erik
>
Thanks for your reviewing and directions, Erik!
And I have found examples you mentioned.
Have a nice day!
Shi Lei
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] util: set OOM in virCopyLastError if error is not set
On 19.07.2018 20:30, John Ferlan wrote:
>
>
> On 07/19/2018 04:15 AM, Nikolay Shirokovskiy wrote:
>>
>>
>> On 17.07.2018 22:28, John Ferlan wrote:
>>>
>>>
>> +} else {
>> +to->code = VIR_ERR_NO_MEMORY;
>> +to->domain = VIR_FROM_NONE;
>> +to->level = VIR_ERR_ERROR;
>
> Should we do a VIR_FREE(to->message); so that nothing that was there
> before somehow remains... I don't think either agent or monitor>
> "lastError" is reset until Dispose time.
Won't hurt but probably will not be used by monitor or agent. If thread
error is not allocated message is NULL upon return, after error is
allocated we never
hit this OOM branch anymore. Of course hypotetical client can bring @to
with message already set so this a bit future proof.
I think then we can leave reset and then set these 3 fields.
Nikolay
>>>
>>> You have commit access and my R-by regardless of whether you add the
>>> VIR_FREE or not. I leave the rest to you ;-)
>>>
>>
>> Thanx! Pushed with reset added.
>>
>> Nikolay
>>
>
> Oh no, you pushed a local .gnulib too!
>
> git log -p
> ...
> commit 1bff5bbe25eb7a7e7a4e0067c4ca7cbc1cb34999
> Author: Nikolay Shirokovskiy
> Date: Mon Jul 2 14:16:52 2018 +0300
>
> ...
> diff --git a/.gnulib b/.gnulib
> index cdbf3d385a..d6397dde2e 16
> --- a/.gnulib
> +++ b/.gnulib
> @@ -1 +1 @@
> -Subproject commit cdbf3d385a32ff904c96f20c26f3470bd8345248
> +Subproject commit d6397dde2e127e246e3eeb5254a21f42cac783c8
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index f198f27957..5f26d59777 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
>
>
> ...
Sorry, hope I'll be more careful in the future.
Nikolay
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tools: Fix typo generating adapter_wwpn field
On Thu, Jul 19, 2018 at 02:03:44PM -0400, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1601377 > > Fix typo from commit id d45bee449 for the parent_wwpn field > resulting in parent_wwnn being printed twice. > > Signed-off-by: John Ferlan > --- Reviewed-by: Erik Skultety (trivial) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] replace 'if' type conditions with 'switch' for VIR_NETWORK_FORWARD_*
On Fri, Jul 20, 2018 at 12:00:53PM +0800, Shi Lei wrote:
> Hi, everyone!
> For VIR_NETWORK_FORWARD_*, I try to replace 'if' type conditions
> with typed 'switch()'.
> It might be more clear.
>
> Signed-off-by: Shi Lei
>
> ---
...
> +networkRemoveFirewallRules(def);
> +if (networkAddFirewallRules(def) < 0) {
> +/* failed to add but already logged */
> +}
^This if should go away, just call networkAddFirewallRules(def);
...
Conceptually, I agree with your changes, however, since you're already doing
the convert, recently we've changed our practice on how we approach the
switches. First, you should typecast def->forward.type to virNetworkForwardType
explicitly, so that we can utilize compile time checks. Second, we use the
default case only to report an out of range error with virReportEnumRangeError
(mainly because the values you care about are all already enumerated), you can
find plenty of examples throughout the code base if you look for the function I
just mentioned. No other objections from my side.
Regards,
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] rpm: modernize the spec file for RHEL
Drop conditionals for RHEL-6 and assume systemd is present Daniel P. Berrangé (2): rpm: increase min required RHEL to 7 rpm: remove conditionals for systemd libvirt.spec.in | 246 1 file changed, 20 insertions(+), 226 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] rpm: increase min required RHEL to 7
We no longer build on RHEL-6, so can bump min required RHEL to 7
removing many conditions.
Signed-off-by: Daniel P. Berrangé
---
libvirt.spec.in | 92 ++---
1 file changed, 11 insertions(+), 81 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index f83e5f8601..150a168514 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -3,7 +3,7 @@
# This spec file assumes you are building on a Fedora or RHEL version
# that's still supported by the vendor. It may work on other distros
# or versions, but no effort will be made to ensure that going forward.
-%define min_rhel 6
+%define min_rhel 7
%define min_fedora 26
%if (0%{?fedora} && 0%{?fedora} >= %{min_fedora}) || (0%{?rhel} && 0%{?rhel}
>= %{min_rhel})
@@ -34,10 +34,7 @@
%if 0%{?rhel}
%define with_qemu_tcg 0
-%define qemu_kvm_arches x86_64
-%if 0%{?rhel} >= 7
-%define qemu_kvm_arches x86_64 %{power64} aarch64 s390x
-%endif
+%define qemu_kvm_arches x86_64 %{power64} aarch64 s390x
%endif
%ifarch %{qemu_kvm_arches}
@@ -58,11 +55,7 @@
%define with_hyperv0%{!?_without_hyperv:1}
# Then the secondary host drivers, which run inside libvirtd
-%if 0%{?fedora} || 0%{?rhel} >= 7
-%define with_storage_rbd 0%{!?_without_storage_rbd:1}
-%else
-%define with_storage_rbd 0
-%endif
+%define with_storage_rbd 0%{!?_without_storage_rbd:1}
%if 0%{?fedora}
%define with_storage_sheepdog 0%{!?_without_storage_sheepdog:1}
%else
@@ -116,7 +109,7 @@
# librados and librbd are built only on x86_64 on rhel
%ifnarch x86_64
-%if 0%{?rhel} >= 7
+%if 0%{?rhel}
%define with_storage_rbd 0
%endif
%endif
@@ -146,20 +139,12 @@
%endif
%endif
-# Fedora 17 / RHEL-7 are first where we use systemd. Although earlier
-# Fedora has systemd, libvirt still used sysvinit there.
-%if 0%{?fedora} || 0%{?rhel} >= 7
-%define with_systemd 1
-%define with_pm_utils 0
-%endif
-
-# Fedora 18 / RHEL-7 are first where firewalld support is enabled
-%if 0%{?fedora} || 0%{?rhel} >= 7
-%define with_firewalld 1
-%endif
+%define with_systemd 1
+%define with_pm_utils 0
+%define with_firewalld 1
# fuse is used to provide virtualized /proc for LXC
-%if %{with_lxc} && 0%{?rhel} != 6
+%if %{with_lxc}
%define with_fuse 0%{!?_without_fuse:1}
%endif
@@ -194,10 +179,7 @@
%define with_libssh 0%{!?_without_libssh:1}
%endif
-# Enable bash-completion for new enough distros
-%if 0%{?fedora} || 0%{?rhel} >= 7
-%define with_bash_completion 0%{!?_without_bash_completion:1}
-%endif
+%define with_bash_completion 0%{!?_without_bash_completion:1}
# Use Python 3 when possible, Python 2 otherwise
%if 0%{?fedora} || 0%{?rhel} > 7
@@ -220,11 +202,7 @@
%define qemu_group qemu
-%if 0%{?fedora} || 0%{?rhel} >= 7
-%define with_systemd_macros 1
-%else
-%define with_systemd_macros 0
-%endif
+%define with_systemd_macros 1
# RHEL releases provide stable tool chains and so it is safe to turn
@@ -315,47 +293,29 @@ BuildRequires: bash-completion >= 2.0
BuildRequires: ncurses-devel
BuildRequires: gettext
BuildRequires: libtasn1-devel
-%if (0%{?rhel} && 0%{?rhel} < 7)
-BuildRequires: libgcrypt-devel
-%endif
BuildRequires: gnutls-devel
BuildRequires: libattr-devel
# For pool-build probing for existing pools
BuildRequires: libblkid-devel >= 2.17
# for augparse, optionally used in testing
BuildRequires: augeas
-%if 0%{?fedora} || 0%{?rhel} >= 7
BuildRequires: systemd-devel >= 185
-%else
-BuildRequires: libudev-devel >= 145
-%endif
BuildRequires: libpciaccess-devel >= 0.10.9
BuildRequires: jansson-devel
%if %{with_sanlock}
BuildRequires: sanlock-devel >= 2.4
%endif
BuildRequires: libpcap-devel
-%if 0%{?rhel} && 0%{?rhel} < 7
-BuildRequires: libnl-devel
-%else
BuildRequires: libnl3-devel
-%endif
BuildRequires: avahi-devel
BuildRequires: libselinux-devel
BuildRequires: dnsmasq >= 2.41
BuildRequires: iptables
-%if 0%{?rhel} && 0%{?rhel} < 7
-BuildRequires: iptables-ipv6
-%endif
BuildRequires: radvd
BuildRequires: ebtables
BuildRequires: module-init-tools
BuildRequires: cyrus-sasl-devel
-%if 0%{?fedora} || 0%{?rhel} >= 7
BuildRequires: polkit >= 0.112
-%else
-BuildRequires: polkit >= 0.93
-%endif
# For mount/umount in FS driver
BuildRequires: util-linux
%if %{with_qemu}
@@ -373,12 +333,8 @@ BuildRequires: parted-devel
# For Multipath support
BuildRequires: device-mapper-devel
%if %{with_storage_rbd}
-%if 0%{?fedora} || 0%{?rhel} >= 7
BuildRequires: librados2-devel
BuildRequires: librbd1-devel
-%else
-BuildRequires: ceph-devel
-%endif
%endif
%if %{with_storage_gluster}
BuildRequires: glusterfs-api-devel >= 3.4.1
@@ -405,11 +361,7 @@ BuildRequires: fuse-devel >= 2.8.6
BuildRequires: libssh2-devel >= 1.3.0
%endif
-%if 0%{?fedora} || 0%{?rhel} >= 7
BuildRequires: netcf-devel >= 0.2.2
-%else
-BuildRequires: netcf-devel >= 0.1.8
-%endif
%if %{with_esx}
BuildRequires: libcurl-devel
[libvirt] [PATCH 2/2] rpm: remove conditionals for systemd
All our supported RHEL and Fedora versions include systemd, so we can
assume it is always present in the spec.
Signed-off-by: Daniel P. Berrangé
---
libvirt.spec.in | 160
1 file changed, 12 insertions(+), 148 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 150a168514..4113579e47 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -74,14 +74,12 @@
# A few optional bits off by default, we enable later
%define with_fuse 0%{!?_without_fuse:0}
%define with_sanlock 0%{!?_without_sanlock:0}
-%define with_systemd 0%{!?_without_systemd:0}
%define with_numad 0%{!?_without_numad:0}
%define with_firewalld 0%{!?_without_firewalld:0}
%define with_libssh2 0%{!?_without_libssh2:0}
%define with_wireshark 0%{!?_without_wireshark:0}
%define with_libssh0%{!?_without_libssh:0}
%define with_bash_completion 0%{!?_without_bash_completion:0}
-%define with_pm_utils 1
# Finally set the OS / architecture specific special cases
@@ -139,8 +137,6 @@
%endif
%endif
-%define with_systemd 1
-%define with_pm_utils 0
%define with_firewalld 1
# fuse is used to provide virtualized /proc for LXC
@@ -202,9 +198,6 @@
%define qemu_group qemu
-%define with_systemd_macros 1
-
-
# RHEL releases provide stable tool chains and so it is safe to turn
# compiler warning into errors without being worried about frequent
# changes in reported warnings
@@ -278,9 +271,7 @@ BuildRequires: perl-interpreter
BuildRequires: perl
%endif
BuildRequires: %{python}
-%if %{with_systemd}
BuildRequires: systemd-units
-%endif
%if %{with_libxl}
BuildRequires: xen-devel
%endif
@@ -443,12 +434,10 @@ Requires: polkit >= 0.112
Requires: dmidecode
%endif
# For service management
-%if %{with_systemd}
Requires(post): systemd-units
Requires(post): systemd-sysv
Requires(preun): systemd-units
Requires(postun): systemd-units
-%endif
%if %{with_numad}
Requires: numad
%endif
@@ -909,10 +898,6 @@ Requires: ncurses
Requires: gettext
# Needed by virt-pki-validate script.
Requires: gnutls-utils
-%if %{with_pm_utils}
-# Needed for probing the power management features of the host.
-Requires: pm-utils
-%endif
%if %{with_bash_completion}
Requires: %{name}-bash-completion = %{version}-%{release}
%endif
@@ -1172,24 +1157,12 @@ exit 1
%define arg_wireshark --without-wireshark-dissector
%endif
-%if %{with_pm_utils}
-%define arg_pm_utils --with-pm-utils
-%else
-%define arg_pm_utils --without-pm-utils
-%endif
-
%define when %(date +"%%F-%%T")
%define where %(hostname)
%define who %{?packager}%{!?packager:Unknown}
%define arg_packager --with-packager="%{who}, %{when}, %{where}"
%define arg_packager_version --with-packager-version="%{release}"
-%if %{with_systemd}
-%define arg_init_script --with-init-script=systemd
-%else
-%define arg_init_script --with-init-script=redhat
-%endif
-
%define arg_selinux_mount --with-selinux-mount="/sys/fs/selinux"
%if 0%{?fedora}
@@ -1262,7 +1235,7 @@ rm -f po/stamp-po
--with-driver-modules \
%{?arg_firewalld} \
%{?arg_wireshark} \
- %{?arg_pm_utils} \
+ --without-pm-utils \
--with-nss-plugin \
%{arg_packager} \
%{arg_packager_version} \
@@ -1272,7 +1245,7 @@ rm -f po/stamp-po
%{?arg_loader_nvram} \
%{?enable_werror} \
--enable-expensive-tests \
- %{arg_init_script} \
+ --with-init-script=systemd \
%{?arg_login_shell}
make %{?_smp_mflags} V=1
gzip -9 ChangeLog
@@ -1396,83 +1369,25 @@ exit 0
%post daemon
-%if %{with_systemd}
-%if %{with_systemd_macros}
-%systemd_post virtlockd.socket virtlockd-admin.socket
-%systemd_post virtlogd.socket virtlogd-admin.socket
-%systemd_post libvirtd.service
-%else
-if [ $1 -eq 1 ] ; then
-# Initial installation
-/bin/systemctl enable \
-virtlockd.socket \
-virtlockd-admin.socket \
-virtlogd.socket \
-virtlogd-admin.socket \
-libvirtd.service >/dev/null 2>&1 || :
-fi
-%endif
-%else
-/sbin/chkconfig --add libvirtd
-/sbin/chkconfig --add virtlogd
-/sbin/chkconfig --add virtlockd
-%endif
+%systemd_post virtlockd.socket virtlockd-admin.socket
+%systemd_post virtlogd.socket virtlogd-admin.socket
+%systemd_post libvirtd.service
# request daemon restart in posttrans
mkdir -p %{_localstatedir}/lib/rpm-state/libvirt || :
touch %{_localstatedir}/lib/rpm-state/libvirt/restart || :
%preun daemon
-%if %{with_systemd}
-%if %{with_systemd_macros}
-%systemd_preun libvirtd.service
-%systemd_preun virtlogd.socket virtlogd-admin.socket virtlogd.service
-%systemd_preun virtlockd.socket virtlockd-admin.socket
virtlockd.service
-%else
-if [ $1 -eq 0 ] ; then
-# Package removal, not upgrade
-/bin/systemctl --no-reload disable \
-
Re: [libvirt] [PATCH 3/3] storage: Implement iscsi_direct pool backend
On Sat, 14 Jul 2018 00:06:15 +0200
c...@lse.epita.fr wrote:
> From: Clementine Hayat
>
> We need here libiscsi for the storgae pool backend.
> For the iscsi-direct storage pool, only checkPool and refreshPool should
> be necessary.
> The pool is state-less and just need the informations within the volume
> to work.
>
> Signed-off-by: Clementine Hayat
> ---
> m4/virt-storage-iscsi-direct.m4| 3 +
> src/storage/Makefile.inc.am| 3 +
> src/storage/storage_backend_iscsi_direct.c | 407 -
> 3 files changed, 410 insertions(+), 3 deletions(-)
>
> diff --git a/m4/virt-storage-iscsi-direct.m4 b/m4/virt-storage-iscsi-direct.m4
> index cc2d490352..dab4414169 100644
> --- a/m4/virt-storage-iscsi-direct.m4
> +++ b/m4/virt-storage-iscsi-direct.m4
> @@ -29,6 +29,9 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_ISCSI_DIRECT], [
> with_storage_iscsi_direct=$with_libiscsi
>fi
>if test "$with_storage_iscsi_direct" = "yes"; then
> +if test "$with_libiscsi" = "no"; then
> + AC_MSG_ERROR([Need libiscsi for iscsi-direct storage driver])
> +fi
> AC_DEFINE_UNQUOTED([WITH_STORAGE_ISCSI_DIRECT], [1],
> [whether iSCSI backend for storage driver is enabled])
>fi
> diff --git a/src/storage/Makefile.inc.am b/src/storage/Makefile.inc.am
> index d81864f5b9..bd5ea06f8b 100644
> --- a/src/storage/Makefile.inc.am
> +++ b/src/storage/Makefile.inc.am
> @@ -202,6 +202,8 @@ endif WITH_STORAGE_ISCSI
> if WITH_STORAGE_ISCSI_DIRECT
> libvirt_storage_backend_iscsi_direct_la_SOURCES =
> $(STORAGE_DRIVER_ISCSI_DIRECT_SOURCES)
> libvirt_storage_backend_iscsi_direct_la_CFLAGS = \
> + -I$(srcdir)/conf \
> + -I$(srcdir)/secret \
> $(LIBISCSI_CFLAGS) \
> $(AM_CFLAGS) \
> $(NULL)
> @@ -210,6 +212,7 @@ storagebackend_LTLIBRARIES +=
> libvirt_storage_backend_iscsi-direct.la
> libvirt_storage_backend_iscsi_direct_la_LDFLAGS = $(AM_LDFLAGS_MOD)
> libvirt_storage_backend_iscsi_direct_la_LIBADD = \
> libvirt.la \
> + $(LIBISCSI_LIBS) \
> ../gnulib/lib/libgnu.la \
> $(NULL)
> endif WITH_STORAGE_ISCSI_DIRECT
> diff --git a/src/storage/storage_backend_iscsi_direct.c
> b/src/storage/storage_backend_iscsi_direct.c
> index e3c1f75b42..f93c8e1e67 100644
> --- a/src/storage/storage_backend_iscsi_direct.c
> +++ b/src/storage/storage_backend_iscsi_direct.c
> @@ -1,34 +1,435 @@
> #include
> #include
> +#include
> +#include
> +#include
> #include
> #include
> #include
>
> #include "datatypes.h"
> #include "driver.h"
> +#include "secret_util.h"
> #include "storage_backend_iscsi_direct.h"
> #include "storage_util.h"
> +#include "viralloc.h"
> +#include "vircommand.h"
> +#include "virerror.h"
> +#include "virfile.h"
> #include "virlog.h"
> #include "virobject.h"
> +#include "virstring.h"
> +#include "virtime.h"
> +#include "viruuid.h"
>From what I see there is a lot of unused headers here:
* fcntl.h
* string.h
* sys/stat.h
* sys/wait.h
* unistd.h
* driver.h
* vircommand.h
* virfile.h
>
> #define VIR_FROM_THIS VIR_FROM_STORAGE
>
> +#define ISCSI_DEFAULT_TARGET_PORT 3260
> +#define VIR_ISCSI_TEST_UNIT_TIMEOUT 30 * 1000
> +
> VIR_LOG_INIT("storage.storage_backend_iscsi_direct");
>
> +static struct iscsi_context *
> +virISCSIDirectCreateContext(const char* initiator_iqn)
> +{
> +struct iscsi_context *iscsi = NULL;
> +
> +iscsi = iscsi_create_context(initiator_iqn);
> +if (!iscsi)
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to create iscsi context for %s"),
> + initiator_iqn);
> +return iscsi;
> +}
> +
> +static char *
> +virStorageBackendISCSIDirectPortal(virStoragePoolSourcePtr source)
> +{
> +char *portal = NULL;
> +
> +if (source->nhost != 1) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Expected exactly 1 host for the storage pool"));
> +return NULL;
> +}
> +if (source->hosts[0].port == 0) {
> +ignore_value(virAsprintf(, "%s:%d",
> + source->hosts[0].name,
> + ISCSI_DEFAULT_TARGET_PORT));
> +} else if (strchr(source->hosts[0].name, ':')) {
> +ignore_value(virAsprintf(, "[%s]:%d",
> + source->hosts[0].name,
> + source->hosts[0].port));
> +} else {
> +ignore_value(virAsprintf(, "%s:%d",
> + source->hosts[0].name,
> + source->hosts[0].port));
> +}
> +return portal;
> +}
> +
> +static int
> +virStorageBackendISCSIDirectSetAuth(struct iscsi_context *iscsi,
> +virStoragePoolSourcePtr source)
> +{
> +unsigned char *secret_value = NULL;
> +size_t secret_size;
> +virStorageAuthDefPtr authdef = source->auth;
> +int ret = -1;
> +virConnectPtr conn = NULL;
> +
> +
Re: [libvirt] [PATCH 4/9] qemu: command: use qemuDomainDiskGetBackendAlias in commandline building
On Wed, Jul 18, 2018 at 05:22:05PM +0200, Peter Krempa wrote: Use the proper backend for the block device both when using -drive and when using -blockdev for disk drives and floppy disks. Shouldn't you use future tense here? Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 24 +++- 1 file changed, 15 insertions(+), 9 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] storage: Introduce iscsi_direct pool type
On Sat, Jul 14, 2018 at 12:06:14AM +0200, c...@lse.epita.fr wrote:
> From: Clementine Hayat
I know that this is more like RFC patch series so before we get to the
actual patch which should be pushed into upstream there should be some
commit message.
> Signed-off-by: Clementine Hayat
> ---
> configure.ac | 6 ++-
> m4/virt-storage-iscsi-direct.m4| 41 +
> src/conf/domain_conf.c | 1 +
> src/conf/storage_conf.c| 31 ++--
> src/conf/storage_conf.h| 1 +
> src/conf/virstorageobj.c | 2 +
> src/storage/Makefile.inc.am| 21 +++
> src/storage/storage_backend.c | 6 +++
> src/storage/storage_backend_iscsi_direct.c | 43 ++
> src/storage/storage_backend_iscsi_direct.h | 6 +++
> src/storage/storage_driver.c | 1 +
> tools/virsh-pool.c | 3 ++
> 12 files changed, 157 insertions(+), 5 deletions(-)
> create mode 100644 m4/virt-storage-iscsi-direct.m4
> create mode 100644 src/storage/storage_backend_iscsi_direct.c
> create mode 100644 src/storage/storage_backend_iscsi_direct.h
[...]
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7396616eda..0a9509de0b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30163,6 +30163,7 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr
> def)
>
> break;
>
> +case VIR_STORAGE_POOL_ISCSI_DIRECT:
> case VIR_STORAGE_POOL_ISCSI:
> if (def->startupPolicy) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
This will not be good enough. We need to set the default
def->src->srcpool->mode to VIR_STORAGE_SOURCE_POOL_MODE_DIRECT if the
storage pool is "iscsi-direct" and if the mode is already configured in
domain XML we need to check whether it's "direct" mode if the storage
pool is "iscsi-direct".
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 5036ab9ef8..7a4b00ad8c 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -62,9 +62,9 @@ VIR_ENUM_IMPL(virStoragePool,
>VIR_STORAGE_POOL_LAST,
>"dir", "fs", "netfs",
>"logical", "disk", "iscsi",
> - "scsi", "mpath", "rbd",
> - "sheepdog", "gluster", "zfs",
> - "vstorage")
> + "iscsi-direct", "scsi", "mpath",
> + "rbd", "sheepdog", "gluster",
> + "zfs", "vstorage")
>
> VIR_ENUM_IMPL(virStoragePoolFormatFileSystem,
>VIR_STORAGE_POOL_FS_LAST,
> @@ -207,6 +207,16 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
> .formatToString = virStoragePoolFormatDiskTypeToString,
>}
> },
> +{.poolType = VIR_STORAGE_POOL_ISCSI_DIRECT,
> + .poolOptions = {
> + .flags = (VIR_STORAGE_POOL_SOURCE_HOST |
> + VIR_STORAGE_POOL_SOURCE_NETWORK |
> + VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN),
We need to use VIR_STORAGE_POOL_SOURCE_DEVICE as well, otherwise it
would not be formatted back.
> + },
> + .volOptions = {
> + .formatToString = virStoragePoolFormatDiskTypeToString,
> + }
> +},
> {.poolType = VIR_STORAGE_POOL_SCSI,
> .poolOptions = {
> .flags = (VIR_STORAGE_POOL_SOURCE_ADAPTER),
> @@ -802,6 +812,18 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
> "./target/permissions") < 0)
> goto error;
> }
One empty line would be nice.
> +if (ret->type == VIR_STORAGE_POOL_ISCSI_DIRECT) {
> +if (!ret->source.initiator.iqn) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing storage pool initiator iqn"));
> +goto error;
> +}
> +if (!ret->source.ndevice) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing storage pool device path"));
> +goto error;
> +}
> +}
>
> cleanup:
> VIR_FREE(uuid);
> @@ -1004,7 +1026,8 @@ virStoragePoolDefFormatBuf(virBufferPtr buf,
> * files, so they don't have a target */
> if (def->type != VIR_STORAGE_POOL_RBD &&
> def->type != VIR_STORAGE_POOL_SHEEPDOG &&
> -def->type != VIR_STORAGE_POOL_GLUSTER) {
> +def->type != VIR_STORAGE_POOL_GLUSTER &&
> +def->type != VIR_STORAGE_POOL_ISCSI_DIRECT) {
> virBufferAddLit(buf, "\n");
> virBufferAdjustIndent(buf, 2);
>
> diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
> index 15dfd8becf..858623783d 100644
> --- a/src/conf/storage_conf.h
> +++ b/src/conf/storage_conf.h
> @@ -85,6 +85,7 @@ typedef enum {
> VIR_STORAGE_POOL_LOGICAL, /* Logical volume groups / volumes */
> VIR_STORAGE_POOL_DISK, /* Disk partitions */
> VIR_STORAGE_POOL_ISCSI,
[libvirt] [PATCH v2 6/8] src: Make virStr*cpy*() functions return an int
Currently, the functions return a pointer to the
destination buffer on success or NULL on failure.
Not only does this kind of error handling look quite
alien in the context of libvirt, where most functions
return zero on success and a negative int on failure,
but it's also somewhat pointless because unless there's
been a failure the returned pointer will be the same
one passed in by the user, thus offering no additional
value.
Change the functions so that they return an int
instead.
Signed-off-by: Andrea Bolognani
---
docs/hacking.html.in | 2 +-
src/conf/capabilities.c | 2 +-
src/conf/netdev_vport_profile_conf.c | 2 +-
src/conf/nwfilter_conf.c | 2 +-
src/esx/esx_driver.c | 6 +++---
src/esx/esx_vi.c | 2 +-
src/esx/esx_vi_types.c| 2 +-
src/hyperv/hyperv_driver.c| 2 +-
src/libxl/libxl_conf.c| 2 +-
src/locking/lock_driver_sanlock.c | 22 ++---
src/lxc/lxc_driver.c | 8
src/nwfilter/nwfilter_dhcpsnoop.c | 2 +-
src/nwfilter/nwfilter_ebiptables_driver.c | 6 +++---
src/nwfilter/nwfilter_learnipaddr.c | 2 +-
src/openvz/openvz_conf.c | 8
src/qemu/qemu_agent.c | 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_monitor.c | 2 +-
src/remote/remote_driver.c| 4 ++--
src/rpc/virnetlibsshsession.c | 4 ++--
src/rpc/virnetsocket.c| 4 ++--
src/security/security_apparmor.c | 2 +-
src/security/virt-aa-helper.c | 2 +-
src/test/test_driver.c| 4 ++--
src/uml/uml_driver.c | 4 ++--
src/util/virfdstream.c| 4 ++--
src/util/virhostcpu.c | 4 ++--
src/util/virhostmem.c | 6 +++---
src/util/virlog.c | 2 +-
src/util/virnetdev.c | 10 +-
src/util/virnetdevbridge.c| 6 +++---
src/util/virnetdevtap.c | 4 ++--
src/util/virnetdevvportprofile.c | 6 +++---
src/util/virstring.c | 20 +--
src/util/virstring.h | 4 ++--
src/util/virtypedparam.c | 8
src/xenapi/xenapi_driver.c| 6 +++---
src/xenconfig/xen_common.c| 24 +++
src/xenconfig/xen_sxpr.c | 2 +-
src/xenconfig/xen_xl.c| 20 +--
src/xenconfig/xen_xm.c| 2 +-
41 files changed, 113 insertions(+), 115 deletions(-)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index fbeea3eb75..6c1a5121a4 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1134,7 +1134,7 @@
respectively. The last argument is the number of bytes
available in the destination string; if a copy of the source
string (including a \0) will not fit into the destination, no
- bytes are copied and the routine returns NULL. Otherwise, n
+ bytes are copied and the routine returns 0. Otherwise, n
bytes from the source are copied into the destination and a
trailing \0 is appended.
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 7a810efa66..0f96500294 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1262,7 +1262,7 @@ virCapabilitiesGetNodeInfo(virNodeInfoPtr nodeinfo)
memset(nodeinfo, 0, sizeof(*nodeinfo));
-if (virStrcpyStatic(nodeinfo->model, virArchToString(hostarch)) == NULL)
+if (virStrcpyStatic(nodeinfo->model, virArchToString(hostarch)) < 0)
return -1;
if (virHostMemGetInfo(, NULL) < 0)
diff --git a/src/conf/netdev_vport_profile_conf.c
b/src/conf/netdev_vport_profile_conf.c
index 58a50793c2..24052bf784 100644
--- a/src/conf/netdev_vport_profile_conf.c
+++ b/src/conf/netdev_vport_profile_conf.c
@@ -134,7 +134,7 @@ virNetDevVPortProfileParse(xmlNodePtr node, unsigned int
flags)
}
if (virtPortProfileID &&
-!virStrcpyStatic(virtPort->profileID, virtPortProfileID)) {
+virStrcpyStatic(virtPort->profileID, virtPortProfileID) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("profileid parameter too long"));
goto error;
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 36a7315880..120ca5ec14 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -966,7 +966,7 @@ ipsetValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED,
{
const char *errmsg = NULL;
-if (virStrcpyStatic(item->u.ipset.setname, val->c) == NULL) {
+if (virStrcpyStatic(item->u.ipset.setname, val->c) < 0) {
errmsg = _("ipset name is too long");
goto
[libvirt] [PATCH v2 0/8] virStr*cpy*() related fixes and cleanups
Changes from [v1]: * catch a bunch more suboptimal uses of the original API; * improve the API so that it's both more sensible and fits better with the rest of libvirt. [v1] https://www.redhat.com/archives/libvir-list/2018-July/msg01044.html Andrea Bolognani (8): src: Use virStrcpyStatic() to avoid truncation src: Use virStrcpyStatic() wherever possible src: Use VIR_STRDUP() wherever possible src: Use virStrcpy() wherever possible src: Don't rely on strncpy()-like behavior src: Make virStr*cpy*() functions return an int esx: Use memcpy() in esxVI_CURL_Debug() util: Improve virStrncpy() implementation docs/hacking.html.in | 29 +- src/conf/capabilities.c | 2 +- src/conf/netdev_vport_profile_conf.c | 2 +- src/conf/nwfilter_conf.c | 3 +- src/esx/esx_driver.c | 8 +-- src/esx/esx_vi.c | 6 +- src/esx/esx_vi_types.c| 2 +- src/hyperv/hyperv_driver.c| 3 +- src/libxl/libxl_conf.c| 2 +- src/locking/lock_driver_sanlock.c | 25 src/lxc/lxc_driver.c | 8 +-- src/nwfilter/nwfilter_dhcpsnoop.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 6 +- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/openvz/openvz_conf.c | 8 +-- src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_monitor.c | 2 +- src/remote/remote_daemon_dispatch.c | 4 +- src/remote/remote_driver.c| 4 +- src/rpc/virnetlibsshsession.c | 4 +- src/rpc/virnetsocket.c| 4 +- src/security/security_apparmor.c | 2 +- src/security/virt-aa-helper.c | 2 +- src/test/test_driver.c| 4 +- src/uml/uml_driver.c | 4 +- src/util/virfdstream.c| 4 +- src/util/virhostcpu.c | 4 +- src/util/virhostmem.c | 6 +- src/util/virlog.c | 5 +- src/util/virnetdev.c | 12 ++-- src/util/virnetdevbridge.c| 6 +- src/util/virnetdevtap.c | 4 +- src/util/virnetdevvportprofile.c | 6 +- src/util/virstring.c | 70 +++ src/util/virstring.h | 4 +- src/util/virtypedparam.c | 8 +-- src/xenapi/xenapi_driver.c| 4 +- src/xenconfig/xen_common.c| 38 ++-- src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c| 41 ++--- src/xenconfig/xen_xm.c| 2 +- 42 files changed, 179 insertions(+), 179 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/8] src: Don't rely on strncpy()-like behavior
The strncpy() function has this quirk where it will copy
*up* to the requested number of bytes, that is, it will
stop early if it encounters a NULL byte in the source
string.
This makes it legal to pass the size of the destination
buffer (minus one byte needed for the string terminator)
as the number of bytes to copy and still get something
somewhat reasonable out of the operation; unfortunately,
it also makes the function difficult to reason about
and way too easy to misuse.
We want to move away from the way strncpy() behaves and
towards better defined semantics, where virStrncpy()
will always copy *exactly* the number of bytes it's
been asked to copy; before we can do that, though, we
have to change a few of the callers.
Signed-off-by: Andrea Bolognani
---
src/locking/lock_driver_sanlock.c | 3 ++-
src/xenapi/xenapi_driver.c| 4 +++-
src/xenconfig/xen_common.c| 14 +++---
src/xenconfig/xen_xl.c| 12 ++--
4 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/src/locking/lock_driver_sanlock.c
b/src/locking/lock_driver_sanlock.c
index 345cf0a772..3f3a587541 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -1004,7 +1004,8 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr
lock,
/* sanlock doesn't use owner_name for anything, so it's safe to take just
* the first SANLK_NAME_LEN - 1 characters from vm_name */
ignore_value(virStrncpy(opt->owner_name, priv->vm_name,
-SANLK_NAME_LEN - 1, SANLK_NAME_LEN));
+MIN(strlen(priv->vm_name), SANLK_NAME_LEN - 1),
+SANLK_NAME_LEN));
if (state && STRNEQ(state, "")) {
if ((rv = sanlock_state_to_args((char *)state,
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 42b305d316..f4375c5874 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -430,7 +430,9 @@ xenapiNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
if (xen_host_cpu_get_all(session, _cpu_set)) {
host_cpu = host_cpu_set->contents[0];
xen_host_cpu_get_modelname(session, , host_cpu);
-if (!virStrncpy(info->model, modelname, LIBVIRT_MODELNAME_LEN - 1,
LIBVIRT_MODELNAME_LEN)) {
+if (!virStrncpy(info->model, modelname,
+MIN(strlen(modelname), LIBVIRT_MODELNAME_LEN - 1),
+LIBVIRT_MODELNAME_LEN)) {
virReportOOMError();
xen_host_cpu_set_free(host_cpu_set);
VIR_FREE(modelname);
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index 4a94127da1..815ccd030e 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -879,7 +879,7 @@ xenParseVif(char *entry, const char *vif_typename)
data++;
if (STRPREFIX(key, "mac=")) {
-int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
+int len = nextkey ? (nextkey - data) : strlen(data);
if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("MAC address %s too big for destination"),
@@ -887,7 +887,7 @@ xenParseVif(char *entry, const char *vif_typename)
return NULL;
}
} else if (STRPREFIX(key, "bridge=")) {
-int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1;
+int len = nextkey ? (nextkey - data) : strlen(data);
if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Bridge %s too big for destination"),
@@ -900,7 +900,7 @@ xenParseVif(char *entry, const char *vif_typename)
if (VIR_STRNDUP(script, data, len) < 0)
return NULL;
} else if (STRPREFIX(key, "model=")) {
-int len = nextkey ? (nextkey - data) : sizeof(model) - 1;
+int len = nextkey ? (nextkey - data) : strlen(data);
if (virStrncpy(model, data, len, sizeof(model)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Model %s too big for destination"),
@@ -908,7 +908,7 @@ xenParseVif(char *entry, const char *vif_typename)
return NULL;
}
} else if (STRPREFIX(key, "type=")) {
-int len = nextkey ? (nextkey - data) : sizeof(type) - 1;
+int len = nextkey ? (nextkey - data) : strlen(data);
if (virStrncpy(type, data, len, sizeof(type)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Type %s too big for destination"),
@@ -916,7 +916,7 @@ xenParseVif(char *entry, const char *vif_typename)
return NULL;
}
} else if (STRPREFIX(key, "vifname=")) {
-int len = nextkey ? (nextkey - data) :
[libvirt] [PATCH v2 2/8] src: Use virStrcpyStatic() wherever possible
This convenience macro was created for the simple cases
where the length of the source string and the size of the
destination buffer can be figued out with strlen() and
sizeof() respectively, so we should use it wherever
possible instead of open-coding parts of it.
Signed-off-by: Andrea Bolognani
---
src/conf/nwfilter_conf.c | 3 +--
src/util/virfdstream.c | 2 +-
src/util/virlog.c| 5 ++---
src/util/virnetdev.c | 3 +--
src/xenconfig/xen_xl.c | 17 -
5 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 706e803a25..36a7315880 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -966,8 +966,7 @@ ipsetValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED,
{
const char *errmsg = NULL;
-if (virStrcpy(item->u.ipset.setname, val->c,
- sizeof(item->u.ipset.setname)) == NULL) {
+if (virStrcpyStatic(item->u.ipset.setname, val->c) == NULL) {
errmsg = _("ipset name is too long");
goto arg_err_exit;
}
diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index 8189559964..f4777cfd12 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -1183,7 +1183,7 @@ int virFDStreamConnectUNIX(virStreamPtr st,
goto error;
sa.sun_path[0] = '\0';
} else {
-if (virStrcpy(sa.sun_path, path, sizeof(sa.sun_path)) == NULL)
+if (virStrcpyStatic(sa.sun_path, path) == NULL)
goto error;
}
diff --git a/src/util/virlog.c b/src/util/virlog.c
index e008dd9c54..9d569057ae 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -284,8 +284,7 @@ virLogOnceInit(void)
*/
r = gethostname(virLogHostname, sizeof(virLogHostname));
if (r == -1) {
-ignore_value(virStrcpy(virLogHostname,
- "(unknown)", sizeof(virLogHostname)));
+ignore_value(virStrcpyStatic(virLogHostname, "(unknown)"));
} else {
NUL_TERMINATE(virLogHostname);
}
@@ -1027,7 +1026,7 @@ virLogOutputToJournald(virLogSourcePtr source,
memset(, 0, sizeof(sa));
sa.sun_family = AF_UNIX;
-if (!virStrcpy(sa.sun_path, "/run/systemd/journal/socket",
sizeof(sa.sun_path)))
+if (!virStrcpyStatic(sa.sun_path, "/run/systemd/journal/socket"))
return;
memset(, 0, sizeof(mh));
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index c20022fbc9..57ebd0ec03 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -914,8 +914,7 @@ int virNetDevGetIndex(const char *ifname, int *ifindex)
memset(, 0, sizeof(ifreq));
-if (virStrncpy(ifreq.ifr_name, ifname, strlen(ifname),
- sizeof(ifreq.ifr_name)) == NULL) {
+if (virStrcpyStatic(ifreq.ifr_name, ifname) == NULL) {
virReportSystemError(ERANGE,
_("invalid interface name %s"),
ifname);
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index f0d9177cec..807fe621d6 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -475,15 +475,12 @@ xenParseXLVnuma(virConfPtr conf,
data++;
if (*data) {
-size_t len;
char vtoken[64];
if (STRPREFIX(str, "pnode")) {
unsigned int cellid;
-len = strlen(data);
-if (!virStrncpy(vtoken, data,
-len, sizeof(vtoken))) {
+if (!virStrcpyStatic(vtoken, data)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("vnuma vnode %zu pnode '%s' too
long for destination"),
vnodeCnt, data);
@@ -499,9 +496,7 @@ xenParseXLVnuma(virConfPtr conf,
}
pnode = cellid;
} else if (STRPREFIX(str, "size")) {
-len = strlen(data);
-if (!virStrncpy(vtoken, data,
-len, sizeof(vtoken))) {
+if (!virStrcpyStatic(vtoken, data)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("vnuma vnode %zu size '%s' too
long for destination"),
vnodeCnt, data);
@@ -514,9 +509,7 @@ xenParseXLVnuma(virConfPtr conf,
virDomainNumaSetNodeMemorySize(numa, vnodeCnt, (kbsize
* 1024));
} else if (STRPREFIX(str, "vcpus")) {
-len = strlen(data);
-if (!virStrncpy(vtoken, data,
-len, sizeof(vtoken))) {
+if (!virStrcpyStatic(vtoken, data)) {
Re: [libvirt] [PATCH] utils: storage: Add copying of PR definition to virStorageSource
On Tue, Jul 17, 2018 at 04:03:49PM +0200, Peter Krempa wrote: Despite the warning that virStorageSourceCopy needs to be populated on additions to the structure commit 687730540e4 neglected to implement the copy function. Signed-off-by: Peter Krempa --- src/util/virstoragefile.c | 27 +++ 1 file changed, 27 insertions(+) Reviewed-by: Ján Tomko Also not copied: bool authInherited; bool encryptionInherited; bool nocow; bool sparse; and all the properties from the last section (starting at 'iomode') It would be nice to mention the properties that do not need to be copied and copy them in the same order they are declared in. Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Expose vfio device display/migration to libvirt and above, was Re: [PATCH 0/3] sample: vfio mdev display devices.
Hi Gerd, Can I know your status on the boot display support work? I'm interested to try it in some real use cases. Thanks, Henry > -Original Message- > From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On > Behalf Of Gerd Hoffmann > Sent: Monday, May 7, 2018 2:26 PM > To: Alex Williamson > Cc: Neo Jia ; k...@vger.kernel.org; Erik Skultety > ; libvirt ; Dr. David Alan > Gilbert ; Zhang, Tina ; Kirti > Wankhede ; Laine Stump ; > Daniel P. Berrange ; Jiri Denemark > ; intel-gvt-...@lists.freedesktop.org > Subject: Re: Expose vfio device display/migration to libvirt and above, was > Re: > [PATCH 0/3] sample: vfio mdev display devices. > > Hi, > > > This raises another question, is the configuration of the emulated > > graphics a factor in the handling the mdev device's display option? > > AFAIK, neither vGPU vendor provides a VBIOS for boot graphics, so even > > with a display option, we're mostly targeting a secondary graphics > > head, otherwise the user will be running headless until the guest OS > > drivers initialize. > > Right now yes, no boot display for vgpu devices. I'm trying to fix that with > ramfb. There are a bunch of rough edges still and details to hashed out. > It'll > probably be uefi only. > > cheers, > Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] lcitool: Update OS when building Docker images
The last build of the Fedora Rawhide Docker image failed
with
Transaction check error:
file /usr/lib64/libgdbm_compat.so.4.0.0 from install
of gdbm-libs-1:1.16-1.fc29.x86_64 conflicts with file
from package gdbm-1:1.14.1-3.fc28.x86_64
caused by the gdbm package having recently been split.
Especially with fast-moving distribution such as Fedora
Rawhide and Debian Sid, we can't rely on the base OS not
changing over time, so merely installing the packages we
need on top of that is not enough.
Additionally, updating the OS brings the Docker images
closer to the guests running on CentOS CI, which are
updated daily.
Signed-off-by: Andrea Bolognani
---
guests/lcitool | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/guests/lcitool b/guests/lcitool
index 22b08dd..33c1afe 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -478,13 +478,15 @@ class Application:
if package_format == "deb":
sys.stdout.write(textwrap.dedent("""
RUN apt-get update && \\
+apt-get dist-upgrade -y && \\
apt-get install -y ${PACKAGES} && \\
apt-get autoremove -y && \\
apt-get autoclean -y
"""))
elif package_format == "rpm":
sys.stdout.write(textwrap.dedent("""
-RUN yum install -y ${PACKAGES} && \\
+RUN yum update -y && \\
+yum install -y ${PACKAGES} && \\
yum autoremove -y && \\
yum clean all -y
"""))
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/9] qemu: monitor: Add the 'query-nodes' argument for query-blockstats
On Wed, Jul 18, 2018 at 05:22:06PM +0200, Peter Krempa wrote: The 'query-blockstats' command does not return statistics for the explicitly named nodes unless the new argument is specified. Add Hopefully nobody will need to use profanity in node names. infrastrucuture that will allow us to use the new approach if desired. Signed-off-by: Peter Krempa --- src/qemu/qemu_block.c| 2 +- src/qemu/qemu_monitor.c | 8 ++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 9 ++--- src/qemu/qemu_monitor_json.h | 3 ++- 5 files changed, 17 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/9] qemu: domain: Move out clearing of backing chain in qemuDomainDetermineDiskChain
On Wed, Jul 18, 2018 at 05:22:03PM +0200, Peter Krempa wrote: In some cases backing chain needs to be cleared prior to re-detection. Move this step out of qemuDomainDetermineDiskChain as only certain places need it and the function itself is able to skip to the end of the chain to perform detection. Signed-off-by: Peter Krempa --- src/qemu/qemu_blockjob.c | 4 ++-- src/qemu/qemu_domain.c | 4 src/qemu/qemu_domain.h | 1 - src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 7 +-- 6 files changed, 10 insertions(+), 12 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] qemu: monitor: Split out code to gather data from 'query-block'
On Wed, Jul 18, 2018 at 05:22:08PM +0200, Peter Krempa wrote: Extract the code for future reuse. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 43 +++ 1 file changed, 27 insertions(+), 16 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/9] utils: storage: Add helper for checking if storage source is the same
On Wed, Jul 18, 2018 at 05:22:09PM +0200, Peter Krempa wrote: To allow checking whether a storage source points to the same location add a helper which checks the relevant fields. This will allow replacing a similar check done by formatting the command line arguments for qemu-like syntax. Signed-off-by: Peter Krempa --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 43 +++ src/util/virstoragefile.h | 3 +++ 3 files changed, 47 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 9/9] qemu: Replace qemuDomainDiskSourceDiffers by virStorageSourceIsSameLocation
On Wed, Jul 18, 2018 at 05:22:10PM +0200, Peter Krempa wrote: Now that we have a saner replacement for checking if the disk source is the same use it instead of formatting qemu command-line chunks. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 34 -- src/qemu/qemu_domain.h | 3 --- src/qemu/qemu_driver.c | 2 +- 3 files changed, 1 insertion(+), 38 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/9] qemu: driver: Reuse qemuDomainBlocksStatsGather in qemuDomainGetBlockInfo
On Wed, Jul 18, 2018 at 05:22:02PM +0200, Peter Krempa wrote: Allow updating capacity for the block devices returned by qemuDomainBlocksStatsGather and replace the open-coded call to qemuMonitorGetAllBlockStatsInfo by the helper. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 41 - 1 file changed, 12 insertions(+), 29 deletions(-) Reviewed-by: Ján Tomko Jano 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/9] qemu: domain: Add helper for getting the disk backend alias
On Wed, Jul 18, 2018 at 05:22:04PM +0200, Peter Krempa wrote:
The disk backend alias was historically the alias of the -drive backing
the storage. For setups with -blockdev this will become more complex as
it will depend on other configs and generally will differ.
---
src/qemu/qemu_domain.c | 28
src/qemu/qemu_domain.h | 6 ++
2 files changed, 34 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c92ac8c926..f8a621a5c9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8449,6 +8449,34 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
}
+/**
+ * qemuDomainDiskGetBackendAlias:
+ * @disk: disk definition
+ * @qemuCaps: emulator capabilities
+ * @backendAlias: filled with the alias of the disk storage backend
+ *
+ * Returns the correct alias for the disk backend. This may be the alias of
+ * -drive for legacy setup or the correct node name for -blockdev setups.
+ *
+ * @backendAlias may be NULL on success if the backend does not exist
+ * (disk is empty).
I presume this will be true in the future,
Caller is responsible for freeing @backendAlias.
+ *
+ * Returns 0 on success, -1 on error with libvirt error reported.
+ */
+int
+qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
+ virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
and the capabilities will be used.
+ char **backendAlias)
+{
+*backendAlias = NULL;
+
+if (!(*backendAlias = qemuAliasDiskDriveFromDisk(disk)))
+return -1;
+
+return 0;
+}
+
+
/**
* qemuDomainDiskChainElementRevoke:
*
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/9] qemu: json: Extract gathering of block statistics
On Wed, Jul 18, 2018 at 05:22:07PM +0200, Peter Krempa wrote: The code is useful also when gathering statistics per node name, so extract it to a separate functions. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 51 ++-- 1 file changed, 35 insertions(+), 16 deletions(-) Reviewed-by: Ján Tomko Jano 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/2] rpm: modernize the spec file for RHEL
On Fri, Jul 20, 2018 at 12:55 PM, Daniel P. Berrangé wrote: > Drop conditionals for RHEL-6 and assume systemd is present > > Daniel P. Berrangé (2): > rpm: increase min required RHEL to 7 > rpm: remove conditionals for systemd While you're on this wouldn't worth to also bump the minimum fedora version from 26 to 27 as f26 reached its EOL? > > libvirt.spec.in | 246 > 1 file changed, 20 insertions(+), 226 deletions(-) > > -- > 2.17.1 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list Best Regards, -- Fabiano Fidêncio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/8] src: Use virStrcpyStatic() to avoid truncation
The way virStrncpy() is called here will never result in
buffer overflow, but it won't prevent or detect truncation
either, despite what the error message might suggest. Use
virStrcpyStatic(), which does all of the above, instead.
Signed-off-by: Andrea Bolognani
---
src/esx/esx_driver.c | 4 +---
src/hyperv/hyperv_driver.c | 3 +--
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 947b7c1a31..edd21b9d28 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -1317,9 +1317,7 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr
nodeinfo)
++ptr;
}
-if (!virStrncpy(nodeinfo->model, dynamicProperty->val->string,
-sizeof(nodeinfo->model) - 1,
-sizeof(nodeinfo->model))) {
+if (!virStrcpyStatic(nodeinfo->model,
dynamicProperty->val->string)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("CPU Model %s too long for destination"),
dynamicProperty->val->string);
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index a85943668c..6f74adf372 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -307,8 +307,7 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
}
/* Fill struct */
-if (virStrncpy(info->model, processorList->data.common->Name,
- sizeof(info->model) - 1, sizeof(info->model)) == NULL) {
+if (virStrcpyStatic(info->model, processorList->data.common->Name) ==
NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("CPU model %s too long for destination"),
processorList->data.common->Name);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 8/8] util: Improve virStrncpy() implementation
We finally get rid of the strncpy()-like semantics
and implement our own, more sensible ones instead.
As a bonus, this also fixes compilation on MinGW.
Signed-off-by: Andrea Bolognani
---
docs/hacking.html.in | 29 ++---
src/util/virstring.c | 62 ++--
2 files changed, 55 insertions(+), 36 deletions(-)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 6c1a5121a4..f99d143b7b 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1121,22 +1121,22 @@
Do not use the strncpy function. According to the man page, it
does not guarantee a NULL-terminated buffer, which makes
- it extremely dangerous to use. Instead, use one of the
- functionally equivalent functions:
+ it extremely dangerous to use. Instead, use one of the replacement
+ functions provided by libvirt:
virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
- The first three arguments have the same meaning as for strncpy;
- namely the destination, source, and number of bytes to copy,
- respectively. The last argument is the number of bytes
- available in the destination string; if a copy of the source
- string (including a \0) will not fit into the destination, no
- bytes are copied and the routine returns 0. Otherwise, n
- bytes from the source are copied into the destination and a
- trailing \0 is appended.
+ The first two arguments have the same meaning as for strncpy,
+ namely the destination and source of the copy operation. Unlike
+ strncpy, the function will always copy exactly the number of bytes
+ requested and make sure the destination is NULL-terminated, as the
+ source is required to be; sanity checks are performed to ensure the
+ size of the destination, as specified by the last argument, is
+ sufficient for the operation to succeed. On success, 0 is returned;
+ on failure, a value 0 is returned instead.
@@ -1144,10 +1144,8 @@
Use this variant if you know you want to copy the entire src
- string into dest. Note that this is a macro, so arguments could
- be evaluated more than once. This is equivalent to
- virStrncpy(dest, src, strlen(src), destbytes)
-
+ string into dest.
+
virStrcpyStatic(char *dest, const char *src)
@@ -1157,8 +1155,7 @@
string into dest and you know that your destination string is
a static string (i.e. that sizeof(dest) returns something
meaningful). Note that this is a macro, so arguments could be
- evaluated more than once. This is equivalent to
- virStrncpy(dest, src, strlen(src), sizeof(dest)).
+ evaluated more than once.
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 3e2f85465f..93fda69d7f 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -769,44 +769,66 @@ virAsprintfInternal(bool report,
}
/**
- * virStrncpy
+ * virStrncpy:
*
- * A safe version of strncpy. The last parameter is the number of bytes
- * available in the destination string, *not* the number of bytes you want
- * to copy. If the destination is not large enough to hold all n of the
- * src string bytes plus a \0, <0 is returned and no data is copied.
- * If the destination is large enough to hold the n bytes plus \0, then the
- * string is copied and 0 is returned.
+ * @dest: destination buffer
+ * @src: source buffer
+ * @n: number of bytes to copy
+ * @destbytes: number of bytes the destination can accomodate
+ *
+ * Copies the first @n bytes of @src to @dest.
+ *
+ * @src must be NULL-terminated; if successful, @dest is guaranteed to
+ * be NULL-terminated as well.
+ *
+ * @n must be a reasonable value, that is, it must not exceed either
+ * the length of @src or the size of @dest. For the latter constraint,
+ * the fact that @dest needs to accomodate a NULL byte in addition to
+ * the bytes copied from @src must be taken into account.
+ *
+ * If you want to copy *all* of @src to @dest, use virStrcpy() or
+ * virStrcpyStatic() instead.
+ *
+ * Returns: 0 on success, <0 on failure.
*/
int
virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
{
-if (n > (destbytes - 1))
+size_t src_len = strlen(src);
+
+/* As a special case, -1 means "copy the entire string".
+ *
+ * This is to avoid calling strlen() twice, once in the virStrcpy()
+ * wrapper and once here for bound checking purposes. */
+if (n == -1)
+n = src_len;
+
+if (n <= 0 || n > src_len || n > (destbytes - 1))
return -1;
-strncpy(dest, src, n);
-/* strncpy NULL terminates iff the last character is \0. Therefore
- * force the last byte to be \0
- */
+memcpy(dest, src, n);
dest[n] = '\0';
return 0;
}
/**
- * virStrcpy
+ * virStrcpy:
+ *
+ * @dest: destination buffer
+ * @src: source buffer
+ *
[libvirt] [PATCH v2 3/8] src: Use VIR_STRDUP() wherever possible
virStrcpy() and friends are useful when the destination
buffer has already been allocated, eg. as part of a struct;
if we have to allocate it on the spot, VIR_STRDUP() is a
better choice.
Signed-off-by: Andrea Bolognani
---
src/remote/remote_daemon_dispatch.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/remote/remote_daemon_dispatch.c
b/src/remote/remote_daemon_dispatch.c
index 4a93f09a7d..e62ebfb596 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -2309,9 +2309,7 @@ remoteDispatchDomainGetSecurityLabelList(virNetServerPtr
server ATTRIBUTE_UNUSED
for (i = 0; i < len; i++) {
size_t label_len = strlen(seclabels[i].label) + 1;
remote_domain_get_security_label_ret *cur = >labels.labels_val[i];
-if (VIR_ALLOC_N(cur->label.label_val, label_len) < 0)
-goto cleanup;
-if (virStrcpy(cur->label.label_val, seclabels[i].label, label_len) ==
NULL) {
+if (VIR_STRDUP(cur->label.label_val, seclabels[i].label) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("failed to copy security label"));
goto cleanup;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 7/8] esx: Use memcpy() in esxVI_CURL_Debug()
We're going to change virStrncpy() in a way that
requires the source string to be NULL-terminated, so
we'll no longer be able to use in this context.
Signed-off-by: Andrea Bolognani
---
src/esx/esx_vi.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 727d76e89d..a816c3a4f9 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -185,10 +185,8 @@ esxVI_CURL_Debug(CURL *curl ATTRIBUTE_UNUSED,
curl_infotype type,
if (VIR_ALLOC_N(buffer, size + 1) < 0)
return 0;
-if (virStrncpy(buffer, info, size, size + 1) < 0) {
-VIR_FREE(buffer);
-return 0;
-}
+memcpy(buffer, info, size);
+buffer[size] = '\0';
switch (type) {
case CURLINFO_TEXT:
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/8] src: Use virStrcpy() wherever possible
virStrncpy() allows us to copy a substring, but if we're
going to copy the entire thing it's much more convenient
to use virStrcpy() instead.
Signed-off-by: Andrea Bolognani
---
src/util/virnetdev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 57ebd0ec03..017786ce43 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -2763,8 +2763,7 @@ static int virNetDevParseMcast(char *buf,
virNetDevMcastEntryPtr mcast)
mcast->idx = num;
break;
case VIR_MCAST_TYPE_NAME_TOKEN:
-if (virStrncpy(mcast->name, token, strlen(token),
-VIR_MCAST_NAME_LEN) == NULL) {
+if (virStrcpy(mcast->name, token, VIR_MCAST_NAME_LEN) == NULL)
{
virReportSystemError(EINVAL,
_("Failed to parse network device
name from '%s'"),
buf);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] rpm: modernize the spec file for RHEL
On Fri, 2018-07-20 at 11:55 +0100, Daniel P. Berrangé wrote: > Drop conditionals for RHEL-6 and assume systemd is present > > Daniel P. Berrangé (2): > rpm: increase min required RHEL to 7 > rpm: remove conditionals for systemd > > libvirt.spec.in | 246 > 1 file changed, 20 insertions(+), 226 deletions(-) Love it! Reviewed-by: Andrea Bolognani And now either you or Fabiano can bump min_fedora as he suggested :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] src: Fix memory leak in virNWFilterBindingDispose
https://bugzilla.redhat.com/show_bug.cgi?id=1603025
Commit b57a9aec neglected to VIR_FREE(binding->filtername) as seen
in the following valgrind report
==6423== 17,328 bytes in 1,083 blocks are definitely lost in loss record 2,275
of 2,297
==6423==at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==6423==by 0x83B20C9: strdup (in /usr/lib64/libc-2.17.so)
==6423==by 0x533C144: virStrdup (virstring.c:977)
==6423==by 0x54BDD53: virGetNWFilterBinding (datatypes.c:865)
==6423==by 0x318D633C: nwfilterBindingCreateXML (nwfilter_driver.c:767)
==6423==by 0x54F3FC5: virNWFilterBindingCreateXML (libvirt-nwfilter.c:701)
==6423==by 0x539CE29: virDomainConfNWFilterInstantiate
(domain_nwfilter.c:116)
==6423==by 0x31E516C2: qemuInterfaceBridgeConnect (qemu_interface.c:589)
==6423==by 0x31D98B56: qemuBuildInterfaceCommandLine (qemu_command.c:8418)
==6423==by 0x31D9F783: qemuBuildNetCommandLine (qemu_command.c:8673)
==6423==by 0x31D9F783: qemuBuildCommandLine (qemu_command.c:10354)
==6423==by 0x31DE355F: qemuProcessLaunch (qemu_process.c:6292)
==6423==by 0x31DE7881: qemuProcessStart (qemu_process.c:6686)
and
==6423== 17,328 bytes in 1,083 blocks are definitely lost in loss record 2,276
of 2,297
==6423==at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==6423==by 0x83B20C9: strdup (in /usr/lib64/libc-2.17.so)
==6423==by 0x533C144: virStrdup (virstring.c:977)
==6423==by 0x54BDD53: virGetNWFilterBinding (datatypes.c:865)
==6423==by 0x318D641F: nwfilterBindingLookupByPortDev
(nwfilter_driver.c:678)
==6423==by 0x54F3B63: virNWFilterBindingLookupByPortDev
(libvirt-nwfilter.c:593)
==6423==by 0x539CBC5: virDomainConfNWFilterTeardownImpl.isra.0
(domain_nwfilter.c:136)
==6423==by 0x539CFA5: virDomainConfVMNWFilterTeardown
(domain_nwfilter.c:170)
==6423==by 0x31DE5651: qemuProcessStop (qemu_process.c:6912)
==6423==by 0x31E37974: qemuDomainDestroyFlags (qemu_driver.c:2229)
==6423==by 0x54C24BB: virDomainDestroy (libvirt-domain.c:475)
==6423==by 0x1589A2: remoteDispatchDomainDestroy
(remote_daemon_dispatch_stubs.h:4827)
==6423==by 0x1589A2: remoteDispatchDomainDestroyHelper
(remote_daemon_dispatch_stubs.h:4803)
Signed-off-by: John Ferlan
---
src/datatypes.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/datatypes.c b/src/datatypes.c
index 878a1c5b5f..caf035f178 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -893,6 +893,7 @@ virNWFilterBindingDispose(void *obj)
VIR_DEBUG("release binding %p %s", binding, binding->portdev);
VIR_FREE(binding->portdev);
+VIR_FREE(binding->filtername);
virObjectUnref(binding->conn);
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] src: Fix memory leak in virNWFilterBindingDispose
On Fri, Jul 20, 2018 at 11:06:48AM -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1603025
>
> Commit b57a9aec neglected to VIR_FREE(binding->filtername) as seen
> in the following valgrind report
>
> ==6423== 17,328 bytes in 1,083 blocks are definitely lost in loss record
> 2,275 of 2,297
> ==6423==at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
> ==6423==by 0x83B20C9: strdup (in /usr/lib64/libc-2.17.so)
> ==6423==by 0x533C144: virStrdup (virstring.c:977)
> ==6423==by 0x54BDD53: virGetNWFilterBinding (datatypes.c:865)
> ==6423==by 0x318D633C: nwfilterBindingCreateXML (nwfilter_driver.c:767)
> ==6423==by 0x54F3FC5: virNWFilterBindingCreateXML (libvirt-nwfilter.c:701)
> ==6423==by 0x539CE29: virDomainConfNWFilterInstantiate
> (domain_nwfilter.c:116)
> ==6423==by 0x31E516C2: qemuInterfaceBridgeConnect (qemu_interface.c:589)
> ==6423==by 0x31D98B56: qemuBuildInterfaceCommandLine (qemu_command.c:8418)
> ==6423==by 0x31D9F783: qemuBuildNetCommandLine (qemu_command.c:8673)
> ==6423==by 0x31D9F783: qemuBuildCommandLine (qemu_command.c:10354)
> ==6423==by 0x31DE355F: qemuProcessLaunch (qemu_process.c:6292)
> ==6423==by 0x31DE7881: qemuProcessStart (qemu_process.c:6686)
>
> and
>
> ==6423== 17,328 bytes in 1,083 blocks are definitely lost in loss record
> 2,276 of 2,297
> ==6423==at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
> ==6423==by 0x83B20C9: strdup (in /usr/lib64/libc-2.17.so)
> ==6423==by 0x533C144: virStrdup (virstring.c:977)
> ==6423==by 0x54BDD53: virGetNWFilterBinding (datatypes.c:865)
> ==6423==by 0x318D641F: nwfilterBindingLookupByPortDev
> (nwfilter_driver.c:678)
> ==6423==by 0x54F3B63: virNWFilterBindingLookupByPortDev
> (libvirt-nwfilter.c:593)
> ==6423==by 0x539CBC5: virDomainConfNWFilterTeardownImpl.isra.0
> (domain_nwfilter.c:136)
> ==6423==by 0x539CFA5: virDomainConfVMNWFilterTeardown
> (domain_nwfilter.c:170)
> ==6423==by 0x31DE5651: qemuProcessStop (qemu_process.c:6912)
> ==6423==by 0x31E37974: qemuDomainDestroyFlags (qemu_driver.c:2229)
> ==6423==by 0x54C24BB: virDomainDestroy (libvirt-domain.c:475)
> ==6423==by 0x1589A2: remoteDispatchDomainDestroy
> (remote_daemon_dispatch_stubs.h:4827)
> ==6423==by 0x1589A2: remoteDispatchDomainDestroyHelper
> (remote_daemon_dispatch_stubs.h:4803)
>
> Signed-off-by: John Ferlan
> ---
> src/datatypes.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/datatypes.c b/src/datatypes.c
> index 878a1c5b5f..caf035f178 100644
> --- a/src/datatypes.c
> +++ b/src/datatypes.c
> @@ -893,6 +893,7 @@ virNWFilterBindingDispose(void *obj)
> VIR_DEBUG("release binding %p %s", binding, binding->portdev);
>
> VIR_FREE(binding->portdev);
> +VIR_FREE(binding->filtername);
> virObjectUnref(binding->conn);
> }
Reviewed-by: Daniel P. Berrangé
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 15/17] Implement InterfaceChangeRollback method for Connect Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Connect.xml | 5 +
src/connect.c| 22 ++
2 files changed, 27 insertions(+)
diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 8c88cc8..7c97117 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -158,6 +158,11 @@
value="See
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceChangeCommit"/>
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceChangeRollback"/>
+
+
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceDefineXML"/>
diff --git a/src/connect.c b/src/connect.c
index 2acc657..ebcfa38 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -777,6 +777,27 @@ virtDBusConnectInterfaceChangeCommit(GVariant *inArgs,
virtDBusUtilSetLastVirtError(error);
}
+static void
+virtDBusConnectInterfaceChangeRollback(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath G_GNUC_UNUSED,
+ gpointer userData,
+ GVariant **outArgs G_GNUC_UNUSED,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+guint flags;
+
+g_variant_get(inArgs, "(u)", );
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+if (virInterfaceChangeRollback(connect->connection, flags) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
static void
virtDBusConnectInterfaceDefineXML(GVariant *inArgs,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1876,6 +1897,7 @@ static virtDBusGDBusMethodTable
virtDBusConnectMethodTable[] = {
{ "GetSysinfo", virtDBusConnectGetSysinfo },
{ "InterfaceChangeBegin", virtDBusConnectInterfaceChangeBegin },
{ "InterfaceChangeCommit", virtDBusConnectInterfaceChangeCommit },
+{ "InterfaceChangeRollback", virtDBusConnectInterfaceChangeRollback },
{ "InterfaceDefineXML", virtDBusConnectInterfaceDefineXML },
{ "ListDomains", virtDBusConnectListDomains },
{ "ListInterfaces", virtDBusConnectListInterfaces },
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 11/17] Implement Active property for Interface Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Interface.xml | 5 +
src/interface.c| 22 ++
tests/test_interface.py| 1 +
3 files changed, 28 insertions(+)
diff --git a/data/org.libvirt.Interface.xml b/data/org.libvirt.Interface.xml
index 921e07f..6dd7c68 100644
--- a/data/org.libvirt.Interface.xml
+++ b/data/org.libvirt.Interface.xml
@@ -3,6 +3,11 @@
+
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceIsActive"/>
+
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetMACString"/>
diff --git a/src/interface.c b/src/interface.c
index 4caafcf..191b8be 100644
--- a/src/interface.c
+++ b/src/interface.c
@@ -70,6 +70,27 @@ virtDBusInterfaceDestroy(GVariant *inArgs,
virtDBusUtilSetLastVirtError(error);
}
+static void
+virtDBusInterfaceGetActive(const gchar *objectPath,
+ gpointer userData,
+ GVariant **value,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterface) interface = NULL;
+gint active;
+
+interface = virtDBusInterfaceGetVirInterface(connect, objectPath, error);
+if (!interface)
+return;
+
+active = virInterfaceIsActive(interface);
+if (active < 0)
+return virtDBusUtilSetLastVirtError(error);
+
+*value = g_variant_new("b", !!active);
+}
+
static void
virtDBusInterfaceGetMAC(const gchar *objectPath,
gpointer userData,
@@ -160,6 +181,7 @@ virtDBusInterfaceUndefine(GVariant *inArgs G_GNUC_UNUSED,
}
static virtDBusGDBusPropertyTable virtDBusInterfacePropertyTable[] = {
+{ "Active", virtDBusInterfaceGetActive, NULL },
{ "MAC", virtDBusInterfaceGetMAC, NULL },
{ "Name", virtDBusInterfaceGetName, NULL },
{ 0 }
diff --git a/tests/test_interface.py b/tests/test_interface.py
index 325f889..4c7e3fb 100755
--- a/tests/test_interface.py
+++ b/tests/test_interface.py
@@ -33,6 +33,7 @@ class TestInterface(libvirttest.BaseTestClass):
props = obj.GetAll('org.libvirt.Interface',
dbus_interface=dbus.PROPERTIES_IFACE)
assert isinstance(props['Name'], dbus.String)
assert isinstance(props['MAC'], dbus.String)
+assert isinstance(props['Active'], dbus.Boolean)
if __name__ == '__main__':
libvirttest.run()
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 10/17] Implement MAC property for Interface Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Interface.xml | 5 +
src/interface.c| 22 ++
tests/test_interface.py| 1 +
3 files changed, 28 insertions(+)
diff --git a/data/org.libvirt.Interface.xml b/data/org.libvirt.Interface.xml
index c53c110..921e07f 100644
--- a/data/org.libvirt.Interface.xml
+++ b/data/org.libvirt.Interface.xml
@@ -3,6 +3,11 @@
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetMACString"/>
+
+
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetName"/>
diff --git a/src/interface.c b/src/interface.c
index a597fe5..4caafcf 100644
--- a/src/interface.c
+++ b/src/interface.c
@@ -70,6 +70,27 @@ virtDBusInterfaceDestroy(GVariant *inArgs,
virtDBusUtilSetLastVirtError(error);
}
+static void
+virtDBusInterfaceGetMAC(const gchar *objectPath,
+gpointer userData,
+GVariant **value,
+GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterface) interface = NULL;
+const gchar *mac;
+
+interface = virtDBusInterfaceGetVirInterface(connect, objectPath, error);
+if (!interface)
+return;
+
+mac = virInterfaceGetMACString(interface);
+if (!mac)
+return virtDBusUtilSetLastVirtError(error);
+
+*value = g_variant_new("s", mac);
+}
+
static void
virtDBusInterfaceGetName(const gchar *objectPath,
gpointer userData,
@@ -139,6 +160,7 @@ virtDBusInterfaceUndefine(GVariant *inArgs G_GNUC_UNUSED,
}
static virtDBusGDBusPropertyTable virtDBusInterfacePropertyTable[] = {
+{ "MAC", virtDBusInterfaceGetMAC, NULL },
{ "Name", virtDBusInterfaceGetName, NULL },
{ 0 }
};
diff --git a/tests/test_interface.py b/tests/test_interface.py
index c11a9be..325f889 100755
--- a/tests/test_interface.py
+++ b/tests/test_interface.py
@@ -32,6 +32,7 @@ class TestInterface(libvirttest.BaseTestClass):
obj = self.bus.get_object('org.libvirt', test_interface_path)
props = obj.GetAll('org.libvirt.Interface',
dbus_interface=dbus.PROPERTIES_IFACE)
assert isinstance(props['Name'], dbus.String)
+assert isinstance(props['MAC'], dbus.String)
if __name__ == '__main__':
libvirttest.run()
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 06/17] Test Destroy method for Interface Interface
Signed-off-by: Anya Harter --- tests/test_interface.py | 4 1 file changed, 4 insertions(+) diff --git a/tests/test_interface.py b/tests/test_interface.py index 88be5dc..62fd517 100755 --- a/tests/test_interface.py +++ b/tests/test_interface.py @@ -7,6 +7,10 @@ class TestInterface(libvirttest.BaseTestClass): """ Tests for methods and properties of the Interface interface """ +def test_interface_destroy(self): +_,interface_obj = self.interface_create() +interface_obj.Destroy(0) + def test_interface_create(self): _,interface_obj = self.interface_create() interface_obj.Destroy(0) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 04/17] Implement Destroy method for Interface Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Interface.xml | 5 +
src/interface.c| 24
2 files changed, 29 insertions(+)
diff --git a/data/org.libvirt.Interface.xml b/data/org.libvirt.Interface.xml
index d98a646..f5ec281 100644
--- a/data/org.libvirt.Interface.xml
+++ b/data/org.libvirt.Interface.xml
@@ -8,5 +8,10 @@
value="See
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceCreate"/>
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceDestroy"/>
+
+
diff --git a/src/interface.c b/src/interface.c
index 4a5e431..077b10d 100644
--- a/src/interface.c
+++ b/src/interface.c
@@ -47,12 +47,36 @@ virtDBusInterfaceCreate(GVariant *inArgs,
virtDBusUtilSetLastVirtError(error);
}
+static void
+virtDBusInterfaceDestroy(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath,
+ gpointer userData,
+ GVariant **outArgs G_GNUC_UNUSED,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterface) interface = NULL;
+guint flags;
+
+g_variant_get(inArgs, "(u)", );
+
+interface = virtDBusInterfaceGetVirInterface(connect, objectPath, error);
+if (!interface)
+return;
+
+if (virInterfaceDestroy(interface, flags) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
static virtDBusGDBusPropertyTable virtDBusInterfacePropertyTable[] = {
{ 0 }
};
static virtDBusGDBusMethodTable virtDBusInterfaceMethodTable[] = {
{ "Create", virtDBusInterfaceCreate },
+{ "Destroy", virtDBusInterfaceDestroy },
{ 0 }
};
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 09/17] Implement Name property for Interface Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Interface.xml | 5 +
src/interface.c| 22 ++
tests/test_interface.py| 8
3 files changed, 35 insertions(+)
diff --git a/data/org.libvirt.Interface.xml b/data/org.libvirt.Interface.xml
index dc03258..c53c110 100644
--- a/data/org.libvirt.Interface.xml
+++ b/data/org.libvirt.Interface.xml
@@ -3,6 +3,11 @@
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetName"/>
+
+
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceCreate"/>
diff --git a/src/interface.c b/src/interface.c
index fcec623..a597fe5 100644
--- a/src/interface.c
+++ b/src/interface.c
@@ -70,6 +70,27 @@ virtDBusInterfaceDestroy(GVariant *inArgs,
virtDBusUtilSetLastVirtError(error);
}
+static void
+virtDBusInterfaceGetName(const gchar *objectPath,
+ gpointer userData,
+ GVariant **value,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterface) interface = NULL;
+const gchar *name;
+
+interface = virtDBusInterfaceGetVirInterface(connect, objectPath, error);
+if (!interface)
+return;
+
+name = virInterfaceGetName(interface);
+if (!name)
+return virtDBusUtilSetLastVirtError(error);
+
+*value = g_variant_new("s", name);
+}
+
static void
virtDBusInterfaceGetXMLDesc(GVariant *inArgs,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -118,6 +139,7 @@ virtDBusInterfaceUndefine(GVariant *inArgs G_GNUC_UNUSED,
}
static virtDBusGDBusPropertyTable virtDBusInterfacePropertyTable[] = {
+{ "Name", virtDBusInterfaceGetName, NULL },
{ 0 }
};
diff --git a/tests/test_interface.py b/tests/test_interface.py
index 0b5a943..c11a9be 100755
--- a/tests/test_interface.py
+++ b/tests/test_interface.py
@@ -25,5 +25,13 @@ class TestInterface(libvirttest.BaseTestClass):
_,interface_obj = self.interface_create()
assert isinstance(interface_obj.GetXMLDesc(0), dbus.String)
+def test_interface_properties_type(self):
+""" Ensure correct return type for Interface properties
+"""
+test_interface_path,_ = self.interface_create()
+obj = self.bus.get_object('org.libvirt', test_interface_path)
+props = obj.GetAll('org.libvirt.Interface',
dbus_interface=dbus.PROPERTIES_IFACE)
+assert isinstance(props['Name'], dbus.String)
+
if __name__ == '__main__':
libvirttest.run()
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 13/17] Implement InterfaceChangeBegin method for Connect Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Connect.xml | 5 +
src/connect.c| 22 ++
2 files changed, 27 insertions(+)
diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 791e1f5..604afea 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -148,6 +148,11 @@
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceChangeBegin"/>
+
+
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceDefineXML"/>
diff --git a/src/connect.c b/src/connect.c
index 9be8264..0e19e91 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -735,6 +735,27 @@ virtDBusConnectGetSysinfo(GVariant *inArgs,
*outArgs = g_variant_new("(s)", sysinfo);
}
+static void
+virtDBusConnectInterfaceChangeBegin(GVariant *inArgs,
+GUnixFDList *inFDs G_GNUC_UNUSED,
+const gchar *objectPath G_GNUC_UNUSED,
+gpointer userData,
+GVariant **outArgs G_GNUC_UNUSED,
+GUnixFDList **outFDs G_GNUC_UNUSED,
+GError **error)
+{
+virtDBusConnect *connect = userData;
+guint flags;
+
+g_variant_get(inArgs, "(u)", );
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+if (virInterfaceChangeBegin(connect->connection, flags) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
static void
virtDBusConnectInterfaceDefineXML(GVariant *inArgs,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1832,6 +1853,7 @@ static virtDBusGDBusMethodTable
virtDBusConnectMethodTable[] = {
{ "GetCPUModelNames", virtDBusConnectGetCPUModelNames },
{ "GetDomainCapabilities", virtDBusConnectGetDomainCapabilities },
{ "GetSysinfo", virtDBusConnectGetSysinfo },
+{ "InterfaceChangeBegin", virtDBusConnectInterfaceChangeBegin },
{ "InterfaceDefineXML", virtDBusConnectInterfaceDefineXML },
{ "ListDomains", virtDBusConnectListDomains },
{ "ListInterfaces", virtDBusConnectListInterfaces },
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 16/17] Implement InterfaceLookupByName method for Connect Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Connect.xml | 6 ++
src/connect.c| 29 +
tests/test_connect.py| 12
3 files changed, 47 insertions(+)
diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 7c97117..f99e205 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -170,6 +170,12 @@
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceLookupByName"/>
+
+
+
https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectListAllDomains"/>
diff --git a/src/connect.c b/src/connect.c
index ebcfa38..ae64d59 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -827,6 +827,34 @@ virtDBusConnectInterfaceDefineXML(GVariant *inArgs,
*outArgs = g_variant_new("(o)", path);
}
+static void
+virtDBusConnectInterfaceLookupByName(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath G_GNUC_UNUSED,
+ gpointer userData,
+ GVariant **outArgs,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterface) interface = NULL;
+g_autofree gchar *path = NULL;
+const gchar *name;
+
+g_variant_get(inArgs, "()", );
+
+if (!virtDBusConnectOpen(connect, NULL))
+return;
+
+interface = virInterfaceLookupByName(connect->connection, name);
+if (!interface)
+return virtDBusUtilSetLastVirtError(error);
+
+path = virtDBusUtilBusPathForVirInterface(interface,
connect->interfacePath);
+
+*outArgs = g_variant_new("(o)", path);
+}
+
static void
virtDBusConnectListDomains(GVariant *inArgs,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1899,6 +1927,7 @@ static virtDBusGDBusMethodTable
virtDBusConnectMethodTable[] = {
{ "InterfaceChangeCommit", virtDBusConnectInterfaceChangeCommit },
{ "InterfaceChangeRollback", virtDBusConnectInterfaceChangeRollback },
{ "InterfaceDefineXML", virtDBusConnectInterfaceDefineXML },
+{ "InterfaceLookupByName", virtDBusConnectInterfaceLookupByName },
{ "ListDomains", virtDBusConnectListDomains },
{ "ListInterfaces", virtDBusConnectListInterfaces },
{ "ListNetworks", virtDBusConnectListNetworks },
diff --git a/tests/test_connect.py b/tests/test_connect.py
index d21b366..7e33b2a 100755
--- a/tests/test_connect.py
+++ b/tests/test_connect.py
@@ -88,6 +88,18 @@ class TestConnect(libvirttest.BaseTestClass):
path = self.connect.InterfaceDefineXML(xmldata.minimal_interface_xml,
0)
assert isinstance(path, dbus.ObjectPath)
+@pytest.mark.parametrize("lookup_method_name,lookup_item", [
+("InterfaceLookupByName", 'Name'),
+])
+def test_connect_interface_lookup_by_property(self, lookup_method_name,
lookup_item):
+"""Parameterized test for all InterfaceLookupBy* API calls of Connect
interface
+"""
+original_path,_ = self.interface_create()
+obj = self.bus.get_object('org.libvirt', original_path)
+prop = obj.Get('org.libvirt.Interface', lookup_item,
dbus_interface=dbus.PROPERTIES_IFACE)
+path = getattr(self.connect, lookup_method_name)(prop)
+assert original_path == path
+
def test_connect_list_networks(self):
networks = self.connect.ListNetworks(0)
assert isinstance(networks, dbus.Array)
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 00/17] Implement and Test Interface APIs
original patch series cover letter: https://www.redhat.com/archives/libvir-list/2018-July/msg00364.html changes since v1: * all commits "make" and "make check" * MACString property => MAC property * various corrections * add tests for all but virConnectListAllInterfaces, virInterfaceChangeBegin, virInterfaceChangeCommit, and virInterfaceChangeRollback * commits were reordered to introduce functions before they are needed by the test suite https://libvirt.org/html/libvirt-libvirt-interface.html The following functions have been implemented: - virConnectListAllInterfaces (connect method) - virInterfaceChangeBegin (connect method) - virInterfaceChangeCommit (connect method) - virInterfaceChangeRollback(connect method) - virInterfaceCreate(interface method) - virInterfaceDefineXML (connect method) - virInterfaceDestroy (interface method) - virInterfaceGetMACString (property) - virInterfaceGetName (property) - virInterfaceGetXMLDesc(interface method) - virInterfaceIsActive (property) - virInterfaceLookupByMACString (connect method) - virInterfaceLookupByName (connect method) - virInterfaceUndefine (interface method) Anya Harter (17): Introduce Interface Interface Implement InterfaceDefineXML method for Connect Interface Implement Create method for Interface Interface Implement Destroy method for Interface Interface Introduce Interface Tests & Test Create method for Interface Interface Test Destroy method for Interface Interface Implement Undefine method for Interface Interface Implement GetXMLDesc method for Interface Interface Implement Name property for Interface Interface Implement MAC property for Interface Interface Implement Active property for Interface Interface Implement ListInterfaces method for Connect Interface Implement InterfaceChangeBegin method for Connect Interface Implement InterfaceChangeCommit method for Connect Interface Implement InterfaceChangeRollback method for Connect Interface Implement InterfaceLookupByName method for Connect Interface Implement InterfaceLookupByMAC method for Connect Interface data/Makefile.am | 1 + data/org.libvirt.Connect.xml | 40 ++ data/org.libvirt.Interface.xml | 42 ++ src/Makefile.am| 2 + src/connect.c | 198 ++ src/connect.h | 1 + src/interface.c| 249 + src/interface.h| 9 ++ src/util.c | 35 + src/util.h | 15 ++ tests/Makefile.am | 1 + tests/libvirttest.py | 12 ++ tests/test_connect.py | 17 +++ tests/test_interface.py| 39 ++ tests/xmldata.py | 12 ++ 15 files changed, 673 insertions(+) create mode 100644 data/org.libvirt.Interface.xml create mode 100644 src/interface.c create mode 100644 src/interface.h create mode 100755 tests/test_interface.py -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 07/17] Implement Undefine method for Interface Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Interface.xml | 4
src/interface.c| 21 +
tests/test_interface.py| 5 +
3 files changed, 30 insertions(+)
diff --git a/data/org.libvirt.Interface.xml b/data/org.libvirt.Interface.xml
index f5ec281..4534b97 100644
--- a/data/org.libvirt.Interface.xml
+++ b/data/org.libvirt.Interface.xml
@@ -13,5 +13,9 @@
value="See
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceDestroy"/>
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceUndefine"/>
+
diff --git a/src/interface.c b/src/interface.c
index 077b10d..a2d1cef 100644
--- a/src/interface.c
+++ b/src/interface.c
@@ -70,6 +70,26 @@ virtDBusInterfaceDestroy(GVariant *inArgs,
virtDBusUtilSetLastVirtError(error);
}
+static void
+virtDBusInterfaceUndefine(GVariant *inArgs G_GNUC_UNUSED,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath,
+ gpointer userData,
+ GVariant **outArgs G_GNUC_UNUSED,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterface) interface = NULL;
+
+interface = virtDBusInterfaceGetVirInterface(connect, objectPath, error);
+if (!interface)
+return;
+
+if (virInterfaceUndefine(interface) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
static virtDBusGDBusPropertyTable virtDBusInterfacePropertyTable[] = {
{ 0 }
};
@@ -77,6 +97,7 @@ static virtDBusGDBusPropertyTable
virtDBusInterfacePropertyTable[] = {
static virtDBusGDBusMethodTable virtDBusInterfaceMethodTable[] = {
{ "Create", virtDBusInterfaceCreate },
{ "Destroy", virtDBusInterfaceDestroy },
+{ "Undefine", virtDBusInterfaceUndefine },
{ 0 }
};
diff --git a/tests/test_interface.py b/tests/test_interface.py
index 62fd517..4a83672 100755
--- a/tests/test_interface.py
+++ b/tests/test_interface.py
@@ -7,6 +7,11 @@ class TestInterface(libvirttest.BaseTestClass):
""" Tests for methods and properties of the Interface interface
"""
+def test_interface_undefine(self):
+_,interface_obj = self.interface_create()
+interface_obj.Destroy(0)
+interface_obj.Undefine(0)
+
def test_interface_destroy(self):
_,interface_obj = self.interface_create()
interface_obj.Destroy(0)
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 14/17] Implement InterfaceChangeCommit method for Connect Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Connect.xml | 5 +
src/connect.c| 22 ++
2 files changed, 27 insertions(+)
diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 604afea..8c88cc8 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -153,6 +153,11 @@
value="See
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceChangeBegin"/>
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceChangeCommit"/>
+
+
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceDefineXML"/>
diff --git a/src/connect.c b/src/connect.c
index 0e19e91..2acc657 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -756,6 +756,27 @@ virtDBusConnectInterfaceChangeBegin(GVariant *inArgs,
virtDBusUtilSetLastVirtError(error);
}
+static void
+virtDBusConnectInterfaceChangeCommit(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath G_GNUC_UNUSED,
+ gpointer userData,
+ GVariant **outArgs G_GNUC_UNUSED,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+guint flags;
+
+g_variant_get(inArgs, "(u)", );
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+if (virInterfaceChangeCommit(connect->connection, flags) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
static void
virtDBusConnectInterfaceDefineXML(GVariant *inArgs,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1854,6 +1875,7 @@ static virtDBusGDBusMethodTable
virtDBusConnectMethodTable[] = {
{ "GetDomainCapabilities", virtDBusConnectGetDomainCapabilities },
{ "GetSysinfo", virtDBusConnectGetSysinfo },
{ "InterfaceChangeBegin", virtDBusConnectInterfaceChangeBegin },
+{ "InterfaceChangeCommit", virtDBusConnectInterfaceChangeCommit },
{ "InterfaceDefineXML", virtDBusConnectInterfaceDefineXML },
{ "ListDomains", virtDBusConnectListDomains },
{ "ListInterfaces", virtDBusConnectListInterfaces },
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 05/17] Introduce Interface Tests & Test Create method for Interface Interface
Signed-off-by: Anya Harter
---
tests/Makefile.am | 1 +
tests/libvirttest.py| 12
tests/test_interface.py | 16
3 files changed, 29 insertions(+)
create mode 100755 tests/test_interface.py
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 09c3e2e..cd1fbd7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -11,6 +11,7 @@ test_helpers = \
test_programs = \
test_connect.py \
test_domain.py \
+ test_interface.py \
test_network.py \
test_nodedev.py \
test_storage.py \
diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index 3741abd..2a09182 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -71,6 +71,18 @@ class BaseTestClass():
if self.timeout:
raise TimeoutError()
+def interface_create(self):
+""" Fixture to define dummy interface on the test driver
+
+This fixture should be used in the setup of every test manipulating
+with interfaces.
+"""
+path = self.connect.InterfaceDefineXML(xmldata.minimal_interface_xml,
0)
+obj = self.bus.get_object('org.libvirt', path)
+interface_obj = dbus.Interface(obj, 'org.libvirt.Interface')
+interface_obj.Create(0)
+return path, interface_obj
+
@pytest.fixture
def node_device_create(self):
""" Fixture to create dummy node device on the test driver
diff --git a/tests/test_interface.py b/tests/test_interface.py
new file mode 100755
index 000..88be5dc
--- /dev/null
+++ b/tests/test_interface.py
@@ -0,0 +1,16 @@
+#!/usr/bin/python3
+
+import dbus
+import libvirttest
+
+class TestInterface(libvirttest.BaseTestClass):
+""" Tests for methods and properties of the Interface interface
+"""
+
+def test_interface_create(self):
+_,interface_obj = self.interface_create()
+interface_obj.Destroy(0)
+interface_obj.Create(0)
+
+if __name__ == '__main__':
+libvirttest.run()
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 12/17] Implement ListInterfaces method for Connect Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Connect.xml | 6 ++
src/connect.c| 38
2 files changed, 44 insertions(+)
diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index f690bab..791e1f5 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -161,6 +161,12 @@
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virConnectListAllInterfaces"/>
+
+
+
https://libvirt.org/html/libvirt-libvirt-network.html#virConnectListAllNetworks"/>
diff --git a/src/connect.c b/src/connect.c
index 304a56f..9be8264 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -801,6 +801,43 @@ virtDBusConnectListDomains(GVariant *inArgs,
*outArgs = g_variant_new_tuple(, 1);
}
+static void
+virtDBusConnectListInterfaces(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath G_GNUC_UNUSED,
+ gpointer userData,
+ GVariant **outArgs,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterfacePtr) interfaces = NULL;
+guint flags;
+GVariantBuilder builder;
+GVariant *ginterfaces;
+
+g_variant_get(inArgs, "(u)", );
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+if (virConnectListAllInterfaces(connect->connection, , flags) <
0)
+return virtDBusUtilSetLastVirtError(error);
+
+g_variant_builder_init(, G_VARIANT_TYPE("ao"));
+
+for (gint i = 0; interfaces[i]; i++) {
+g_autofree gchar *path = NULL;
+path = virtDBusUtilBusPathForVirInterface(interfaces[i],
+ connect->interfacePath);
+
+g_variant_builder_add(, "o", path);
+}
+
+ginterfaces = g_variant_builder_end();
+*outArgs = g_variant_new_tuple(, 1);
+}
+
static void
virtDBusConnectListNetworks(GVariant *inArgs,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1797,6 +1834,7 @@ static virtDBusGDBusMethodTable
virtDBusConnectMethodTable[] = {
{ "GetSysinfo", virtDBusConnectGetSysinfo },
{ "InterfaceDefineXML", virtDBusConnectInterfaceDefineXML },
{ "ListDomains", virtDBusConnectListDomains },
+{ "ListInterfaces", virtDBusConnectListInterfaces },
{ "ListNetworks", virtDBusConnectListNetworks },
{ "ListNodeDevices", virtDBusConnectListNodeDevices },
{ "ListNWFilters", virtDBusConnectListNWFilters },
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 03/17] Implement Create method for Interface Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Interface.xml | 5
src/interface.c| 45 ++
2 files changed, 50 insertions(+)
diff --git a/data/org.libvirt.Interface.xml b/data/org.libvirt.Interface.xml
index 93fa32f..d98a646 100644
--- a/data/org.libvirt.Interface.xml
+++ b/data/org.libvirt.Interface.xml
@@ -3,5 +3,10 @@
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceCreate"/>
+
+
diff --git a/src/interface.c b/src/interface.c
index 3c32f0f..4a5e431 100644
--- a/src/interface.c
+++ b/src/interface.c
@@ -3,11 +3,56 @@
#include
+static virInterfacePtr
+virtDBusInterfaceGetVirInterface(virtDBusConnect *connect,
+ const gchar *objectPath,
+ GError **error)
+{
+virInterfacePtr interface;
+
+if (virtDBusConnectOpen(connect, error) < 0)
+return NULL;
+
+interface = virtDBusUtilVirInterfaceFromBusPath(connect->connection,
+objectPath,
+connect->interfacePath);
+if (!interface) {
+virtDBusUtilSetLastVirtError(error);
+return NULL;
+}
+
+return interface;
+}
+
+static void
+virtDBusInterfaceCreate(GVariant *inArgs,
+GUnixFDList *inFDs G_GNUC_UNUSED,
+const gchar *objectPath,
+gpointer userData,
+GVariant **outArgs G_GNUC_UNUSED,
+GUnixFDList **outFDs G_GNUC_UNUSED,
+GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterface) interface = NULL;
+guint flags;
+
+g_variant_get(inArgs, "(u)", );
+
+interface = virtDBusInterfaceGetVirInterface(connect, objectPath, error);
+if (!interface)
+return;
+
+if (virInterfaceCreate(interface, flags) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
static virtDBusGDBusPropertyTable virtDBusInterfacePropertyTable[] = {
{ 0 }
};
static virtDBusGDBusMethodTable virtDBusInterfaceMethodTable[] = {
+{ "Create", virtDBusInterfaceCreate },
{ 0 }
};
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 17/17] Implement InterfaceLookupByMAC method for Connect Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Connect.xml | 6 ++
src/connect.c| 29 +
tests/test_connect.py| 1 +
3 files changed, 36 insertions(+)
diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index f99e205..3447538 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -170,6 +170,12 @@
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceLookupByMACString"/>
+
+
+
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceLookupByName"/>
diff --git a/src/connect.c b/src/connect.c
index ae64d59..f0da1dd 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -827,6 +827,34 @@ virtDBusConnectInterfaceDefineXML(GVariant *inArgs,
*outArgs = g_variant_new("(o)", path);
}
+static void
+virtDBusConnectInterfaceLookupByMAC(GVariant *inArgs,
+GUnixFDList *inFDs G_GNUC_UNUSED,
+const gchar *objectPath G_GNUC_UNUSED,
+gpointer userData,
+GVariant **outArgs,
+GUnixFDList **outFDs G_GNUC_UNUSED,
+GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterface) interface = NULL;
+g_autofree gchar *path = NULL;
+const gchar *mac;
+
+g_variant_get(inArgs, "()", );
+
+if (!virtDBusConnectOpen(connect, NULL))
+return;
+
+interface = virInterfaceLookupByMACString(connect->connection, mac);
+if (!interface)
+return virtDBusUtilSetLastVirtError(error);
+
+path = virtDBusUtilBusPathForVirInterface(interface,
connect->interfacePath);
+
+*outArgs = g_variant_new("(o)", path);
+}
+
static void
virtDBusConnectInterfaceLookupByName(GVariant *inArgs,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1927,6 +1955,7 @@ static virtDBusGDBusMethodTable
virtDBusConnectMethodTable[] = {
{ "InterfaceChangeCommit", virtDBusConnectInterfaceChangeCommit },
{ "InterfaceChangeRollback", virtDBusConnectInterfaceChangeRollback },
{ "InterfaceDefineXML", virtDBusConnectInterfaceDefineXML },
+{ "InterfaceLookupByMAC", virtDBusConnectInterfaceLookupByMAC },
{ "InterfaceLookupByName", virtDBusConnectInterfaceLookupByName },
{ "ListDomains", virtDBusConnectListDomains },
{ "ListInterfaces", virtDBusConnectListInterfaces },
diff --git a/tests/test_connect.py b/tests/test_connect.py
index 7e33b2a..042c568 100755
--- a/tests/test_connect.py
+++ b/tests/test_connect.py
@@ -90,6 +90,7 @@ class TestConnect(libvirttest.BaseTestClass):
@pytest.mark.parametrize("lookup_method_name,lookup_item", [
("InterfaceLookupByName", 'Name'),
+("InterfaceLookupByMAC", 'MAC'),
])
def test_connect_interface_lookup_by_property(self, lookup_method_name,
lookup_item):
"""Parameterized test for all InterfaceLookupBy* API calls of Connect
interface
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 02/17] Implement InterfaceDefineXML method for Connect Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Connect.xml | 7 +++
src/connect.c| 30 ++
tests/test_connect.py| 4
tests/xmldata.py | 12
4 files changed, 53 insertions(+)
diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 7014a09..f690bab 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -148,6 +148,13 @@
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceDefineXML"/>
+
+
+
+
https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectListAllDomains"/>
diff --git a/src/connect.c b/src/connect.c
index 3dc434d..304a56f 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -735,6 +735,35 @@ virtDBusConnectGetSysinfo(GVariant *inArgs,
*outArgs = g_variant_new("(s)", sysinfo);
}
+static void
+virtDBusConnectInterfaceDefineXML(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath G_GNUC_UNUSED,
+ gpointer userData,
+ GVariant **outArgs,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterface) interface = NULL;
+g_autofree gchar *path = NULL;
+const gchar *xml;
+guint flags;
+
+g_variant_get(inArgs, "()", , );
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+interface = virInterfaceDefineXML(connect->connection, xml, flags);
+if (!interface)
+return virtDBusUtilSetLastVirtError(error);
+
+path = virtDBusUtilBusPathForVirInterface(interface,
connect->interfacePath);
+
+*outArgs = g_variant_new("(o)", path);
+}
+
static void
virtDBusConnectListDomains(GVariant *inArgs,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1766,6 +1795,7 @@ static virtDBusGDBusMethodTable
virtDBusConnectMethodTable[] = {
{ "GetCPUModelNames", virtDBusConnectGetCPUModelNames },
{ "GetDomainCapabilities", virtDBusConnectGetDomainCapabilities },
{ "GetSysinfo", virtDBusConnectGetSysinfo },
+{ "InterfaceDefineXML", virtDBusConnectInterfaceDefineXML },
{ "ListDomains", virtDBusConnectListDomains },
{ "ListNetworks", virtDBusConnectListNetworks },
{ "ListNodeDevices", virtDBusConnectListNodeDevices },
diff --git a/tests/test_connect.py b/tests/test_connect.py
index 2299b8a..d21b366 100755
--- a/tests/test_connect.py
+++ b/tests/test_connect.py
@@ -84,6 +84,10 @@ class TestConnect(libvirttest.BaseTestClass):
sysinfo = self.connect.GetSysinfo(0)
assert isinstance(sysinfo, dbus.String)
+def test_connect_interface_define_xml(self):
+path = self.connect.InterfaceDefineXML(xmldata.minimal_interface_xml,
0)
+assert isinstance(path, dbus.ObjectPath)
+
def test_connect_list_networks(self):
networks = self.connect.ListNetworks(0)
assert isinstance(networks, dbus.Array)
diff --git a/tests/xmldata.py b/tests/xmldata.py
index 25bb089..a70bac1 100644
--- a/tests/xmldata.py
+++ b/tests/xmldata.py
@@ -12,6 +12,18 @@ minimal_domain_xml = '''
'''
+minimal_interface_xml = '''
+
+
+
+
+
+
+
+
+
+'''
+
minimal_network_xml = '''
bar
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 08/17] Implement GetXMLDesc method for Interface Interface
Signed-off-by: Anya Harter
---
data/org.libvirt.Interface.xml | 6 ++
src/interface.c| 28
tests/test_interface.py| 4
3 files changed, 38 insertions(+)
diff --git a/data/org.libvirt.Interface.xml b/data/org.libvirt.Interface.xml
index 4534b97..dc03258 100644
--- a/data/org.libvirt.Interface.xml
+++ b/data/org.libvirt.Interface.xml
@@ -13,6 +13,12 @@
value="See
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceDestroy"/>
+
+ https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetXMLDesc"/>
+
+
+
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceUndefine"/>
diff --git a/src/interface.c b/src/interface.c
index a2d1cef..fcec623 100644
--- a/src/interface.c
+++ b/src/interface.c
@@ -70,6 +70,33 @@ virtDBusInterfaceDestroy(GVariant *inArgs,
virtDBusUtilSetLastVirtError(error);
}
+static void
+virtDBusInterfaceGetXMLDesc(GVariant *inArgs,
+GUnixFDList *inFDs G_GNUC_UNUSED,
+const gchar *objectPath,
+gpointer userData,
+GVariant **outArgs,
+GUnixFDList **outFDs G_GNUC_UNUSED,
+GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterface) interface = NULL;
+g_autofree gchar *xml = NULL;
+guint flags;
+
+g_variant_get(inArgs, "(u)", );
+
+interface = virtDBusInterfaceGetVirInterface(connect, objectPath, error);
+if (!interface)
+return;
+
+xml = virInterfaceGetXMLDesc(interface, flags);
+if (!xml)
+return virtDBusUtilSetLastVirtError(error);
+
+*outArgs = g_variant_new("(s)", xml);
+}
+
static void
virtDBusInterfaceUndefine(GVariant *inArgs G_GNUC_UNUSED,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -97,6 +124,7 @@ static virtDBusGDBusPropertyTable
virtDBusInterfacePropertyTable[] = {
static virtDBusGDBusMethodTable virtDBusInterfaceMethodTable[] = {
{ "Create", virtDBusInterfaceCreate },
{ "Destroy", virtDBusInterfaceDestroy },
+{ "GetXMLDesc", virtDBusInterfaceGetXMLDesc },
{ "Undefine", virtDBusInterfaceUndefine },
{ 0 }
};
diff --git a/tests/test_interface.py b/tests/test_interface.py
index 4a83672..0b5a943 100755
--- a/tests/test_interface.py
+++ b/tests/test_interface.py
@@ -21,5 +21,9 @@ class TestInterface(libvirttest.BaseTestClass):
interface_obj.Destroy(0)
interface_obj.Create(0)
+def test_interface_get_xml_description(self):
+_,interface_obj = self.interface_create()
+assert isinstance(interface_obj.GetXMLDesc(0), dbus.String)
+
if __name__ == '__main__':
libvirttest.run()
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH v2 01/17] Introduce Interface Interface
Signed-off-by: Anya Harter
---
data/Makefile.am | 1 +
data/org.libvirt.Interface.xml | 7
src/Makefile.am| 2 ++
src/connect.c | 6
src/connect.h | 1 +
src/interface.c| 65 ++
src/interface.h| 9 +
src/util.c | 35 ++
src/util.h | 15
9 files changed, 141 insertions(+)
create mode 100644 data/org.libvirt.Interface.xml
create mode 100644 src/interface.c
create mode 100644 src/interface.h
diff --git a/data/Makefile.am b/data/Makefile.am
index 7b523da..35a0bbd 100644
--- a/data/Makefile.am
+++ b/data/Makefile.am
@@ -33,6 +33,7 @@ polkit_DATA = \
interfaces_files = \
org.libvirt.Connect.xml \
org.libvirt.Domain.xml \
+ org.libvirt.Interface.xml \
org.libvirt.Network.xml \
org.libvirt.NodeDevice.xml \
org.libvirt.NWFilter.xml \
diff --git a/data/org.libvirt.Interface.xml b/data/org.libvirt.Interface.xml
new file mode 100644
index 000..93fa32f
--- /dev/null
+++ b/data/org.libvirt.Interface.xml
@@ -0,0 +1,7 @@
+http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd;>
+
+
+
+
+
diff --git a/src/Makefile.am b/src/Makefile.am
index b5bf129..d0e8f0d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -41,6 +41,8 @@ libvirt_dbus_SOURCES = \
events.h \
gdbus.c \
gdbus.h \
+ interface.c \
+ interface.h \
main.c \
network.c \
network.h \
diff --git a/src/connect.c b/src/connect.c
index 9275121..3dc434d 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -1,6 +1,7 @@
#include "connect.h"
#include "domain.h"
#include "events.h"
+#include "interface.h"
#include "network.h"
#include "nodedev.h"
#include "nwfilter.h"
@@ -1809,6 +1810,7 @@ virtDBusConnectFree(virtDBusConnect *connect)
virtDBusConnectClose(connect, TRUE);
g_free(connect->domainPath);
+g_free(connect->interfacePath);
g_free(connect->networkPath);
g_free(connect->nodeDevPath);
g_free(connect->nwfilterPath);
@@ -1869,6 +1871,10 @@ virtDBusConnectNew(virtDBusConnect **connectp,
if (error && *error)
return;
+virtDBusInterfaceRegister(connect, error);
+if (error && *error)
+return;
+
virtDBusNetworkRegister(connect, error);
if (error && *error)
return;
diff --git a/src/connect.h b/src/connect.h
index 0b9ae10..961b115 100644
--- a/src/connect.h
+++ b/src/connect.h
@@ -13,6 +13,7 @@ struct virtDBusConnect {
const gchar *uri;
const gchar *connectPath;
gchar *domainPath;
+gchar *interfacePath;
gchar *networkPath;
gchar *nodeDevPath;
gchar *nwfilterPath;
diff --git a/src/interface.c b/src/interface.c
new file mode 100644
index 000..3c32f0f
--- /dev/null
+++ b/src/interface.c
@@ -0,0 +1,65 @@
+#include "interface.h"
+#include "util.h"
+
+#include
+
+static virtDBusGDBusPropertyTable virtDBusInterfacePropertyTable[] = {
+{ 0 }
+};
+
+static virtDBusGDBusMethodTable virtDBusInterfaceMethodTable[] = {
+{ 0 }
+};
+
+static gchar **
+virtDBusInterfaceEnumerate(gpointer userData)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virInterfacePtr) interfaces = NULL;
+gint num = 0;
+gchar **ret = NULL;
+
+if (!virtDBusConnectOpen(connect, NULL))
+return NULL;
+
+num = virConnectListAllInterfaces(connect->connection, , 0);
+if (num < 0)
+return NULL;
+
+if (num == 0)
+return NULL;
+
+ret = g_new0(gchar *, num + 1);
+
+for (gint i = 0; i < num; i++) {
+ret[i] = virtDBusUtilBusPathForVirInterface(interfaces[i],
+connect->interfacePath);
+}
+
+return ret;
+}
+
+static GDBusInterfaceInfo *interfaceInfo;
+
+void
+virtDBusInterfaceRegister(virtDBusConnect *connect,
+ GError **error)
+{
+connect->interfacePath = g_strdup_printf("%s/interface",
+ connect->connectPath);
+
+if (!interfaceInfo) {
+interfaceInfo =
virtDBusGDBusLoadIntrospectData(VIRT_DBUS_INTERFACE_INTERFACE,
+error);
+if (!interfaceInfo)
+return;
+}
+
+virtDBusGDBusRegisterSubtree(connect->bus,
+ connect->interfacePath,
+ interfaceInfo,
+ virtDBusInterfaceEnumerate,
+ virtDBusInterfaceMethodTable,
+ virtDBusInterfacePropertyTable,
+ connect);
+}
diff --git a/src/interface.h b/src/interface.h
new file mode 100644
index 000..8e5ee0a
--- /dev/null
+++ b/src/interface.h
@@ -0,0 +1,9 @@
+#pragma once
+
+#include "connect.h"
+
+#define VIRT_DBUS_INTERFACE_INTERFACE
Re: [libvirt] [PATCHv2 11/11] qemu_driver: BaselineHypervisorCPU supports S390 using QEMU/QMP
Quoting David Hildenbrand (2018-07-18 02:26:24)
> On 18.07.2018 00:39, Collin Walling wrote:
> > On 07/17/2018 05:01 PM, David Hildenbrand wrote:
> >> On 13.07.2018 18:00, Jiri Denemark wrote:
> >>> On Mon, Jul 09, 2018 at 22:56:55 -0500, Chris Venteicher wrote:
> Transient S390 configurations require using QEMU to compute CPU Model
> Baseline and to do CPU Feature Expansion.
>
> Start and use a single QEMU instance to do both the baseline and
> expansion transactions required by BaselineHypervisorCPU.
>
> CPU Feature Expansion uses true / false to indicate if property is/isn't
> included in model. Baseline only returns property list where all
> enumerated properties are included.
> >>>
> >>> So are you saying on s390 there's no chance there would be a CPU model
> >>> with some feature which is included in the CPU model disabled for some
> >>> reason? Sounds too good to be true :-) (This is the question I referred
> >>> to in one of my replies to the other patches.)
> >>
> >> Giving some background information: When we expand/baseline CPU models,
> >> we always expand them to the "-base" variants of our CPU models, which
> >> contain some set of features we expect to be around in all sane
> >> configurations ("minimal feature set").
> >>
> >> It is very unlikely that we ever have something like
> >> "z14-base,featx=off", but it could happen
> >> - When using an emulator (TCG)
> >> - When running nested and the guest hypervisor is started with such a
> >>strange CPU model
> >> - When something in the HW is very wrong or eventually removed in the
> >>future (unlikely but possible)
> >>
> >> On some very weird inputs to a baseline request, such a strange model
> >> can also be the result. But it is very unusual.
> >>
> >> I assume something like "baseline z14-base,featx=off with z14-base" will
> >> result in "z14-base,featx=off", too.
> >>
> >>
> >
> > That's correct. A CPU model with a feature disabled that is baseline with a
> > CPU
> > model with that same feature enabled will omit that feature in the QMP
> > response.
> >
> > e.g. if z14-base has features x, y, z then
> >
> > "baseline z14-base,featx=off with z14-base" will result in
> > "z14-base,featy=on,featz=on"
I am runing tests on both S390 and X86 (hacked QEMU to enable baseline).
I don't see a "false" property in the baseline response in any of the logs.
I did try to slip a "zpci":false into the query-cpu-model-baseline but I still
don't get a false in the response.
Here is the request/response for reference.
{"execute":"query-cpu-model-baseline",
"arguments":{"modela":{"name":"z14"},
"modelb":{"name":"z13","props":{"msa5":true,"exrl":true,"zpci":false}}},
"id":"libvirt-2"}
{"return": {"model": {"name": "z13-base","props": {"aen": true, "aefsi": true,
"msa5": true, "msa4": true, "msa3": true, "msa2": true, "msa1": true, "sthyi":
true, "edat": true, "ri": true, "edat2": true, "vx": true, "ipter": true,
"esop": true, "cte": true, "sea_esop2": true, "te": true, "cmm": true}}}, "id":
"libvirt-2"}
> Usually we try to not chose a model with stripped off base features ("we
> try to produce a model that looks sane"), but instead fallback to some
> very ancient CPU model. E.g.
>
> { "execute": "query-cpu-model-baseline", "arguments" : { "modela": {
> "name": "z14-base", "props": {"msa" : false}}, "modelb": { "name": "z14"}} }
>
> -> {"return": {"model": {"name": "z800-base", "props": {"etf2": true,
> "ldisp": true
>
> We might want to change that behavior in the future however (or maybe it
> already is like this for some corner cases) - assume some base feature
> gets dropped by HW in a new CPU generation. We don't always want to
> fallback to a z900 or so when baselining. So one should assume that we
> can have disabled features here.
>
> Especially as there is a BUG in QEMU I'll have to fix:
>
> { "execute": "query-cpu-model-baseline", "arguments" : { "modela": {
> "name": "z14-base", "props": {"esan3" : false}}, "modelb": { "name":
> "z14"}} }
>
> -> Segmentation fault
>
> This would have to produce a model with esan3 disabled (very very
> unlikely to ever happen in real life :) )
>
> The result should be something like {"model": {"name": "z900-base",
> "props": {"esan3": false}}} or an error that they cannot be baselined.
>
Seems like were saying I should be filtering out or otherwise property excluding
any false properties that are returned. Please correct if I have this wrong.
I currently do filtering / exclusion on the result of expansion but seems like I
should be doing filtering on baseline output too if we don't do the expansion
just in case baseline would return a false property for some reason.
>
> >
> > Since baseline will just report a base cpu and its minimal feature set... I
> > wonder if it
> > makes sense for libvirt to just report the features resulting from the
> > baseline command
> > instead of later calling expansion?
Re: [libvirt] [PATCHv2 03/11] qemu_monitor: Indicate when CPUModelInfo props report migratablity
Quoting Jiri Denemark (2018-07-12 06:59:18)
> On Mon, Jul 09, 2018 at 22:56:47 -0500, Chris Venteicher wrote:
> > Renamed variable in CPUModelInfo such that
> > props_migratable_valid is true when properties in CPUModelInfo
> > have been updated to accurately indicate if property is / isn't
> > migratable.
> >
> > Property migratability is not returned directly in QMP messages but
> > rather is sometimes calculated within Libvirt by other means and then
> > stored in CPUModelInfo properties by Libvirt. props_migratable_valid is
> > set to true when this calculation has been done by Libvirt.
> > ---
> > src/qemu/qemu_capabilities.c | 10 +-
> > src/qemu/qemu_monitor.c | 2 +-
> > src/qemu/qemu_monitor.h | 2 +-
> > 3 files changed, 7 insertions(+), 7 deletions(-)
> >
> ...
> > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> > index 18b59be985..208a7f5d21 100644
> > --- a/src/qemu/qemu_monitor.h
> > +++ b/src/qemu/qemu_monitor.h
> > @@ -1005,7 +1005,7 @@ struct _qemuMonitorCPUModelInfo {
> > char *name;
> > size_t nprops;
> > qemuMonitorCPUPropertyPtr props;
> > -bool migratability;
> > +bool props_migratable_valid;
> > };
>
> I don't see a reason for renaming the variable. The new name is uglier
> and may be confusing in exactly the same way you found migratability to
> be confusing. Just add a comment which would explain that the
> migratability tells whether we can use the prop->migratable value to
> check if a particular feature is migratable.
>
I still have some concerns about the use of the migratability
(props_migratability_valid) variable.
There seems to be a very narrow case where the CPUModelInfo is non-migratable
(contains non-migratable props) ... arch:X86 + name:host.
In all other cases, specific model names (ex IvyBridge) or other Archs, the
CPUModelInfo is by default migratable yet the variable migratability
(props_migratable_valid) is false.
There seems to be multiple cases in existing code base where the CPUModelInfo
is
actually migratable but is treated as non-migratable because the migratability
(props_migratability_valid) is false.
Not sure if this is really a problem or if I am just missing something yet.
Seems like the most efficient thing is to try to sort it out in a stand alone
patch that's easy to give feedback on.
Regardless, I will try to keep the naming consistent (migratability) and use a
comment if needed.
Chris
> Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
