[libvirt] about support quorum and replication in libvirt

[wang.guang55]

hi


we wiill use quorum and replication in qemu.


but we found that libvirt does not support quorum and replication in xml.


so we want submit some patches for supporting quorum and replication in xml.


how about it??.--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Plans for next release

[Daniel Veillard]

On Wed, Jul 26, 2017 at 10:15:37PM +0200, Peter Krempa wrote:
> On Tue, Jul 25, 2017 at 23:47:12 +0800, Daniel Veillard wrote:
> >   The end of the month is next week, so if we want to release around
> > Aug 1st, I think we should enter freeze on Thursday morning, then
> > I could make rc2 on Saturday and we could have the release on the Tuesday
> > or Wed.
> >   Usually this is not the busiest time of the year but if this is a problem
> > please raise this,
> 
> I've messed up linking of the apparmor helper program. Fix is here:
> 
> https://www.redhat.com/archives/libvir-list/2017-July/msg01197.html
> 
> Since it's coupled with a fairly invasive cleanup I might need to respin
> because of that.

  If you can get the ACKs and push Thursday morning your time, I will wait
for this for the freeze, ping me when you're done,

  thanks,

Daniel




-- 
Daniel Veillard  | Red Hat Developers Tools http://developer.redhat.com/
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

[libvirt] question about support quorum and replication in libvirt

[wang.guang55]

hl

we wiill use quorum and replication in qemu.

but we found that libvirt does not support quorum and replication .

so we want submit some patches for supporing quorum and replication 

how about it??.--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/3] libxl: Add a test suite for libxl_domain_config generator

[Marek Marczykowski-Górecki]

On Fri, Jul 14, 2017 at 05:59:57PM -0600, Jim Fehlig wrote:
> On 07/01/2017 08:16 PM, Marek Marczykowski-Górecki wrote:
> > On Sun, Feb 26, 2017 at 07:02:24PM -0700, Jim Fehlig wrote:
> > > Long ago danpb posted some patches to test libvirt domXML to
> > > libxl_domain_config conversion
> > > 
> > > https://www.redhat.com/archives/libvir-list/2014-May/msg01102.html
> > > 
> > > Some of the prerequisite patches were pushed, but we've never managed
> > > to push patches actually providing the conversion tests. I sent several
> > > follow-ups to Dan's work but never converged on a satisfactory solution
> > > for all the Xen versions supported by libvirt. The last attempt was in
> > > Sept 2014
> > > 
> > > https://www.redhat.com/archives/libvir-list/2014-September/msg00698.html
> > > 
> > > I tried to revive the work in Jan 2015, but that also stalled
> > > 
> > > https://www.redhat.com/archives/libvir-list/2015-January/msg00924.html
> > > 
> > > Fast-forward over 2.5 years from the first attempt and libvirt no longer
> > > supports older Xen versions 4.2 and 4.3 that were proving to be 
> > > problematic.
> > > Starting with Xen 4.5 libxl added support for 
> > > libxl_domain_config_from_json,
> > > which provides a way to implement the conversion tests that work with all
> > > Xen versions >= 4.5 (including latest xen.git master).
> > 
> > Few more months have passed...
> 
> And a few more weeks :-). Sorry for the delay. Slowly catching up on libvirt
> mail after some time away...

It's me this time...

> > FWIW, I've tested it with Xen 4.6. The patch needs very minor update:
> >   - s/VIRT_TEST_MAIN_PRELOAD/VIR_TEST_MAIN_PRELOAD/
> >   - add xencaps argument to libxlBuildDomainConfig cal >
> > 
> > After that, it works! When I made some test to fail, reported error is
> > not so helpful ("libvirt: Xen Light Driver error : internal error:
> > Expected and actual libxl_domain_config objects do not compare"), but it
> > do catch failures.
> > Then, if I change strcmp to virTestCompareToString, the output is much
> > more helpful.
> 
> Thanks for the improvements. I noticed you included them in your V2 of patch
> 3/3. I assume you needed patches 1 and 2 as well?

Yes, in this tests shape. But see below.

> > I'd really love to have it merged, mostly because I want to add more tests
> > using this framework (see "Add setting CPU features (CPUID) with
> > libxenlight driver" thread).
> 
> I really dislike patches 1 and 2. They remove very useful checks IMO.
> Shortly after posting the series in Feb, I tried mocking the emulator
> checks. I failed quite a bit before coming up with something that worked,
> yet not so satisfying :-). I'll attach the (now rather old) patches for
> reference.

My independent try to this test approach ("tests: check domain XML to
libxl structures conversion") had much simpler (but uglier) solution for
this problem: use /bin/true as emulator path (replaced in runtime, just
after loading XML from file). While proper mocking this check would be
better, apparently it isn't trivial.

> > Is there anything I can do to make it happen?
> 
> Do you have time to update these old patches, test, and repost to the list?
> Or perhaps have better ideas on mocking, or approaches that avoid the need
> for 1/3 and 2/3.

If using /bin/true as emulator path is acceptable, I can finish this and
post updated version.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


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

[libvirt] status of support for cache allocation technology?

[Chris Friesen]

Hi,

I'm just wondering what the current status is about exposing/controlling cache 
banks.  Looking at the code, it appears that we report the banks as part of 
"virsh capabilities".  Is it possible to associate a particular bank with a 
particular domain, or has that not yet merged?


Is the "[PATH V10 00/12] Support cache tune in libvirt" patch series the most 
recent set of patches?


Thanks,
Chris

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

Re: [libvirt] [PATCH 00/24] qemu: refactor node name detection

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Since query-blockstats provides full hierarchy of node names without any 
> doubtful
> matching algorithm, use it to make the node name detector reliable.
> 
> Unfortunately some of the changes are ugly when viewed as a diff since they
> delete and rewrite chunks of code.
> 
> This series can be fetched at:
> 
>  git fetch git://pipo.sk/pipo/libvirt.git node-name-detect-refactor-1

So far, I've just reviewed the patches, and found a few things to fix
although the overall impression was good.  I still hope to build and see
if I can manage to find problems in my own testing, but don't let that
be a bottleneck if you are trying to get this in before Thursday
morning's freeze (ie. we can do followup patches if I find something
later, if you can't wait for me to report results).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 24/24] tests: qemumonitorjson: Test extraction of iSCSI device node names

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> ---
>  .../qemumonitorjson-nodename-iscsi-blockstats.json | 113 
>  ...qemumonitorjson-nodename-iscsi-named-nodes.json | 114 
> +
>  .../qemumonitorjson-nodename-iscsi.result  |  13 +++
>  tests/qemumonitorjsontest.c|   1 +
>  4 files changed, 241 insertions(+)
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi-blockstats.json
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi-named-nodes.json
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi.result
> 

> +++ 
> b/tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi-named-nodes.json
> @@ -0,0 +1,114 @@
> +[
> +{
> +  "iops_rd": 0,
> +  "detect_zeroes": "off",
> +  "image": {
> +"virtual-size": 1073741824,
> +"filename": "json:{\"driver\": \"raw\", \"file\": {\"lun\": \"0\", 
> \"portal\": \"example.com:3260\", \"driver\": \"iscsi\", \"transport\": 
> \"tcp\", \"target\": \"iqn.2016-09.com.example:server\"}}",

Another fun name.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 23/24] tests: qemumonitorjson: Test extraction of LUKS node names

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> ---
>  .../qemumonitorjson-nodename-luks-blockstats.json  |  58 +++
>  .../qemumonitorjson-nodename-luks-named-nodes.json | 109 
> +
>  .../qemumonitorjson-nodename-luks.result   |   6 ++
>  tests/qemumonitorjsontest.c|   1 +
>  4 files changed, 174 insertions(+)
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-luks-blockstats.json
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-luks-named-nodes.json
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-luks.result
> 

> +++ b/tests/qemumonitorjsondata/qemumonitorjson-nodename-luks-named-nodes.json
> @@ -0,0 +1,109 @@
> +[
> +{
> +  "iops_rd": 0,
> +  "detect_zeroes": "off",
> +  "image": {
> +"virtual-size": 10485760,
> +"filename": "json:{\"driver\": \"luks\", \"file\": {\"driver\": 
> \"file\", \"filename\": \"/var/lib/libvirt/images/luks\"}, \"key-secret\": 
> \"virtio-disk0-luks-secret0\"}",

What a fun name.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 22/24] tests: qemumonitorjson: Old and empty test case for node name detection

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Prior to qemu 2.5 the node names would not be generated, thus would be
> missing from 'query-blockstats' and 'query-named-block-nodes'. Test that
> the code correctly detects nothing.
> 
> Additionally make sure that a VM without disks does not cause problems.
> 
> The test case change is necessary as our test file checker does not play
> well with empty files.

Can you use a file consisting of solely backslash-newline?  That is not
empty, but with \\\n folding becomes an empty string.  That might be a
bit simpler than:

> +++ b/tests/qemumonitorjsondata/qemumonitorjson-nodename-empty.result
> @@ -0,0 +1 @@
> +nothing


> +++ b/tests/qemumonitorjsontest.c
> @@ -2763,6 +2763,9 @@ testBlockNodeNameDetect(const void *opaque)
> 
>  virBufferTrim(&buf, "\n", -1);
> 
> +if (virBufferUse(&buf) == 0)
> +virBufferAddLit(&buf, "nothing\n");

having to special case "nothing".

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 21/24] tests: qemumonitorjson: Add test case for node name detection with blockjob

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> qemu 2.9 returns an extra layer in the backing data if a block job is
> active. Add a test case to see whether our code properly detects and
> ignores such layer.
> ---
>  ...mumonitorjson-nodename-blockjob-blockstats.json | 301 +
>  ...umonitorjson-nodename-blockjob-named-nodes.json | 682 
> +
>  .../qemumonitorjson-nodename-blockjob.result   |  26 +
>  tests/qemumonitorjsontest.c|   1 +
>  4 files changed, 1010 insertions(+)
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-blockjob-blockstats.json
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-blockjob-named-nodes.json
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-blockjob.result
> 

Again, the sequence of steps used to get to this point (as setting up a
block job is not necessarily trivial) might be useful reference material
when reading git log in the future.

> +++ b/tests/qemumonitorjsondata/qemumonitorjson-nodename-blockjob.result
> @@ -0,0 +1,26 @@
> +drive-ide0-0-0
> +filename: '/var/lib/libvirt/images/d.1499152698'
> +format node : '#block179'
> +format drv  : 'qcow2'
> +storage node: '#block042'
> +storage drv : 'file'
> +  filename: '/var/lib/libvirt/images/d.1499152668'
> +  format node : '#block312'

Looks like two snapshots where libvirt created the filename via
timestamps (which of course is not trivially reproducible, but easy to
understand when I get higher numbers), on top of a chain you built...

> +  format drv  : 'qcow2'
> +  storage node: '#block259'
> +  storage drv : 'file'
> +filename: '/var/lib/libvirt/images/d'
> +format node : '#block551'
> +format drv  : 'qcow2'
> +storage node: '#block449'
> +storage drv : 'file'
> +  filename: '/var/lib/libvirt/images/c'
> +  format node : '#block717'
> +  format drv  : 'qcow2'
> +  storage node: '#block618'
> +  storage drv : 'file'
> +filename: '/var/lib/libvirt/images/a'

of 'a'<-'c'<-'d' - I suppose you've already merged 'b' from some other
operation?

At any rate, the new test is useful, and adding a backstory to the
commit message won't change the patch body, so:
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 20/24] tests: qemumonitorjson: Fix 'gluster' node name detection test case

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Add the blockstats data and fix the expected output.
> ---
>  ...emumonitorjson-nodename-gluster-blockstats.json | 111 
> +
>  ...mumonitorjson-nodename-gluster-named-nodes.json |  90 -
>  .../qemumonitorjson-nodename-gluster.result|  15 ++-
>  tests/qemumonitorjsontest.c|   2 +-
>  4 files changed, 167 insertions(+), 51 deletions(-)
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-gluster-blockstats.json

Reviewed-by: Eric Blake 

> 
> diff --git 
> a/tests/qemumonitorjsondata/qemumonitorjson-nodename-gluster-blockstats.json 
> b/tests/qemumonitorjsondata/qemumonitorjson-nodename-gluster-blockstats.json
> new file mode 100644
> index 0..665a6b05b
> --- /dev/null
> +++ 
> b/tests/qemumonitorjsondata/qemumonitorjson-nodename-gluster-blockstats.json
> @@ -0,0 +1,111 @@
> +[
> +{
> +  "device": "drive-virtio-disk0",
> +  "parent": {
> +"stats": {
> +  "flush_total_time_ns": 0,

This output was obviously generated by loading a simple  XML and
then getting query-blockstats output.  I'm wondering, if for
reproducibility (modulo the change of random nodename suffixes), you
should enhance the commit message here (and in similar other patches) to
call out the steps you used to create the domain (did you have to use
qemu-img, and what  spec did you use).  It's not a hard request,
as I've already given reviews, but it may be something we appreciate in
the future when doing git archaeology.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 19/24] tests: qemumonitorjson: Fix 'relative' node name detection test case

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Add the blockstats data and fix the expected output.
> ---
>  ...mumonitorjson-nodename-relative-blockstats.json | 329 
> +
>  ...umonitorjson-nodename-relative-named-nodes.json |  26 +-
>  .../qemumonitorjson-nodename-relative.result   |  44 ++-
>  tests/qemumonitorjsontest.c|   2 +-
>  4 files changed, 372 insertions(+), 29 deletions(-)
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-relative-blockstats.json
> 

> +++ 
> b/tests/qemumonitorjsondata/qemumonitorjson-nodename-relative-named-nodes.json
> @@ -41,7 +41,7 @@
>},
>"iops_wr": 0,
>"ro": false,
> -  "node-name": "#block1177",
> +  "node-name": "#block1290",

Hmm - here you created one extra node in the regeneration in comparison
to how you set it up previously.  Not fatal to the patch, though.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 18/24] tests: qemumonitorjson: Add data and fix 'same-backing' node detection case

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> With the new approach we are actually able to correctly detect node
> names for the two instances of the same backing file.
> ---
>  ...nitorjson-nodename-same-backing-blockstats.json | 221 
> +
>  ...itorjson-nodename-same-backing-named-nodes.json |  16 +-
>  .../qemumonitorjson-nodename-same-backing.result   |  28 ++-
>  tests/qemumonitorjsontest.c|   2 +-
>  4 files changed, 252 insertions(+), 15 deletions(-)
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-same-backing-blockstats.json
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 17/24] qemu: block: store and test driver names for detected storage nodes

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Store the 'drv' field both for the storage node and for the format node
> and format them in the test case.
> ---
>  src/qemu/qemu_block.c  | 14 
> +++---
>  src/qemu/qemu_block.h  |  4 
>  .../qemumonitorjson-nodename-basic.result  |  6 ++
>  tests/qemumonitorjsontest.c|  2 ++
>  4 files changed, 23 insertions(+), 3 deletions(-)
> +++ b/src/qemu/qemu_block.h
> @@ -34,6 +34,10 @@ struct qemuBlockNodeNameBackingChainData {
>  char *nodeformat;   /* node name of the format layer */
>  char *nodestorage;  /* node name of the storage backing the format node 
> */
> 
> +/* for testing purposes */
> +char *drvformat;
> +char *drvstorage;
> +

Should those two fields be listed...

>  qemuBlockNodeNameBackingChainDataPtr backing;

...after this one, to make it obvious where the 'for testing purposes' ends?

>  filename: '/var/lib/libvirt/images/rhel7.3.qcow2'
>  format node : '#block558'
> +format drv  : 'qcow2'
>  storage node: '#block481'
> +storage drv : 'file'

But I definitely see how it will be useful when we mix more than one
protocol, or where a backing chain ends in raw instead of qcow2.
Rearranging fields in a declaration is minor, so you are still okay adding:

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 16/24] tests: qemumonitorjson: Simplify node name detection test

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> We can now iterate the hash table and print all detected backing chains.
> This simplifies calling of the test cases.
> ---
>  .../qemumonitorjson-nodename-basic.result  |  1 +
>  tests/qemumonitorjsontest.c| 55 
> +-
>  2 files changed, 23 insertions(+), 33 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 15/24] tests: Extract mock library for making hash table deterministic

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> virHashNew calls virRandomBits to initialize seed for the hashing
> function. If a test uses iteration through the hash table to produce
> results they may/will be non-deterministic. Extract the mock library
> which was used for mac address mapping to be universal.
> ---
>  tests/Makefile.am | 17 +
>  tests/{virmacmapmock.c => virdeterministichashmock.c} |  0
>  tests/virmacmaptest.c |  2 +-
>  3 files changed, 10 insertions(+), 9 deletions(-)
>  rename tests/{virmacmapmock.c => virdeterministichashmock.c} (100%)
> 

Long name, but works for me.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 14/24] qemu: block: Refactor node name detection code

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Remove the complex and unreliable code which inferred the node name
> hierarchy only from data returned by 'query-named-block-nodes'. It turns
> out that query-blockstats contain the full hierarchy of nodes as
> perceived by qemu so the inference code is not necessary.
> 
> In query blockstats, the 'parent' object corresponds to the storage
> behind a storage volume and 'backing' corresponds to the lower level of
> backing chain. Since all have node names this data can be really easily
> used to detect node names.
> 
> In addition to the code refactoring the one remaining test case needed
> to be fixed along.

The diff is hard to read given how much changed; it's easier to just
focus on the code additions to see if the new stuff makes sense,
ignoring what used to be there.

> ---
>  src/qemu/qemu_block.c  | 290 
> -
>  src/qemu/qemu_block.h  |  10 +-
>  .../qemumonitorjson-nodename-basic-blockstats.json | 166 
>  ...qemumonitorjson-nodename-basic-named-nodes.json |  18 +-
>  .../qemumonitorjson-nodename-basic.result  |  12 +-
>  tests/qemumonitorjsontest.c|  22 +-
>  6 files changed, 313 insertions(+), 205 deletions(-)
>  create mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-basic-blockstats.json
> 

> -
> +/* list of driver names of layers that qemu automatically adds into the
> + * backing chain */
> +static const char *qemuBlockDriversBlockjob[] = {
> +"mirror_top", "commit_top", NULL };

This list may change over time; also, qemu 2.10 will hide these implicit
nodes (or did 2.9 expose them, so we are stuck dealing with them?)

> +
> +/* qemu 2.9 reports layers in the backing chain which don't 
> correspond
> + * to files. skip them */
> +if (qemuBlockDriverMatch(drvname, qemuBlockDriversBlockjob)) {

And that answers my question.


> +qemuBlockNodeNameGetBackingChainDisk(size_t pos ATTRIBUTE_UNUSED,
> + virJSONValuePtr item,
> + void *opaque)

> +if (qemuBlockNodeNameGetBackingChainBacking(item, data->nodenamestable,
> +&devicedata) < 0)

Sounds like we're stuttering on Backing; but I guess it makes sense (we
are doing pieces of 'GetBackingChain', with one piece getting the disk
and the other getting the next backing level).


>  /**
>   * qemuBlockNodeNameGetBackingChain:
> - * @json: JSON array of data returned from 'query-named-block-nodes'
> + * @namednodes: JSON array of data returned from 'query-named-block-nodes'
> + * @blockstats: JSON array of data returned from 'query-blockstats'

So we really have to query two pieces of information, then thread them
together (but at least the threading is easy to do, now that qemu
auto-names every node); definitely better than our old code of
reconstructing the threading by filename guesses.  But it also goes to
show why Kevin is working on a new query-block-something for 2.11 that
will be less painful; so while we'll have to keep this cruft for as long
as we support 2.9/2.10, we'll want to give feedback to Kevin's
improvements that will let us rebuild things in less time.  Or even
better, we reach the point where we assign all node names ourselves, and
don't even have to do reconstructive queries.  But that's for down the
road; now back to reviewing this patch.

> +++ 
> b/tests/qemumonitorjsondata/qemumonitorjson-nodename-basic-named-nodes.json
> @@ -21,7 +21,7 @@
>},
>"iops_wr": 0,
>"ro": true,
> -  "node-name": "#block567",
> +  "node-name": "#block558",

Trivia: qemu randomizes the last two digits (so you HAVE to query rather
than making hard-coded assumptions about what node name will be
auto-assigned); but the leading digits are merely a counter of how many
nodes qemu previously had to name (this node thus had 5 preceding nodes)
- so having the same #block5XX means you created your new trace using
the same steps of node creation as the old trace ;)

> -DO_TEST_BLOCK_NODE_DETECT("basic", "#block118");
> +DO_TEST_BLOCK_NODE_DETECT("basic", "drive-virtio-disk0");

The real payout of your new scheme: you get to start your query from a
block device name (under libvirt control) rather than from an
intermediate generated node name!

I didn't spot anything blatantly wrong, and the new code DOES seem less
convoluted (yeah, we have to bounce between two arrays to piece things
together, but we aren't reconstructing threading by hand), and, as the
commit message says, you're now able to handle a case that previously
failed without the use of query-blockstats.  I'd still like to test
things once I've gone over the whole series, but if I get through my
review to 24/24 without any further red flags, you can add:

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.  

[libvirt] [PATCH v4] qemu: Check for existence of provided *_tls_x509_cert_dir

[John Ferlan]

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

Introduce virQEMUDriverConfigTLSDirResetDefaults in order to check
if the defaultTLSx509certdir was changed, then change the default
for any other *TLSx509certdir that was not set to the default default.

Introduce virQEMUDriverConfigValidate to validate the existence of
any of the *_tls_x509_cert_dir values that were uncommented/set,
incuding the default.

Update the qemu.conf description for default to describe the consequences
if the default directory path does not exist.

Signed-off-by: John Ferlan 
---
 v3: https://www.redhat.com/archives/libvir-list/2017-July/msg00915.html

 Changes since v3 - rework even more based on code review.

 src/qemu/qemu.conf |   8 
 src/qemu/qemu_conf.c   | 105 -
 src/qemu/qemu_conf.h   |   4 ++
 src/qemu/qemu_driver.c |   3 ++
 4 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 1d81472..f977e3b 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -13,6 +13,14 @@
 #
 #  dh-params.pem - the DH params configuration file
 #
+# If the directory does not exist or contain the necessary files, QEMU
+# domains will fail to start if they are configured to use TLS.
+#
+# In order to overwrite the default path alter the following. This path
+# definition will be used as the default path for other *_tls_x509_cert_dir
+# configuration settings if their default path does not exist or is not
+# specifically set.
+#
 #default_tls_x509_cert_dir = "/etc/pki/qemu"
 
 
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index c4714ed..1a4a998 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -425,6 +425,43 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
 }
 
 
+/**
+ * @cfg: Just read config TLS values
+ *
+ * If the default_tls_x509_cert_dir was uncommented or changed from
+ * the default value assigned to the *_tls_x509_cert_dir values when
+ * virQEMUDriverConfigNew was executed, we need to check if we need
+ * to update the other defaults.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+static int
+virQEMUDriverConfigTLSDirResetDefaults(virQEMUDriverConfigPtr cfg)
+{
+/* Not changed or set to the default default, nothing to do */
+if (!cfg->checkdefaultTLSx509certdir ||
+STREQ(cfg->defaultTLSx509certdir, SYSCONFDIR "/pki/qemu"))
+return 0;
+
+#define CHECK_RESET_CERT_DIR_DEFAULT(val) \
+do {  \
+if (STREQ(cfg->val ## TLSx509certdir, SYSCONFDIR "/pki/qemu")) {  \
+VIR_FREE(cfg->val ## TLSx509certdir); \
+if (VIR_STRDUP(cfg->val ## TLSx509certdir,\
+   cfg->defaultTLSx509certdir) < 0)   \
+return -1;\
+} \
+} while (0)
+
+CHECK_RESET_CERT_DIR_DEFAULT(vnc);
+CHECK_RESET_CERT_DIR_DEFAULT(spice);
+CHECK_RESET_CERT_DIR_DEFAULT(chardev);
+CHECK_RESET_CERT_DIR_DEFAULT(migrate);
+
+return 0;
+}
+
+
 int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 const char *filename,
 bool privileged)
@@ -452,8 +489,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 if (!(conf = virConfReadFile(filename, 0)))
 goto cleanup;
 
-if (virConfGetValueString(conf, "default_tls_x509_cert_dir", 
&cfg->defaultTLSx509certdir) < 0)
+if ((rv = virConfGetValueString(conf, "default_tls_x509_cert_dir", 
&cfg->defaultTLSx509certdir)) < 0)
 goto cleanup;
+cfg->checkdefaultTLSx509certdir = (rv == 1);
 if (virConfGetValueBool(conf, "default_tls_x509_verify", 
&cfg->defaultTLSx509verify) < 0)
 goto cleanup;
 if (virConfGetValueString(conf, "default_tls_x509_secret_uuid",
@@ -549,6 +587,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 
 #undef GET_CONFIG_TLS_CERTINFO
 
+if (virQEMUDriverConfigTLSDirResetDefaults(cfg) < 0)
+goto cleanup;
+
 if (virConfGetValueUInt(conf, "remote_websocket_port_min", 
&cfg->webSocketPortMin) < 0)
 goto cleanup;
 if (cfg->webSocketPortMin < QEMU_WEBSOCKET_PORT_MIN) {
@@ -873,6 +914,68 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 return ret;
 }
 
+
+/**
+ * @cfg: Recently read config values
+ *
+ * Validate the recently read configuration values.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
+{
+/* If the default entry was uncommented, then validate existence */
+if (cfg->checkdefaultTLSx509certdir) {
+if (!virFileExists(cfg->defaultTLSx509certdir)) {
+virReportError(VIR_ERR_CONF_SYNTAX,
+  

Re: [libvirt] [PATCH 13/24] util: storagefile: rename 'nodebacking' to 'nodestorage' in virStorageSource

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Make it less confusing by naming the field which refers to the storage
> object as 'nodestorage'.
> ---
>  src/qemu/qemu_block.c | 10 +-
>  src/qemu/qemu_driver.c| 10 +-
>  src/util/virstoragefile.c |  6 +++---
>  src/util/virstoragefile.h |  2 +-
>  4 files changed, 14 insertions(+), 14 deletions(-)
> 

Yes, that's better.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 12/24] qemu: block: Rename qemuBlockFillNodeData and move it to the top

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> The same operation will become useful in other places so rename the
> function to be more generic and move it to the top so that it can be
> reused earlier in the file.
> ---
>  src/qemu/qemu_block.c | 38 +++---
>  1 file changed, 19 insertions(+), 19 deletions(-)

> 
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 22de70657..3afcbde94 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -27,6 +27,24 @@
>  #define VIR_FROM_THIS VIR_FROM_QEMU
> 
> @@ -419,7 +419,7 @@ qemuBlockGetNodeData(virJSONValuePtr data)
>  if (!(ret = virHashCreate(50, virJSONValueHashFree)))
>  return NULL;
> 
> -if (virJSONValueArrayForeachSteal(data, qemuBlockFillNodeData, ret) < 0)
> +if (virJSONValueArrayForeachSteal(data, qemuBlockNamedNodesArrayToHash, 
> ret) < 0)

Long line; worth wrapping?

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] Plans for next release

[Peter Krempa]

On Tue, Jul 25, 2017 at 23:47:12 +0800, Daniel Veillard wrote:
>   The end of the month is next week, so if we want to release around
> Aug 1st, I think we should enter freeze on Thursday morning, then
> I could make rc2 on Saturday and we could have the release on the Tuesday
> or Wed.
>   Usually this is not the busiest time of the year but if this is a problem
> please raise this,

I've messed up linking of the apparmor helper program. Fix is here:

https://www.redhat.com/archives/libvir-list/2017-July/msg01197.html

Since it's coupled with a fairly invasive cleanup I might need to respin
because of that.


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

[libvirt] [PATCH RFC 1/2] make: Drop building without driver modules

[Peter Krempa]

Driver modules proved to be reliable for a long time. Since support for
not building modules complicates the code and makefiles drop the support
for not building drivers as modules.
---

Notes:
This was suggested a while ago by Dan:

https://www.redhat.com/archives/libvir-list/2017-March/msg00917.html

I actually did not try to build this on windows, since I don't have the
environment ready (do we actually even build the daemon on windows?).

It simplifies the next patch so I thought it may be the right time to do 
this.

 daemon/Makefile.am|  57 ---
 daemon/libvirtd.c |  54 +--
 m4/virt-driver-modules.m4 |  24 +++
 src/Makefile.am   | 158 --
 src/driver.c  |   8 +--
 src/storage/storage_backend.c |  11 +--
 src/vbox/vbox_driver.c|   2 +-
 src/vbox/vbox_driver.h|   6 +-
 tests/Makefile.am |   6 --
 tests/testutils.c |   2 -
 tools/virsh.c |   3 -
 11 files changed, 21 insertions(+), 310 deletions(-)

diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index d02ab33bd..697778e56 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -208,63 +208,6 @@ libvirtd_LDADD += \
../src/libvirt_driver_remote.la \
$(NULL)

-if ! WITH_DRIVER_MODULES
-if WITH_QEMU
-libvirtd_LDADD += ../src/libvirt_driver_qemu.la
-if WITH_DTRACE_PROBES
-libvirtd_LDADD += ../src/libvirt_qemu_probes.lo
-endif WITH_DTRACE_PROBES
-endif WITH_QEMU
-
-if WITH_LXC
-libvirtd_LDADD += ../src/libvirt_driver_lxc.la
-endif WITH_LXC
-
-if WITH_XEN
-libvirtd_LDADD += ../src/libvirt_driver_xen.la
-endif WITH_XEN
-
-if WITH_LIBXL
-libvirtd_LDADD += ../src/libvirt_driver_libxl.la
-endif WITH_LIBXL
-
-if WITH_UML
-libvirtd_LDADD += ../src/libvirt_driver_uml.la
-endif WITH_UML
-
-if WITH_VBOX
-libvirtd_LDADD += ../src/libvirt_driver_vbox.la
-endif WITH_VBOX
-
-if WITH_VZ
-libvirtd_LDADD += ../src/libvirt_driver_vz.la
-endif WITH_VZ
-
-if WITH_STORAGE
-libvirtd_LDADD += ../src/libvirt_driver_storage.la
-endif WITH_STORAGE
-
-if WITH_NETWORK
-libvirtd_LDADD += ../src/libvirt_driver_network.la
-endif WITH_NETWORK
-
-if WITH_INTERFACE
-libvirtd_LDADD += ../src/libvirt_driver_interface.la
-endif WITH_INTERFACE
-
-if WITH_NODE_DEVICES
-libvirtd_LDADD += ../src/libvirt_driver_nodedev.la
-endif WITH_NODE_DEVICES
-
-if WITH_SECRETS
-libvirtd_LDADD += ../src/libvirt_driver_secret.la
-endif WITH_SECRETS
-
-if WITH_NWFILTER
-libvirtd_LDADD += ../src/libvirt_driver_nwfilter.la
-endif WITH_NWFILTER
-endif ! WITH_DRIVER_MODULES
-
 libvirtd_LDADD += ../src/libvirt.la

 if WITH_POLKIT
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index a55845873..7e5d7af69 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -60,52 +60,7 @@
 #include "virgettext.h"
 #include "util/virnetdevopenvswitch.h"

-#ifdef WITH_DRIVER_MODULES
-# include "driver.h"
-#else
-# ifdef WITH_QEMU
-#  include "qemu/qemu_driver.h"
-# endif
-# ifdef WITH_LXC
-#  include "lxc/lxc_driver.h"
-# endif
-# ifdef WITH_XEN
-#  include "xen/xen_driver.h"
-# endif
-# ifdef WITH_LIBXL
-#  include "libxl/libxl_driver.h"
-# endif
-# ifdef WITH_UML
-#  include "uml/uml_driver.h"
-# endif
-# ifdef WITH_VBOX
-#  include "vbox/vbox_driver.h"
-# endif
-# ifdef WITH_BHYVE
-#  include "bhyve/bhyve_driver.h"
-# endif
-# ifdef WITH_NETWORK
-#  include "network/bridge_driver.h"
-# endif
-# ifdef WITH_INTERFACE
-#  include "interface/interface_driver.h"
-# endif
-# ifdef WITH_STORAGE
-#  include "storage/storage_driver.h"
-# endif
-# ifdef WITH_NODE_DEVICES
-#  include "node_device/node_device_driver.h"
-# endif
-# ifdef WITH_SECRETS
-#  include "secret/secret_driver.h"
-# endif
-# ifdef WITH_NWFILTER
-#  include "nwfilter/nwfilter_driver.h"
-# endif
-#endif
-#ifdef WITH_VZ
-# include "vz/vz_driver.h"
-#endif
+#include "driver.h"

 #include "configmake.h"

@@ -341,13 +296,8 @@ static int daemonErrorLogFilter(virErrorPtr err, int 
priority)
 }


-#ifdef WITH_DRIVER_MODULES
-# define VIR_DAEMON_LOAD_MODULE(func, module) \
+#define VIR_DAEMON_LOAD_MODULE(func, module) \
 virDriverLoadModule(module, #func)
-#else
-# define VIR_DAEMON_LOAD_MODULE(func, module) \
-func()
-#endif
 static void daemonInitialize(void)
 {
 /*
diff --git a/m4/virt-driver-modules.m4 b/m4/virt-driver-modules.m4
index ba65c3ba6..8bf8ecf2b 100644
--- a/m4/virt-driver-modules.m4
+++ b/m4/virt-driver-modules.m4
@@ -19,7 +19,7 @@ dnl

 AC_DEFUN([LIBVIRT_ARG_DRIVER_MODULES], [
   LIBVIRT_ARG_WITH([DRIVER_MODULES], [build drivers as loadable modules],
-   [check])
+   [yes])
 ])

 AC_DEFUN([LIBVIRT_CHECK_DRIVER_MODULES], [
@@ -27,28 +27,26 @@ AC_DEFUN([LIBVIRT_CHECK_DRIVER_MODULES], [

   if test "$with_libvirtd" = "no" ; then
 with_driver_modules=no
+  else
+if test "$with_driver_modules" = "no"; then
+  AC_MSG_ERROR([B

[libvirt] [PATCH 0/2] Resolve linking issue of storage driver to apparmor helper

[Peter Krempa]

Also drop support for building without loadable modules.

Peter Krempa (2):
  make: Drop building without driver modules
  security: apparmor: load the storage driver dynamically

 daemon/Makefile.am|  57 ---
 daemon/libvirtd.c |  54 +-
 m4/virt-driver-modules.m4 |  24 +++
 src/Makefile.am   | 160 +-
 src/driver.c  |   8 +--
 src/security/virt-aa-helper.c |  10 +--
 src/storage/storage_backend.c |  11 +--
 src/vbox/vbox_driver.c|   2 +-
 src/vbox/vbox_driver.h|   6 +-
 tests/Makefile.am |   6 --
 tests/testutils.c |   2 -
 tools/virsh.c |   3 -
 12 files changed, 27 insertions(+), 316 deletions(-)

-- 
2.13.2

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

[libvirt] [PATCH 2/2] security: apparmor: load the storage driver dynamically

[Peter Krempa]

In commit 5e515b542d I've attempted to fix the inability to access
storage from the apparmor helper program by linking with the storage
driver. By linking with the .so the linker complains that it's not
portable. Fix this by loading the module dynamically as we are supposed
to do.
---

Notes:
This patch is possible even with the previous patch, but it would be 
slightly
more complex, since it would need the logic to determine whether to load the
module or just initialize it.

 src/Makefile.am   |  2 +-
 src/security/virt-aa-helper.c | 10 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 471be40d1..b8e875482 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -3124,7 +3124,7 @@ virt_aa_helper_LDADD =
\
libvirt.la  \
libvirt_conf.la \
libvirt_util.la \
-   libvirt_driver_storage.la   \
+   libvirt_driver_storage_impl.la  \
../gnulib/lib/libgnu.la
 if WITH_DTRACE_PROBES
 virt_aa_helper_LDADD += libvirt_probes.lo
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index a751d6deb..35dcb35bc 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -41,6 +41,7 @@
 #include "viralloc.h"
 #include "vircommand.h"
 #include "virlog.h"
+#include "driver.h"

 #include "security_driver.h"
 #include "security_apparmor.h"
@@ -56,7 +57,6 @@
 #include "virgettext.h"

 #include "storage/storage_source.h"
-#include "storage/storage_backend.h"

 #define VIR_FROM_THIS VIR_FROM_SECURITY

@@ -927,10 +927,10 @@ get_files(vahControl * ctl)
 goto cleanup;
 }

-if (virStorageBackendDriversRegister(false) < 0) {
-vah_error(ctl, 0, _("failed to register storage driver backend"));
-goto cleanup;
-}
+/* load the storage driver so that backing store can be accessed */
+#ifdef WITH_STORAGE
+virDriverLoadModule("storage", "storageRegister");
+#endif

 for (i = 0; i < ctl->def->ndisks; i++) {
 virDomainDiskDefPtr disk = ctl->def->disks[i];
-- 
2.13.2

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

[libvirt] [PATCH 06/12] docs: remove use of — entity

[Daniel P. Berrange]

A handful of places in the docs choose to use — instead
of '-' for no clear reason. Remove this inconsistency.

Signed-off-by: Daniel P. Berrange 
---
 docs/formatdomain.html.in  | 34 +-
 docs/formatnetwork.html.in |  2 +-
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fb22dd1db..0ed12ecb0 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6914,10 +6914,10 @@ qemu-kvm -net nic,model=? /dev/null
 QEMU and KVM support:
 
 
-   'i6300esb' — the recommended device,
+  'i6300esb' - the recommended device,
 emulating a PCI Intel 6300ESB 
-   'ib700' — emulating an ISA iBase IB700 
-   'diag288' — emulating an S390 DIAG288 device
+  'ib700' - emulating an ISA iBase IB700 
+  'diag288' - emulating an S390 DIAG288 device
 Since 1.2.17
 
   
@@ -6932,15 +6932,15 @@ qemu-kvm -net nic,model=? /dev/null
 QEMU and KVM support:
 
 
-  'reset' — default, forcefully reset the guest
-  'shutdown' — gracefully shutdown the guest
+  'reset' - default, forcefully reset the guest
+  'shutdown' - gracefully shutdown the guest
 (not recommended) 
-  'poweroff' — forcefully power off the guest
-  'pause' — pause the guest
-  'none' — do nothing
-  'dump' — automatically dump the guest
+  'poweroff' - forcefully power off the guest
+  'pause' - pause the guest
+  'none' - do nothing
+  'dump' - automatically dump the guest
 Since 0.8.7
-  'inject-nmi' — inject a non-maskable interrupt
+  'inject-nmi' - inject a non-maskable interrupt
 into the guest
 Since 1.2.17
 
@@ -7005,8 +7005,8 @@ qemu-kvm -net nic,model=? /dev/null
   the virtualization platform
 
 
-  'virtio' — default with QEMU/KVM
-  'xen' — default with Xen
+  'virtio' - default with QEMU/KVM
+  'xen' - default with Xen
 
   
   autodeflate
@@ -7078,7 +7078,7 @@ qemu-kvm -net nic,model=? /dev/null
   the virtualization platform:
 
 
-  'virtio' — supported by qemu and virtio-rng kernel 
module
+  'virtio' - supported by qemu and virtio-rng kernel module
 
   
   rate
@@ -7270,11 +7270,11 @@ qemu-kvm -net nic,model=? /dev/null
   is missing depends on the hypervisor and guest arch.
 
 
-  'isa' — for ISA pvpanic device
-  'pseries' — default and valid only for pSeries guests.
-  'hyperv' — for Hyper-V crash CPU feature.
+  'isa' - for ISA pvpanic device
+  'pseries' - default and valid only for pSeries guests.
+  'hyperv' - for Hyper-V crash CPU feature.
 Since 1.3.0, QEMU and KVM only
-  's390' — default for S390 guests.
+  's390' - default for S390 guests.
 Since 1.3.5
 
   
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index e8e618e42..9c07e5e24 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -997,7 +997,7 @@
 prefix attribute, which is an integer (for example,
 netmask='255.255.255.0' could also be given as
 prefix='24'). The family attribute is used
-to specify the type of address — ipv4 or
+to specify the type of address - ipv4 or
 ipv6; if no family is given,
 ipv4 is assumed. More than one address of each family can
 be defined for a network. The optional localPtr attribute
-- 
2.13.3

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

[libvirt] [PATCH 12/12] docs: add full set of "favicon" files to support modern clients

[Daniel P. Berrange]

Use of the relation "shortcut" for a favicon was an Internet
Explorer only feature. Other browsers just require "icon".

The new icons & metadata are generated using

  https://realfavicongenerator.net/

which is user tested to work well across all modern clients

Signed-off-by: Daniel P. Berrange 
---
 docs/android-chrome-192x192.png | Bin 0 -> 13057 bytes
 docs/android-chrome-256x256.png | Bin 0 -> 16597 bytes
 docs/apple-touch-icon.png   | Bin 0 -> 10489 bytes
 docs/browserconfig.xml  |   9 +
 docs/favicon-16x16.png  | Bin 0 -> 872 bytes
 docs/favicon-32x32.png  | Bin 0 -> 1793 bytes
 docs/favicon.ico| Bin 0 -> 15086 bytes
 docs/manifest.json  |  18 ++
 docs/mstile-150x150.png | Bin 0 -> 7579 bytes
 docs/page.xsl   |   6 +-
 10 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 docs/android-chrome-192x192.png
 create mode 100644 docs/android-chrome-256x256.png
 create mode 100644 docs/apple-touch-icon.png
 create mode 100644 docs/browserconfig.xml
 create mode 100644 docs/favicon-16x16.png
 create mode 100644 docs/favicon-32x32.png
 create mode 100644 docs/favicon.ico
 create mode 100644 docs/manifest.json
 create mode 100644 docs/mstile-150x150.png

diff --git a/docs/android-chrome-192x192.png b/docs/android-chrome-192x192.png
new file mode 100644
index 
..2edadf93f1f270a9ed4d2c01e9f71847078b8409
GIT binary patch
literal 13057
zcmX|n19W9guG2w~r6Wg|JYm%ARwmGr&=D&B{*K60VuCJ@wtJmJu
z1yWLwM1seM2LJ#_(o$k7|9Jm@1Pk%+Z4REz@sB}T3d;)v01a^n@5WI7WH1*MNfAKp
z6yfPV0nSlM+vT6}@_z&-LX+hI06+kgHQ!0|1%kxnXz$k5Rg#r{-JaXOunJf
z*tmFJK_H-Ch^>>Gt8eg6cV9Lx9%HMY=623X8oK|Abd4?KRW$Gjh-8)3`Gmx+99(JX
znYelRr4?0q1VnIf@nsZM)N~9r^-b8hxXo=H*u0zohQ_{OF<)O_|N7$>9(!_f!s_V|
z2#N<7o3MMk_=HBYxY+~rjm)g=FV4@6%&i%nZ6Khbd3bodLn4Et6S;!DEbN?ZUEITB
zlNmki=`4-(jLluW0|C1FacSAy0Ul9F8EH9%U#9>t4QD${I7#=>ot(mn0P(@9BV|%BrsRc|$_21-NU*7=a`GPYUC0CCCc>S}BuUVyMjV`~Q)IYm%#$kNIxH7%_ID9Ak^
zj8{x_YIdGLT9okHH_xIR0H1(|m$zuJw=2+%o{_PCda|Rl)85{m@&|`eatw!oHlL*_
zi;$pZT*MDq>ABTaKR-VVB_%7rVDEsCwm+jG6~$CyV%0>iHRa;oP7MzqKA^+c
z;{3)8L`GYafVzqRAJpk87@M!0PcA*Vvr=^jK|*8Auit2>HkvX&e@53;HWq>7S=oxZ
zc`AT}VSkuNGEs1&_FD0WB9joF&PsxTQiuim!Ypq=8uQ!
zuZQHHR%F|2u5mFA`-@n9Ws6n+Hj22bih?Qt7lQ~M0vQ|(hLp*S<2z-fJzs^eu7XZq!frKk8{_2Uye1HgzwmFkddkU7jpQ+i*a_p3
z=Pt3n<-2<8zIyX96nwK2d^G*PXa9>FUCF0&Wq%(fKO*kyUd!kLGz-AaLxz#!LNWNB
zGIB$j;>Su@|I=}9e{4)fR(5<&oMu=EocqEP9zaY<%xb8kqL-eg8Lz3OrFW~Y
zEmZ*Togs|tYf{)=U0hFMSj&maZu3Jt_1O0#ZK3&|IrKS%pMl7_u(`UqU8vUn8!jQC
z(Cm6?nax2WJOQfmcHf67WAKlLXyw^1$my}-eL!+GzlkVDP
zT8t2X@!7C5GWG0m#~S0BSo==dW&X7GL&%nk}VSlQ29{399l{o2HlO=$IK!%6*HBf^HtKqnF;TBDb1Bf
zVRnAauZgI>ipT5h91ZH%UOa7C%=g!9OOj1{H1Q{OZE2^#PWyb9>tkvOdLo6{*+^%U
zdh1LQD=3x3APK~^X0wNeWb%sB!c7L@;j`Gt&inG+`9ny
zbYXkLrtbIk{qUv&B)2-qgn}e34KMJSwrJPKEe^c2WiWnh0Vzv5FU;mbh_M2%pMFbf
zstHcabZqv^GWCmR0oS^($Hd36nK2~qcWAp+n68LE&^W>+N(<8&^kIcef{K3>HgnL2
zb%=oLJMU6c<;}wh(~oRP^H`Niy!nzIz@L
z-5~aQ282=MTXL1thO%|ZCQ&oRc{;P6F(U_HHzU^2xuMq~JZoCW5wXuk^fQq>RysgF
z7J8;S2IP5fotK%A*@XjF9#S4S{SyM;c~Czgl9Tk{3?NOgkb_X}q$7JKFw
zAd1pp)fZeSOCv`j60S0Wfi;pu$-s`HIJLI^<5OHf0J(MMsyOkem?gyHP|suhn`U)M
zeIoQQdrCb^t^%=9Sl3F6O4n(=yX?f{r$$wj!ne%=b<{8>Y|0YKG7Ae$4Gnd5b&YR-
z1d?ACL4(fe&yP3RO-CKLRd<~k(!8Rq8Iw`pHfxP&FhvHR90kIsA%^uL0HQ_e*J6tB
zRXY$IAt-79!^GU|%p7$fRb75JUtf1ONnK@S=Si=dqsiVG#{A4}R?SqZ<1;^F&`1%6
zt7(e1J>56z@kZNvc<`W_s=TfeO1kUs&q|Uwfq9vp#kX?80@BJ9@R6Nw%Ta5ApX(?C
zXJV6sRW%B7j)=!D;1D@5r+ywoa2N=mm!kN$p|XIg9e1Sap#5+*K}Rf-caq$DGe|*0
zQ>7%41wOssIR?!7S-N_WXaXM=&+JHZ4KQG=O_8vW8P_e&T34)wSHUmf#|)6*z$(QQsmA^NuvPxW;!p9;fFOJ9_-ZGk{reAkH42iV=7H!=
z(S4WpU<~^Lo4ahSgd15@8wPQrzp~10L3Qxri%rW&{xSg>B}pHfKo5)dcCd@!%jk(R
zjDuG-a+@K@X}%M{X9vq0_)sI{&AY)EM^F=Vd;}%BZ#|&UZ3*Y`ej5L|zla%USQQgl
zK6{_X%SEy!!-KmYrc&x!pT&Bxu~UQa%2|xDavIc6eb9jHJxds*o?lQOrn0@QKmq$4
zr=EnlBMC|G^?eQsNz~NT{o96}?~fPu-9wYDCXWl$@3{duC2Kg}Du$4EyVBzjkyNV-
z%*m}z%tpdj;K2OH8qV+|lkcGFwaCZ{{n9(R+>Vc0@8?744%XqfFCC4)iq?i9aSuMZ
z^7T(i&P;grBHrHx9?`pVo~89&UPUj&MELF<(C4Px4xN8UTYJ3Ar%DkgA+-n@Igf+o
zm|K7Pw4jw7Y~AbDWa%XX+`hiWBVPv0;yFv;dhoMWSwd`?^0HX6286K=x{-Zu%Ck-Q
zH&FcGrQCOgqSPFh{t?$#Ax{?((aHC4KNaJ~{_2T3)j~qAUB@4dvV?&;*kdCq%PAM&
z{&k(^NRHI8VwL3Gb0P$&0<43!4`U3)F2n|Tvn<%ZROzY|N?r
zSzSya>47unG!tmdM?jR8*Fn65Xd1e<~nfidI9zqye#c7CR4Wm7-=PW
z%(ySu*!TBMfT2=>G76*>%p9sz=)>N__|GiO_wUSqDj3}M_;0e
z4MfLUlmcpNk1XtqAq6v>;CMDZ1CRs2O<;CJJ@mW9HP$rn@If-zjtG!i)XqPVGp}?@
zY~T9LpVb|e??e|Ay2l;zPBdU*!zX)|^D_M2ZZ?X>G0y-T6~a(fVKcCZXSI5wl+O-E
z9@yxow;`O|nIq2{B;zu}1JF|XoE}<5Ly)j0NCHA`;BJGFf=52&%=ryD@kwR9KDNAQ
zy`!UZ!h%Lf_lElI4<~K@pU-VBsw8AF8g$eTxc;)Ty2Wu!_yv%cwQya)l)&iD-Crg?
z{R0??EA<{$Kj$MCC{g=OL~(?h9TvyCZJ=(ej4BljZd%>QvVE+dMI4QMEIOw>F3}Ws
zmZ4D-whBuQu%r9BbagTCN<>Xje!hE~Ermg&?CT@gGryxbc})K4P(G`vqOaCo_#L?G
z2<7^X^6Jz0y4TtIF17j5`z5rk5hH1&$rpFBl_WgsnFZb<`OuUjo
z-b-aO^v1nSOw5&7;xxEq%d=|}K^_iW1
zk2iKcc4^7YawF#hn)+C1a913im4g-Cv5W9`%?<35`2j6iv1$^ORHWTyfVQ21fXWg6EUjiD<3pW3p0K~}Z
z-$q7o#r_WMz_&exQ;3!z!PxW{Cnu998g$x
z>aR$6IEGQk>ZS4sSi2Hu|15Ly$i+O)ROILM_8|#&8kD2Hc6_OhX6CF0Q?Eg$@ho-!
zf?UBIqgdigfjAkP4a

[libvirt] [PATCH 04/12] docs: fix typo s/∧/&/

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange 
---
 docs/hooks.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/hooks.html.in b/docs/hooks.html.in
index 7a04ac198..6cc47a6c5 100644
--- a/docs/hooks.html.in
+++ b/docs/hooks.html.in
@@ -289,7 +289,7 @@
   Since 1.2.2, before a network is started,
 this script is called as:
   /etc/libvirt/hooks/network network_name start begin -
-  After the network is started, up ∧ running, the script is
+  After the network is started, up & running, the script is
 called as:
   /etc/libvirt/hooks/network network_name started begin 
-
   When a network is shut down, this script is called as:
-- 
2.13.3

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

[libvirt] [PATCH 09/12] docs: generate pretty indented HTML for API docs

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange 
---
 docs/newapi.xsl | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index a55736d32..a2f0e0714 100644
--- a/docs/newapi.xsl
+++ b/docs/newapi.xsl
@@ -814,6 +814,7 @@
 
   
 
@@ -828,6 +829,7 @@
   
 
   
-- 
2.13.3

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

[libvirt] [PATCH 11/12] docs: explicitly declare pages as being UTF-8 format

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange 
---
 docs/page.xsl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/page.xsl b/docs/page.xsl
index 4d49be085..d9be66b93 100644
--- a/docs/page.xsl
+++ b/docs/page.xsl
@@ -83,6 +83,7 @@
 Do not edit this file. Changes will be lost.
   
   
+
 
 
 libvirt: 
-- 
2.13.3

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

[libvirt] [PATCH 03/12] docs: make xmllint & xsltproc compulsory

[Daniel P. Berrange]

We already require libxml to be installed, so it is not unreasonable
to require xmllint and xsltproc to be installed too - any platform
with the former will have the latter too.

Signed-off-by: Daniel P. Berrange 
---
 docs/Makefile.am | 44 +++-
 m4/virt-external-programs.m4 | 12 ++--
 2 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index f478d9505..d6c9d0091 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -221,17 +221,14 @@ $(srcdir)/hvsupport.html.in: $(srcdir)/hvsupport.pl 
$(api_DATA) \
 news.html.in: \
  $(srcdir)/news.xml \
  $(srcdir)/news-html.xsl
-   $(AM_V_GEN) \
-   if [ -x $(XSLTPROC) ]; then \
- $(XSLTPROC) --nonet \
+   $(AM_V_GEN)$(XSLTPROC) --nonet \
$(srcdir)/news-html.xsl \
$(srcdir)/news.xml \
  >$@-tmp \
|| { rm -f $@-tmp; exit 1; }; \
  sed 's/ xmlns=""//g' $@-tmp >$@ \
|| { rm -f $@-tmp; exit 1; }; \
- rm -f $@-tmp; \
-   fi
+ rm -f $@-tmp
 EXTRA_DIST += \
$(srcdir)/news.xml \
$(srcdir)/news.rng \
@@ -244,9 +241,7 @@ MAINTAINERCLEANFILES += \
 
 %.html.tmp: %.html.in site.xsl subsite.xsl page.xsl \
$(acl_generated)
-   @if [ -x $(XSLTPROC) ] ; then \
- echo "Generating $@"; \
- name=`echo $@ | sed -e 's/.tmp//'`; \
+   $(AM_V_GEN)name=`echo $@ | sed -e 's/.tmp//'`; \
  dir=`dirname $@` ; \
  if test "$$dir" = "."; \
  then \
@@ -257,42 +252,33 @@ MAINTAINERCLEANFILES += \
  fi; \
  $(XSLTPROC) --stringparam pagename $$name --nonet \
$(top_srcdir)/docs/$$style $< > $@ \
-   || { rm $@ && exit 1; }; fi
+   || { rm $@ && exit 1; }
 
 %.html: %.html.tmp
-   @if test -x $(XMLLINT) ; then \
- echo "Validating $@" ; \
- $(XMLLINT) --nonet --format $< > $(srcdir)/$@ \
- || { rm $(srcdir)/$@ && exit 1; }; fi
+   $(AM_V_GEN)$(XMLLINT) --nonet --format $< > $(srcdir)/$@ \
+ || { rm $(srcdir)/$@ && exit 1; }
 
 %.php.tmp: %.php.in site.xsl page.xsl
-   @if [ -x $(XSLTPROC) ] ; then \
- echo "Generating $@"; \
- $(XSLTPROC) --stringparam pagename $(@:.tmp=) --nonet \
+   $(AM_V_GEN)$(XSLTPROC) --stringparam pagename $(@:.tmp=) --nonet \
$(top_srcdir)/docs/site.xsl $< > $@ \
-   || { rm $@ && exit 1; }; fi
+   || { rm $@ && exit 1; }
 
 %.php: %.php.tmp %.php.code.in
-   @if [ -x $(XSLTPROC) ] ; then \
- echo "Scripting $@"; \
-   sed -e '/<\/span>/r 
'"$(srcdir)/$@.code.in" \
+   $(AM_V_GEN)sed -e '/<\/span>/r 
'"$(srcdir)/$@.code.in" \
-e /php_placeholder/d < $@.tmp > $(srcdir)/$@ \
-   || { rm $(srcdir)/$@ && exit 1; }; fi
+   || { rm $(srcdir)/$@ && exit 1; }
 
 $(apihtml_generated): html/index.html
 
 html/index.html: libvirt-api.xml newapi.xsl page.xsl $(APIBUILD_STAMP)
-   $(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \
- $(XSLTPROC) --nonet -o $(srcdir)/ \
+   $(AM_V_GEN)$(XSLTPROC) --nonet -o $(srcdir)/ \
  --stringparam builddir '$(abs_top_builddir)' \
- $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml ; fi && \
-   if test -x $(XMLLINT) ; then \
- $(XMLLINT) --nonet --noout $(srcdir)/html/*.html ; fi
+ $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml && \
+ $(XMLLINT) --nonet --noout $(srcdir)/html/*.html
 
 $(addprefix $(srcdir)/,$(devhelphtml)): $(srcdir)/libvirt-api.xml $(devhelpxsl)
-   $(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \
- $(XSLTPROC) --nonet -o $(srcdir)/devhelp/ \
- $(top_srcdir)/docs/devhelp/devhelp.xsl $(srcdir)/libvirt-api.xml ; fi
+   $(AM_V_GEN)$(XSLTPROC) --nonet -o $(srcdir)/devhelp/ \
+ $(top_srcdir)/docs/devhelp/devhelp.xsl $(srcdir)/libvirt-api.xml
 
 
 python_generated_files = \
diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4
index 4a10c85ad..ab6149288 100644
--- a/m4/virt-external-programs.m4
+++ b/m4/virt-external-programs.m4
@@ -23,8 +23,16 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
   AM_CONDITIONAL([HAVE_RPCGEN], [test "x$ac_cv_path_RPCGEN" != "xno"])
 
   dnl Miscellaneous external programs.
-  AC_PATH_PROG([XMLLINT], [xmllint], [/usr/bin/xmllint])
-  AC_PATH_PROG([XSLTPROC], [xsltproc], [/usr/bin/xsltproc])
+  AC_PATH_PROG([XMLLINT], [xmllint], [])
+  if test -z "$XMLLINT"
+  then
+AC_MSG_ERROR("xmllint is required to build libvirt")
+  fi
+  AC_PATH_PROG([XSLTPROC], [xsltproc], [])
+  if test -z "$XSLTPROC"
+  then
+AC_MSG_ERROR("xsltproc is required to build libvirt")
+  fi
   AC_PATH_PROG([AUGPARSE], [augparse], [/usr/bin/augparse])
   AC_PROG_MKDIR_P
   AC_PROG_LN_S
-- 
2.13.3

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

[libvirt] [PATCH 02/12] docs: drop XHTML 1.0 validation of website

[Daniel P. Berrange]

The HTML pages are currently validated against an XHTML 1.0 DTD.
This makes it impossible to take advantage of features that are
introduced in HTML 5, because they'll fail validation.

There is intentionally no DTD defined for HTML 5, so there's no
alternative to XHTML 1.0 DTD that we could switch to. The only
options are to stick with XHTML 1.0 forever, or drop the DTD
validation, and we pick the latter.

Signed-off-by: Daniel P. Berrange 
---
 .travis.yml  |  1 -
 docs/Makefile.am | 18 +-
 libvirt.spec.in  |  1 -
 m4/virt-external-programs.m4 |  1 -
 4 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 5a3e76510..f05ba8454 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -38,7 +38,6 @@ addons:
   - libapparmor-dev
   - dnsmasq-base
   - librbd-dev
-  - w3c-dtd-xhtml
 
 notifications:
   irc:
diff --git a/docs/Makefile.am b/docs/Makefile.am
index e32758f4a..f478d9505 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -260,14 +260,10 @@ MAINTAINERCLEANFILES += \
|| { rm $@ && exit 1; }; fi
 
 %.html: %.html.tmp
-   @if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \
- if $(XMLCATALOG) '$(XML_CATALOG_FILE)' \
-   "-//W3C//DTD XHTML 1.0 Strict//EN" > /dev/null ; then \
+   @if test -x $(XMLLINT) ; then \
  echo "Validating $@" ; \
- SGML_CATALOG_FILES='$(XML_CATALOG_FILE)' \
- $(XMLLINT) --catalogs --nonet --format --valid $< > $(srcdir)/$@ \
- || { rm $(srcdir)/$@ && exit 1; }; \
- else echo "missing XHTML1 DTD"; cat $< > $(srcdir)/$@ ; fi ; fi
+ $(XMLLINT) --nonet --format $< > $(srcdir)/$@ \
+ || { rm $(srcdir)/$@ && exit 1; }; fi
 
 %.php.tmp: %.php.in site.xsl page.xsl
@if [ -x $(XSLTPROC) ] ; then \
@@ -290,12 +286,8 @@ html/index.html: libvirt-api.xml newapi.xsl page.xsl 
$(APIBUILD_STAMP)
  $(XSLTPROC) --nonet -o $(srcdir)/ \
  --stringparam builddir '$(abs_top_builddir)' \
  $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml ; fi && \
-   if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \
- if $(XMLCATALOG) '$(XML_CATALOG_FILE)' "-//W3C//DTD XHTML 1.0 
Strict//EN" \
-   > /dev/null ; then \
- SGML_CATALOG_FILES='$(XML_CATALOG_FILE)' \
- $(XMLLINT) --catalogs --nonet --valid --noout $(srcdir)/html/*.html ; 
\
- else echo "missing XHTML1 DTD"; cat $< > $(srcdir)/$@ ; fi ; fi
+   if test -x $(XMLLINT) ; then \
+ $(XMLLINT) --nonet --noout $(srcdir)/html/*.html ; fi
 
 $(addprefix $(srcdir)/,$(devhelphtml)): $(srcdir)/libvirt-api.xml $(devhelpxsl)
$(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \
diff --git a/libvirt.spec.in b/libvirt.spec.in
index b074bd171..d1cff4caf 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -301,7 +301,6 @@ BuildRequires: systemd-units
 BuildRequires: xen-devel
 %endif
 BuildRequires: libxml2-devel
-BuildRequires: xhtml1-dtds
 BuildRequires: libxslt
 BuildRequires: readline-devel
 BuildRequires: ncurses-devel
diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4
index f2f62f492..4a10c85ad 100644
--- a/m4/virt-external-programs.m4
+++ b/m4/virt-external-programs.m4
@@ -24,7 +24,6 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
 
   dnl Miscellaneous external programs.
   AC_PATH_PROG([XMLLINT], [xmllint], [/usr/bin/xmllint])
-  AC_PATH_PROG([XMLCATALOG], [xmlcatalog], [/usr/bin/xmlcatalog])
   AC_PATH_PROG([XSLTPROC], [xsltproc], [/usr/bin/xsltproc])
   AC_PATH_PROG([AUGPARSE], [augparse], [/usr/bin/augparse])
   AC_PROG_MKDIR_P
-- 
2.13.3

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

[libvirt] [PATCH 07/12] docs: use UTF-8 instead of HTML entities for decorated letters

[Daniel P. Berrange]

We have files which use HTML entities for decorating letters
with unlauts, accents, etc. Other files just use UTF-8
characters directly for this. Remove the HTML entities since
they have no benefit and use UTF-8 instead.

Signed-off-by: Daniel P. Berrange 
---
 docs/apps.html.in   | 2 +-
 docs/csharp.html.in | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index 760004715..e54ab5372 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -353,7 +353,7 @@
   
   http://honk.sigxcpu.org/projects/libvirt/#munin";>Munin
   
-The plugins provided by Guido Günther allow to monitor various 
things
+The plugins provided by Guido Günther allow to monitor various things
 like network and block I/O with
 http://munin.projects.linpro.no/";>Munin.
   
diff --git a/docs/csharp.html.in b/docs/csharp.html.in
index 722b02911..72746762a 100644
--- a/docs/csharp.html.in
+++ b/docs/csharp.html.in
@@ -115,7 +115,7 @@ git clone git://libvirt.org/libvirt-csharp.git
 
   The C# bindings are the work of Arnaud Champion
   arnaud.champion AT 
devatom.fr>,
-  based upon the previous work of Jaromír Červenka.
+  based upon the previous work of Jaromír Červenka.
 
 
 Test Configuration
-- 
2.13.3

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

[libvirt] [PATCH 10/12] docs: remove bogus 'shape' attribute on links

[Daniel P. Berrange]

The 'shape' attribute on  is used together with a 'coords'
attribute to create hot-zones in image maps. We're not using
image maps so our inclusion of a 'shape' attribute is bogus.
Furthermore this is forbidden in HTML5.

Signed-off-by: Daniel P. Berrange 
---
 docs/apps.html.in  |  4 ++--
 docs/format.html.in| 20 ++--
 docs/index.html.in | 20 ++--
 docs/news-2005.html.in |  4 ++--
 docs/news-2006.html.in |  4 ++--
 docs/news-2007.html.in |  4 ++--
 docs/news-2008.html.in |  4 ++--
 docs/news-2009.html.in |  4 ++--
 docs/news-2010.html.in |  4 ++--
 docs/news-2011.html.in |  4 ++--
 docs/news-2012.html.in |  4 ++--
 docs/news-2013.html.in |  4 ++--
 12 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index 2b91c4c03..1ced03c3d 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -365,10 +365,10 @@
 your Xen or QEMU/KVM guests, or to integrate with your existing Nagios
 installation.
   
-  http://www.pcp.io/man/man1/pmdalibvirt.1.html"; 
shape="rect">PCP
+  http://www.pcp.io/man/man1/pmdalibvirt.1.html";>PCP
   
 The PCP libvirt PMDA (plugin) is part of the
-http://pcp.io/"; shape="rect">PCP toolkit and provides
+http://pcp.io/";>PCP toolkit and provides
 hypervisor and guest information and complete set of guest performance
 metrics. It supports pCPU, vCPU, memory, block device, network 
interface,
 and performance event metrics for each virtual guest.
diff --git a/docs/format.html.in b/docs/format.html.in
index 11e0defb5..22b23e3fc 100644
--- a/docs/format.html.in
+++ b/docs/format.html.in
@@ -14,16 +14,16 @@
 
 
 
-  Domains
-  Networks
-  Network filtering
-  Storage
-  Storage 
encryption
-  Capabilities
-  Domain 
capabilities
-  Node devices
-  Secrets
-  Snapshots
+  Domains
+  Networks
+  Network filtering
+  Storage
+  Storage encryption
+  Capabilities
+  Domain capabilities
+  Node devices
+  Secrets
+  Snapshots
 
 
 Command line validation
diff --git a/docs/index.html.in b/docs/index.html.in
index 200f8eb10..8333cf6bb 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -59,16 +59,16 @@
 
 XML configuration
 Description of the XML schemas for
-  domains,
-  networks,
-  network filtering,
-  storage,
-  storage 
encryption,
-  capabilities,
-  domain capabilities,
-  node devices,
-  secrets,
-  snapshots
+  domains,
+  networks,
+  network filtering,
+  storage,
+  storage encryption,
+  capabilities,
+  domain capabilities,
+  node devices,
+  secrets,
+  snapshots
 http://wiki.libvirt.org";>Wiki
 Read further community contributed content
   
diff --git a/docs/news-2005.html.in b/docs/news-2005.html.in
index 6804cf2c9..44d8c4cc4 100644
--- a/docs/news-2005.html.in
+++ b/docs/news-2005.html.in
@@ -9,9 +9,9 @@
 Here is the list of official releases made during the year 2005.
 
 It is also possible to just use
-the GIT version or snapshot,
+the GIT version or snapshot,
 contact the mailing list and check
-the http://libvirt.org/git/?p=libvirt.git;a=log"; shape="rect">GIT 
log
+the http://libvirt.org/git/?p=libvirt.git;a=log";>GIT log
 to gauge progress.
 
 
diff --git a/docs/news-2006.html.in b/docs/news-2006.html.in
index 558f55a98..6556d4fed 100644
--- a/docs/news-2006.html.in
+++ b/docs/news-2006.html.in
@@ -10,9 +10,9 @@
 A similar list for 2005 is also available.
 
 It is also possible to just use
-the GIT version or snapshot,
+the GIT version or snapshot,
 contact the mailing list and check
-the http://libvirt.org/git/?p=libvirt.git;a=log"; shape="rect">GIT 
log
+the http://libvirt.org/git/?p=libvirt.git;a=log";>GIT log
 to gauge progress.
 
 
diff --git a/docs/news-2007.html.in b/docs/news-2007.html.in
index 46d8457cb..c12449abf 100644
--- a/docs/news-2007.html.in
+++ b/docs/news-2007.html.in
@@ -10,9 +10,9 @@
 A similar list for 2006 is also available.
 
 It is also possible to just use
-the GIT version or snapshot,
+the GIT version or snapshot,
 contact the mailing list and check
-the http://libvirt.org/git/?p=libvirt.git;a=log"; shape="rect">GIT 
log
+the http://libvirt.org/git/?p=libvirt.git;a=log";>GIT log
 to gauge progress.
 
 
diff --git a/docs/news-2008.html.in b/docs/news-2008.html.in
index 8081b82f1..aefbf3873 100644
--- a/docs/news-2008.html.in
+++ b/docs/news-2008.html.in
@@ -10,9 +10,9 @@
 A similar list for 2007 is also available.
 
 It is also possible to just use
-the GIT version or snapshot,
+the GIT version or snapshot,
 contact the mailing list and check
-  

[libvirt] [PATCH 05/12] docs: remove use of in docs

[Daniel P. Berrange]

Some docs pages were using   to add arbitrary whitespace
in the page. This is something that should be done by CSS if needed,
but it is not needed here, so delete it.

There was also use of   which adds no value at all
when we have CSS to prettify tables.

Signed-off-by: Daniel P. Berrange 
---
 docs/csharp.html.in  | 612 +++
 docs/virshcmdref.html.in |   6 -
 2 files changed, 300 insertions(+), 318 deletions(-)

diff --git a/docs/csharp.html.in b/docs/csharp.html.in
index e1c0fefba..722b02911 100644
--- a/docs/csharp.html.in
+++ b/docs/csharp.html.in
@@ -19,8 +19,6 @@
   to libvirt.
 
 
- 
-
 Requirements
 
 
@@ -31,8 +29,6 @@
   compiling libvirt for windows.
 
 
- 
-
 

[libvirt] [PATCH 00/12] Cleanup website generation & add favicons

[Daniel P. Berrange]

This started as an attempt to add modern favicon support to
the website. This requires use of HTML5 only syntax, which
lead to the massive cleanup to stop using XHTML 1.0, which
forms all of this series except the last patch

Daniel P. Berrange (12):
  docs: switch to using 'id' attribute instead of 'name' for links
  docs: drop XHTML 1.0 validation of website
  docs: make xmllint & xsltproc compulsory
  docs: fix typo s/∧/&/
  docs: remove use of   in docs
  docs: remove use of — entity
  docs: use UTF-8 instead of HTML entities for decorated letters
  docs: switch to using HTML5 doctype declaration
  docs: generate pretty indented HTML for API docs
  docs: remove bogus 'shape' attribute on links
  docs: explicitly declare pages as being UTF-8 format
  docs: add full set of "favicon" files to support modern clients

 .travis.yml  |   1 -
 docs/404.html.in |   2 +-
 docs/Makefile.am |  52 +--
 docs/acl.html.in |   8 +-
 docs/aclpolkit.html.in   |  34 +-
 docs/android-chrome-192x192.png  | Bin 0 -> 13057 bytes
 docs/android-chrome-256x256.png  | Bin 0 -> 16597 bytes
 docs/api.html.in |  10 +-
 docs/api_extension.html.in   |  14 +-
 docs/apple-touch-icon.png| Bin 0 -> 10489 bytes
 docs/apps.html.in|  38 +--
 docs/architecture.html.in|   8 +-
 docs/auditlog.html.in|  40 +--
 docs/auth.html.in|  16 +-
 docs/bindings.html.in|   2 +-
 docs/browserconfig.xml   |   9 +
 docs/bugs.html.in|  12 +-
 docs/cgroups.html.in |  26 +-
 docs/compiling.html.in   |   8 +-
 docs/contact.html.in |   8 +-
 docs/contribute.html.in  |  12 +-
 docs/csharp.html.in  | 634 +--
 docs/devguide.html.in|   2 +-
 docs/docs.html.in|   2 +-
 docs/downloads.html.in   |  14 +-
 docs/drivers.html.in |   6 +-
 docs/drvbhyve.html.in|  22 +-
 docs/drvesx.html.in  |  46 +--
 docs/drvhyperv.html.in   |  12 +-
 docs/drvlxc.html.in  |  54 +--
 docs/drvnodedev.html.in  |  10 +-
 docs/drvopenvz.html.in   |  14 +-
 docs/drvphyp.html.in |   8 +-
 docs/drvqemu.html.in |  34 +-
 docs/drvremote.html.in   |   2 +-
 docs/drvtest.html.in |   2 +-
 docs/drvuml.html.in  |   4 +-
 docs/drvvbox.html.in |   6 +-
 docs/drvvirtuozzo.html.in|   8 +-
 docs/drvvmware.html.in   |   6 +-
 docs/drvxen.html.in  |  16 +-
 docs/errors.html.in  |   2 +-
 docs/favicon-16x16.png   | Bin 0 -> 872 bytes
 docs/favicon-32x32.png   | Bin 0 -> 1793 bytes
 docs/favicon.ico | Bin 0 -> 15086 bytes
 docs/firewall.html.in|   8 +-
 docs/format.html.in  |  22 +-
 docs/formatcaps.html.in  |  10 +-
 docs/formatdomain.html.in| 228 ++---
 docs/formatdomaincaps.html.in|  26 +-
 docs/formatnetwork.html.in   |  36 +-
 docs/formatnode.html.in  |   6 +-
 docs/formatnwfilter.html.in  |  70 ++--
 docs/formatsecret.html.in|  12 +-
 docs/formatsnapshot.html.in  |   6 +-
 docs/formatstorage.html.in   |  30 +-
 docs/formatstorageencryption.html.in |  12 +-
 docs/genaclperms.pl  |   2 +-
 docs/goals.html.in   |   2 +-
 docs/governance.html.in  |  14 +-
 docs/hacking.html.in |  42 +--
 docs/hooks.html.in   |  34 +-
 docs/hvsupport.pl|   2 +-
 docs/index.html.in   |  22 +-
 docs/internals.html.in   |   2 +-
 docs/internals/command.html.in   |  34 +-
 docs/internals/eventloop.html.in |   8 +-
 docs/internals/locking.html.in   |  18 +-
 docs/internals/oomtesting.html.in|  10 +-
 docs/internals/rpc.html.in   |  44 +--
 docs/java.html.in|   2 +-
 docs/locking-lockd.html.in   |  10 +-
 docs/locking-sanlock.html.in |  12 +-
 docs/locking.html.in |   4 +-
 docs/logging.html.in |  16 +-
 docs/manifest.json   |  18 +
 docs/migration.html.in   |  38 +--
 docs/mstile-150x150.png  | Bin 0 -> 7579 bytes
 docs/newapi.xsl  |  19 +-
 docs/news-2005.html.in   |   6 +-
 docs/news-2006.html.in   |   6 +-
 docs/news-2007.html.in   |   6 +-
 docs/news-2008.html.in   |   6 +-
 docs/news-2009.html.in   |   6 +-
 docs/news-2010.html.in   |   6 +-
 docs/news-2011.html.in   

Re: [libvirt] [RFC]Add new mdev interface for QoS

[Alex Williamson]

[cc +libvir-list]

On Wed, 26 Jul 2017 21:16:59 +0800
"Gao, Ping A"  wrote:

> The vfio-mdev provide the capability to let different guest share the
> same physical device through mediate sharing, as result it bring a
> requirement about how to control the device sharing, we need a QoS
> related interface for mdev to management virtual device resource.
> 
> E.g. In practical use, vGPUs assigned to different quests almost has
> different performance requirements, some guests may need higher priority
> for real time usage, some other may need more portion of the GPU
> resource to get higher 3D performance, corresponding we can define some
> interfaces like weight/cap for overall budget control, priority for
> single submission control.
> 
> So I suggest to add some common attributes which are vendor agnostic in
> mdev core sysfs for QoS purpose.

I think what you're asking for is just some standardization of a QoS
attribute_group which a vendor can optionally include within the
existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
transparently enable this, but it really only provides the standard,
all of the support code is left for the vendor.  I'm fine with that,
but of course the trouble with and sort of standardization is arriving
at an agreed upon standard.  Are there QoS knobs that are generic
across any mdev device type?  Are there others that are more specific
to vGPU?  Are there existing examples of this that we can steal their
specification?

Also, mdev devices are not necessarily the exclusive users of the
hardware, we can have a native user such as a local X client.  They're
not an mdev user, so we can't support them via the mdev_attr_group.
Does there need to be a per mdev parent QoS attribute_group standard
for somehow defining the QoS of all the child mdev devices, or perhaps
representing the remaining host QoS attributes?

Ultimately libvirt and upper level management tools would be the
consumer of these control knobs, so let's immediately get libvirt
involved in the discussion.  Thanks,

Alex

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

Re: [libvirt] [PATCH 2/2] conf: Introduce the virDomainNetModel enumeration

[Peter Krempa]

On Wed, Jul 26, 2017 at 14:20:05 +0200, Andrea Bolognani wrote:
> Up until now, we have stored the model name for network
> interfaces as raw strings in virDomainNetDef: this is
> suboptimal for a number of reasons, such as having to
> perform relatively expensive string allocations and
> comparisons all the time and not giving the compiler
> the opportunity to have our back in certain situations.
> 
> Replace the strings with an enumeration similar to the
> ones we already use for pretty much everything else.
> 
> Signed-off-by: Andrea Bolognani 
> ---
> Most drivers already performed pretty strict validation
> of the model name, so there should be no problems there;
> the QEMU driver, however, though it would be a good idea
> to just accept any string that possibly kinda resembled
> a valid model name.
> 
> The models I've included in the enumeration are those
> that were already referenced somewhere else in libvirt,
> but there's no guarantee that other model names are not
> used in the wild.
> 
> One possibility would be to also add (a subset of) all
> models QEMU ever supported, even though some of them
> might have never been used, just to be safe; on the
> other side of the spectrum, we could go with the minimal
> set and possibly add more if breakages are reported.

I don't think that upstream would ever consider the last option.

> @@ -10330,18 +10349,20 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr 
> xmlopt,
>   * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio
>   * QEMU PPC64 supports spapr-vlan
>   */
> -if (model != NULL) {
> -if (strspn(model, NET_MODEL_CHARS) < strlen(model)) {
> -virReportError(VIR_ERR_INVALID_ARG, "%s",
> -   _("Model name contains invalid characters"));
> +if (model) {
> +if ((val = virDomainNetModelTypeFromString(model)) < 0 ||
> +val == VIR_DOMAIN_NET_MODEL_NONE) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("Unknown interface  "
> + "has been specified"),
> +   model);
>  goto error;

So this is the infamous case, when we will drop configs which were
previously accepted. I'm not entirely persuaded that this is a great
idea, since we usually try to avoid this at all costs.

That's most probably also the reason why nobody bothered to change it
yet.

I think you'll need to store the original string in case when it can't
be parsed and use that one and accept such definition even in case when
the model is unknown.

Thanks to the validation callback you can then make sure that drivers
accept only values which can be parsed and users still have an option to
edit the definition and replace it.

Losing it is not acceptable.

NACK to full removal of the string.


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

Re: [libvirt] [PATCH 11/24] tests: qemumontitorjson: temporarily disable node name detection tests

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> To simplify the refactoring patches disable the tests. This will allow
> to add test data later.

s/to add/adding/ ("allow to VERB" is not idiomatic English; it's either
"allow WHO to VERB", or the shorter "allow VERBing")

> ---
>  tests/qemumonitorjsontest.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index 268c15f4b..c3e86419d 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -2930,9 +2930,9 @@ mymain(void)
>  } while (0)
> 
>  DO_TEST_BLOCK_NODE_DETECT("basic", "#block118");
> -DO_TEST_BLOCK_NODE_DETECT("same-backing", "#block170,#block574");
> -DO_TEST_BLOCK_NODE_DETECT("relative", "#block153,#block1177");
> -DO_TEST_BLOCK_NODE_DETECT("gluster", "#block1008");
> +/*DO_TEST_BLOCK_NODE_DETECT("same-backing", "#block170,#block574"); */
> +/*DO_TEST_BLOCK_NODE_DETECT("relative", "#block153,#block1177"); */
> +/*DO_TEST_BLOCK_NODE_DETECT("gluster", "#block1008"); */
> 
>  #undef DO_TEST_BLOCK_NODE_DETECT
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 10/24] tests: qemumonitorjson: simplify path handling in testBlockNodeNameDetect

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Extract the test prefix path into a variable and reuse
> virTestLoadFileJSON to load the sample json files rather than doing it
> manually.
> ---
>  tests/qemumonitorjsontest.c | 19 +--
>  1 file changed, 5 insertions(+), 14 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

[libvirt] [PATCH] build: Distribute tests/{qemucpumock.c, testutilshostcpus.h}

[Martin Kletzander]

Missed by 13554a9e7f4e.

Signed-off-by: Martin Kletzander 
---
Pushed under the build-breaker rule.

 tests/Makefile.am | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 562c1c77178e..11a380163177 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -560,7 +560,7 @@ libqemutestdriver_la_LDFLAGS = $(QEMULIB_LDFLAGS)
 libqemutestdriver_la_LIBADD = $(qemu_LDADDS)

 qemucpumock_la_SOURCES = \
-   qemucpumock.c
+   qemucpumock.c testutilshostcpus.h
 qemucpumock_la_CFLAGS = $(AM_CFLAGS)
 qemucpumock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
 qemucpumock_la_LIBADD = $(MOCKLIBS_LIBS)
@@ -674,7 +674,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c 
qemuargv2xmltest.c \
qemumonitorjsontest.c qemuhotplugtest.c \
qemuagenttest.c qemucapabilitiestest.c \
qemucaps2xmltest.c qemucommandutiltest.c \
-   qemumemlocktest.c \
+   qemumemlocktest.c qemucpumock.c testutilshostcpus.h \
$(QEMUMONITORTESTUTILS_SOURCES)
 endif ! WITH_QEMU

--
2.13.3

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

Re: [libvirt] [PATCH 07/24] tests: utils: Add virTestLoadFilePath helper

[Eric Blake]

On 07/26/2017 08:39 AM, Eric Blake wrote:
> On 07/26/2017 05:00 AM, Peter Krempa wrote:
>> This new helper loads and returns a file from 'abs_srcdir'. By using
>> variable arguments for the function, it's not necessary to format the
>> path separately in the test cases.
>> ---
>>  tests/testutils.c | 51 +++
>>  tests/testutils.h |  2 ++
>>  2 files changed, 53 insertions(+)
>>
> 
>> +
>> +/**
>> + * virTestLoadFilePath:
>> + * @...: file name components.
> 
> Mention that it must end in NULL...
> 
>> + *
>> + * Constructs the test file path from variable arguments and loads the file.
>> + * 'abs_srcdir' is automatically prepended.
>> + */
>> +char *
>> +virTestLoadFilePath(const char *p, ...)
> 
> and gcc has an attribute to mark vararg functions that require a NULL
> sentinel, to let the compiler enforce correct usage.

Looking back at the patch, I see you did use it, but that I missed it
because it was must later in the email:

> 
> --- a/tests/testutils.h
> +++ b/tests/testutils.h
> @@ -52,6 +52,8 @@ int virTestRun(const char *title,
> int (*body)(const void *data),
> const void *data);
>  int virTestLoadFile(const char *file, char **buf);
> +char *virTestLoadFilePath(const char *p, ...)
> +ATTRIBUTE_SENTINEL;

I like to use git's orderfile directive, so that my patches always list
.h changes first (when reviewing, it's nicer to see the interface
changes before the implementations); maybe libvirt should copy this idea
from qemu:
https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg06438.html

So I still think the comment should mention that the list must end in
NULL, but with that, you can add

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 09/24] tests: utils: Add virTestLoadFileJSON helper

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> This new helper loads, parses and returns a JSON file from 'abs_srcdir'
> By using variable arguments for the function, it's not necessary to
> format the path separately in the test cases.
> ---
>  tests/testutils.c | 34 ++
>  tests/testutils.h |  4 
>  2 files changed, 38 insertions(+)
> 
> diff --git a/tests/testutils.c b/tests/testutils.c
> index f193cdf8b..75f69e1c3 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -407,6 +407,40 @@ virTestLoadFilePath(const char *p, ...)
>  }
> 
> 
> +/**
> + * virTestLoadFileJSON:
> + * @...: name components

Same comment as in 7/24: document that it must end in NULL.  With that,

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 15/16] network: Modify naming for virNetworkObjList* fetching APIs

[Pavel Hrdina]

On Wed, Jul 26, 2017 at 10:57:30AM -0400, John Ferlan wrote:
> 
> 
> On 07/24/2017 07:49 AM, Pavel Hrdina wrote:
> > On Fri, May 19, 2017 at 09:03:23AM -0400, John Ferlan wrote:
> >> Use the structure names in the @data setup - makes it easier that going
> >> back to find the struct.
> >>
> >> Use the @maxnames instead of @nnames since that's what it is.
> > 
> > Please use camelCase -> @maxNames.
> > 
> 
> This I disagree with as maxnames is used *liberally* throughout many
> places in libvirt and in particular as arguments to functions. In
> particular, follow this back to :
> 
> 
> virDrvConnectListNetworks
> virDrvConnectListDefinedNetworks
> 
> and
> 
> virConnectListNetworks
> virConnectListDefinedNetworks
> 
> which all use @maxnames.
> 
> But I will separate and describe appropriately.

On the other hand there is for example maxMemory which uses camelCase.
I have to disagree with your disagreement :) but argument that something
is used in a certain way is not a good argument because the old usage
may be wrong.

> 
> >>
> >> Modify the @filter to be @aclfilter and change the typedef from
> >> virNetworkObjListFilter to virNetworkObjListACLFilter.
> > 
> > NACK to this change, even though it's used only to filter by ACLs, it
> > can be used to filter by anything.
> 
> Again, I disagree. I've been using @aclfilter in other drivers and
> taking this route makes things consistent.

Now that I've checked it, even virDomainObjListExport has the type
with ACL in it.  So I take my NACK back :).  Anyway, for the name I
would prefer to use aclFilter but since the ship has sailed I give up.
It can be done as a followup cleanup.  But please keep this in mind that
the camleCase is preferred.  Having no separation between words in the
variable name reduces readability.

Pavel

> Besides, look at a few of the vir*ObjListExport* type functions where
> there's actually a second filter that would take an @obj and a @flags
> argument and could be defined "generically" as "@filter".  Now if there
> was a "generic" ObjListExport routine, that @filter could be an element
> to a common structure too...
> 
> I will though separate it out.
> 
> Tks -
> 
> John
> 
> > 
> > This patch does three things in one, so it should be three separate
> > patches.  Since the last change is not correct split the remaining
> > changes into two patches.
> > 
> > Pavel
> > 


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

Re: [libvirt] [PATCH] docs: Add build timestamps to generated html/php pages

[Daniel P. Berrange]

On Wed, Jul 26, 2017 at 05:13:08PM +0200, Martin Kletzander wrote:
> In order not to make the build even less reproducible, honour
> SOURCE_DATE_EPOCH environment variable as specified:
> 
>   https://reproducible-builds.org/specs/source-date-epoch/
> 
> Signed-off-by: Martin Kletzander 
> ---
>  docs/Makefile.am | 13 +
>  docs/newapi.xsl  |  2 ++
>  docs/page.xsl|  4 
>  docs/site.xsl|  1 +
>  docs/subsite.xsl |  1 +
>  5 files changed, 17 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrange 



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] [PATCH] docs: Add build timestamps to generated html/php pages

[Martin Kletzander]

In order not to make the build even less reproducible, honour
SOURCE_DATE_EPOCH environment variable as specified:

  https://reproducible-builds.org/specs/source-date-epoch/

Signed-off-by: Martin Kletzander 
---
 docs/Makefile.am | 13 +
 docs/newapi.xsl  |  2 ++
 docs/page.xsl|  4 
 docs/site.xsl|  1 +
 docs/subsite.xsl |  1 +
 5 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index e32758f4aa36..67a945a334f3 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -196,6 +196,8 @@ MAINTAINERCLEANFILES = \
   $(addprefix $(srcdir)/,$(dot_php)) \
   $(srcdir)/hvsupport.html.in $(srcdir)/aclperms.htmlinc
 
+timestamp="$(shell if test -n "$$SOURCE_DATE_EPOCH"; then date 
--date="@$$SOURCE_DATE_EPOCH"; else date; fi)"
+
 all-am: web
 
 api: $(srcdir)/libvirt-api.xml $(srcdir)/libvirt-refs.xml
@@ -255,7 +257,8 @@ MAINTAINERCLEANFILES += \
$(MKDIR_P) $$dir; \
style=subsite.xsl; \
  fi; \
- $(XSLTPROC) --stringparam pagename $$name --nonet \
+ $(XSLTPROC) --stringparam pagename $$name \
+   --stringparam timestamp $(timestamp) --nonet \
$(top_srcdir)/docs/$$style $< > $@ \
|| { rm $@ && exit 1; }; fi
 
@@ -270,9 +273,10 @@ MAINTAINERCLEANFILES += \
  else echo "missing XHTML1 DTD"; cat $< > $(srcdir)/$@ ; fi ; fi
 
 %.php.tmp: %.php.in site.xsl page.xsl
-   @if [ -x $(XSLTPROC) ] ; then \
+   if [ -x $(XSLTPROC) ] ; then \
  echo "Generating $@"; \
- $(XSLTPROC) --stringparam pagename $(@:.tmp=) --nonet \
+ $(XSLTPROC) --stringparam pagename $(@:.tmp=) \
+   --stringparam timestamp $(timestamp) --nonet \
$(top_srcdir)/docs/site.xsl $< > $@ \
|| { rm $@ && exit 1; }; fi
 
@@ -289,6 +293,7 @@ html/index.html: libvirt-api.xml newapi.xsl page.xsl 
$(APIBUILD_STAMP)
$(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \
  $(XSLTPROC) --nonet -o $(srcdir)/ \
  --stringparam builddir '$(abs_top_builddir)' \
+ --stringparam timestamp $(timestamp) \
  $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml ; fi && \
if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \
  if $(XMLCATALOG) '$(XML_CATALOG_FILE)' "-//W3C//DTD XHTML 1.0 
Strict//EN" \
@@ -299,7 +304,7 @@ html/index.html: libvirt-api.xml newapi.xsl page.xsl 
$(APIBUILD_STAMP)
 
 $(addprefix $(srcdir)/,$(devhelphtml)): $(srcdir)/libvirt-api.xml $(devhelpxsl)
$(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \
- $(XSLTPROC) --nonet -o $(srcdir)/devhelp/ \
+ $(XSLTPROC) --stringparam timestamp $(timestamp) --nonet -o 
$(srcdir)/devhelp/ \
  $(top_srcdir)/docs/devhelp/devhelp.xsl $(srcdir)/libvirt-api.xml ; fi
 
 
diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index 9dd961507af4..d45bfc192d63 100644
--- a/docs/newapi.xsl
+++ b/docs/newapi.xsl
@@ -818,6 +818,7 @@
   doctype-system="http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd";>
   
 
+
   
 
 
@@ -834,6 +835,7 @@
 
doctype-system="http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd";>
 
   
+  
 
   
 
diff --git a/docs/page.xsl b/docs/page.xsl
index 1d662c670686..5d8e6e8263c3 100644
--- a/docs/page.xsl
+++ b/docs/page.xsl
@@ -75,11 +75,15 @@
   
   
 
+
 
   
 This file is autogenerated from .in
 Do not edit this file. Changes will be lost.
   
+  
+This page was generated at .
+  
   
 
 
diff --git a/docs/site.xsl b/docs/site.xsl
index ce354c32fb59..b84630e39029 100644
--- a/docs/site.xsl
+++ b/docs/site.xsl
@@ -29,6 +29,7 @@
   
 
   
+  
 
   
 
diff --git a/docs/subsite.xsl b/docs/subsite.xsl
index 108d0d83e509..6d2453f98e9c 100644
--- a/docs/subsite.xsl
+++ b/docs/subsite.xsl
@@ -19,6 +19,7 @@
   
 
   
+  
 
   
 
-- 
2.13.3

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

[libvirt] [PATCH v2 20/20] network: Privatize virNetworkObj

[John Ferlan]

Move from virnetworkobj.h into virnetworkobj.c

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c | 21 +
 src/conf/virnetworkobj.h | 20 
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index aab737b..b44f604 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -37,6 +37,27 @@ VIR_LOG_INIT("conf.virnetworkobj");
 /* currently, /sbin/tc implementation allows up to 16 bits for minor class 
size */
 #define CLASS_ID_BITMAP_SIZE (1<<16)
 
+struct _virNetworkObj {
+virObjectLockable parent;
+
+pid_t dnsmasqPid;
+pid_t radvdPid;
+bool active;
+int autostart;
+bool persistent;
+
+virNetworkDefPtr def; /* The current definition */
+virNetworkDefPtr newDef; /* New definition to activate at shutdown */
+
+virBitmapPtr classIdMap; /* bitmap of class IDs for QoS */
+unsigned long long floor_sum; /* sum of all 'floor'-s of attached NICs */
+
+unsigned int taint;
+
+/* Immutable pointer, self locking APIs */
+virMacMapPtr macmap;
+};
+
 struct _virNetworkObjList {
 virObjectLockable parent;
 
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index d7199fd..857228a 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -26,26 +26,6 @@
 
 typedef struct _virNetworkObj virNetworkObj;
 typedef virNetworkObj *virNetworkObjPtr;
-struct _virNetworkObj {
-virObjectLockable parent;
-
-pid_t dnsmasqPid;
-pid_t radvdPid;
-bool active;
-int autostart;
-bool persistent;
-
-virNetworkDefPtr def; /* The current definition */
-virNetworkDefPtr newDef; /* New definition to activate at shutdown */
-
-virBitmapPtr classIdMap; /* bitmap of class IDs for QoS */
-unsigned long long floor_sum; /* sum of all 'floor'-s of attached NICs */
-
-unsigned int taint;
-
-/* Immutable pointer, self locking APIs */
-virMacMapPtr macmap;
-};
 
 virNetworkObjPtr
 virNetworkObjNew(void);
-- 
2.9.4

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

[libvirt] [PATCH v2 17/20] network: Use @maxnames instead of @nnames

[John Ferlan]

To be consistent with the API definition, use the @maxnames instead
of @nnames when describing/comparing against the maximum names to
be provided for the *ConnectList[Defined]Networks APIs.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c| 10 +-
 src/conf/virnetworkobj.h|  2 +-
 src/network/bridge_driver.c |  8 
 src/test/test_driver.c  |  8 
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 43fc2cf..d288dd0 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1438,7 +1438,7 @@ struct virNetworkObjListGetHelperData {
 virConnectPtr conn;
 virNetworkObjListFilter filter;
 char **names;
-int nnames;
+int maxnames;
 bool active;
 int got;
 bool error;
@@ -1455,8 +1455,8 @@ virNetworkObjListGetHelper(void *payload,
 if (data->error)
 return 0;
 
-if (data->nnames >= 0 &&
-data->got == data->nnames)
+if (data->maxnames >= 0 &&
+data->got == data->maxnames)
 return 0;
 
 virObjectLock(obj);
@@ -1485,14 +1485,14 @@ int
 virNetworkObjListGetNames(virNetworkObjListPtr nets,
   bool active,
   char **names,
-  int nnames,
+  int maxnames,
   virNetworkObjListFilter filter,
   virConnectPtr conn)
 {
 int ret = -1;
 
 struct virNetworkObjListGetHelperData data = {
-conn, filter, names, nnames, active, 0, false};
+conn, filter, names, maxnames, active, 0, false};
 
 virObjectLock(nets);
 virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index 96f9eff..f7ed387 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -235,7 +235,7 @@ int
 virNetworkObjListGetNames(virNetworkObjListPtr nets,
   bool active,
   char **names,
-  int nnames,
+  int maxnames,
   virNetworkObjListFilter filter,
   virConnectPtr conn);
 
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 2433e4f..d2d983d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2945,7 +2945,7 @@ networkConnectNumOfNetworks(virConnectPtr conn)
 static int
 networkConnectListNetworks(virConnectPtr conn,
char **const names,
-   int nnames)
+   int maxnames)
 {
 virNetworkDriverStatePtr driver = networkGetDriver();
 int got = 0;
@@ -2954,7 +2954,7 @@ networkConnectListNetworks(virConnectPtr conn,
 return -1;
 
 got = virNetworkObjListGetNames(driver->networks,
-true, names, nnames,
+true, names, maxnames,
 virConnectListNetworksCheckACL,
 conn);
 
@@ -2983,7 +2983,7 @@ networkConnectNumOfDefinedNetworks(virConnectPtr conn)
 static int
 networkConnectListDefinedNetworks(virConnectPtr conn,
   char **const names,
-  int nnames)
+  int maxnames)
 {
 virNetworkDriverStatePtr driver = networkGetDriver();
 int got = 0;
@@ -2992,7 +2992,7 @@ networkConnectListDefinedNetworks(virConnectPtr conn,
 return -1;
 
 got = virNetworkObjListGetNames(driver->networks,
-false, names, nnames,
+false, names, maxnames,
 virConnectListDefinedNetworksCheckACL,
 conn);
 return got;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e306f1e..4b8699b 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3299,13 +3299,13 @@ testConnectNumOfNetworks(virConnectPtr conn)
 static int
 testConnectListNetworks(virConnectPtr conn,
 char **const names,
-int nnames)
+int maxnames)
 {
 testDriverPtr privconn = conn->privateData;
 int n;
 
 n = virNetworkObjListGetNames(privconn->networks,
-  true, names, nnames, NULL, conn);
+  true, names, maxnames, NULL, conn);
 return n;
 }
 
@@ -3325,13 +3325,13 @@ testConnectNumOfDefinedNetworks(virConnectPtr conn)
 static int
 testConnectListDefinedNetworks(virConnectPtr conn,
char **const names,
-   int nnames)
+   int maxnames)
 {
 testDriverPtr privconn = conn->privateData;
 int n;
 
 n = virNetworkObjListGetNames(privco

[libvirt] [PATCH v2 18/20] network: Rename @filter to @aclfilter

[John Ferlan]

Rename the virNetworkObjListFilter to be virNetworkObjListACLFilter
since that's more representative of what it is. Also modify the
variable @filter to be @aclfilter. In the future adding the ability
to describe a generic @filter routine for the Export functions
could be a useful thing.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c | 24 
 src/conf/virnetworkobj.h | 10 +-
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index d288dd0..2be48dd 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1307,7 +1307,7 @@ virNetworkMatch(virNetworkObjPtr obj,
 struct virNetworkObjListData {
 virConnectPtr conn;
 virNetworkPtr *nets;
-virNetworkObjListFilter filter;
+virNetworkObjListACLFilter aclfilter;
 unsigned int flags;
 int nnets;
 bool error;
@@ -1327,8 +1327,8 @@ virNetworkObjListPopulate(void *payload,
 
 virObjectLock(obj);
 
-if (data->filter &&
-!data->filter(data->conn, obj->def))
+if (data->aclfilter &&
+!data->aclfilter(data->conn, obj->def))
 goto cleanup;
 
 if (!virNetworkMatch(obj, data->flags))
@@ -1356,11 +1356,11 @@ int
 virNetworkObjListExport(virConnectPtr conn,
 virNetworkObjListPtr netobjs,
 virNetworkPtr **nets,
-virNetworkObjListFilter filter,
+virNetworkObjListACLFilter aclfilter,
 unsigned int flags)
 {
 int ret = -1;
-struct virNetworkObjListData data = { conn, NULL, filter, flags, 0, false};
+struct virNetworkObjListData data = { conn, NULL, aclfilter, flags, 0, 
false};
 
 virObjectLock(netobjs);
 if (nets && VIR_ALLOC_N(data.nets, virHashSize(netobjs->objs) + 1) < 0)
@@ -1436,7 +1436,7 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
 
 struct virNetworkObjListGetHelperData {
 virConnectPtr conn;
-virNetworkObjListFilter filter;
+virNetworkObjListACLFilter aclfilter;
 char **names;
 int maxnames;
 bool active;
@@ -1461,8 +1461,8 @@ virNetworkObjListGetHelper(void *payload,
 
 virObjectLock(obj);
 
-if (data->filter &&
-!data->filter(data->conn, obj->def))
+if (data->aclfilter &&
+!data->aclfilter(data->conn, obj->def))
 goto cleanup;
 
 if ((data->active && virNetworkObjIsActive(obj)) ||
@@ -1486,13 +1486,13 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
   bool active,
   char **names,
   int maxnames,
-  virNetworkObjListFilter filter,
+  virNetworkObjListACLFilter aclfilter,
   virConnectPtr conn)
 {
 int ret = -1;
 
 struct virNetworkObjListGetHelperData data = {
-conn, filter, names, maxnames, active, 0, false};
+conn, aclfilter, names, maxnames, active, 0, false};
 
 virObjectLock(nets);
 virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
@@ -1514,11 +1514,11 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
 int
 virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
bool active,
-   virNetworkObjListFilter filter,
+   virNetworkObjListACLFilter aclfilter,
virConnectPtr conn)
 {
 struct virNetworkObjListGetHelperData data = {
-conn, filter, NULL, -1, active, 0, false};
+conn, aclfilter, NULL, -1, active, 0, false};
 
 virObjectLock(nets);
 virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index f7ed387..d7199fd 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -153,8 +153,8 @@ virNetworkObjTaint(virNetworkObjPtr obj,
virNetworkTaintFlags taint);
 
 typedef bool
-(*virNetworkObjListFilter)(virConnectPtr conn,
-   virNetworkDefPtr def);
+(*virNetworkObjListACLFilter)(virConnectPtr conn,
+  virNetworkDefPtr def);
 
 virNetworkObjPtr
 virNetworkObjAssignDef(virNetworkObjListPtr nets,
@@ -219,7 +219,7 @@ int
 virNetworkObjListExport(virConnectPtr conn,
 virNetworkObjListPtr netobjs,
 virNetworkPtr **nets,
-virNetworkObjListFilter filter,
+virNetworkObjListACLFilter aclfilter,
 unsigned int flags);
 
 typedef int
@@ -236,13 +236,13 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
   bool active,
   char **names,
   int maxnames,
-  virNetworkObjListFilter filter,
+  virNetworkObjListACLFilter aclfilter,
  

[libvirt] [PATCH v2 14/20] network: Consistent use of @obj for virnetworkobj

[John Ferlan]

Consistently use @obj for a virNetworkObjPtr.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c | 333 +++
 1 file changed, 166 insertions(+), 167 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 37b76e5..edea6f5 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -71,39 +71,39 @@ VIR_ONCE_GLOBAL_INIT(virNetworkObj)
 virNetworkObjPtr
 virNetworkObjNew(void)
 {
-virNetworkObjPtr net;
+virNetworkObjPtr obj;
 
 if (virNetworkObjInitialize() < 0)
 return NULL;
 
-if (!(net = virObjectLockableNew(virNetworkObjClass)))
+if (!(obj = virObjectLockableNew(virNetworkObjClass)))
 return NULL;
 
-if (!(net->classIdMap = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
+if (!(obj->classIdMap = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
 goto error;
 
 /* The first three class IDs are already taken */
-ignore_value(virBitmapSetBit(net->classIdMap, 0));
-ignore_value(virBitmapSetBit(net->classIdMap, 1));
-ignore_value(virBitmapSetBit(net->classIdMap, 2));
+ignore_value(virBitmapSetBit(obj->classIdMap, 0));
+ignore_value(virBitmapSetBit(obj->classIdMap, 1));
+ignore_value(virBitmapSetBit(obj->classIdMap, 2));
 
-return net;
+return obj;
 
  error:
-virObjectUnref(net);
+virObjectUnref(obj);
 return NULL;
 }
 
 
 void
-virNetworkObjEndAPI(virNetworkObjPtr *net)
+virNetworkObjEndAPI(virNetworkObjPtr *obj)
 {
-if (!*net)
+if (!*obj)
 return;
 
-virObjectUnlock(*net);
-virObjectUnref(*net);
-*net = NULL;
+virObjectUnlock(*obj);
+virObjectUnref(*obj);
+*obj = NULL;
 }
 
 
@@ -336,15 +336,15 @@ virNetworkObjPtr
 virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
   const unsigned char *uuid)
 {
-virNetworkObjPtr ret = NULL;
+virNetworkObjPtr obj = NULL;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
 virUUIDFormat(uuid, uuidstr);
 
-ret = virHashLookup(nets->objs, uuidstr);
-if (ret)
-virObjectRef(ret);
-return ret;
+obj = virHashLookup(nets->objs, uuidstr);
+if (obj)
+virObjectRef(obj);
+return obj;
 }
 
 
@@ -362,14 +362,14 @@ virNetworkObjPtr
 virNetworkObjFindByUUID(virNetworkObjListPtr nets,
 const unsigned char *uuid)
 {
-virNetworkObjPtr ret;
+virNetworkObjPtr obj;
 
 virObjectLock(nets);
-ret = virNetworkObjFindByUUIDLocked(nets, uuid);
+obj = virNetworkObjFindByUUIDLocked(nets, uuid);
 virObjectUnlock(nets);
-if (ret)
-virObjectLock(ret);
-return ret;
+if (obj)
+virObjectLock(obj);
+return obj;
 }
 
 
@@ -378,13 +378,13 @@ virNetworkObjSearchName(const void *payload,
 const void *name ATTRIBUTE_UNUSED,
 const void *data)
 {
-virNetworkObjPtr net = (virNetworkObjPtr) payload;
+virNetworkObjPtr obj = (virNetworkObjPtr) payload;
 int want = 0;
 
-virObjectLock(net);
-if (STREQ(net->def->name, (const char *)data))
+virObjectLock(obj);
+if (STREQ(obj->def->name, (const char *)data))
 want = 1;
-virObjectUnlock(net);
+virObjectUnlock(obj);
 return want;
 }
 
@@ -402,12 +402,12 @@ virNetworkObjPtr
 virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
   const char *name)
 {
-virNetworkObjPtr ret = NULL;
+virNetworkObjPtr obj = NULL;
 
-ret = virHashSearch(nets->objs, virNetworkObjSearchName, name, NULL);
-if (ret)
-virObjectRef(ret);
-return ret;
+obj = virHashSearch(nets->objs, virNetworkObjSearchName, name, NULL);
+if (obj)
+virObjectRef(obj);
+return obj;
 }
 
 
@@ -425,14 +425,14 @@ virNetworkObjPtr
 virNetworkObjFindByName(virNetworkObjListPtr nets,
 const char *name)
 {
-virNetworkObjPtr ret;
+virNetworkObjPtr obj;
 
 virObjectLock(nets);
-ret = virNetworkObjFindByNameLocked(nets, name);
+obj = virNetworkObjFindByNameLocked(nets, name);
 virObjectUnlock(nets);
-if (ret)
-virObjectLock(ret);
-return ret;
+if (obj)
+virObjectLock(obj);
+return obj;
 }
 
 
@@ -451,21 +451,21 @@ virNetworkObjTaint(virNetworkObjPtr obj,
 
 
 static void
-virNetworkObjDispose(void *obj)
+virNetworkObjDispose(void *opaque)
 {
-virNetworkObjPtr net = obj;
+virNetworkObjPtr obj = opaque;
 
-virNetworkDefFree(net->def);
-virNetworkDefFree(net->newDef);
-virBitmapFree(net->classIdMap);
-virObjectUnref(net->macmap);
+virNetworkDefFree(obj->def);
+virNetworkDefFree(obj->newDef);
+virBitmapFree(obj->classIdMap);
+virObjectUnref(obj->macmap);
 }
 
 
 static void
-virNetworkObjListDispose(void *obj)
+virNetworkObjListDispose(void *opaque)
 {
-virNetworkObjListPtr nets = obj;
+virNetworkObjListPtr nets = opaque;
 
 virHashFree(nets->objs);
 }
@@ -488,7 +488,7 @@ vi

[libvirt] [PATCH v2 16/20] network: Move virObjectRef during AssignDef processing

[John Ferlan]

Move the virObjectRef in virNetworkObjAssignDefLocked to after
the virHashAddEntry to make it "clearer" why the @ref is being
incremented. Upon return from the ObjNew we will have 1 ref on
the object already, adding it to the hash table requires the
increment.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 037186e..43fc2cf 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -595,10 +595,10 @@ virNetworkObjAssignDefLocked(virNetworkObjListPtr nets,
 virUUIDFormat(def->uuid, uuidstr);
 if (virHashAddEntry(nets->objs, uuidstr, obj) < 0)
 goto cleanup;
+virObjectRef(obj);
 
 obj->def = def;
 obj->persistent = !(flags & VIR_NETWORK_OBJ_LIST_ADD_LIVE);
-virObjectRef(obj);
 }
 
 ret = obj;
-- 
2.9.4

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

[libvirt] [PATCH v2 19/20] network: Modify naming for virNetworkObjList* fetching APIs

[John Ferlan]

Use the structure names in the @data setup - makes it easier than
going back to find the struct fields to make sure the order of the
data is correct.

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

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 2be48dd..aab737b 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1360,7 +1360,9 @@ virNetworkObjListExport(virConnectPtr conn,
 unsigned int flags)
 {
 int ret = -1;
-struct virNetworkObjListData data = { conn, NULL, aclfilter, flags, 0, 
false};
+struct virNetworkObjListData data = {
+.conn = conn, .nets = NULL, .aclfilter = aclfilter, .flags = flags,
+.nnets = 0, .error = false };
 
 virObjectLock(netobjs);
 if (nets && VIR_ALLOC_N(data.nets, virHashSize(netobjs->objs) + 1) < 0)
@@ -1426,7 +1428,8 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
  virNetworkObjListIterator callback,
  void *opaque)
 {
-struct virNetworkObjListForEachHelperData data = {callback, opaque, 0};
+struct virNetworkObjListForEachHelperData data = {
+.callback = callback, .opaque = opaque, .ret = 0};
 virObjectLock(nets);
 virHashForEach(nets->objs, virNetworkObjListForEachHelper, &data);
 virObjectUnlock(nets);
@@ -1492,7 +1495,8 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
 int ret = -1;
 
 struct virNetworkObjListGetHelperData data = {
-conn, aclfilter, names, maxnames, active, 0, false};
+.conn = conn, .aclfilter = aclfilter, .names = names,
+.maxnames = maxnames, .active = active, .got = 0, .error = false};
 
 virObjectLock(nets);
 virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
@@ -1518,7 +1522,8 @@ virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
virConnectPtr conn)
 {
 struct virNetworkObjListGetHelperData data = {
-conn, aclfilter, NULL, -1, active, 0, false};
+.conn = conn, .aclfilter = aclfilter, .names = NULL,
+.maxnames = -1, .active = active, .got = 0, .error = false};
 
 virObjectLock(nets);
 virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
-- 
2.9.4

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

[libvirt] [PATCH v2 13/20] network: Introduce virNetworkObjIsPersistent

[John Ferlan]

In preparation to privatize the virNetworkObj - create an accessor function
to get the current @persistent value.  Also change the value to a bool rather
than an unsigned int (since that's how it's generated anyway).

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c| 7 +++
 src/conf/virnetworkobj.h| 5 -
 src/libvirt_private.syms| 1 +
 src/network/bridge_driver.c | 9 +
 src/test/test_driver.c  | 4 ++--
 5 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index a195494..37b76e5 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -144,6 +144,13 @@ virNetworkObjSetActive(virNetworkObjPtr obj,
 }
 
 
+bool
+virNetworkObjIsPersistent(virNetworkObjPtr obj)
+{
+return obj->persistent;
+}
+
+
 int
 virNetworkObjGetAutostart(virNetworkObjPtr obj)
 {
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index 8b778c8..96f9eff 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -33,7 +33,7 @@ struct _virNetworkObj {
 pid_t radvdPid;
 bool active;
 int autostart;
-unsigned int persistent : 1;
+bool persistent;
 
 virNetworkDefPtr def; /* The current definition */
 virNetworkDefPtr newDef; /* New definition to activate at shutdown */
@@ -67,6 +67,9 @@ void
 virNetworkObjSetActive(virNetworkObjPtr obj,
bool active);
 
+bool
+virNetworkObjIsPersistent(virNetworkObjPtr obj);
+
 int
 virNetworkObjGetAutostart(virNetworkObjPtr obj);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8796258..2497af0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -954,6 +954,7 @@ virNetworkObjGetNewDef;
 virNetworkObjGetPersistentDef;
 virNetworkObjGetRadvdPid;
 virNetworkObjIsActive;
+virNetworkObjIsPersistent;
 virNetworkObjListExport;
 virNetworkObjListForEach;
 virNetworkObjListGetNames;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index f423c82..2433e4f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3099,7 +3099,7 @@ networkIsPersistent(virNetworkPtr net)
 if (virNetworkIsPersistentEnsureACL(net->conn, virNetworkObjGetDef(obj)) < 
0)
 goto cleanup;
 
-ret = obj->persistent;
+ret = virNetworkObjIsPersistent(obj);
 
  cleanup:
 virNetworkObjEndAPI(&obj);
@@ -3597,7 +3597,7 @@ networkUndefine(virNetworkPtr net)
 if (virNetworkObjIsActive(obj))
 active = true;
 
-if (!obj->persistent) {
+if (!virNetworkObjIsPersistent(obj)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("can't undefine transient network"));
 goto cleanup;
@@ -3875,7 +3875,8 @@ networkDestroy(virNetworkPtr net)
 VIR_NETWORK_EVENT_STOPPED,
 0);
 
-if (!obj->persistent && networkRemoveInactive(driver, obj) < 0) {
+if (!virNetworkObjIsPersistent(obj) &&
+networkRemoveInactive(driver, obj) < 0) {
 ret = -1;
 goto cleanup;
 }
@@ -3990,7 +3991,7 @@ networkSetAutostart(virNetworkPtr net,
 if (virNetworkSetAutostartEnsureACL(net->conn, def) < 0)
 goto cleanup;
 
-if (!obj->persistent) {
+if (!virNetworkObjIsPersistent(obj)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cannot set autostart for transient network"));
 goto cleanup;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 698488e..e306f1e 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3377,7 +3377,7 @@ testNetworkIsPersistent(virNetworkPtr net)
 if (!(obj = testNetworkObjFindByUUID(privconn, net->uuid)))
 goto cleanup;
 
-ret = obj->persistent;
+ret = virNetworkObjIsPersistent(obj);
 
  cleanup:
 virNetworkObjEndAPI(&obj);
@@ -3576,7 +3576,7 @@ testNetworkDestroy(virNetworkPtr net)
 event = virNetworkEventLifecycleNew(def->name, def->uuid,
 VIR_NETWORK_EVENT_STOPPED,
 0);
-if (!obj->persistent)
+if (!virNetworkObjIsPersistent(obj))
 virNetworkObjRemoveInactive(privconn->networks, obj);
 
 ret = 0;
-- 
2.9.4

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

[libvirt] [PATCH v2 12/20] network: Introduce virNetworkObj{Is|Set}Active

[John Ferlan]

In order to privatize the virNetworkObj create accessors in virnetworkobj
in order to handle the get/set of the active value.

Also rather than an unsigned int, convert it to a boolean to match other
drivers representation and the reality of what it is.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c| 17 -
 src/conf/virnetworkobj.h| 15 ---
 src/libvirt_private.syms|  2 ++
 src/network/bridge_driver.c | 10 +-
 src/test/test_driver.c  |  8 
 5 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 36d4bff..a195494 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -129,6 +129,21 @@ virNetworkObjGetNewDef(virNetworkObjPtr obj)
 }
 
 
+bool
+virNetworkObjIsActive(virNetworkObjPtr obj)
+{
+return obj->active;
+}
+
+
+void
+virNetworkObjSetActive(virNetworkObjPtr obj,
+   bool active)
+{
+obj->active = active;
+}
+
+
 int
 virNetworkObjGetAutostart(virNetworkObjPtr obj)
 {
@@ -942,7 +957,7 @@ virNetworkLoadState(virNetworkObjListPtr nets,
 net->floor_sum = floor_sum_val;
 
 net->taint = taint;
-net->active = 1; /* any network with a state file is by definition active 
*/
+net->active = true; /* any network with a state file is by definition 
active */
 
  cleanup:
 VIR_FREE(configFile);
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index a526d30..8b778c8 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -31,7 +31,7 @@ struct _virNetworkObj {
 
 pid_t dnsmasqPid;
 pid_t radvdPid;
-unsigned int active : 1;
+bool active;
 int autostart;
 unsigned int persistent : 1;
 
@@ -60,6 +60,13 @@ virNetworkObjSetDef(virNetworkObjPtr obj,
 virNetworkDefPtr
 virNetworkObjGetNewDef(virNetworkObjPtr obj);
 
+bool
+virNetworkObjIsActive(virNetworkObjPtr obj);
+
+void
+virNetworkObjSetActive(virNetworkObjPtr obj,
+   bool active);
+
 int
 virNetworkObjGetAutostart(virNetworkObjPtr obj);
 
@@ -119,12 +126,6 @@ virNetworkObjEndAPI(virNetworkObjPtr *net);
 typedef struct _virNetworkObjList virNetworkObjList;
 typedef virNetworkObjList *virNetworkObjListPtr;
 
-static inline int
-virNetworkObjIsActive(const virNetworkObj *net)
-{
-return net->active;
-}
-
 virNetworkObjListPtr
 virNetworkObjListNew(void);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8a3284f..8796258 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -953,6 +953,7 @@ virNetworkObjGetMacMap;
 virNetworkObjGetNewDef;
 virNetworkObjGetPersistentDef;
 virNetworkObjGetRadvdPid;
+virNetworkObjIsActive;
 virNetworkObjListExport;
 virNetworkObjListForEach;
 virNetworkObjListGetNames;
@@ -967,6 +968,7 @@ virNetworkObjNew;
 virNetworkObjRemoveInactive;
 virNetworkObjReplacePersistentDef;
 virNetworkObjSaveStatus;
+virNetworkObjSetActive;
 virNetworkObjSetAutostart;
 virNetworkObjSetDef;
 virNetworkObjSetDefTransient;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index eb814d3..f423c82 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -451,7 +451,7 @@ networkUpdateState(virNetworkObjPtr obj,
 case VIR_NETWORK_FORWARD_OPEN:
 /* If bridge doesn't exist, then mark it inactive */
 if (!(def->bridge && virNetDevExists(def->bridge) == 1))
-obj->active = 0;
+virNetworkObjSetActive(obj, false);
 
 if (!(macMapFile = virMacMapFileName(driver->dnsmasqStateDir,
  def->bridge)))
@@ -467,7 +467,7 @@ networkUpdateState(virNetworkObjPtr obj,
 case VIR_NETWORK_FORWARD_BRIDGE:
 if (def->bridge) {
 if (virNetDevExists(def->bridge) != 1)
-obj->active = 0;
+virNetworkObjSetActive(obj, false);
 break;
 }
 /* intentionally drop through to common case for all
@@ -486,7 +486,7 @@ networkUpdateState(virNetworkObjPtr obj,
 }
 
 /* Try and read dnsmasq/radvd pids of active networks */
-if (obj->active && def->ips && (def->nips > 0)) {
+if (virNetworkObjIsActive(obj) && def->ips && (def->nips > 0)) {
 pid_t radvdPid;
 pid_t dnsmasqPid;
 char *radvdpidbase;
@@ -2789,7 +2789,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
 if (virNetworkObjSaveStatus(driver->stateDir, obj) < 0)
 goto cleanup;
 
-obj->active = 1;
+virNetworkObjSetActive(obj, true);
 VIR_INFO("Network '%s' started up", def->name);
 ret = 0;
 
@@ -2859,7 +2859,7 @@ networkShutdownNetwork(virNetworkDriverStatePtr driver,
 networkRunHook(obj, NULL, NULL, VIR_HOOK_NETWORK_OP_STOPPED,
VIR_HOOK_SUBOP_END);
 
-obj->active = 0;
+virNetworkObjSetActive(obj, false);
 virNetworkObjUnsetDefTransient(obj);
 return ret;
 }
diff --git a/src/test/test_driver.c b/src/test/tes

[libvirt] [PATCH v2 15/20] network: Have virNetworkObjNew lock the returned object

[John Ferlan]

Forces callers to use the virNetworkObjEndAPI properly.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c| 4 ++--
 tests/networkxml2conftest.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index edea6f5..037186e 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -87,6 +87,8 @@ virNetworkObjNew(void)
 ignore_value(virBitmapSetBit(obj->classIdMap, 1));
 ignore_value(virBitmapSetBit(obj->classIdMap, 2));
 
+virObjectLock(obj);
+
 return obj;
 
  error:
@@ -590,8 +592,6 @@ virNetworkObjAssignDefLocked(virNetworkObjListPtr nets,
 if (!(obj = virNetworkObjNew()))
   goto cleanup;
 
-virObjectLock(obj);
-
 virUUIDFormat(def->uuid, uuidstr);
 if (virHashAddEntry(nets->objs, uuidstr, obj) < 0)
 goto cleanup;
diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c
index 6841810..4251a22 100644
--- a/tests/networkxml2conftest.c
+++ b/tests/networkxml2conftest.c
@@ -67,7 +67,7 @@ testCompareXMLToConfFiles(const char *inxml, const char 
*outconf, dnsmasqCapsPtr
 VIR_FREE(actual);
 VIR_FREE(pidfile);
 virCommandFree(cmd);
-virObjectUnref(obj);
+virNetworkObjEndAPI(&obj);
 dnsmasqContextFree(dctx);
 return ret;
 }
-- 
2.9.4

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

[libvirt] [PATCH v2 11/20] network: Introduce virNetworkObj{Get|Set}Autostart

[John Ferlan]

In preparation for privatizing the virNetworkObj structure, create
accessors for the obj->autostart.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c| 15 +++
 src/conf/virnetworkobj.h|  9 -
 src/libvirt_private.syms|  2 ++
 src/network/bridge_driver.c | 20 ++--
 src/test/test_driver.c  |  5 +++--
 5 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 631f8cd..36d4bff 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -129,6 +129,21 @@ virNetworkObjGetNewDef(virNetworkObjPtr obj)
 }
 
 
+int
+virNetworkObjGetAutostart(virNetworkObjPtr obj)
+{
+return obj->autostart;
+}
+
+
+void
+virNetworkObjSetAutostart(virNetworkObjPtr obj,
+  int autostart)
+{
+obj->autostart = autostart;
+}
+
+
 pid_t
 virNetworkObjGetDnsmasqPid(virNetworkObjPtr obj)
 {
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index 90ce0ca..a526d30 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -32,7 +32,7 @@ struct _virNetworkObj {
 pid_t dnsmasqPid;
 pid_t radvdPid;
 unsigned int active : 1;
-unsigned int autostart : 1;
+int autostart;
 unsigned int persistent : 1;
 
 virNetworkDefPtr def; /* The current definition */
@@ -60,6 +60,13 @@ virNetworkObjSetDef(virNetworkObjPtr obj,
 virNetworkDefPtr
 virNetworkObjGetNewDef(virNetworkObjPtr obj);
 
+int
+virNetworkObjGetAutostart(virNetworkObjPtr obj);
+
+void
+virNetworkObjSetAutostart(virNetworkObjPtr obj,
+  int autostart);
+
 virMacMapPtr
 virNetworkObjGetMacMap(virNetworkObjPtr obj);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 84e3eb7..8a3284f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -944,6 +944,7 @@ virNetworkObjFindByName;
 virNetworkObjFindByNameLocked;
 virNetworkObjFindByUUID;
 virNetworkObjFindByUUIDLocked;
+virNetworkObjGetAutostart;
 virNetworkObjGetClassIdMap;
 virNetworkObjGetDef;
 virNetworkObjGetDnsmasqPid;
@@ -966,6 +967,7 @@ virNetworkObjNew;
 virNetworkObjRemoveInactive;
 virNetworkObjReplacePersistentDef;
 virNetworkObjSaveStatus;
+virNetworkObjSetAutostart;
 virNetworkObjSetDef;
 virNetworkObjSetDefTransient;
 virNetworkObjSetDnsmasqPid;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b4fbfc5..eb814d3 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -525,7 +525,7 @@ networkAutostartConfig(virNetworkObjPtr obj,
 int ret = -1;
 
 virObjectLock(obj);
-if (obj->autostart &&
+if (virNetworkObjGetAutostart(obj) &&
 !virNetworkObjIsActive(obj) &&
 networkStartNetwork(driver, obj) < 0)
 goto cleanup;
@@ -3963,7 +3963,7 @@ networkGetAutostart(virNetworkPtr net,
 if (virNetworkGetAutostartEnsureACL(net->conn, virNetworkObjGetDef(obj)) < 
0)
 goto cleanup;
 
-*autostart = obj->autostart;
+*autostart = virNetworkObjGetAutostart(obj);
 ret = 0;
 
  cleanup:
@@ -3974,12 +3974,13 @@ networkGetAutostart(virNetworkPtr net,
 
 static int
 networkSetAutostart(virNetworkPtr net,
-int autostart)
+int new_autostart)
 {
 virNetworkDriverStatePtr driver = networkGetDriver();
 virNetworkObjPtr obj;
 virNetworkDefPtr def;
 char *configFile = NULL, *autostartLink = NULL;
+int cur_autostart;
 int ret = -1;
 
 if (!(obj = networkObjFromNetwork(net)))
@@ -3995,9 +3996,9 @@ networkSetAutostart(virNetworkPtr net,
 goto cleanup;
 }
 
-autostart = (autostart != 0);
-
-if (obj->autostart != autostart) {
+new_autostart = (new_autostart != 0);
+cur_autostart = virNetworkObjGetAutostart(obj);
+if (cur_autostart != new_autostart) {
 if ((configFile = virNetworkConfigFile(driver->networkConfigDir,
def->name)) == NULL)
 goto cleanup;
@@ -4005,7 +4006,7 @@ networkSetAutostart(virNetworkPtr net,
   def->name)) == NULL)
 goto cleanup;
 
-if (autostart) {
+if (new_autostart) {
 if (virFileMakePath(driver->networkAutostartDir) < 0) {
 virReportSystemError(errno,
  _("cannot create autostart directory 
'%s'"),
@@ -4028,13 +4029,12 @@ networkSetAutostart(virNetworkPtr net,
 }
 }
 
-obj->autostart = autostart;
+virNetworkObjSetAutostart(obj, new_autostart);
 }
+
 ret = 0;
 
  cleanup:
-VIR_FREE(configFile);
-VIR_FREE(autostartLink);
 virNetworkObjEndAPI(&obj);
 return ret;
 }
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 868aa27..8e841b2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3647,7 +3647,7 @@ testNetworkGetAutostart(virNetworkPtr net,
 if (!(obj = testNetwo

[libvirt] [PATCH v2 07/20] network: Alter virNetworkObj @class_id to be @classIdMap

[John Ferlan]

Change the variable name to be a bit more descriptive and less confusing
when used with the data.network.actual->class_id.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c| 39 ---
 src/conf/virnetworkobj.h|  2 +-
 src/network/bridge_driver.c | 10 +-
 3 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 533ed26..fb533b9 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -79,13 +79,13 @@ virNetworkObjNew(void)
 if (!(net = virObjectLockableNew(virNetworkObjClass)))
 return NULL;
 
-if (!(net->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
+if (!(net->classIdMap = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
 goto error;
 
 /* The first three class IDs are already taken */
-ignore_value(virBitmapSetBit(net->class_id, 0));
-ignore_value(virBitmapSetBit(net->class_id, 1));
-ignore_value(virBitmapSetBit(net->class_id, 2));
+ignore_value(virBitmapSetBit(net->classIdMap, 0));
+ignore_value(virBitmapSetBit(net->classIdMap, 1));
+ignore_value(virBitmapSetBit(net->classIdMap, 2));
 
 return net;
 
@@ -376,7 +376,7 @@ virNetworkObjDispose(void *obj)
 
 virNetworkDefFree(net->def);
 virNetworkDefFree(net->newDef);
-virBitmapFree(net->class_id);
+virBitmapFree(net->classIdMap);
 virObjectUnref(net->macmap);
 }
 
@@ -716,17 +716,17 @@ virNetworkObjFormat(virNetworkObjPtr net,
 unsigned int flags)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
-char *class_id = virBitmapFormat(net->class_id);
+char *classIdStr = virBitmapFormat(net->classIdMap);
 size_t i;
 
-if (!class_id)
+if (!classIdStr)
 goto error;
 
 virBufferAddLit(&buf, "\n");
 virBufferAdjustIndent(&buf, 2);
-virBufferAsprintf(&buf, "\n", class_id);
+virBufferAsprintf(&buf, "\n", classIdStr);
 virBufferAsprintf(&buf, "\n", net->floor_sum);
-VIR_FREE(class_id);
+VIR_FREE(classIdStr);
 
 for (i = 0; i < VIR_NETWORK_TAINT_LAST; i++) {
 if (net->taint & (1 << i))
@@ -783,7 +783,7 @@ virNetworkLoadState(virNetworkObjListPtr nets,
 xmlDocPtr xml = NULL;
 xmlNodePtr node = NULL, *nodes = NULL;
 xmlXPathContextPtr ctxt = NULL;
-virBitmapPtr class_id_map = NULL;
+virBitmapPtr classIdMap = NULL;
 unsigned long long floor_sum_val = 0;
 unsigned int taint = 0;
 int n;
@@ -820,18 +820,19 @@ virNetworkLoadState(virNetworkObjListPtr nets,
 if (xmlStrEqual(node->name, BAD_CAST "networkstatus")) {
 /* Newer network status file. Contains useful
  * info which are not to be found in bare config XML */
-char *class_id = NULL;
+char *classIdStr = NULL;
 char *floor_sum = NULL;
 
 ctxt->node = node;
-if ((class_id = virXPathString("string(./class_id[1]/@bitmap)", 
ctxt))) {
-if (virBitmapParse(class_id, &class_id_map,
+if ((classIdStr = virXPathString("string(./class_id[1]/@bitmap)",
+ ctxt))) {
+if (virBitmapParse(classIdStr, &classIdMap,
CLASS_ID_BITMAP_SIZE) < 0) {
-VIR_FREE(class_id);
+VIR_FREE(classIdStr);
 goto error;
 }
 }
-VIR_FREE(class_id);
+VIR_FREE(classIdStr);
 
 floor_sum = virXPathString("string(./floor[1]/@sum)", ctxt);
 if (floor_sum &&
@@ -873,9 +874,9 @@ virNetworkLoadState(virNetworkObjListPtr nets,
 /* do not put any "goto error" below this comment */
 
 /* assign status data stored in the network object */
-if (class_id_map) {
-virBitmapFree(net->class_id);
-net->class_id = class_id_map;
+if (classIdMap) {
+virBitmapFree(net->classIdMap);
+net->classIdMap = classIdMap;
 }
 
 if (floor_sum_val > 0)
@@ -892,7 +893,7 @@ virNetworkLoadState(virNetworkObjListPtr nets,
 
  error:
 VIR_FREE(nodes);
-virBitmapFree(class_id_map);
+virBitmapFree(classIdMap);
 virNetworkDefFree(def);
 goto cleanup;
 }
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index a07c7bf..0192913 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -38,7 +38,7 @@ struct _virNetworkObj {
 virNetworkDefPtr def; /* The current definition */
 virNetworkDefPtr newDef; /* New definition to activate at shutdown */
 
-virBitmapPtr class_id; /* bitmap of class IDs for QoS */
+virBitmapPtr classIdMap; /* bitmap of class IDs for QoS */
 unsigned long long floor_sum; /* sum of all 'floor'-s of attached NICs */
 
 unsigned int taint;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5367773..ff0ad9c 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -5347,9 +5347,9 @@ networkNextClassID(virNetworkObjPtr obj)
 {
 ssize_t r

[libvirt] [PATCH v2 10/20] network: Add virNetworkObj Get/Set API's for @def and @newDef

[John Ferlan]

In preparation for making the object private, create a couple of API's
to get the obj->def & obj->newDef and set the obj->def.

While altering networkxml2conftest.c to use the virNetworkObjSetDef
API, fix the name of the variable from @dev to @def

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c|  22 ++
 src/conf/virnetworkobj.h|  10 +
 src/libvirt_private.syms|   3 +
 src/network/bridge_driver.c | 505 
 src/test/test_driver.c  |  58 +++--
 tests/networkxml2conftest.c |   9 +-
 6 files changed, 349 insertions(+), 258 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index ce571bc..631f8cd 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -107,6 +107,28 @@ virNetworkObjEndAPI(virNetworkObjPtr *net)
 }
 
 
+virNetworkDefPtr
+virNetworkObjGetDef(virNetworkObjPtr obj)
+{
+return obj->def;
+}
+
+
+void
+virNetworkObjSetDef(virNetworkObjPtr obj,
+virNetworkDefPtr def)
+{
+obj->def = def;
+}
+
+
+virNetworkDefPtr
+virNetworkObjGetNewDef(virNetworkObjPtr obj)
+{
+return obj->newDef;
+}
+
+
 pid_t
 virNetworkObjGetDnsmasqPid(virNetworkObjPtr obj)
 {
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index 12d0a86..90ce0ca 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -50,6 +50,16 @@ struct _virNetworkObj {
 virNetworkObjPtr
 virNetworkObjNew(void);
 
+virNetworkDefPtr
+virNetworkObjGetDef(virNetworkObjPtr obj);
+
+void
+virNetworkObjSetDef(virNetworkObjPtr obj,
+virNetworkDefPtr def);
+
+virNetworkDefPtr
+virNetworkObjGetNewDef(virNetworkObjPtr obj);
+
 virMacMapPtr
 virNetworkObjGetMacMap(virNetworkObjPtr obj);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 35dfe5e..84e3eb7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -945,9 +945,11 @@ virNetworkObjFindByNameLocked;
 virNetworkObjFindByUUID;
 virNetworkObjFindByUUIDLocked;
 virNetworkObjGetClassIdMap;
+virNetworkObjGetDef;
 virNetworkObjGetDnsmasqPid;
 virNetworkObjGetFloorSum;
 virNetworkObjGetMacMap;
+virNetworkObjGetNewDef;
 virNetworkObjGetPersistentDef;
 virNetworkObjGetRadvdPid;
 virNetworkObjListExport;
@@ -964,6 +966,7 @@ virNetworkObjNew;
 virNetworkObjRemoveInactive;
 virNetworkObjReplacePersistentDef;
 virNetworkObjSaveStatus;
+virNetworkObjSetDef;
 virNetworkObjSetDefTransient;
 virNetworkObjSetDnsmasqPid;
 virNetworkObjSetFloorSum;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 602492e..b4fbfc5 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -213,6 +213,7 @@ networkRunHook(virNetworkObjPtr obj,
int op,
int sub_op)
 {
+virNetworkDefPtr def = virNetworkObjGetDef(obj);
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 char *xml = NULL, *net_xml = NULL, *dom_xml = NULL;
 int hookret;
@@ -229,7 +230,7 @@ networkRunHook(virNetworkObjPtr obj,
 virBufferAdjustIndent(&buf, 2);
 if (iface && virDomainNetDefFormat(&buf, iface, NULL, 0) < 0)
 goto cleanup;
-if (virNetworkDefFormatBuf(&buf, obj->def, 0) < 0)
+if (virNetworkDefFormatBuf(&buf, def, 0) < 0)
 goto cleanup;
 if (dom && virDomainDefFormatInternal(dom, NULL, 0, &buf) < 0)
 goto cleanup;
@@ -241,7 +242,7 @@ networkRunHook(virNetworkObjPtr obj,
 goto cleanup;
 
 xml = virBufferContentAndReset(&buf);
-hookret = virHookCall(VIR_HOOK_DRIVER_NETWORK, obj->def->name,
+hookret = virHookCall(VIR_HOOK_DRIVER_NETWORK, def->name,
   op, sub_op, NULL, xml, NULL);
 
 /*
@@ -429,6 +430,7 @@ static int
 networkUpdateState(virNetworkObjPtr obj,
void *opaque)
 {
+virNetworkDefPtr def;
 virNetworkDriverStatePtr driver = opaque;
 dnsmasqCapsPtr dnsmasq_caps = networkGetDnsmasqCaps(driver);
 virMacMapPtr macmap;
@@ -440,18 +442,19 @@ networkUpdateState(virNetworkObjPtr obj,
 ret = 0;
 goto cleanup;
 }
+def = virNetworkObjGetDef(obj);
 
-switch (obj->def->forward.type) {
+switch (def->forward.type) {
 case VIR_NETWORK_FORWARD_NONE:
 case VIR_NETWORK_FORWARD_NAT:
 case VIR_NETWORK_FORWARD_ROUTE:
 case VIR_NETWORK_FORWARD_OPEN:
 /* If bridge doesn't exist, then mark it inactive */
-if (!(obj->def->bridge && virNetDevExists(obj->def->bridge) == 1))
+if (!(def->bridge && virNetDevExists(def->bridge) == 1))
 obj->active = 0;
 
 if (!(macMapFile = virMacMapFileName(driver->dnsmasqStateDir,
- obj->def->bridge)))
+ def->bridge)))
 goto cleanup;
 
 if (!(macmap = virMacMapNew(macMapFile)))
@@ -462,8 +465,8 @@ networkUpdateState(virNetworkObjPtr obj,
 break;
 
 case VIR_NETWORK_FORWARD_BR

[libvirt] [PATCH v2 00/20] Make the virNetworkObjPtr private

[John Ferlan]

v1: https://www.redhat.com/archives/libvir-list/2017-May/msg00701.html
(but reviewed much more recently)

NOTE from v1:
Patches 1-3 already pushed

Former patch 4:
 * Patch 1 (NEW) - splits out the formatting change in bridge_driver.h
 * Patch 2 - Remainder of the change for consistent naming
 NB: Without the split, there was a R-B, but I didn't push so that the
 split could be "seen".

Former patch 5:
 * Patch 3 (NEW) - from review create virMacMapFileName in src/util/virmacmap.c
 * Patch 4 - Remainder of the previous change w/ adjusted name of course
 * Patch 5 (NEW) - from review, alter virNetworkObjUnrefMacMap

Former patch 6:
 * Patch 6 - No change

Former patch 7:
 * Patch 7 (NEW) - Split out the @class_id to @classIdMap change
 * Patch 8 - Remainder of previous change

Former patch 8:
 * Patch 9 - No change

Former patch 9:
 * Patch 10 - Make suggested naming adjustments
  Add/use virNetworkObjSetDef API

Former patch 10:
 * Patch 11 - Move code back to driver, just have accessors for @autostart

Former patch 11:
 * Patch 12 - No change

Former patch 12:
 * Patch 13 - Use virNetworkObjIsPersistent in networkSetAutostart

Former patch 13:
 * Patch 14 - No change

Former patch 14:
 * Patch 15 - Just have the virNetworkObjNew lock the object now and make
  use of that with using virNetworkObjEndAPI in networkxml2conftest
  NB: Since we'll have a refcnt=1 and lock=true after New the
  EndAPI is proper
 * Patch 16 (NEW) - Just move the virObjectRef - makes it clearer why it's
being ref'd
Former patch 15:
 * Patch 17 (NEW) - Split out the rename of @nnames to @maxnames and explain
the reason better
 * Patch 18 (NEW) - Split out the rename of @filter to @aclnames and explain
the reason better
 * Patch 19 - The remainder of the former patch

Former patch 16:
 * Patch 20 - No change (other than merge conflict resolution)


John Ferlan (20):
  network: Perform some formatting cleanup in bridge_driver.h
  network: Use consistent naming in bridge_driver for virNetwork objects
  network: Move and rename networkMacMgrFileName
  network: Move macmap mgmt from bridge_driver to virnetworkobj
  network: Unconditionally initialize macmap when stopping virtual
network
  network: Add virNetworkObj Get/Set API's for @dnsmasqPid and @radvdPid
  network: Alter virNetworkObj @class_id to be @classIdMap
  network: Introduce virNetworkObjGetClassIdMap
  network: Add virNetworkObj Get/Set API's for @floor_sum
  network: Add virNetworkObj Get/Set API's for @def and @newDef
  network: Introduce virNetworkObj{Get|Set}Autostart
  network: Introduce virNetworkObj{Is|Set}Active
  network: Introduce virNetworkObjIsPersistent
  network: Consistent use of @obj for virnetworkobj
  network: Have virNetworkObjNew lock the returned object
  network: Move virObjectRef during AssignDef processing
  network: Use @maxnames instead of @nnames
  network: Rename @filter to @aclfilter
  network: Modify naming for virNetworkObjList* fetching APIs
  network: Privatize virNetworkObj

 src/conf/virnetworkobj.c|  614 +++---
 src/conf/virnetworkobj.h|  105 ++--
 src/libvirt_private.syms|   21 +
 src/network/bridge_driver.c | 1218 ++-
 src/network/bridge_driver.h |   50 +-
 src/test/test_driver.c  |   83 +--
 src/util/virmacmap.c|   12 +
 src/util/virmacmap.h|4 +
 tests/networkxml2conftest.c |   11 +-
 9 files changed, 1231 insertions(+), 887 deletions(-)

-- 
2.9.4

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

[libvirt] [PATCH v2 04/20] network: Move macmap mgmt from bridge_driver to virnetworkobj

[John Ferlan]

In preparation for having a private virNetworkObj - let's create/move some
API's that handle the obj->macmap. The API's will be renamed to have a
virNetworkObj prefix to follow conventions and the arguments slightly
modified to accept what's necessary to complete their task.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c| 85 +
 src/conf/virnetworkobj.h| 22 
 src/libvirt_private.syms|  5 +++
 src/network/bridge_driver.c | 80 +++---
 4 files changed, 124 insertions(+), 68 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index ccde72e..5578ac3 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -107,6 +107,91 @@ virNetworkObjEndAPI(virNetworkObjPtr *net)
 }
 
 
+virMacMapPtr
+virNetworkObjGetMacMap(virNetworkObjPtr obj)
+{
+return obj->macmap;
+}
+
+
+void
+virNetworkObjSetMacMap(virNetworkObjPtr obj,
+   virMacMapPtr macmap)
+{
+obj->macmap = macmap;
+}
+
+
+void
+virNetworkObjUnrefMacMap(virNetworkObjPtr obj)
+{
+if (!virObjectUnref(obj->macmap))
+obj->macmap = NULL;
+}
+
+
+int
+virNetworkObjMacMgrAdd(virNetworkObjPtr obj,
+   const char *dnsmasqStateDir,
+   const char *domain,
+   const virMacAddr *mac)
+{
+char macStr[VIR_MAC_STRING_BUFLEN];
+char *file = NULL;
+int ret = -1;
+
+if (!obj->macmap)
+return 0;
+
+virMacAddrFormat(mac, macStr);
+
+if (!(file = virMacMapFileName(dnsmasqStateDir, obj->def->bridge)))
+goto cleanup;
+
+if (virMacMapAdd(obj->macmap, domain, macStr) < 0)
+goto cleanup;
+
+if (virMacMapWriteFile(obj->macmap, file) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+VIR_FREE(file);
+return ret;
+}
+
+
+int
+virNetworkObjMacMgrDel(virNetworkObjPtr obj,
+   const char *dnsmasqStateDir,
+   const char *domain,
+   const virMacAddr *mac)
+{
+char macStr[VIR_MAC_STRING_BUFLEN];
+char *file = NULL;
+int ret = -1;
+
+if (!obj->macmap)
+return 0;
+
+virMacAddrFormat(mac, macStr);
+
+if (!(file = virMacMapFileName(dnsmasqStateDir, obj->def->bridge)))
+goto cleanup;
+
+if (virMacMapRemove(obj->macmap, domain, macStr) < 0)
+goto cleanup;
+
+if (virMacMapWriteFile(obj->macmap, file) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+VIR_FREE(file);
+return ret;
+}
+
+
 virNetworkObjListPtr
 virNetworkObjListNew(void)
 {
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index 8090c2e..5c3d9a0 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -50,6 +50,28 @@ struct _virNetworkObj {
 virNetworkObjPtr
 virNetworkObjNew(void);
 
+virMacMapPtr
+virNetworkObjGetMacMap(virNetworkObjPtr obj);
+
+void
+virNetworkObjSetMacMap(virNetworkObjPtr obj,
+   virMacMapPtr macmap);
+
+void
+virNetworkObjUnrefMacMap(virNetworkObjPtr obj);
+
+int
+virNetworkObjMacMgrAdd(virNetworkObjPtr obj,
+   const char *dnsmasqStateDir,
+   const char *domain,
+   const virMacAddr *mac);
+
+int
+virNetworkObjMacMgrDel(virNetworkObjPtr obj,
+   const char *dnsmasqStateDir,
+   const char *domain,
+   const virMacAddr *mac);
+
 void
 virNetworkObjEndAPI(virNetworkObjPtr *net);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c293f38..320c3be 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -944,6 +944,7 @@ virNetworkObjFindByName;
 virNetworkObjFindByNameLocked;
 virNetworkObjFindByUUID;
 virNetworkObjFindByUUIDLocked;
+virNetworkObjGetMacMap;
 virNetworkObjGetPersistentDef;
 virNetworkObjListExport;
 virNetworkObjListForEach;
@@ -953,12 +954,16 @@ virNetworkObjListNumOfNetworks;
 virNetworkObjListPrune;
 virNetworkObjLoadAllConfigs;
 virNetworkObjLoadAllState;
+virNetworkObjMacMgrAdd;
+virNetworkObjMacMgrDel;
 virNetworkObjNew;
 virNetworkObjRemoveInactive;
 virNetworkObjReplacePersistentDef;
 virNetworkObjSaveStatus;
 virNetworkObjSetDefTransient;
+virNetworkObjSetMacMap;
 virNetworkObjTaint;
+virNetworkObjUnrefMacMap;
 virNetworkObjUnsetDefTransient;
 virNetworkObjUpdate;
 virNetworkObjUpdateAssignDef;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index de2e83c..a730742 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -400,68 +400,6 @@ networkRemoveInactive(virNetworkDriverStatePtr driver,
 }
 
 
-static int
-networkMacMgrAdd(virNetworkDriverStatePtr driver,
- virNetworkObjPtr obj,
- const char *domain,
- const virMacAddr *mac)
-{
-char macStr[VIR_MAC_STRING_BUFLEN];
-char *file = NULL;
-int ret = -1;
-
-if (!obj->macmap)
-return

[libvirt] [PATCH v2 01/20] network: Perform some formatting cleanup in bridge_driver.h

[John Ferlan]

Alter prototypes to utilize some more recent guidelines.

Signed-off-by: John Ferlan 
---
 src/network/bridge_driver.h | 50 -
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h
index 7832b60..23d0fac 100644
--- a/src/network/bridge_driver.h
+++ b/src/network/bridge_driver.h
@@ -31,37 +31,49 @@
 # include "virdnsmasq.h"
 # include "virnetworkobj.h"
 
-int networkRegister(void);
+int
+networkRegister(void);
 
 # if WITH_NETWORK
-int networkAllocateActualDevice(virDomainDefPtr dom,
-virDomainNetDefPtr iface)
+int
+networkAllocateActualDevice(virDomainDefPtr dom,
+virDomainNetDefPtr iface)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-void networkNotifyActualDevice(virDomainDefPtr dom,
-   virDomainNetDefPtr iface)
+
+void
+networkNotifyActualDevice(virDomainDefPtr dom,
+  virDomainNetDefPtr iface)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-int networkReleaseActualDevice(virDomainDefPtr dom,
-   virDomainNetDefPtr iface)
+
+int
+networkReleaseActualDevice(virDomainDefPtr dom,
+   virDomainNetDefPtr iface)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
-int networkGetNetworkAddress(const char *netname, char **netaddr)
+int
+networkGetNetworkAddress(const char *netname,
+ char **netaddr)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
-int networkGetActualType(virDomainNetDefPtr iface)
+int
+networkGetActualType(virDomainNetDefPtr iface)
 ATTRIBUTE_NONNULL(1);
 
-int networkDnsmasqConfContents(virNetworkObjPtr network,
-const char *pidfile,
-char **configstr,
-dnsmasqContext *dctx,
-dnsmasqCapsPtr caps);
+int
+networkDnsmasqConfContents(virNetworkObjPtr network,
+   const char *pidfile,
+   char **configstr,
+   dnsmasqContext *dctx,
+   dnsmasqCapsPtr caps);
 
-bool networkBandwidthChangeAllowed(virDomainNetDefPtr iface,
-   virNetDevBandwidthPtr newBandwidth)
+bool
+networkBandwidthChangeAllowed(virDomainNetDefPtr iface,
+  virNetDevBandwidthPtr newBandwidth)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
-int networkBandwidthUpdate(virDomainNetDefPtr iface,
-   virNetDevBandwidthPtr newBandwidth)
+int
+networkBandwidthUpdate(virDomainNetDefPtr iface,
+   virNetDevBandwidthPtr newBandwidth)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 # else
@@ -80,7 +92,7 @@ networkNotifyActualDevice(virDomainDefPtr dom 
ATTRIBUTE_UNUSED,
 
 static inline int
 networkReleaseActualDevice(virDomainDefPtr dom ATTRIBUTE_UNUSED,
-  virDomainNetDefPtr iface ATTRIBUTE_UNUSED)
+   virDomainNetDefPtr iface ATTRIBUTE_UNUSED)
 {
 return 0;
 }
-- 
2.9.4

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

[libvirt] [PATCH v2 03/20] network: Move and rename networkMacMgrFileName

[John Ferlan]

Move networkMacMgrFileName into src/util/virmacmap.c and rename to
virMacMapFileName. We're about to move some more MacMgr processing
files into virnetworkobj and it doesn't make sense to have this helper
in the driver or in virnetworkobj.

Signed-off-by: John Ferlan 
---
 src/libvirt_private.syms|  1 +
 src/network/bridge_driver.c | 24 +++-
 src/util/virmacmap.c| 12 
 src/util/virmacmap.h|  4 
 4 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0dca0a8..c293f38 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2067,6 +2067,7 @@ virMacAddrSetRaw;
 # util/virmacmap.h
 virMacMapAdd;
 virMacMapDumpStr;
+virMacMapFileName;
 virMacMapLookup;
 virMacMapNew;
 virMacMapRemove;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index fd0e6ca..de2e83c 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -322,18 +322,6 @@ networkRadvdConfigFileName(virNetworkDriverStatePtr driver,
 }
 
 
-static char *
-networkMacMgrFileName(virNetworkDriverStatePtr driver,
-  const char *bridge)
-{
-char *filename;
-
-ignore_value(virAsprintf(&filename, "%s/%s.macs",
- driver->dnsmasqStateDir, bridge));
-return filename;
-}
-
-
 /* do needed cleanup steps and remove the network from the list */
 static int
 networkRemoveInactive(virNetworkDriverStatePtr driver,
@@ -375,7 +363,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver,
 if (!(statusfile = virNetworkConfigFile(driver->stateDir, def->name)))
 goto cleanup;
 
-if (!(macMapFile = networkMacMgrFileName(driver, def->bridge)))
+if (!(macMapFile = virMacMapFileName(driver->dnsmasqStateDir, 
def->bridge)))
 goto cleanup;
 
 /* dnsmasq */
@@ -427,7 +415,7 @@ networkMacMgrAdd(virNetworkDriverStatePtr driver,
 
 virMacAddrFormat(mac, macStr);
 
-if (!(file = networkMacMgrFileName(driver, obj->def->bridge)))
+if (!(file = virMacMapFileName(driver->dnsmasqStateDir, obj->def->bridge)))
 goto cleanup;
 
 if (virMacMapAdd(obj->macmap, domain, macStr) < 0)
@@ -458,7 +446,7 @@ networkMacMgrDel(virNetworkDriverStatePtr driver,
 
 virMacAddrFormat(mac, macStr);
 
-if (!(file = networkMacMgrFileName(driver, obj->def->bridge)))
+if (!(file = virMacMapFileName(driver->dnsmasqStateDir, obj->def->bridge)))
 goto cleanup;
 
 if (virMacMapRemove(obj->macmap, domain, macStr) < 0)
@@ -523,7 +511,8 @@ networkUpdateState(virNetworkObjPtr obj,
 if (!(obj->def->bridge && virNetDevExists(obj->def->bridge) == 1))
 obj->active = 0;
 
-if (!(macMapFile = networkMacMgrFileName(driver, obj->def->bridge)))
+if (!(macMapFile = virMacMapFileName(driver->dnsmasqStateDir,
+ obj->def->bridge)))
 goto cleanup;
 
 if (!(obj->macmap = virMacMapNew(macMapFile)))
@@ -2380,7 +2369,8 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr 
driver,
 }
 }
 
-if (!(macMapFile = networkMacMgrFileName(driver, obj->def->bridge)) ||
+if (!(macMapFile = virMacMapFileName(driver->dnsmasqStateDir,
+ obj->def->bridge)) ||
 !(obj->macmap = virMacMapNew(macMapFile)))
 goto err1;
 
diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c
index a9697e3..42890ba 100644
--- a/src/util/virmacmap.c
+++ b/src/util/virmacmap.c
@@ -294,6 +294,18 @@ virMacMapWriteFileLocked(virMacMapPtr mgr,
 }
 
 
+char *
+virMacMapFileName(const char *dnsmasqStateDir,
+  const char *bridge)
+{
+char *filename;
+
+ignore_value(virAsprintf(&filename, "%s/%s.macs", dnsmasqStateDir, 
bridge));
+
+return filename;
+}
+
+
 #define VIR_MAC_HASH_TABLE_SIZE 10
 
 virMacMapPtr
diff --git a/src/util/virmacmap.h b/src/util/virmacmap.h
index 82da833..e6f754e 100644
--- a/src/util/virmacmap.h
+++ b/src/util/virmacmap.h
@@ -27,6 +27,10 @@
 typedef struct virMacMap virMacMap;
 typedef virMacMap *virMacMapPtr;
 
+char *
+virMacMapFileName(const char *dnsmasqStateDir,
+  const char *bridge);
+
 virMacMapPtr virMacMapNew(const char *file);
 
 int virMacMapAdd(virMacMapPtr mgr,
-- 
2.9.4

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

[libvirt] [PATCH v2 05/20] network: Unconditionally initialize macmap when stopping virtual network

[John Ferlan]

Since we can only ever have one reference to obj->macmap, rather
than only clearing obj->macmap during virNetworkObjUnrefMacMap
(e.g. virtual network from networkShutdownNetwork), let's just
unconditionally clear the obj->macmap to ensure that some future
change that created it's own reference to obj->macmap wouldn't
have that reference disappear if virNetworkObjDispose got called.

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

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 5578ac3..bb7225c 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -125,8 +125,8 @@ virNetworkObjSetMacMap(virNetworkObjPtr obj,
 void
 virNetworkObjUnrefMacMap(virNetworkObjPtr obj)
 {
-if (!virObjectUnref(obj->macmap))
-obj->macmap = NULL;
+virObjectUnref(obj->macmap);
+obj->macmap = NULL;
 }
 
 
-- 
2.9.4

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

[libvirt] [PATCH v2 06/20] network: Add virNetworkObj Get/Set API's for @dnsmasqPid and @radvdPid

[John Ferlan]

In preparation for making the object private, create/use a couple of API's
to get/set the obj->dnsmasqPid and obj->radvdPid.

NB: Since the pid's can sometimes changed based on intervening functions,
be sure to always fetch the latest value.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c| 30 ++
 src/conf/virnetworkobj.h| 14 +++
 src/libvirt_private.syms|  4 ++
 src/network/bridge_driver.c | 96 -
 4 files changed, 109 insertions(+), 35 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index bb7225c..533ed26 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -107,6 +107,36 @@ virNetworkObjEndAPI(virNetworkObjPtr *net)
 }
 
 
+pid_t
+virNetworkObjGetDnsmasqPid(virNetworkObjPtr obj)
+{
+return obj->dnsmasqPid;
+}
+
+
+void
+virNetworkObjSetDnsmasqPid(virNetworkObjPtr obj,
+   pid_t dnsmasqPid)
+{
+obj->dnsmasqPid = dnsmasqPid;
+}
+
+
+pid_t
+virNetworkObjGetRadvdPid(virNetworkObjPtr obj)
+{
+return obj->radvdPid;
+}
+
+
+void
+virNetworkObjSetRadvdPid(virNetworkObjPtr obj,
+ pid_t radvdPid)
+{
+obj->radvdPid = radvdPid;
+}
+
+
 virMacMapPtr
 virNetworkObjGetMacMap(virNetworkObjPtr obj)
 {
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index 5c3d9a0..a07c7bf 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -53,6 +53,20 @@ virNetworkObjNew(void);
 virMacMapPtr
 virNetworkObjGetMacMap(virNetworkObjPtr obj);
 
+pid_t
+virNetworkObjGetDnsmasqPid(virNetworkObjPtr obj);
+
+void
+virNetworkObjSetDnsmasqPid(virNetworkObjPtr obj,
+   pid_t dnsmasqPid);
+
+pid_t
+virNetworkObjGetRadvdPid(virNetworkObjPtr obj);
+
+void
+virNetworkObjSetRadvdPid(virNetworkObjPtr obj,
+ pid_t radvdPid);
+
 void
 virNetworkObjSetMacMap(virNetworkObjPtr obj,
virMacMapPtr macmap);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 320c3be..923960b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -944,8 +944,10 @@ virNetworkObjFindByName;
 virNetworkObjFindByNameLocked;
 virNetworkObjFindByUUID;
 virNetworkObjFindByUUIDLocked;
+virNetworkObjGetDnsmasqPid;
 virNetworkObjGetMacMap;
 virNetworkObjGetPersistentDef;
+virNetworkObjGetRadvdPid;
 virNetworkObjListExport;
 virNetworkObjListForEach;
 virNetworkObjListGetNames;
@@ -961,7 +963,9 @@ virNetworkObjRemoveInactive;
 virNetworkObjReplacePersistentDef;
 virNetworkObjSaveStatus;
 virNetworkObjSetDefTransient;
+virNetworkObjSetDnsmasqPid;
 virNetworkObjSetMacMap;
+virNetworkObjSetRadvdPid;
 virNetworkObjTaint;
 virNetworkObjUnrefMacMap;
 virNetworkObjUnsetDefTransient;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index a730742..5367773 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -484,12 +484,15 @@ networkUpdateState(virNetworkObjPtr obj,
 
 /* Try and read dnsmasq/radvd pids of active networks */
 if (obj->active && obj->def->ips && (obj->def->nips > 0)) {
+pid_t radvdPid;
+pid_t dnsmasqPid;
 char *radvdpidbase;
 
 ignore_value(virPidFileReadIfAlive(driver->pidDir,
obj->def->name,
-   &obj->dnsmasqPid,
+   &dnsmasqPid,

dnsmasqCapsGetBinaryPath(dnsmasq_caps)));
+virNetworkObjSetDnsmasqPid(obj, dnsmasqPid);
 
 radvdpidbase = networkRadvdPidfileBasename(obj->def->name);
 if (!radvdpidbase)
@@ -497,7 +500,8 @@ networkUpdateState(virNetworkObjPtr obj,
 
 ignore_value(virPidFileReadIfAlive(driver->pidDir,
radvdpidbase,
-   &obj->radvdPid, RADVD));
+   &radvdPid, RADVD));
+virNetworkObjSetRadvdPid(obj, radvdPid);
 VIR_FREE(radvdpidbase);
 }
 
@@ -1480,7 +1484,7 @@ 
networkBuildDhcpDaemonCommandLine(virNetworkDriverStatePtr driver,
 char *configstr = NULL;
 char *leaseshelper_path = NULL;
 
-obj->dnsmasqPid = -1;
+virNetworkObjSetDnsmasqPid(obj, -1);
 
 if (networkDnsmasqConfContents(obj, pidfile, &configstr,
dctx, dnsmasq_caps) < 0)
@@ -1533,6 +1537,7 @@ networkStartDhcpDaemon(virNetworkDriverStatePtr driver,
 bool needDnsmasq = false;
 virCommandPtr cmd = NULL;
 char *pidfile = NULL;
+pid_t dnsmasqPid;
 int ret = -1;
 dnsmasqContext *dctx = NULL;
 
@@ -1601,9 +1606,10 @@ networkStartDhcpDaemon(virNetworkDriverStatePtr driver,
  * pid
  */
 
-ret = virPidFileRead(driver->pidDir, obj->def->name, &obj->dnsmasqPid);
+ret = virPidFileRead(driver->pidDir, obj->def->name, &dnsmasqPid);
 if (ret < 0)
 goto cleanup;
+vi

[libvirt] [PATCH v2 02/20] network: Use consistent naming in bridge_driver for virNetwork objects

[John Ferlan]

Use @obj for a virNetworkObjPtr

Use @net for a virNetworkPtr

Signed-off-by: John Ferlan 
---
 src/network/bridge_driver.c | 966 ++--
 src/network/bridge_driver.h |   2 +-
 2 files changed, 481 insertions(+), 487 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d05e08f..fd0e6ca 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -148,25 +148,25 @@ networkStateCleanup(void);
 
 static int
 networkStartNetwork(virNetworkDriverStatePtr driver,
-virNetworkObjPtr network);
+virNetworkObjPtr obj);
 
 static int
 networkShutdownNetwork(virNetworkDriverStatePtr driver,
-   virNetworkObjPtr network);
+   virNetworkObjPtr obj);
 
 static int
 networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
-   virNetworkObjPtr network);
+   virNetworkObjPtr obj);
 
 static int
 networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver,
-  virNetworkObjPtr network);
+  virNetworkObjPtr obj);
 
 static int
-networkStartNetworkExternal(virNetworkObjPtr network);
+networkStartNetworkExternal(virNetworkObjPtr obj);
 
 static int
-networkShutdownNetworkExternal(virNetworkObjPtr network);
+networkShutdownNetworkExternal(virNetworkObjPtr obj);
 
 static void
 networkReloadFirewallRules(virNetworkDriverStatePtr driver);
@@ -175,15 +175,15 @@ static void
 networkRefreshDaemons(virNetworkDriverStatePtr driver);
 
 static int
-networkPlugBandwidth(virNetworkObjPtr net,
+networkPlugBandwidth(virNetworkObjPtr obj,
  virDomainNetDefPtr iface);
 
 static int
-networkUnplugBandwidth(virNetworkObjPtr net,
+networkUnplugBandwidth(virNetworkObjPtr obj,
virDomainNetDefPtr iface);
 
 static void
-networkNetworkObjTaint(virNetworkObjPtr net,
+networkNetworkObjTaint(virNetworkObjPtr obj,
virNetworkTaintFlags taint);
 
 
@@ -191,23 +191,23 @@ static virNetworkObjPtr
 networkObjFromNetwork(virNetworkPtr net)
 {
 virNetworkDriverStatePtr driver = networkGetDriver();
-virNetworkObjPtr network;
+virNetworkObjPtr obj;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-if (!network) {
+obj = virNetworkObjFindByUUID(driver->networks, net->uuid);
+if (!obj) {
 virUUIDFormat(net->uuid, uuidstr);
 virReportError(VIR_ERR_NO_NETWORK,
_("no network with matching uuid '%s' (%s)"),
uuidstr, net->name);
 }
 
-return network;
+return obj;
 }
 
 
 static int
-networkRunHook(virNetworkObjPtr network,
+networkRunHook(virNetworkObjPtr obj,
virDomainDefPtr dom,
virDomainNetDefPtr iface,
int op,
@@ -219,8 +219,8 @@ networkRunHook(virNetworkObjPtr network,
 int ret = -1;
 
 if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) {
-if (!network) {
-VIR_DEBUG("Not running hook as @network is NULL");
+if (!obj) {
+VIR_DEBUG("Not running hook as @obj is NULL");
 ret = 0;
 goto cleanup;
 }
@@ -229,7 +229,7 @@ networkRunHook(virNetworkObjPtr network,
 virBufferAdjustIndent(&buf, 2);
 if (iface && virDomainNetDefFormat(&buf, iface, NULL, 0) < 0)
 goto cleanup;
-if (virNetworkDefFormatBuf(&buf, network->def, 0) < 0)
+if (virNetworkDefFormatBuf(&buf, obj->def, 0) < 0)
 goto cleanup;
 if (dom && virDomainDefFormatInternal(dom, NULL, 0, &buf) < 0)
 goto cleanup;
@@ -241,7 +241,7 @@ networkRunHook(virNetworkObjPtr network,
 goto cleanup;
 
 xml = virBufferContentAndReset(&buf);
-hookret = virHookCall(VIR_HOOK_DRIVER_NETWORK, network->def->name,
+hookret = virHookCall(VIR_HOOK_DRIVER_NETWORK, obj->def->name,
   op, sub_op, NULL, xml, NULL);
 
 /*
@@ -250,7 +250,7 @@ networkRunHook(virNetworkObjPtr network,
 if (hookret < 0)
 goto cleanup;
 
-networkNetworkObjTaint(network, VIR_NETWORK_TAINT_HOOK);
+networkNetworkObjTaint(obj, VIR_NETWORK_TAINT_HOOK);
 }
 
 ret = 0;
@@ -337,7 +337,7 @@ networkMacMgrFileName(virNetworkDriverStatePtr driver,
 /* do needed cleanup steps and remove the network from the list */
 static int
 networkRemoveInactive(virNetworkDriverStatePtr driver,
-  virNetworkObjPtr net)
+  virNetworkObjPtr obj)
 {
 char *leasefile = NULL;
 char *customleasefile = NULL;
@@ -347,7 +347,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver,
 char *statusfile = NULL;
 char *macMapFile = NULL;
 dnsmasqContext *dctx = NULL;
-virNetworkDefPtr def = virNetworkObjGetPersistentDef(net);
+   

[libvirt] [PATCH v2 09/20] network: Add virNetworkObj Get/Set API's for @floor_sum

[John Ferlan]

In preparation for making the object private, create a couple of API's
to get/set the obj->floor_sum.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c| 15 +++
 src/conf/virnetworkobj.h|  7 +++
 src/libvirt_private.syms|  2 ++
 src/network/bridge_driver.c | 35 +++
 4 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 1fedb1c..ce571bc 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -151,6 +151,21 @@ virNetworkObjGetMacMap(virNetworkObjPtr obj)
 }
 
 
+unsigned long long
+virNetworkObjGetFloorSum(virNetworkObjPtr obj)
+{
+return obj->floor_sum;
+}
+
+
+void
+virNetworkObjSetFloorSum(virNetworkObjPtr obj,
+ unsigned long long floor_sum)
+{
+obj->floor_sum = floor_sum;
+}
+
+
 void
 virNetworkObjSetMacMap(virNetworkObjPtr obj,
virMacMapPtr macmap)
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index dc8a1cd..12d0a86 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -70,6 +70,13 @@ virNetworkObjSetRadvdPid(virNetworkObjPtr obj,
 virBitmapPtr
 virNetworkObjGetClassIdMap(virNetworkObjPtr obj);
 
+unsigned long long
+virNetworkObjGetFloorSum(virNetworkObjPtr obj);
+
+void
+virNetworkObjSetFloorSum(virNetworkObjPtr obj,
+ unsigned long long floor_sum);
+
 void
 virNetworkObjSetMacMap(virNetworkObjPtr obj,
virMacMapPtr macmap);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 147d78d..35dfe5e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -946,6 +946,7 @@ virNetworkObjFindByUUID;
 virNetworkObjFindByUUIDLocked;
 virNetworkObjGetClassIdMap;
 virNetworkObjGetDnsmasqPid;
+virNetworkObjGetFloorSum;
 virNetworkObjGetMacMap;
 virNetworkObjGetPersistentDef;
 virNetworkObjGetRadvdPid;
@@ -965,6 +966,7 @@ virNetworkObjReplacePersistentDef;
 virNetworkObjSaveStatus;
 virNetworkObjSetDefTransient;
 virNetworkObjSetDnsmasqPid;
+virNetworkObjSetFloorSum;
 virNetworkObjSetMacMap;
 virNetworkObjSetRadvdPid;
 virNetworkObjTaint;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 4a1c258..602492e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -5271,7 +5271,7 @@ networkCheckBandwidth(virNetworkObjPtr obj,
 {
 int ret = -1;
 virNetDevBandwidthPtr netBand = obj->def->bandwidth;
-unsigned long long tmp_floor_sum = obj->floor_sum;
+unsigned long long tmp_floor_sum = virNetworkObjGetFloorSum(obj);
 unsigned long long tmp_new_rate = 0;
 char ifmac[VIR_MAC_STRING_BUFLEN];
 
@@ -5365,6 +5365,7 @@ networkPlugBandwidthImpl(virNetworkObjPtr obj,
 {
 virNetworkDriverStatePtr driver = networkGetDriver();
 virBitmapPtr classIdMap = virNetworkObjGetClassIdMap(obj);
+unsigned long long tmp_floor_sum = virNetworkObjGetFloorSum(obj);
 ssize_t class_id = 0;
 int plug_ret;
 int ret = -1;
@@ -5386,17 +5387,19 @@ networkPlugBandwidthImpl(virNetworkObjPtr obj,
 /* QoS was set, generate new class ID */
 iface->data.network.actual->class_id = class_id;
 /* update sum of 'floor'-s of attached NICs */
-obj->floor_sum += ifaceBand->in->floor;
+tmp_floor_sum += ifaceBand->in->floor;
+virNetworkObjSetFloorSum(obj, tmp_floor_sum);
 /* update status file */
 if (virNetworkObjSaveStatus(driver->stateDir, obj) < 0) {
 ignore_value(virBitmapClearBit(classIdMap, class_id));
-obj->floor_sum -= ifaceBand->in->floor;
+tmp_floor_sum -= ifaceBand->in->floor;
+virNetworkObjSetFloorSum(obj, tmp_floor_sum);
 iface->data.network.actual->class_id = 0;
 ignore_value(virNetDevBandwidthUnplug(obj->def->bridge, class_id));
 goto cleanup;
 }
 /* update rate for non guaranteed NICs */
-new_rate -= obj->floor_sum;
+new_rate -= tmp_floor_sum;
 if (virNetDevBandwidthUpdateRate(obj->def->bridge, 2,
  obj->def->bandwidth, new_rate) < 0)
 VIR_WARN("Unable to update rate for 1:2 class on %s bridge",
@@ -5454,6 +5457,7 @@ networkUnplugBandwidth(virNetworkObjPtr obj,
virDomainNetDefPtr iface)
 {
 virBitmapPtr classIdMap = virNetworkObjGetClassIdMap(obj);
+unsigned long long tmp_floor_sum = virNetworkObjGetFloorSum(obj);
 virNetworkDriverStatePtr driver = networkGetDriver();
 int ret = 0;
 unsigned long long new_rate;
@@ -5477,19 +5481,22 @@ networkUnplugBandwidth(virNetworkObjPtr obj,
 if (ret < 0)
 goto cleanup;
 /* update sum of 'floor'-s of attached NICs */
-obj->floor_sum -= ifaceBand->in->floor;
+tmp_floor_sum -= ifaceBand->in->floor;
+virNetworkObjSetFloorSum(obj, tmp_floor_sum);
+
 /* return class ID */
 ignore_value(virBitmapClearBit(classIdMap,
   

[libvirt] [PATCH v2 08/20] network: Introduce virNetworkObjGetClassIdMap

[John Ferlan]

In preparation for privatizing virNetworkObj, create accessor function to
fetch the @classIdMap.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c|  7 +++
 src/conf/virnetworkobj.h|  3 +++
 src/libvirt_private.syms|  1 +
 src/network/bridge_driver.c | 13 -
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index fb533b9..1fedb1c 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -137,6 +137,13 @@ virNetworkObjSetRadvdPid(virNetworkObjPtr obj,
 }
 
 
+virBitmapPtr
+virNetworkObjGetClassIdMap(virNetworkObjPtr obj)
+{
+return obj->classIdMap;
+}
+
+
 virMacMapPtr
 virNetworkObjGetMacMap(virNetworkObjPtr obj)
 {
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index 0192913..dc8a1cd 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -67,6 +67,9 @@ void
 virNetworkObjSetRadvdPid(virNetworkObjPtr obj,
  pid_t radvdPid);
 
+virBitmapPtr
+virNetworkObjGetClassIdMap(virNetworkObjPtr obj);
+
 void
 virNetworkObjSetMacMap(virNetworkObjPtr obj,
virMacMapPtr macmap);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 923960b..147d78d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -944,6 +944,7 @@ virNetworkObjFindByName;
 virNetworkObjFindByNameLocked;
 virNetworkObjFindByUUID;
 virNetworkObjFindByUUIDLocked;
+virNetworkObjGetClassIdMap;
 virNetworkObjGetDnsmasqPid;
 virNetworkObjGetMacMap;
 virNetworkObjGetPersistentDef;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index ff0ad9c..4a1c258 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -5346,10 +5346,11 @@ static ssize_t
 networkNextClassID(virNetworkObjPtr obj)
 {
 ssize_t ret = 0;
+virBitmapPtr classIdMap = virNetworkObjGetClassIdMap(obj);
 
-ret = virBitmapNextClearBit(obj->classIdMap, -1);
+ret = virBitmapNextClearBit(classIdMap, -1);
 
-if (ret < 0 || virBitmapSetBit(obj->classIdMap, ret) < 0)
+if (ret < 0 || virBitmapSetBit(classIdMap, ret) < 0)
 return -1;
 
 return ret;
@@ -5363,6 +5364,7 @@ networkPlugBandwidthImpl(virNetworkObjPtr obj,
  unsigned long long new_rate)
 {
 virNetworkDriverStatePtr driver = networkGetDriver();
+virBitmapPtr classIdMap = virNetworkObjGetClassIdMap(obj);
 ssize_t class_id = 0;
 int plug_ret;
 int ret = -1;
@@ -5387,7 +5389,7 @@ networkPlugBandwidthImpl(virNetworkObjPtr obj,
 obj->floor_sum += ifaceBand->in->floor;
 /* update status file */
 if (virNetworkObjSaveStatus(driver->stateDir, obj) < 0) {
-ignore_value(virBitmapClearBit(obj->classIdMap, class_id));
+ignore_value(virBitmapClearBit(classIdMap, class_id));
 obj->floor_sum -= ifaceBand->in->floor;
 iface->data.network.actual->class_id = 0;
 ignore_value(virNetDevBandwidthUnplug(obj->def->bridge, class_id));
@@ -5451,6 +5453,7 @@ static int
 networkUnplugBandwidth(virNetworkObjPtr obj,
virDomainNetDefPtr iface)
 {
+virBitmapPtr classIdMap = virNetworkObjGetClassIdMap(obj);
 virNetworkDriverStatePtr driver = networkGetDriver();
 int ret = 0;
 unsigned long long new_rate;
@@ -5476,12 +5479,12 @@ networkUnplugBandwidth(virNetworkObjPtr obj,
 /* update sum of 'floor'-s of attached NICs */
 obj->floor_sum -= ifaceBand->in->floor;
 /* return class ID */
-ignore_value(virBitmapClearBit(obj->classIdMap,
+ignore_value(virBitmapClearBit(classIdMap,
iface->data.network.actual->class_id));
 /* update status file */
 if (virNetworkObjSaveStatus(driver->stateDir, obj) < 0) {
 obj->floor_sum += ifaceBand->in->floor;
-ignore_value(virBitmapSetBit(obj->classIdMap,
+ignore_value(virBitmapSetBit(classIdMap,
  
iface->data.network.actual->class_id));
 goto cleanup;
 }
-- 
2.9.4

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

Re: [libvirt] [PATCH 1/2] tests: Prepare for stricter NIC model validation

[Ján Tomko]

On Wed, Jul 26, 2017 at 02:20:04PM +0200, Andrea Bolognani wrote:

While using "definitely-not-virtio" as a model name is very
cute,


◕‿◕


it will also cause the relevant test to fail once we
introduce stricter validation.

Use "e1000", which is definitely not virtio but also a valid
model name, instead.

Signed-off-by: Andrea Bolognani 
---
tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml | 2 +-
tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)



ACK

Jan


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

Re: [libvirt] [PATCH 15/16] network: Modify naming for virNetworkObjList* fetching APIs

[John Ferlan]

On 07/24/2017 07:49 AM, Pavel Hrdina wrote:
> On Fri, May 19, 2017 at 09:03:23AM -0400, John Ferlan wrote:
>> Use the structure names in the @data setup - makes it easier that going
>> back to find the struct.
>>
>> Use the @maxnames instead of @nnames since that's what it is.
> 
> Please use camelCase -> @maxNames.
> 

This I disagree with as maxnames is used *liberally* throughout many
places in libvirt and in particular as arguments to functions. In
particular, follow this back to :


virDrvConnectListNetworks
virDrvConnectListDefinedNetworks

and

virConnectListNetworks
virConnectListDefinedNetworks

which all use @maxnames.

But I will separate and describe appropriately.

>>
>> Modify the @filter to be @aclfilter and change the typedef from
>> virNetworkObjListFilter to virNetworkObjListACLFilter.
> 
> NACK to this change, even though it's used only to filter by ACLs, it
> can be used to filter by anything.

Again, I disagree. I've been using @aclfilter in other drivers and
taking this route makes things consistent.

Besides, look at a few of the vir*ObjListExport* type functions where
there's actually a second filter that would take an @obj and a @flags
argument and could be defined "generically" as "@filter".  Now if there
was a "generic" ObjListExport routine, that @filter could be an element
to a common structure too...

I will though separate it out.

Tks -

John

> 
> This patch does three things in one, so it should be three separate
> patches.  Since the last change is not correct split the remaining
> changes into two patches.
> 
> Pavel
> 

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

Re: [libvirt] [PATCH 0/3] properly handle '=' in the VNC socket path

[Pavel Hrdina]

On Wed, Jul 26, 2017 at 03:41:13PM +0200, Ján Tomko wrote:
> On Fri, Jul 21, 2017 at 08:11:28PM +0200, Pavel Hrdina wrote:
> >Pavel Hrdina (3):
> >  qemu: capabilities: introduce QEMU_CAPS_VNC_MULTI_SERVERS
> >  qemu: properly handle '=' in the VNC socket path
> >  tests: add test case for VNC unix path with '='
> >
> 
> ACK series
> 
> Jan

Thanks, but I have to SNACK it.  We can safely use the old syntax if
QEMU_CAPS_VNC_MULTI_SERVERS is not available.  I'll send a v2.

Pavel


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

Re: [libvirt] [PATCH] docs: Fix documentation related to memory locking

[Ján Tomko]

On Wed, Jul 26, 2017 at 03:24:25PM +0530, Nitesh Konkar wrote:

Signed-off-by: Nitesh Konkar 
---
docs/formatdomain.html.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



ACK and pushed with slight commit summary tuning.

Jan


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

Re: [libvirt] [PATCH] qemu_cgroup: Remove unnecessary virQEMUDriverPtr arguments

[Ján Tomko]

On Tue, Jul 25, 2017 at 05:49:43PM +0200, Martin Kletzander wrote:

Since commit 2e6ecba1bcac, the pointer to the qemu driver is saved in
domain object's private data and hence does not have to be passed as
yet another parameter if domain object is already one of them.

This is a first (example) patch of this kind of clean up, others will
hopefully follow.

Signed-off-by: Martin Kletzander 
---
src/qemu/qemu_cgroup.c  | 33 ++---
src/qemu/qemu_cgroup.h  |  6 ++
src/qemu/qemu_process.c |  4 ++--
3 files changed, 18 insertions(+), 25 deletions(-)



ACK

Jan


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

Re: [libvirt] [PATCH v3] qemu: Check for existence of provided *_tls_x509_cert_dir

[Ján Tomko]

On Fri, Jul 21, 2017 at 02:23:25PM -0400, John Ferlan wrote:

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

Introduce virQEMUDriverConfigTLSDirValidateResetDefault to validate
that if any of the *_tls_x509_cert_dir values were set properly and
reset the default value if the default_tls_x509_cert_dir changed.

Update the qemu.conf description for default to describe the consequences
if the default directory path does not exist.

Signed-off-by: John Ferlan 
---



[...]



src/qemu/qemu.conf |  8 +++
src/qemu/qemu_conf.c   | 65 +-
src/qemu/qemu_conf.h   |  4 
src/qemu/qemu_driver.c |  3 +++
4 files changed, 79 insertions(+), 1 deletion(-)



[...]


diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 6f44cbf..87d2c2d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c


[...]


@@ -872,6 +873,68 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
return ret;
}

+
+/**
+ * @cfg: Recently config values
+ *
+ * Validate the recently read *_tls_x509_cert_dir values and if necessary
+ * update the default value to match the default_tls_x509_cert_dir
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virQEMUDriverConfigTLSDirValidateResetDefault(virQEMUDriverConfigPtr cfg)


Or just virQEMUDriverConfigValidate, for future expansion.


+{
+bool newDefault = false;
+
+/* If the default entry was uncommented, then validate existence */
+if (cfg->checkdefaultTLSx509certdir) {
+if (!virFileExists(cfg->defaultTLSx509certdir)) {
+virReportError(VIR_ERR_CONF_SYNTAX,
+   _("default_tls_x509_cert_dir directory '%s' "
+ "does not exist"),
+   cfg->defaultTLSx509certdir);
+return -1;
+}
+if (STRNEQ(cfg->defaultTLSx509certdir, SYSCONFDIR "/pki/qemu"))
+newDefault = true;


This bool is only used to decide if we need to stdup the value to the
more specific directory variables. If the user took the time to put
the default value in the uncommented variable, it's only fair to have
them suffer the performance penalty of one extra strdup in exchange for
making this code clearer by dropping this bool.

cfg->checkdefaultTLSx509certdir can be used (or renamed to
'defaultTLSx509certdirProvided'.
Alternatively, the values can be left at NULL in virQEMUDriverConfigNew,
this function would only validate the user-provided values and the
defaults would be filled in by another function. (This has the benefit
of the TLSdir logic being in one place. On the other hand, 
virQEMUDriverConfigNew
would not give us a complete default config)


+}
+
+/* We know virQEMUDriverConfigNew set the particular value to either
+ * it's default or default_tls_x509_cert_dir's default. So, if not the
+ * default default and the directory doesn't exist, then the entry was
+ * set in the config file to something that doesn't exist, so error.
+ *
+ * Also, if the defaultTLSx509certdir value was changed from the default,
+ * then we need to update the default for each setting as well to match
+ * the default_tls_x509_cert_dir.
+ */
+#define VALIDATE_TLS_X509_CERT_DIR(val)   \
+do {  \
+if (STRNEQ(cfg->val ## TLSx509certdir, SYSCONFDIR "/pki/qemu") && \


Okay, here we assume that SYSCONFDIR "/pki/qemu" means the value was
untouched by the user for the simplicity of the code.


+!virFileExists(cfg->val ## TLSx509certdir)) { \
+virReportError(VIR_ERR_CONF_SYNTAX,   \
+   _(#val"_tls_x509_cert_dir directory '%s' " \
+ "does not exist"),   \


This translatable string is untranslatable:
msgid "_tls_x509_cert_dir directory '%s' does not exist"

Either supply the whole variable name, or abolish macros and open-code
the error message like we do for {state,lib,cache}Dir in
qemuStateInitialize.


+   cfg->val ## TLSx509certdir);   \
+return -1;\



+} else if (newDefault) {  \
+VIR_FREE(cfg->val ## TLSx509certdir); \
+if (VIR_STRDUP(cfg->val ## TLSx509certdir,\
+   cfg->defaultTLSx509certdir) < 0)   \
+return -1;\
+} \


This should not be in a macro called VALIDATE.
Maybe virQEMUDriverConfigTLSDirSetDefaults?


+} while (0)
+
+VALIDATE_TLS_X509_CERT_DIR(vnc);
+VALIDATE_TLS_X509_CERT_DIR(spice);
+VALIDATE_TLS_X509_CERT_DIR(chardev);
+V

Re: [libvirt] [PATCH v2 7/7] qemu: privatize _virQEMUCapsCachePriv struct

[Pavel Hrdina]

On Wed, Jul 26, 2017 at 02:33:15PM +0200, Jiri Denemark wrote:
> On Fri, Jul 21, 2017 at 15:30:15 +0200, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina 
> > ---
> > 
> > Notes:
> > Changes in v2:
> > - the body of structure was changed
> > 
> >  src/qemu/qemu_capabilities.c | 10 ++
> >  src/qemu/qemu_capspriv.h | 12 
> >  2 files changed, 10 insertions(+), 12 deletions(-)
> 
> Reviewed-by: Jiri Denemark 

Thanks, I've pushed the series.

Pavel


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

Re: [libvirt] [PATCH 0/3] properly handle '=' in the VNC socket path

[Ján Tomko]

On Fri, Jul 21, 2017 at 08:11:28PM +0200, Pavel Hrdina wrote:

Pavel Hrdina (3):
 qemu: capabilities: introduce QEMU_CAPS_VNC_MULTI_SERVERS
 qemu: properly handle '=' in the VNC socket path
 tests: add test case for VNC unix path with '='



ACK series

Jan


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

Re: [libvirt] [PATCH 08/24] tests: qemuhelp: convert to virTestLoadFilePath

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> As a sample usage of the new helper convert the calls in qemuhelptest to
> the new helper.
> ---
>  tests/qemuhelptest.c | 14 ++
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 

Nice!
Reviewed-by: Eric Blake 

> +if (!(help = virTestLoadFilePath("qemuhelpdata/", info->name, "-device", 
> NULL)))
>  goto cleanup;

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 07/24] tests: utils: Add virTestLoadFilePath helper

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> This new helper loads and returns a file from 'abs_srcdir'. By using
> variable arguments for the function, it's not necessary to format the
> path separately in the test cases.
> ---
>  tests/testutils.c | 51 +++
>  tests/testutils.h |  2 ++
>  2 files changed, 53 insertions(+)
> 

> +
> +/**
> + * virTestLoadFilePath:
> + * @...: file name components.

Mention that it must end in NULL...

> + *
> + * Constructs the test file path from variable arguments and loads the file.
> + * 'abs_srcdir' is automatically prepended.
> + */
> +char *
> +virTestLoadFilePath(const char *p, ...)

and gcc has an attribute to mark vararg functions that require a NULL
sentinel, to let the compiler enforce correct usage.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 06/24] util: buffer: Add virBufferStrcatVArgs

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Split out the worker loop into a separate function and export it.
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virbuffer.c | 27 +--
>  src/util/virbuffer.h |  2 ++
>  3 files changed, 24 insertions(+), 6 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

[libvirt] [PATCH 7/8] conf: check for buffer errors before virBufferUse

[Ján Tomko]

After an OOM error, virBuffer* APIs set buf->use to zero.
Adding a buffer to the parent buffer only if use is non-zero
would quitely drop data on error.

Check the error beforehand to make sure buf->use is zero
because we have not attempted to add anything to it.
---
 src/conf/capabilities.c |  5 +
 src/conf/cpu_conf.c |  4 
 src/conf/domain_conf.c  | 31 +--
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 0f99f3096..db7efffdf 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -930,6 +930,11 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
   bank->controls[j]->max_allocation);
 }
 
+if (virBufferCheckError(&controlBuf) < 0) {
+VIR_FREE(cpus_str);
+return -1;
+}
+
 if (virBufferUse(&controlBuf)) {
 virBufferAddLit(buf, ">\n");
 virBufferAddBuffer(buf, &controlBuf);
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index da40e9ba9..065b4df99 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -646,6 +646,10 @@ virCPUDefFormatBufFull(virBufferPtr buf,
 if (virDomainNumaDefCPUFormat(&childrenBuf, numa) < 0)
 goto cleanup;
 
+if (virBufferCheckError(&attributeBuf) < 0 ||
+virBufferCheckError(&childrenBuf) < 0)
+goto cleanup;
+
 /* Put it all together */
 if (virBufferUse(&attributeBuf) || virBufferUse(&childrenBuf)) {
 virBufferAddLit(buf, "virtio);
 
+if (virBufferCheckError(&driverBuf) < 0)
+return -1;
+
 if (virBufferUse(&driverBuf)) {
 virBufferAddLit(buf, "virtio);
 
-if (virBufferUse(&driverBuf)) {
+if (virBufferError(&driverBuf) != 0 || virBufferUse(&driverBuf)) {
 virBufferAddLit(buf, "\n");
@@ -21891,6 +21894,9 @@ virDomainControllerDefFormat(virBufferPtr buf,
   "pcihole64>\n", def->opts.pciopts.pcihole64size);
 }
 
+if (virBufferCheckError(&childBuf) < 0)
+return -1;
+
 if (virBufferUse(&childBuf)) {
 virBufferAddLit(buf, ">\n");
 virBufferAddBuffer(buf, &childBuf);
@@ -21962,6 +21968,9 @@ virDomainFSDefFormat(virBufferPtr buf,
 
 virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
 
+if (virBufferCheckError(&driverBuf) < 0)
+return -1;
+
 if (virBufferUse(&driverBuf)) {
 virBufferAddLit(buf, "\n");
 } else {
@@ -23309,6 +23321,10 @@ static int virDomainPanicDefFormat(virBufferPtr buf,
 virBufferAdjustIndent(&childrenBuf, indent + 2);
 if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0)
 return -1;
+
+if (virBufferCheckError(&childrenBuf) < 0)
+return -1;
+
 if (virBufferUse(&childrenBuf)) {
 virBufferAddLit(buf, ">\n");
 virBufferAddBuffer(buf, &childrenBuf);
@@ -23655,6 +23671,9 @@ virDomainInputDefFormat(virBufferPtr buf,
 if (virDomainDeviceInfoFormat(&childbuf, &def->info, flags) < 0)
 return -1;
 
+if (virBufferCheckError(&childbuf) < 0)
+return -1;
+
 if (!virBufferUse(&childbuf)) {
 virBufferAddLit(buf, "/>\n");
 } else {
@@ -24596,6 +24615,9 @@ virDomainCputuneDefFormat(virBufferPtr buf,
  def->iothreadids[i]->iothread_id);
 }
 
+if (virBufferCheckError(&childrenBuf) < 0)
+return -1;
+
 if (virBufferUse(&childrenBuf)) {
 virBufferAddLit(buf, "\n");
 virBufferAddBuffer(buf, &childrenBuf);
@@ -24709,7 +24731,8 @@ virDomainIOMMUDefFormat(virBufferPtr buf,
 
 virBufferAsprintf(buf, "model));
-if (virBufferUse(&childBuf)) {
+
+if (virBufferError(&childBuf) != 0 || virBufferUse(&childBuf)) {
 virBufferAddLit(buf, ">\n");
 virBufferAddBuffer(buf, &childBuf);
 virBufferAddLit(buf, "\n");
@@ -24847,6 +24870,10 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 virBufferAdjustIndent(&childrenBuf, -2);
 virBufferAddLit(&childrenBuf, "\n");
 }
+
+if (virBufferCheckError(&childrenBuf) < 0)
+goto error;
+
 if (virBufferUse(&childrenBuf)) {
 virBufferAddLit(buf, "\n");
 virBufferAddBuffer(buf, &childrenBuf);
-- 
2.13.0

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

Re: [libvirt] [PATCH 05/24] tests: utils: Don't calculate file size in virTestLoadFile

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> The callers don't use it so don't waste a strlen(). Also fix the comment
> for the function.
> ---
>  tests/testutils.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/testutils.c b/tests/testutils.c
> index ed01136a0..7f1c4672b 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -289,9 +289,17 @@ virTestRun(const char *title,
>  return ret;
>  }
> 
> -/* Allocate BUF to the size of FILE. Read FILE into buffer BUF.
> -   Upon any failure, diagnose it and return -1, but don't bother trying
> -   to preserve errno. Otherwise, return the number of bytes copied into BUF. 
> */
> +
> +/**
> + * virTestLoadFile:
> + * @file: name of the file to load
> + * @buf: buffer to load the file into
> + *
> + * Allocates @buf to the size of FILE. Reads FILE into buffer BUF.
> + * Upon any failure, error is printed to stderr and -1 is returned. 'errno' 
> is
> + * not preserved. On success 0 is returned. Caller is responsible for freeing
> + * @buf.
> + */

Callers that know the file won't contain NUL bytes don't use the
returned length, but if we ever want to parse a file that DOES contain
NUL, then it becomes important.  Then again, such a file is that much
more annoying to store in git (it is treated as binary rather than
text), so our testsuite is better off if it avoids files with NUL.

>  int
>  virTestLoadFile(const char *file, char **buf)
>  {
> @@ -345,7 +353,7 @@ virTestLoadFile(const char *file, char **buf)
>  }
> 
>  VIR_FORCE_FCLOSE(fp);
> -return strlen(*buf);
> +return 0;

And really, strlen() is wrong if you have embedded NUL bytes anyways, so
it's pretty certain we weren't relying on a correct length for a file
that contains NUL.  So

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

[libvirt] [PATCH 8/8] Turn virDomainDeviceInfoFormat into void

[Ján Tomko]

The rombar attribute was already validated at the time of parsing
the XML.
---
 src/conf/domain_conf.c | 94 --
 1 file changed, 30 insertions(+), 64 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4dc49fdb0..460776ec6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5332,7 +5332,7 @@ virDomainVirtioOptionsFormat(virBufferPtr buf,
 }
 
 
-static int ATTRIBUTE_NONNULL(2)
+static void ATTRIBUTE_NONNULL(2)
 virDomainDeviceInfoFormat(virBufferPtr buf,
   virDomainDeviceInfoPtr info,
   unsigned int flags)
@@ -5360,16 +5360,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 
 virBufferAddLit(buf, "rombar) {
-
 const char *rombar = virTristateSwitchTypeToString(info->rombar);
 
-if (!rombar) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("unexpected rom bar value %d"),
-   info->rombar);
-return -1;
-}
-virBufferAsprintf(buf, " bar='%s'", rombar);
+if (rombar)
+virBufferAsprintf(buf, " bar='%s'", rombar);
 }
 if (info->romfile)
 virBufferEscapeString(buf, " file='%s'", info->romfile);
@@ -5378,7 +5372,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 
 if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
 info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
-return 0;
+return;
 
 virBufferAsprintf(buf, "type));
@@ -5465,7 +5459,6 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 }
 
 virBufferAddLit(buf, "/>\n");
-return 0;
 }
 
 static int
@@ -21711,9 +21704,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
 if (def->src->encryption &&
 virStorageEncryptionFormat(buf, def->src->encryption) < 0)
 return -1;
-if (virDomainDeviceInfoFormat(buf, &def->info,
-  flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 
0)
-return -1;
+virDomainDeviceInfoFormat(buf, &def->info,
+  flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
 
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
@@ -21885,8 +21877,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
 
 virDomainControllerDriverFormat(&childBuf, def);
 
-if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
-return -1;
+virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
 
 if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
 def->opts.pciopts.pcihole64) {
@@ -22018,9 +22009,7 @@ virDomainFSDefFormat(virBufferPtr buf,
 if (def->readonly)
 virBufferAddLit(buf, "\n");
 
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
-return -1;
-
+virDomainDeviceInfoFormat(buf, &def->info, flags);
 
 if (def->space_hard_limit)
 virBufferAsprintf(buf, ""
@@ -22786,10 +22775,9 @@ virDomainNetDefFormat(virBufferPtr buf,
 
 virDomainNetDefCoalesceFormatXML(buf, def->coalesce);
 
-if (virDomainDeviceInfoFormat(buf, &def->info,
-  flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
-  | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) < 0)
-return -1;
+virDomainDeviceInfoFormat(buf, &def->info,
+  flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
+  | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM);
 
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
@@ -23024,8 +23012,7 @@ virDomainChrDefFormat(virBufferPtr buf,
 break;
 }
 
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
-return -1;
+virDomainDeviceInfoFormat(buf, &def->info, flags);
 
 virBufferAdjustIndent(buf, -2);
 virBufferAsprintf(buf, "\n", elementName);
@@ -23074,10 +23061,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
_("unexpected smartcard type %d"), def->type);
 return -1;
 }
-if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) {
-virBufferFreeAndReset(&childBuf);
-return -1;
-}
+virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
 
 if (virBufferCheckError(&childBuf) < 0)
 return -1;
@@ -23134,8 +23118,7 @@ virDomainTPMDefFormat(virBufferPtr buf,
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
 
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
-return -1;
+virDomainDeviceInfoFormat(buf, &def->info, flags);
 
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
@@ -23164,10 +23147,7 @@ virDomainSoundDefFormat(virBufferPtr buf,
 for (i = 0; i < def->ncodecs; i++)
 virDomainSoundCodecDefFormat(&childBuf, def->codecs[i]);
 
-if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) {
-virBufferFreeAndReset

[libvirt] [PATCH 6/8] Use a separate buffer for subelements

[Ján Tomko]

Switch virDomainHubDefFormat to use a separate buffer for subelements.
---
 src/conf/domain_conf.c | 34 +++---
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fdac0e0ba..7728321cb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3529,23 +3529,6 @@ virDomainDeviceGetInfo(virDomainDeviceDefPtr device)
 return NULL;
 }
 
-static bool
-virDomainDeviceInfoNeedsFormat(virDomainDeviceInfoPtr info, unsigned int flags)
-{
-if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
-return true;
-if (info->alias && !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
-return true;
-if (info->mastertype != VIR_DOMAIN_CONTROLLER_MASTER_NONE)
-return true;
-if ((info->rombar != VIR_TRISTATE_SWITCH_ABSENT) ||
-info->romfile)
-return true;
-if (info->bootIndex)
-return true;
-return false;
-}
-
 
 static int
 virDomainDefHasDeviceAddressIterator(virDomainDefPtr def ATTRIBUTE_UNUSED,
@@ -24304,6 +24287,9 @@ virDomainHubDefFormat(virBufferPtr buf,
   unsigned int flags)
 {
 const char *type = virDomainHubTypeToString(def->type);
+virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+
+virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
 
 if (!type) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -24311,14 +24297,16 @@ virDomainHubDefFormat(virBufferPtr buf,
 return -1;
 }
 
-virBufferAsprintf(buf, "info, flags) < 0)
+return -1;
+
+if (virBufferCheckError(&childBuf) < 0)
+return -1;
 
-if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
+virBufferAsprintf(buf, "\n");
-virBufferAdjustIndent(buf, 2);
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
-return -1;
-virBufferAdjustIndent(buf, -2);
+virBufferAddBuffer(buf, &childBuf);
 virBufferAddLit(buf, "\n");
 } else {
 virBufferAddLit(buf, "/>\n");
-- 
2.13.0

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

[libvirt] [PATCH 4/8] Use a separate buffer for subelements

[Ján Tomko]

Convert virDomainSoundDefFormat to use a separate buffer
for subelements.
---
 src/conf/domain_conf.c | 34 ++
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ead94976d..6c4f2f398 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23158,38 +23158,32 @@ virDomainSoundDefFormat(virBufferPtr buf,
 unsigned int flags)
 {
 const char *model = virDomainSoundModelTypeToString(def->model);
-bool children = false;
+virBuffer childBuf = VIR_BUFFER_INITIALIZER;
 size_t i;
 
+virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
+
 if (!model) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected sound model %d"), def->model);
 return -1;
 }
 
-virBufferAsprintf(buf, "ncodecs; i++)
+virDomainSoundCodecDefFormat(&childBuf, def->codecs[i]);
 
-for (i = 0; i < def->ncodecs; i++) {
-if (!children) {
-virBufferAddLit(buf, ">\n");
-virBufferAdjustIndent(buf, 2);
-children = true;
-}
-virDomainSoundCodecDefFormat(buf, def->codecs[i]);
+if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) {
+virBufferFreeAndReset(&childBuf);
+return -1;
 }
 
-if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
-if (!children) {
-virBufferAddLit(buf, ">\n");
-virBufferAdjustIndent(buf, 2);
-children = true;
-}
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
-return -1;
-}
+if (virBufferCheckError(&childBuf) < 0)
+return -1;
 
-if (children) {
-virBufferAdjustIndent(buf, -2);
+virBufferAsprintf(buf, "\n");
+virBufferAddBuffer(buf, &childBuf);
 virBufferAddLit(buf, "\n");
 } else {
 virBufferAddLit(buf, "/>\n");
-- 
2.13.0

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

[libvirt] [PATCH 5/8] Use a separate buffer for subelements

[Ján Tomko]

Convert virDomainWatchdogDefFormat to use a separate
buffer for subelements.
---
 src/conf/domain_conf.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c4f2f398..fdac0e0ba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23275,6 +23275,9 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
 {
 const char *model = virDomainWatchdogModelTypeToString(def->model);
 const char *action = virDomainWatchdogActionTypeToString(def->action);
+virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+
+virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
 
 if (!model) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -23288,15 +23291,18 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
 return -1;
 }
 
+if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
+return -1;
+
+if (virBufferCheckError(&childBuf) < 0)
+return -1;
+
 virBufferAsprintf(buf, "info, flags)) {
+if (virBufferUse(&childBuf)) {
 virBufferAddLit(buf, ">\n");
-virBufferAdjustIndent(buf, 2);
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
-return -1;
-virBufferAdjustIndent(buf, -2);
+virBufferAddBuffer(buf, &childBuf);
 virBufferAddLit(buf, "\n");
 } else {
 virBufferAddLit(buf, "/>\n");
-- 
2.13.0

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

[libvirt] [PATCH 3/8] Use a separate buffer for subelements

[Ján Tomko]

Convert virDomainSmartcardDefFormat to use a separate buffer
for possible subelements, to avoid the need for duplicated
formatting logic in virDomainDeviceInfoNeedsFormat.
---
 src/conf/domain_conf.c | 39 +++
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c958bcf6..ead94976d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23047,37 +23047,32 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
 unsigned int flags)
 {
 const char *mode = virDomainSmartcardTypeToString(def->type);
+virBuffer childBuf = VIR_BUFFER_INITIALIZER;
 size_t i;
 
+virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
+
 if (!mode) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected smartcard type %d"), def->type);
 return -1;
 }
 
-virBufferAsprintf(buf, "type) {
 case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
-if (!virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "/>\n");
-return 0;
-}
-virBufferAddLit(buf, ">\n");
 break;
 
 case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
-virBufferAddLit(buf, ">\n");
-for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++)
-virBufferEscapeString(buf, "%s\n",
+for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
+virBufferEscapeString(&childBuf, "%s\n",
   def->data.cert.file[i]);
-virBufferEscapeString(buf, "%s\n",
+}
+virBufferEscapeString(&childBuf, "%s\n",
   def->data.cert.database);
 break;
 
 case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-if (virDomainChrSourceDefFormat(buf, def->data.passthru, false,
+if (virDomainChrSourceDefFormat(&childBuf, def->data.passthru, false,
 flags) < 0)
 return -1;
 break;
@@ -23087,10 +23082,22 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
_("unexpected smartcard type %d"), def->type);
 return -1;
 }
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) {
+virBufferFreeAndReset(&childBuf);
 return -1;
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
+}
+
+if (virBufferCheckError(&childBuf) < 0)
+return -1;
+
+virBufferAsprintf(buf, "\n");
+virBufferAddBuffer(buf, &childBuf);
+virBufferAddLit(buf, "\n");
+} else {
+virBufferAddLit(buf, "/>\n");
+}
 return 0;
 }
 
-- 
2.13.0

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

[libvirt] [PATCH 2/8] virDomainDeviceInfoFormat: delete outdated comments

[Ján Tomko]

This function has grown to format more than just the address.
Delete the comment completely to avoid failing to update it
in the future.

Also, the indentation is now handled by the virBuffer APIs,
so the comment about indentation no longer makes sense.
---
 src/conf/domain_conf.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 73c4b55e9..6c958bcf6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5349,9 +5349,6 @@ virDomainVirtioOptionsFormat(virBufferPtr buf,
 }
 
 
-/* Generate a string representation of a device address
- * @info address Device address to stringify
- */
 static int ATTRIBUTE_NONNULL(2)
 virDomainDeviceInfoFormat(virBufferPtr buf,
   virDomainDeviceInfoPtr info,
@@ -5400,7 +5397,6 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
 return 0;
 
-/* We'll be in domain/devices/[device type]/ so 3 level indent */
 virBufferAsprintf(buf, "type));
 
-- 
2.13.0

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

[libvirt] [PATCH 1/8] Remove superfluous usage of virDomainDeviceInfoNeedsFormat

[Ján Tomko]

This function returns false if virDomainDeviceInfoFormat
would not format anything.

Using it as the sole condition to decide whether to call
virDomainDeviceInfoFormat or not is pointless, since
the conditions are repeated in virDomainDeviceInfoFormat.
---
 src/conf/domain_conf.c | 33 +++--
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 34c8f45ed..73c4b55e9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21903,8 +21903,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
 
 virDomainControllerDriverFormat(&childBuf, def);
 
-if (virDomainDeviceInfoNeedsFormat(&def->info, flags) &&
-virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
+if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
 return -1;
 
 if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
@@ -23037,10 +23036,8 @@ virDomainChrDefFormat(virBufferPtr buf,
 break;
 }
 
-if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
-return -1;
-}
+if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+return -1;
 
 virBufferAdjustIndent(buf, -2);
 virBufferAsprintf(buf, "\n", elementName);
@@ -23142,10 +23139,8 @@ virDomainTPMDefFormat(virBufferPtr buf,
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
 
-if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
-return -1;
-}
+if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+return -1;
 
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
@@ -23227,8 +23222,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
 if (def->period)
 virBufferAsprintf(&childrenBuf, "\n", def->period);
 
-if (virDomainDeviceInfoNeedsFormat(&def->info, flags) &&
-virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) {
+if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) {
 virBufferFreeAndReset(&childrenBuf);
 return -1;
 }
@@ -23267,8 +23261,7 @@ virDomainNVRAMDefFormat(virBufferPtr buf,
 {
 virBufferAddLit(buf, "\n");
 virBufferAdjustIndent(buf, 2);
-if (virDomainDeviceInfoNeedsFormat(&def->info, flags) &&
-virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
 return -1;
 
 virBufferAdjustIndent(buf, -2);
@@ -23427,10 +23420,8 @@ virDomainRNGDefFormat(virBufferPtr buf,
 virBufferAddLit(buf, "/>\n");
 }
 
-if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
-return -1;
-}
+if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+return -1;
 
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
@@ -23548,10 +23539,8 @@ virDomainMemoryDefFormat(virBufferPtr buf,
 
 virDomainMemoryTargetDefFormat(buf, def);
 
-if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
-if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
-return -1;
-}
+if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+return -1;
 
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
-- 
2.13.0

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

[libvirt] [PATCH 0/8] Delete a function

[Ján Tomko]

*** BLURB HERE ***

Ján Tomko (8):
  Remove superfluous usage of virDomainDeviceInfoNeedsFormat
  virDomainDeviceInfoFormat: delete outdated comments
  Use a separate buffer for  subelements
  Use a separate buffer for  subelements
  Use a separate buffer for  subelements
  Use a separate buffer for  subelements
  conf: check for buffer errors before virBufferUse
  Turn virDomainDeviceInfoFormat into void

 src/conf/capabilities.c |   5 +
 src/conf/cpu_conf.c |   4 +
 src/conf/domain_conf.c  | 237 +---
 3 files changed, 114 insertions(+), 132 deletions(-)

-- 
2.13.0

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

Re: [libvirt] [PATCH v2 6/7] qemu: switch QEMU capabilities to use virFileCache

[Jiri Denemark]

On Fri, Jul 21, 2017 at 15:30:14 +0200, Pavel Hrdina wrote:
> The switch contains considerable amount of changes:
> 
>   virQEMUCapsRememberCached() is removed because this is now handled
>   by virFileCacheSave().
> 
>   virQEMUCapsInitCached() is removed because this is now handled by
>   virFileCacheLoad().
> 
>   virQEMUCapsNewForBinary() is split into two functions,
>   virQEMUCapsNewData() which creates new data if there is nothing
>   cached and virQEMUCapsLoadFile() which loads the cached data.
>   This is now handled by virFileCacheNewData().
> 
>   virQEMUCapsCacheValidate() is removed because this is now handled by
>   virFileCacheValidate().
> 
>   virQEMUCapsCacheFree() is removed because it's no longer required.
> 
>   Add virCapsPtr into virQEMUCapsCachePriv because for each call of
>   virFileCacheLookup*() we need to use current virCapsPtr.
> 
> Signed-off-by: Pavel Hrdina 
> ---
> 
> Notes:
> Changes in v2:
> - removed the need to set/unset virCaps before calling 
> virFileCacheLookup*

Reviewed-by: Jiri Denemark 

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

Re: [libvirt] [PATCH 04/24] tests: qemumonitorjson: Drop redundant data from testBlockNodeNameDetectFormat

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> The node name and backing file name can be inferred from the hierarchy.
> This will also help when converting to detect node names using
> query-blockstats data.
> ---
>  .../qemumonitorjson-nodename-basic.result|  6 --
>  .../qemumonitorjson-nodename-gluster.result  |  4 
>  .../qemumonitorjson-nodename-relative.result | 12 
> 
>  .../qemumonitorjson-nodename-same-backing.result |  4 
>  tests/qemumonitorjsontest.c  |  4 
>  5 files changed, 30 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

[libvirt] [PATCH] docs: Fix documentation related to memory locking

[Nitesh Konkar]

Signed-off-by: Nitesh Konkar 
---
 docs/formatdomain.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index bceddd2..91195be 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -964,7 +964,7 @@
 option opens up to a potential security risk: the host will be unable
 to reclaim the locked memory back from the guest when it's running out
 of memory, which means a malicious guest allocating large amounts of
-locked memory could cause a denial-of-service attach on the host.
+locked memory could cause a denial-of-service attack on the host.
 Because of this, using this option is discouraged unless your workload
 demands it; even then, it's highly recommended to set an
 hard_limit (see
-- 
2.9.4

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

Re: [libvirt] [PATCH] nodedev: Fix double unlock of the driver on udevEnumerateDevices failure

[Erik Skultety]

On Wed, Jul 26, 2017 at 08:36:04AM -0400, John Ferlan wrote:
>
>
> On 07/26/2017 04:45 AM, Erik Skultety wrote:
> > Commit @4cb719b2dc moved the driver locks around since these have become
> > unnecessary at spots where the code handles now self-lockable object
> > list, but missed the possible double unlock if udevEnumerateDevices
> > fails, because at that point the driver lock had been already dropped.
> >
> > Signed-off-by: Erik Skultety 
> > ---
> >  src/node_device/node_device_udev.c | 17 ++---
> >  1 file changed, 10 insertions(+), 7 deletions(-)
> >
>
> Oh yeah - missed that case... thanks.
>
> w/r/t: _hal from Martin's review - that's pre-existing and separable.
>
> Still in both cases you're in Initialization functions with an unlock of
> an unlocked resource with no error checking by the same thread on your
> way to a function that's about to destroy the mutex... and eventual
> libvirtd death.

Sure, but behaviour of an unlock of an unlocked resource is undefined and we
most probably want to terminate the daemon gracefully, or better said,
deterministically.

Erik

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

Re: [libvirt] [PATCH 03/24] tests: qemumonitor: Prepare for more test data in testBlockNodeNameDetect

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Rename 'json' and related variables to 'nodeNameJson'. Also rename the
> test files along. This is a preparation for modifying how we detect node
> names which will require also data from 'query-blockstats'.

s/require also/also require/

> ---
>  ...emumonitorjson-nodename-basic-named-nodes.json} |  0
>  ...umonitorjson-nodename-gluster-named-nodes.json} |  0
>  ...monitorjson-nodename-relative-named-nodes.json} |  0
>  ...torjson-nodename-same-backing-named-nodes.json} |  0
>  tests/qemumonitorjsontest.c| 22 
> +++---
>  5 files changed, 11 insertions(+), 11 deletions(-)
>  rename tests/qemumonitorjsondata/{qemumonitorjson-nodename-basic.json => 
> qemumonitorjson-nodename-basic-named-nodes.json} (100%)
>  rename tests/qemumonitorjsondata/{qemumonitorjson-nodename-gluster.json => 
> qemumonitorjson-nodename-gluster-named-nodes.json} (100%)
>  rename tests/qemumonitorjsondata/{qemumonitorjson-nodename-relative.json => 
> qemumonitorjson-nodename-relative-named-nodes.json} (100%)
>  rename tests/qemumonitorjsondata/{qemumonitorjson-nodename-same-backing.json 
> => qemumonitorjson-nodename-same-backing-named-nodes.json} (100%)

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH] nodedev: Fix double unlock of the driver on udevEnumerateDevices failure

[John Ferlan]

On 07/26/2017 04:45 AM, Erik Skultety wrote:
> Commit @4cb719b2dc moved the driver locks around since these have become
> unnecessary at spots where the code handles now self-lockable object
> list, but missed the possible double unlock if udevEnumerateDevices
> fails, because at that point the driver lock had been already dropped.
> 
> Signed-off-by: Erik Skultety 
> ---
>  src/node_device/node_device_udev.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 

Oh yeah - missed that case... thanks.

w/r/t: _hal from Martin's review - that's pre-existing and separable.

Still in both cases you're in Initialization functions with an unlock of
an unlocked resource with no error checking by the same thread on your
way to a function that's about to destroy the mutex... and eventual
libvirtd death.

Tks -

John

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

Re: [libvirt] [PATCH v2 7/7] qemu: privatize _virQEMUCapsCachePriv struct

[Jiri Denemark]

On Fri, Jul 21, 2017 at 15:30:15 +0200, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
> 
> Notes:
> Changes in v2:
> - the body of structure was changed
> 
>  src/qemu/qemu_capabilities.c | 10 ++
>  src/qemu/qemu_capspriv.h | 12 
>  2 files changed, 10 insertions(+), 12 deletions(-)

Reviewed-by: Jiri Denemark 

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

Re: [libvirt] [PATCH v2 5/7] qemu: pass only host arch instead of the whole virCaps

[Jiri Denemark]

On Fri, Jul 21, 2017 at 15:30:13 +0200, Pavel Hrdina wrote:
> This is a preparation for following patches where we switch to
> virFileCache for QEMU capabilities cache
> 
> The host arch will always remain the same but virCaps may change.  Now
> the host arch is stored while creating new qemu capabilities cache.
> It removes the need to pass virCaps into virQEMUCapsCache*() functions.
> 
> Signed-off-by: Pavel Hrdina 
> ---
> 
> Notes:
> New in v2
> 
>  src/qemu/qemu_capabilities.c | 61 
> +---
>  src/qemu/qemu_capabilities.h |  9 +++
>  src/qemu/qemu_capspriv.h | 11 +---
>  src/qemu/qemu_domain.c   | 17 +---
>  src/qemu/qemu_driver.c   |  8 +++---
>  src/qemu/qemu_process.c  |  9 +++
>  tests/qemucapsprobe.c|  3 ++-
>  tests/qemucpumock.c  |  3 ++-
>  tests/qemuxml2argvtest.c |  9 +--
>  tests/testutilsqemu.c|  5 +++-
>  10 files changed, 67 insertions(+), 68 deletions(-)

Reviewed-by: Jiri Denemark 

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

Re: [libvirt] [PATCH 02/24] tests: qemumonitorjson: Consolidate basic node name detection test cases

[Eric Blake]

On 07/26/2017 05:00 AM, Peter Krempa wrote:
> Test cases named '1' and '2' differed only in the length of the backing
> chain, so remove test case '2' and rename test '1' to 'basic'.
> ---
>  .../qemumonitorjson-nodename-2.json| 2270 
> 
>  .../qemumonitorjson-nodename-2.result  |   60 -
>  ...-1.json => qemumonitorjson-nodename-basic.json} |0
>  ...esult => qemumonitorjson-nodename-basic.result} |0
>  tests/qemumonitorjsontest.c|3 +-
>  5 files changed, 1 insertion(+), 2332 deletions(-)
>  delete mode 100644 tests/qemumonitorjsondata/qemumonitorjson-nodename-2.json
>  delete mode 100644 
> tests/qemumonitorjsondata/qemumonitorjson-nodename-2.result
>  rename tests/qemumonitorjsondata/{qemumonitorjson-nodename-1.json => 
> qemumonitorjson-nodename-basic.json} (100%)
>  rename tests/qemumonitorjsondata/{qemumonitorjson-nodename-1.result => 
> qemumonitorjson-nodename-basic.result} (100%)
> 

Reviewed-by: Eric Blake 

> -  "image": {
> -"backing-image": {
> -  "backing-image": {
> -"backing-image": {
> -  "backing-image": {
> -"backing-image": {
> -  "backing-image": {
> -"backing-image": {
> -  "backing-image": {
> -"backing-image": {
> -  "backing-image": {
> -"backing-image": {
> -  "virtual-size": 9663676416,
> -  "filename": 
> "/var/lib/libvirt/images/rhel7.3.qcow2",
> -  "cluster-size": 65536,

That qemu output is rather verbose.  Here's hoping Kevin's plan to add a
saner interface in 2.11 pans out!

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH v2 4/7] tests: rewrite host CPU mocking

[Jiri Denemark]

On Fri, Jul 21, 2017 at 15:30:12 +0200, Pavel Hrdina wrote:
> Move all the host CPU data into a separate file and rewrite qemucpumock
> to not use passed @caps.  This is preparation for following patch which
> will replace virCaps argument with virArch.
> 
> Signed-off-by: Pavel Hrdina 
> ---
> 
> Notes:
> New in v2
> 
>  tests/domaincapstest.c|  55 +
>  tests/qemucpumock.c   |  13 ++--
>  tests/testutilshostcpus.h | 148 
> ++
>  tests/testutilsqemu.c |  93 +++--
>  tests/testutilsxen.c  |  29 +
>  5 files changed, 167 insertions(+), 171 deletions(-)
>  create mode 100644 tests/testutilshostcpus.h

Reviewed-by: Jiri Denemark 

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

[libvirt] [PATCH 2/2] conf: Introduce the virDomainNetModel enumeration

[Andrea Bolognani]

Up until now, we have stored the model name for network
interfaces as raw strings in virDomainNetDef: this is
suboptimal for a number of reasons, such as having to
perform relatively expensive string allocations and
comparisons all the time and not giving the compiler
the opportunity to have our back in certain situations.

Replace the strings with an enumeration similar to the
ones we already use for pretty much everything else.

Signed-off-by: Andrea Bolognani 
---
Most drivers already performed pretty strict validation
of the model name, so there should be no problems there;
the QEMU driver, however, though it would be a good idea
to just accept any string that possibly kinda resembled
a valid model name.

The models I've included in the enumeration are those
that were already referenced somewhere else in libvirt,
but there's no guarantee that other model names are not
used in the wild.

One possibility would be to also add (a subset of) all
models QEMU ever supported, even though some of them
might have never been used, just to be safe; on the
other side of the spectrum, we could go with the minimal
set and possibly add more if breakages are reported.

 src/bhyve/bhyve_command.c   |  6 ++--
 src/bhyve/bhyve_parse_command.c |  6 +++-
 src/conf/domain_conf.c  | 63 ++---
 src/conf/domain_conf.h  | 28 +-
 src/libvirt_private.syms|  2 ++
 src/libxl/libxl_conf.c  |  7 +++--
 src/qemu/qemu_command.c | 14 +
 src/qemu/qemu_domain.c  | 24 +++-
 src/qemu/qemu_domain_address.c  | 47 +++---
 src/qemu/qemu_driver.c  |  3 +-
 src/qemu/qemu_hotplug.c | 11 ---
 src/qemu/qemu_interface.c   |  8 +++---
 src/qemu/qemu_parse_command.c   | 10 +--
 src/security/virt-aa-helper.c   |  2 +-
 src/vbox/vbox_common.c  | 27 +-
 src/vmx/vmx.c   | 53 --
 src/xenconfig/xen_common.c  | 21 --
 src/xenconfig/xen_sxpr.c| 23 +--
 18 files changed, 225 insertions(+), 130 deletions(-)

diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index b3ae315..ac9ea56 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -57,10 +57,10 @@ bhyveBuildNetArgStr(virConnectPtr conn,
 int ret = -1;
 virDomainNetType actualType = virDomainNetGetActualType(net);
 
-if (STREQ(net->model, "virtio")) {
+if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO) {
 if (VIR_STRDUP(nic_model, "virtio-net") < 0)
 return -1;
-} else if (STREQ(net->model, "e1000")) {
+} else if (net->model == VIR_DOMAIN_NET_MODEL_E1000) {
 if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_NET_E1000) != 0) {
 if (VIR_STRDUP(nic_model, "e1000") < 0)
 return -1;
@@ -73,7 +73,7 @@ bhyveBuildNetArgStr(virConnectPtr conn,
 } else {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("NIC model '%s' is not supported"),
-   net->model);
+   NULLSTR(virDomainNetModelTypeToString(net->model)));
 return -1;
 }
 
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index fcaaed2..f77ed27 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -515,8 +515,12 @@ bhyveParsePCINet(virDomainDefPtr def,
 if (VIR_STRDUP(net->data.bridge.brname, "virbr0") < 0)
 goto error;
 
-if (VIR_STRDUP(net->model, model) < 0)
+if ((net->model = virDomainNetModelTypeFromString(model)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("NIC model '%s' is not supported"),
+   model);
 goto error;
+}
 
 net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
 net->info.addr.pci.slot = pcislot;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 34c8f45..7a2ff8d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -418,6 +418,28 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
   "hostdev",
   "udp")
 
+VIR_ENUM_IMPL(virDomainNetModel, VIR_DOMAIN_NET_MODEL_LAST,
+  "none",
+  "usb-net",
+  "netfront",
+  "vlance",
+  "vmxnet",
+  "vmxnet2",
+  "vmxnet3",
+  "am79c970a",
+  "am79c973",
+  "82540em",
+  "82543gc",
+  "82545em",
+  "spapr-vlan",
+  "smc91c111",
+  "lan9118",
+  "rtl8139",
+  "e1000",
+  "e1000e",
+  "virtio",
+);
+
 VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST,
   "default",
   "qemu",
@@ -1986,7 +2008,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
 if (!def)
 retur

[libvirt] [PATCH 1/2] tests: Prepare for stricter NIC model validation

[Andrea Bolognani]

While using "definitely-not-virtio" as a model name is very
cute, it will also cause the relevant test to fail once we
introduce stricter validation.

Use "e1000", which is definitely not virtio but also a valid
model name, instead.

Signed-off-by: Andrea Bolognani 
---
 tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml | 2 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
index 1d6c0ff..a1532cb 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
@@ -24,7 +24,7 @@
 
 
   
-  
+  
   
   
 
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
index a39b8cb..3a95b60 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
@@ -28,7 +28,7 @@
 
 
   
-  
+  
   
   
 
-- 
2.7.5

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

[libvirt] [PATCH 0/2] Introduce the virDomainNetModel enumeration

[Andrea Bolognani]

More information in the commit message for patch 2/2.
Nobody reads cover letters anyway.

Andrea Bolognani (2):
  tests: Prepare for stricter NIC model validation
  conf: Introduce the virDomainNetModel enumeration

 src/bhyve/bhyve_command.c  |  6 +--
 src/bhyve/bhyve_parse_command.c|  6 ++-
 src/conf/domain_conf.c | 63 --
 src/conf/domain_conf.h | 28 +-
 src/libvirt_private.syms   |  2 +
 src/libxl/libxl_conf.c |  7 +--
 src/qemu/qemu_command.c| 14 +++--
 src/qemu/qemu_domain.c | 24 -
 src/qemu/qemu_domain_address.c | 47 ++--
 src/qemu/qemu_driver.c |  3 +-
 src/qemu/qemu_hotplug.c| 11 ++--
 src/qemu/qemu_interface.c  |  8 +--
 src/qemu/qemu_parse_command.c  | 10 +++-
 src/security/virt-aa-helper.c  |  2 +-
 src/vbox/vbox_common.c | 27 +-
 src/vmx/vmx.c  | 53 +-
 src/xenconfig/xen_common.c | 21 
 src/xenconfig/xen_sxpr.c   | 23 
 .../qemuxml2argv-tap-vhost-incorrect.xml   |  2 +-
 .../qemuxml2xmlout-tap-vhost-incorrect.xml |  2 +-
 20 files changed, 227 insertions(+), 132 deletions(-)

-- 
2.7.5

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