Re: [libvirt] [PATCH v2 RESEND 00/17] Introduce RDT memory bandwidth allocation support

[John Ferlan]

On 07/30/2018 11:54 PM, bing.niu wrote:
> 
> 
> On 2018年07月31日 06:14, John Ferlan wrote:
>>
>>
>> On 07/29/2018 11:12 PM, bing@intel.com wrote:
>>> From: Bing Niu 
>>>
>>> This series is to introduce RDT memory bandwidth allocation support
>>> by extending
>>> current virresctrl implementation.
> 
> []
>>> Bing Niu (17):
>>>    util: Rename some functions of virresctrl
>>>    util: Refactor virResctrlGetInfo in virresctrl
>>>    util: Refactor virResctrlAllocFormat of virresctrl
>>>    util: Add MBA capability information query to resctrl
>>>    util: Add MBA check to virResctrlInfoGetCache
>>>    util: Add MBA allocation to virresctrl
>>>    util: Add MBA schemata parse and format methods
>>>    util: Add support to calculate MBA utilization
>>>    util: Introduce virResctrlAllocForeachMemory
>>>    util: Introduce virResctrlAllocSetMemoryBandwidth
>>>    conf: Rename cachetune to resctrl
>>>    conf: Factor out vcpus parsing part from virDomainCachetuneDefParse
>>>    conf: Factor out vcpus overlapping from virDomainCachetuneDefParse
>>>    conf: Factor out virDomainResctrlDef update from
>>>  virDomainCachetuneDefParse
>>>    conf: Add support for memorytune XML processing for resctrl MBA
>>>    conf: Add return value check to virResctrlAllocForeachCache
>>>    conf: Add memory bandwidth allocation capability of host
>>>
>>>   docs/formatdomain.html.in  |  39 +-
>>>   docs/schemas/capability.rng    |  33 ++
>>>   docs/schemas/domaincommon.rng  |  17 +
>>>   src/conf/capabilities.c    | 107 
>>>   src/conf/capabilities.h    |  11 +
>>>   src/conf/domain_conf.c | 428
>>> ---
>>>   src/conf/domain_conf.h |  10 +-
>>>   src/libvirt_private.syms   |   6 +-
>>>   src/qemu/qemu_domain.c |   2 +-
>>>   src/qemu/qemu_process.c    |  18 +-
>>>   src/util/virresctrl.c  | 611
>>> +++--
>>>   src/util/virresctrl.h  |  55 +-
>>>   .../memorytune-colliding-allocs.xml    |  30 +
>>>   .../memorytune-colliding-cachetune.xml |  32 ++
>>>   tests/genericxml2xmlindata/memorytune.xml  |  33 ++
>>>   tests/genericxml2xmltest.c |   5 +
>>>   .../linux-resctrl/resctrl/info/MB/bandwidth_gran   |   1 +
>>>   .../linux-resctrl/resctrl/info/MB/min_bandwidth    |   1 +
>>>   .../linux-resctrl/resctrl/info/MB/num_closids  |   1 +
>>>   tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml   |   8 +
>>>   tests/virresctrldata/resctrl.schemata  |   1 +
>>>   21 files changed, 1280 insertions(+), 169 deletions(-)
>>>   create mode 100644
>>> tests/genericxml2xmlindata/memorytune-colliding-allocs.xml
>>>   create mode 100644
>>> tests/genericxml2xmlindata/memorytune-colliding-cachetune.xml
>>>   create mode 100644 tests/genericxml2xmlindata/memorytune.xml
>>>   create mode 100644
>>> tests/vircaps2xmldata/linux-resctrl/resctrl/info/MB/bandwidth_gran
>>>   create mode 100644
>>> tests/vircaps2xmldata/linux-resctrl/resctrl/info/MB/min_bandwidth
>>>   create mode 100644
>>> tests/vircaps2xmldata/linux-resctrl/resctrl/info/MB/num_closids
>>>
>>
>> Reviewed-by: John Ferlan 
>> (series)
>>
>> I'll push once the tree is open for 4.7.0 commits unless someone else
>> chimes in with other major issues that need to be addressed.
>>
> 

Tree re-opened after I left for a week away from the virtual world...
Now that I'm back and digging out of email, figured I'd sync this series
up with current top and push.

Please post a followup docs/news.xml article describing the change.

Tks -

John

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

Re: [libvirt] [PATCH 2/2] esx:Fix esxDomainGetMaxVcpus to return correct vcpus

[Matthias Bolte]

2018-08-10 5:56 GMT+02:00 Marcos Paulo de Souza :
> Before this change, esxDomainGetMaxVcpus returned -1, which in turn
> fails in libvirt. This commit reimplements esxDomainGetMaxVcpus instead
> of calling esxDomainGetVcpusFlags. The implementation checks for
> capability.maxSupportedVcpus, but as this one can be ommited in ESXi, we
> also check for capability.maxHostSupportedVcpus. With this change,
> virDomainSetVcpus, virDomainGetMaxVcpus and virDomainGetVcpusFlags and
> returning correct values.
>
> Signed-off-by: Marcos Paulo de Souza 
> ---
>  src/esx/esx_driver.c | 36 ++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index d5e8a7b4eb..3169314fa4 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -2581,8 +2581,40 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned 
> int flags)
>  static int
>  esxDomainGetMaxVcpus(virDomainPtr domain)
>  {
> -return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE |
> -   VIR_DOMAIN_VCPU_MAXIMUM));
> +esxPrivate *priv = domain->conn->privateData;
> +esxVI_String *propertyNameList = NULL;
> +esxVI_ObjectContent *hostSystem = NULL;
> +esxVI_Int *supportedVcpus = NULL;
> +esxVI_Int *hostVcpus = NULL;
> +
> +if (esxVI_EnsureSession(priv->primary) < 0)
> +return -1;
> +
> +priv->maxVcpus = -1;
> +
> +if (esxVI_String_AppendValueToList(,
> +   
> "capability.maxHostSupportedVcpus\0"
> +   "capability.maxSupportedVcpus"
> +  ) < 0 ||
> +esxVI_LookupHostSystemProperties(priv->primary, propertyNameList,
> +  ) < 0 ||
> +esxVI_GetInt(hostSystem, "capability.maxHostSupportedVcpus",
> +  , esxVI_Occurrence_RequiredItem) < 0 ||
> +esxVI_GetInt(hostSystem, "capability.maxSupportedVcpus",
> +  , esxVI_Occurrence_OptionalItem) < 0)
> +
> +goto cleanup;
> +
> +/* as maxSupportedVcpus is optional, check also for 
> maxHostSupportedVcpus */
> +priv->maxVcpus = supportedVcpus ? supportedVcpus->value : 
> hostVcpus->value;
> +
> + cleanup:
> +esxVI_String_Free();
> +esxVI_ObjectContent_Free();
> +esxVI_Int_Free();
> +esxVI_Int_Free();
> +
> +return priv->maxVcpus;
>  }

This is the wrong way to fix the situation. The correct way ist to
make esxDomainGetVcpusFlags handle the VIR_DOMAIN_VCPU_MAXIMUM flag
properly.

-- 
Matthias Bolte
http://photron.blogspot.com

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

Re: [libvirt] [PATCH] bhyve: fix process reconnect

[no-reply]

Hi,

This series was run against 'syntax-check' test by patchew.org, which failed, 
please find the details below:

Type: series
Message-id: 20180805155428.40866-1-bogorods...@gmail.com
Subject: [libvirt] [PATCH] bhyve: fix process reconnect

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
time bash -c './autogen.sh && make syntax-check'
=== TEST SCRIPT END ===

Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01
>From https://github.com/patchew-project/libvirt
 * [new tag]   patchew/cover.1534173734.git.pkre...@redhat.com -> 
patchew/cover.1534173734.git.pkre...@redhat.com
 * [new tag]   
patchew/e4c0c13ab8e42fa3a2571fc6f5ce303dd8fa4363.1534157377.git.mpriv...@redhat.com
 -> 
patchew/e4c0c13ab8e42fa3a2571fc6f5ce303dd8fa4363.1534157377.git.mpriv...@redhat.com
Switched to a new branch 'test'
f034446009 bhyve: fix process reconnect

=== OUTPUT BEGIN ===
Updating submodules...
Submodule 'gnulib' (https://git.savannah.gnu.org/git/gnulib.git/) registered 
for path '.gnulib'
Submodule 'keycodemapdb' (https://gitlab.com/keycodemap/keycodemapdb.git) 
registered for path 'src/keycodemapdb'
Cloning into '/var/tmp/patchew-tester-tmp-4mu61k2o/src/.gnulib'...
Cloning into '/var/tmp/patchew-tester-tmp-4mu61k2o/src/src/keycodemapdb'...
Submodule path '.gnulib': checked out '68df637b5f1b5c10370f6981d2a43a5cf74368df'
Submodule path 'src/keycodemapdb': checked out 
'16e5b0787687d8904dad2c026107409eb9bfcb95'
Running bootstrap...
./bootstrap: Bootstrapping from checked-out libvirt sources...
./bootstrap: consider installing git-merge-changelog from gnulib
./bootstrap: getting gnulib files...
running: libtoolize --install --copy
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/config.guess'
libtoolize: copying file 'build-aux/config.sub'
libtoolize: copying file 'build-aux/install-sh'
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
libtoolize: copying file 'm4/libtool.m4'
libtoolize: copying file 'm4/ltoptions.m4'
libtoolize: copying file 'm4/ltsugar.m4'
libtoolize: copying file 'm4/ltversion.m4'
libtoolize: copying file 'm4/lt~obsolete.m4'
./bootstrap: .gnulib/gnulib-tool--no-changelog   --aux-dir=build-aux   
--doc-base=doc   --lib=libgnu   --m4-base=m4/   --source-base=gnulib/lib/   
--tests-base=gnulib/tests   --local-dir=gnulib/local--lgpl=2 --with-tests 
--makefile-name=gnulib.mk --avoid=pt_chown --avoid=lock-tests   --libtool 
--import ...
Module list with included dependencies (indented):
absolute-header
  accept
accept-tests
alloca
alloca-opt
alloca-opt-tests
allocator
  areadlink
areadlink-tests
arpa_inet
arpa_inet-tests
assure
  autobuild
  base64
base64-tests
binary-io
binary-io-tests
  bind
bind-tests
  bitrotate
bitrotate-tests
btowc
btowc-tests
builtin-expect
  byteswap
byteswap-tests
  c-ctype
c-ctype-tests
  c-strcase
c-strcase-tests
  c-strcasestr
c-strcasestr-tests
  calloc-posix
  canonicalize-lgpl
canonicalize-lgpl-tests
careadlinkat
  chown
chown-tests
  clock-time
cloexec
cloexec-tests
  close
close-tests
  configmake
  connect
connect-tests
  count-leading-zeros
count-leading-zeros-tests
  count-one-bits
count-one-bits-tests
ctype
ctype-tests
  dirname-lgpl
dosname
double-slash-root
dup
dup-tests
dup2
dup2-tests
  environ
environ-tests
errno
errno-tests
error
  execinfo
exitfail
extensions
extern-inline
fatal-signal
  fclose
fclose-tests
  fcntl
  fcntl-h
fcntl-h-tests
fcntl-tests
fd-hook
  fdatasync
fdatasync-tests
fdopen
fdopen-tests
fflush
fflush-tests
  ffs
ffs-tests
  ffsl
ffsl-tests
fgetc-tests
filename
flexmember
float
float-tests
  fnmatch
fnmatch-tests
fpieee
fpucw
fpurge
fpurge-tests
fputc-tests
fread-tests
freading
freading-tests
fseek
fseek-tests
fseeko
fseeko-tests
fstat
fstat-tests
  fsync
fsync-tests
ftell
ftell-tests
ftello
ftello-tests
ftruncate
ftruncate-tests
  func
func-tests
fwrite-tests
  getaddrinfo
getaddrinfo-tests
  getcwd-lgpl
getcwd-lgpl-tests
getdelim
getdelim-tests
getdtablesize
getdtablesize-tests
getgroups
getgroups-tests
  gethostname
gethostname-tests
getline
getline-tests
  getopt-posix
getopt-posix-tests
getpagesize
  getpass
  getpeername
getpeername-tests
getprogname
getprogname-tests
  getsockname
getsockname-tests
getsockopt
getsockopt-tests
gettext-h
  gettimeofday
gettimeofday-tests
getugroups
  gitlog-to-changelog
  gnumakefile
grantpt
grantpt-tests
  

Re: [libvirt] [PATCH] esx: Fix build when libcurl debug is enabled

[Michal Prívozník]

On 08/10/2018 12:18 PM, Marcos Paulo de Souza wrote:
> On Mon, Aug 13, 2018 at 03:51:51PM +0200, Michal Prívozník wrote:
>> On 08/11/2018 04:39 PM, Marcos Paulo de Souza wrote:
>>> When building libvirt with libcurl debug enabled (with
>>> ESX_VI__CURL__ENABLE_DEBUG_OUTPUT set), the message bellow pops up:
>>>
>>> make[3]: Entering directory '/mnt/data/gitroot/libvirt/src'
>>>   CC   esx/libvirt_driver_esx_la-esx_vi.lo
>>> esx/esx_vi.c: In function 'esxVI_CURL_Debug':
>>> esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_SSL_DATA_IN' not 
>>> handled in switch [-Werror=switch-enum]
>>>  switch (type) {
>>>  ^~
>>> esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_SSL_DATA_OUT' not 
>>> handled in switch [-Werror=switch-enum]
>>> esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_END' not handled in 
>>> switch [-Werror=switch-enum]
>>>
>>> Our build requires at least libcurl 7.18.0, which is pretty stable since
>>> it was release in 2008. Fix this problem by handling the mentioned enums
>>> in the code.
>>>
>>> Signed-off-by: Marcos Paulo de Souza 
>>> ---
>>>  src/esx/esx_vi.c | 8 +++-
>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
>>> index a816c3a4f9..588670e137 100644
>>> --- a/src/esx/esx_vi.c
>>> +++ b/src/esx/esx_vi.c
>>> @@ -163,7 +163,7 @@ esxVI_CURL_WriteBuffer(char *data, size_t size, size_t 
>>> nmemb, void *userdata)
>>>  return 0;
>>>  }
>>>  
>>> -#define ESX_VI__CURL__ENABLE_DEBUG_OUTPUT 0
>>> +#define ESX_VI__CURL__ENABLE_DEBUG_OUTPUT 1
>>
>> The part below is fine. However, I'm not sure about this one ^^. This
>> turns curl debugging on by default. I'm not sure that is what we want.
> 
> You are right, this part was included by mistake. Would you want me to send a
> new version with this part removed?


No need. I've removed it just before pushing.

ACKed and pushed.

Michal

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

Re: [libvirt] [PATCH 4/4] apparmor: allow to preserve /dev mountpoints into qemu namespaces

[Jamie Strandboge]

On Mon, 2018-08-13 at 16:39 +0200, Christian Ehrhardt wrote:
> Libvirt now tries to preserve all mounts under /dev in qemu
> namespaces.
> The old rules only listed a set of known paths but those are no more
> enough.
> 
> I found some due to containers like /dev/.lxc/* and such but also
> /dev/console
> and /dev/net/tun.
> 
> Libvirt is correct to do so, but we can no more predict the names
> properly, so
> we modify the rule to allow a wildcard based pattern matching what
> libvirt does.
> 
> Signed-off-by: Christian Ehrhardt 
> ---
>  examples/apparmor/usr.sbin.libvirtd | 16 +---
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/examples/apparmor/usr.sbin.libvirtd
> b/examples/apparmor/usr.sbin.libvirtd
> index 3ff43c32a2..b2e38fe0ad 100644
> --- a/examples/apparmor/usr.sbin.libvirtd
> +++ b/examples/apparmor/usr.sbin.libvirtd
> @@ -33,17 +33,11 @@
>mount options=(rw,rslave)  -> /,
>mount options=(rw, nosuid) -> /{var/,}run/libvirt/qemu/*.dev/,
>  
> -  mount options=(rw, move) /dev/   ->
> /{var/,}run/libvirt/qemu/*.dev/,
> -  mount options=(rw, move) /dev/hugepages/ ->
> /{var/,}run/libvirt/qemu/*.hugepages/,
> -  mount options=(rw, move) /dev/mqueue/->
> /{var/,}run/libvirt/qemu/*.mqueue/,
> -  mount options=(rw, move) /dev/pts/   ->
> /{var/,}run/libvirt/qemu/*.pts/,
> -  mount options=(rw, move) /dev/shm/   ->
> /{var/,}run/libvirt/qemu/*.shm/,
> -
> -  mount options=(rw, move) /{var/,}run/libvirt/qemu/*.dev/   ->
> /dev/,
> -  mount options=(rw, move) /{var/,}run/libvirt/qemu/*.hugepages/ ->
> /dev/hugepages/,
> -  mount options=(rw, move) /{var/,}run/libvirt/qemu/*.mqueue/->
> /dev/mqueue/,
> -  mount options=(rw, move) /{var/,}run/libvirt/qemu/*.pts/   ->
> /dev/pts/,
> -  mount options=(rw, move) /{var/,}run/libvirt/qemu/*.shm/   ->
> /dev/shm/,
> +  # libvirt provides any mounts under /dev to qemu namespaces
> +  mount options=(rw, move) /dev/ -> /{var/,}run/libvirt/qemu/*.dev/,
> +  mount options=(rw, move) /dev/**{/,} ->
> /{var/,}run/libvirt/qemu/*{/,},

What are you trying to convey with this rule? As written, the '{/,}' is
redundant since '**' will match that.

> +  mount options=(rw, move) /{var/,}run/libvirt/qemu/*.dev/ -> /dev/,
> +  mount options=(rw, move) /{var/,}run/libvirt/qemu/*{/,} ->
> /dev/**{/,},

ditto

-- 
Jamie Strandboge | http://www.canonical.com

signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 02/11] util: netlink: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC

[John Ferlan]

On 08/13/2018 07:42 AM, Erik Skultety wrote:
> On Thu, Aug 09, 2018 at 09:42:10AM +0530, Sukrit Bhatnagar wrote:
>> Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
>> src/util/viralloc.h, define a new wrapper around an existing
>> cleanup function which will be called when a variable declared
>> with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant
>> viralloc.h include, since that has moved from the source module into
>> the header.
>>
>> This commit also typedefs virNlMsg to struct nl_msg type for use
>> with the cleanup macros.
>>
>> When a variable of type virNlMsg * is declared using VIR_AUTOPTR,
>> the function nlmsg_free will be run automatically on it when it
>> goes out of scope.
>>
>> Signed-off-by: Sukrit Bhatnagar 
>> ---
>>  src/util/virnetlink.c | 1 -
>>  src/util/virnetlink.h | 5 +
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
>> index 162efe6..ecf62c9 100644
>> --- a/src/util/virnetlink.c
>> +++ b/src/util/virnetlink.c
>> @@ -38,7 +38,6 @@
>>  #include "virnetlink.h"
>>  #include "virnetdev.h"
>>  #include "virlog.h"
>> -#include "viralloc.h"
>>  #include "virthread.h"
>>  #include "virmacaddr.h"
>>  #include "virerror.h"
>> diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
>> index 2a9de0a..8ebeab8 100644
>> --- a/src/util/virnetlink.h
>> +++ b/src/util/virnetlink.h
>> @@ -22,6 +22,7 @@
>>
>>  # include "internal.h"
>>  # include "virmacaddr.h"
>> +# include "viralloc.h"
>>
>>  # if defined(__linux__) && defined(HAVE_LIBNL)
>>
>> @@ -44,6 +45,8 @@ struct nlmsghdr;
>>
>>  # endif /* __linux__ */
>>
>> +typedef struct nl_msg virNlMsg;
> 
> Since the name of the module is virNetlink, I'll rename this to virNetlinkMsg
> and tweak all the affected places across the whole series.
> 
> Reviewed-by: Erik Skultety 
> 
>> +
>>  int virNetlinkStartup(void);
>>  void virNetlinkShutdown(void);
>>
>> @@ -123,4 +126,6 @@ int 
>> virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB,
>>  int virNetlinkEventRemoveClient(int watch, const virMacAddr *macaddr,
>>  unsigned int protocol);
>>
>> +VIR_DEFINE_AUTOPTR_FUNC(virNlMsg, nlmsg_free)
>> +

The freebsd builds are not very happy, for example from...

https://ci.centos.org/view/libvirt/job/libvirt-master-build/systems=libvirt-freebsd-11/1675/

One gets:

...
  CC   util/libvirt_nss_la-viratomic.lo
In file included from ../../src/network/bridge_driver.c:63:
../../src/util/virnetlink.h:129:40: error: use of undeclared identifier
'nlmsg_free'
VIR_DEFINE_AUTOPTR_FUNC(virNetlinkMsg, nlmsg_free)
   ^
1 error generated.


John

>>  #endif /* __VIR_NETLINK_H__ */
>> --
>> 1.8.3.1
>>
>> --
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

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

Re: [libvirt] [PATCH v3 00/11] use GNU C's cleanup attribute in src/util (batch III)

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:08AM +0530, Sukrit Bhatnagar wrote:
> This third series of patches also modifies a few files in src/util
> to use VIR_AUTOFREE and VIR_AUTOPTR for automatic freeing of memory
> and get rid of some VIR_FREE macro invocations and *Free function
> calls.
>
> This is meant as a follow-up of the v1 series [1] of the same batch,
> and contains those patches which were not (completely) pushed upstream.

So I pushed all the patches except for 7 and 8 (technically I could have gone
with 8 too, but there were lots of merge conflicts, so I figured it would be
easier to send it along with 7 again rather than risk a mistake because of a
rebase).

Erik

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

Re: [libvirt] [PATCH 1/2] esx: Make esxDomainGetVcpusFlags return vcpus again

[Matthias Bolte]

2018-08-10 5:56 GMT+02:00 Marcos Paulo de Souza :
> Before this patch, esxDomainGetVcpusFlags was returning -1 since
> "maxSupportedVcpus" can be NULL in ESXi[1]. In order to make it work,
> replicate the same behavior than esxDomainGetInfo that used
> config.hardware.numCPU to return the correct number of vcpus of a VM.
>
> This patch, together with the next one, makes the calls
> virDomainSetVcpus, virDomainGetMaxVcpus and virDomainGetVcpusFlags to
> return successfull again.
>
> [1]:https://pubs.vmware.com/vi-sdk/visdk250/ReferenceGuide/vim.host.Capability.html
>
> Signed-off-by: Marcos Paulo de Souza 
> ---
>  src/esx/esx_driver.c | 36 +++-
>  1 file changed, 11 insertions(+), 25 deletions(-)


Before and after this commit the function did not properly handle the
VIR_DOMAIN_VCPU_MAXIMUM flag. If the flag is present then the function
should return the maximum vCPU Count. If the flag is absent then it
should return the current vCPU count.

-- 
Matthias Bolte
http://photron.blogspot.com

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

Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable

[no-reply]

Hi,

This series was run against 'syntax-check' test by patchew.org, which failed, 
please find the details below:

Type: series
Message-id: 20180810112233.44540-1-i...@conquex.com
Subject: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
time bash -c './autogen.sh && make syntax-check'
=== TEST SCRIPT END ===

Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01
Switched to a new branch 'test'
e0ac4cf637 bhyve: Make LPC slot number configurable

=== OUTPUT BEGIN ===
Updating submodules...
Submodule 'gnulib' (https://git.savannah.gnu.org/git/gnulib.git/) registered 
for path '.gnulib'
Submodule 'keycodemapdb' (https://gitlab.com/keycodemap/keycodemapdb.git) 
registered for path 'src/keycodemapdb'
Cloning into '/var/tmp/patchew-tester-tmp-fryj5ja2/src/.gnulib'...
Cloning into '/var/tmp/patchew-tester-tmp-fryj5ja2/src/src/keycodemapdb'...
Submodule path '.gnulib': checked out '68df637b5f1b5c10370f6981d2a43a5cf74368df'
Submodule path 'src/keycodemapdb': checked out 
'16e5b0787687d8904dad2c026107409eb9bfcb95'
Running bootstrap...
./bootstrap: Bootstrapping from checked-out libvirt sources...
./bootstrap: consider installing git-merge-changelog from gnulib
./bootstrap: getting gnulib files...
running: libtoolize --install --copy
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/config.guess'
libtoolize: copying file 'build-aux/config.sub'
libtoolize: copying file 'build-aux/install-sh'
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
libtoolize: copying file 'm4/libtool.m4'
libtoolize: copying file 'm4/ltoptions.m4'
libtoolize: copying file 'm4/ltsugar.m4'
libtoolize: copying file 'm4/ltversion.m4'
libtoolize: copying file 'm4/lt~obsolete.m4'
./bootstrap: .gnulib/gnulib-tool--no-changelog   --aux-dir=build-aux   
--doc-base=doc   --lib=libgnu   --m4-base=m4/   --source-base=gnulib/lib/   
--tests-base=gnulib/tests   --local-dir=gnulib/local--lgpl=2 --with-tests 
--makefile-name=gnulib.mk --avoid=pt_chown --avoid=lock-tests   --libtool 
--import ...
Module list with included dependencies (indented):
absolute-header
  accept
accept-tests
alloca
alloca-opt
alloca-opt-tests
allocator
  areadlink
areadlink-tests
arpa_inet
arpa_inet-tests
assure
  autobuild
  base64
base64-tests
binary-io
binary-io-tests
  bind
bind-tests
  bitrotate
bitrotate-tests
btowc
btowc-tests
builtin-expect
  byteswap
byteswap-tests
  c-ctype
c-ctype-tests
  c-strcase
c-strcase-tests
  c-strcasestr
c-strcasestr-tests
  calloc-posix
  canonicalize-lgpl
canonicalize-lgpl-tests
careadlinkat
  chown
chown-tests
  clock-time
cloexec
cloexec-tests
  close
close-tests
  configmake
  connect
connect-tests
  count-leading-zeros
count-leading-zeros-tests
  count-one-bits
count-one-bits-tests
ctype
ctype-tests
  dirname-lgpl
dosname
double-slash-root
dup
dup-tests
dup2
dup2-tests
  environ
environ-tests
errno
errno-tests
error
  execinfo
exitfail
extensions
extern-inline
fatal-signal
  fclose
fclose-tests
  fcntl
  fcntl-h
fcntl-h-tests
fcntl-tests
fd-hook
  fdatasync
fdatasync-tests
fdopen
fdopen-tests
fflush
fflush-tests
  ffs
ffs-tests
  ffsl
ffsl-tests
fgetc-tests
filename
flexmember
float
float-tests
  fnmatch
fnmatch-tests
fpieee
fpucw
fpurge
fpurge-tests
fputc-tests
fread-tests
freading
freading-tests
fseek
fseek-tests
fseeko
fseeko-tests
fstat
fstat-tests
  fsync
fsync-tests
ftell
ftell-tests
ftello
ftello-tests
ftruncate
ftruncate-tests
  func
func-tests
fwrite-tests
  getaddrinfo
getaddrinfo-tests
  getcwd-lgpl
getcwd-lgpl-tests
getdelim
getdelim-tests
getdtablesize
getdtablesize-tests
getgroups
getgroups-tests
  gethostname
gethostname-tests
getline
getline-tests
  getopt-posix
getopt-posix-tests
getpagesize
  getpass
  getpeername
getpeername-tests
getprogname
getprogname-tests
  getsockname
getsockname-tests
getsockopt
getsockopt-tests
gettext-h
  gettimeofday
gettimeofday-tests
getugroups
  gitlog-to-changelog
  gnumakefile
grantpt
grantpt-tests
hard-locale
havelib
host-cpu-c-abi
hostent
  ignore-value
ignore-value-tests
include_next
inet_ntop
inet_ntop-tests
  inet_pton
inet_pton-tests
  intprops
intprops-tests
inttypes
inttypes-incomplete
inttypes-tests
  ioctl
ioctl-tests
  isatty
isatty-tests
isblank
isblank-tests

Re: [libvirt] [PATCH 3/4] apparmor: allow expected /tmp access patterns

[Jamie Strandboge]

On Mon, 2018-08-13 at 16:39 +0200, Christian Ehrhardt wrote:
> Several cases were found needing /tmp, for example ceph will try to
> list /tmp
> and the samba feature of qemu will place things in /tmp/qemu-smb.*.
> This is sort of safe because:
>  - While /tmp could contain anything it is not recommended to put
> critical
>data there anyway
>  - We restrict general access to only dir listing and reading of
> files owned
>(intentionally not the full power of user-tmp abstraction)
>  - While it would be hard to predict the PID as part of the string
> for the
>qemu smb feature (this is not exposed through XML so virt-aa-
> helper
>can't help) it is guarded by the "owner" statement and a pretty
> clear
>qemu-smb infix in the path.
> 
> Signed-off-by: Christian Ehrhardt 
> ---
>  examples/apparmor/libvirt-qemu | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/examples/apparmor/libvirt-qemu
> b/examples/apparmor/libvirt-qemu
> index 5caf14e418..c4f231b328 100644
> --- a/examples/apparmor/libvirt-qemu
> +++ b/examples/apparmor/libvirt-qemu
> @@ -180,6 +180,16 @@
># for rbd
>/etc/ceph/ceph.conf r,
>  
> +  # various functions will need /tmp (e.g. ceph), allow the base dir
> and a
> +  # few known functions.
> +  # we want to avoid to give blanket read or even write to
> everything under /tmp
> +  # so users are expected to add site specific addons for more
> uncommon cases.
> +  # allow only dir listing and owner based file read
> +  /{,var/}tmp/ r,
> +  owner /{,var/}tmp/**/ r,
> +  # allow qemu smb feature specific path with write access
> +  owner /tmp/qemu-smb.*/{,**} rw,

The actual rule additions are a compromise between security and
usability. Personally I'd prefer they not be there and let admins add
them or allow distros to patch them. That said, the comments and commit
message don't seem quite right for what you are adding. Eg, you mention
ceph, but there is no ceph rule and the profile comment doesn't mention
ceph only needs to list dirs; the profile comments are formatted a bit
weird too.

You mention 'sort of safe', but because of the '/{,var/}tmp/ r,' rule,
you are allowing guests to enumerate all the /tmp/qemu-smb.*
directories and then there is a 'rw' rule for that path. While this is
an 'owner' match, in practice, VMs all run under the same uid so this
means a rogue vm would be allowed to access files in these directories.
That said, I don't know what qemu is setting up in there-- so maybe it
is designed in such a way that this doesn't matter.

I'd much rather not call this 'sort of safe' but instead call out the
problem, justify why the rule should be there and perhaps add a TODO
that once smb is supported in domain xml that this rule will be added
conditionally.

+0 for rule inclusion provided the comments and commit message are
addressed. +1 if it can be demonstrated that qemu is handling those
dirs safely wrt vm isolation.

-- 
Jamie Strandboge | http://www.canonical.com

signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/2] esx: Fix {g,s}et vcpus

[no-reply]

Hi,

This series was run against 'syntax-check' test by patchew.org, which failed, 
please find the details below:

Type: series
Message-id: 20180810035658.13555-1-marcos.souza@gmail.com
Subject: [libvirt] [PATCH 0/2] esx: Fix {g,s}et vcpus

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
time bash -c './autogen.sh && make syntax-check'
=== TEST SCRIPT END ===

Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01
>From https://github.com/patchew-project/libvirt
   7995fecc25..d8826129fd  master -> master
Switched to a new branch 'test'
296fc25a8f esx:Fix esxDomainGetMaxVcpus to return correct vcpus
25939eab7b esx: Make esxDomainGetVcpusFlags return vcpus again

=== OUTPUT BEGIN ===
Updating submodules...
Submodule 'gnulib' (https://git.savannah.gnu.org/git/gnulib.git/) registered 
for path '.gnulib'
Submodule 'keycodemapdb' (https://gitlab.com/keycodemap/keycodemapdb.git) 
registered for path 'src/keycodemapdb'
Cloning into '/var/tmp/patchew-tester-tmp-75anuei6/src/.gnulib'...
Cloning into '/var/tmp/patchew-tester-tmp-75anuei6/src/src/keycodemapdb'...
Submodule path '.gnulib': checked out '68df637b5f1b5c10370f6981d2a43a5cf74368df'
Submodule path 'src/keycodemapdb': checked out 
'16e5b0787687d8904dad2c026107409eb9bfcb95'
Running bootstrap...
./bootstrap: Bootstrapping from checked-out libvirt sources...
./bootstrap: consider installing git-merge-changelog from gnulib
./bootstrap: getting gnulib files...
running: libtoolize --install --copy
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/config.guess'
libtoolize: copying file 'build-aux/config.sub'
libtoolize: copying file 'build-aux/install-sh'
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
libtoolize: copying file 'm4/libtool.m4'
libtoolize: copying file 'm4/ltoptions.m4'
libtoolize: copying file 'm4/ltsugar.m4'
libtoolize: copying file 'm4/ltversion.m4'
libtoolize: copying file 'm4/lt~obsolete.m4'
./bootstrap: .gnulib/gnulib-tool--no-changelog   --aux-dir=build-aux   
--doc-base=doc   --lib=libgnu   --m4-base=m4/   --source-base=gnulib/lib/   
--tests-base=gnulib/tests   --local-dir=gnulib/local--lgpl=2 --with-tests 
--makefile-name=gnulib.mk --avoid=pt_chown --avoid=lock-tests   --libtool 
--import ...
Module list with included dependencies (indented):
absolute-header
  accept
accept-tests
alloca
alloca-opt
alloca-opt-tests
allocator
  areadlink
areadlink-tests
arpa_inet
arpa_inet-tests
assure
  autobuild
  base64
base64-tests
binary-io
binary-io-tests
  bind
bind-tests
  bitrotate
bitrotate-tests
btowc
btowc-tests
builtin-expect
  byteswap
byteswap-tests
  c-ctype
c-ctype-tests
  c-strcase
c-strcase-tests
  c-strcasestr
c-strcasestr-tests
  calloc-posix
  canonicalize-lgpl
canonicalize-lgpl-tests
careadlinkat
  chown
chown-tests
  clock-time
cloexec
cloexec-tests
  close
close-tests
  configmake
  connect
connect-tests
  count-leading-zeros
count-leading-zeros-tests
  count-one-bits
count-one-bits-tests
ctype
ctype-tests
  dirname-lgpl
dosname
double-slash-root
dup
dup-tests
dup2
dup2-tests
  environ
environ-tests
errno
errno-tests
error
  execinfo
exitfail
extensions
extern-inline
fatal-signal
  fclose
fclose-tests
  fcntl
  fcntl-h
fcntl-h-tests
fcntl-tests
fd-hook
  fdatasync
fdatasync-tests
fdopen
fdopen-tests
fflush
fflush-tests
  ffs
ffs-tests
  ffsl
ffsl-tests
fgetc-tests
filename
flexmember
float
float-tests
  fnmatch
fnmatch-tests
fpieee
fpucw
fpurge
fpurge-tests
fputc-tests
fread-tests
freading
freading-tests
fseek
fseek-tests
fseeko
fseeko-tests
fstat
fstat-tests
  fsync
fsync-tests
ftell
ftell-tests
ftello
ftello-tests
ftruncate
ftruncate-tests
  func
func-tests
fwrite-tests
  getaddrinfo
getaddrinfo-tests
  getcwd-lgpl
getcwd-lgpl-tests
getdelim
getdelim-tests
getdtablesize
getdtablesize-tests
getgroups
getgroups-tests
  gethostname
gethostname-tests
getline
getline-tests
  getopt-posix
getopt-posix-tests
getpagesize
  getpass
  getpeername
getpeername-tests
getprogname
getprogname-tests
  getsockname
getsockname-tests
getsockopt
getsockopt-tests
gettext-h
  gettimeofday
gettimeofday-tests
getugroups
  gitlog-to-changelog
  gnumakefile
grantpt
grantpt-tests
hard-locale
havelib
host-cpu-c-abi
hostent
  ignore-value
ignore-value-tests
include_next
inet_ntop
inet_ntop-tests
  inet_pton
inet_pton-tests
  intprops
 

Re: [libvirt] [PATCH 1/4] cpu: allow include files for CPU definition

[John Ferlan]

On 08/01/2018 01:02 PM, Daniel P. Berrangé wrote:
> Allow for syntax
> 
> 
> 
> to reference other files in the CPU database directory
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  libvirt.spec.in   |  2 +-
>  mingw-libvirt.spec.in |  4 +--
>  src/Makefile.am   |  2 +-
>  src/cpu/cpu_map.c | 84 +--
>  4 files changed, 86 insertions(+), 6 deletions(-)
> 

I'll assume you got the spec/makefile magic correct as it's not in my
wheelhouse!



> diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c
> index d263eb8cdd..9e090919ed 100644
> --- a/src/cpu/cpu_map.c
> +++ b/src/cpu/cpu_map.c
> @@ -70,6 +70,83 @@ static int load(xmlXPathContextPtr ctxt,
>  return ret;
>  }
>  

[...]

> +
> +
> +static int loadIncludes(xmlXPathContextPtr ctxt,
> +cpuMapLoadCallback callback,
> +void *data)

static int
loadIncludes(...)

for consistency

> +{
> +int ret = -1;

[...]

>  
>  int cpuMapLoad(const char *arch,
> cpuMapLoadCallback cb,
> @@ -88,7 +165,7 @@ int cpuMapLoad(const char *arch,
>  PKGDATADIR)))
>  return -1;
>  
> -VIR_DEBUG("Loading CPU map from %s", mapfile);
> +VIR_DEBUG("Loading '%s' CPU map from %s", arch, mapfile);

Considering the subsequent NULL check:

s/arch/NULLSTR(arch)/

or move the VIR_DEBUG after the check (IDC).

I'm not even sure why @mapfile filling is above the argument validation
checks, but that's a different issue and since more changes are about to
come, it's not that important ;-)...  As long there is either a NULLSTR
or moved message, that's fine.

Reviewed-by: John Ferlan 

John

[...]

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

Re: [libvirt] [PATCH 4/4] cpu: split x86 map data into separate files

[John Ferlan]

On 08/01/2018 01:02 PM, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/cpu/cpu_map.xml  | 2374 +-
>  src/cpu/cpu_map_x86_486.xml  |7 +
>  src/cpu/cpu_map_x86_Broadwell-IBRS.xml   |   61 +
>  src/cpu/cpu_map_x86_Broadwell-noTSX-IBRS.xml |   59 +
>  src/cpu/cpu_map_x86_Broadwell-noTSX.xml  |   58 +
>  src/cpu/cpu_map_x86_Broadwell.xml|   60 +
>  src/cpu/cpu_map_x86_Conroe.xml   |   33 +
>  src/cpu/cpu_map_x86_EPYC-IBRS.xml|   73 +
>  src/cpu/cpu_map_x86_EPYC.xml |   72 +
>  src/cpu/cpu_map_x86_Haswell-IBRS.xml |   57 +
>  src/cpu/cpu_map_x86_Haswell-noTSX-IBRS.xml   |   55 +
>  src/cpu/cpu_map_x86_Haswell-noTSX.xml|   54 +
>  src/cpu/cpu_map_x86_Haswell.xml  |   56 +
>  src/cpu/cpu_map_x86_IvyBridge-IBRS.xml   |   51 +
>  src/cpu/cpu_map_x86_IvyBridge.xml|   50 +
>  src/cpu/cpu_map_x86_Nehalem-IBRS.xml |   38 +
>  src/cpu/cpu_map_x86_Nehalem.xml  |   37 +
>  src/cpu/cpu_map_x86_Opteron_G1.xml   |   31 +
>  src/cpu/cpu_map_x86_Opteron_G2.xml   |   35 +
>  src/cpu/cpu_map_x86_Opteron_G3.xml   |   40 +
>  src/cpu/cpu_map_x86_Opteron_G4.xml   |   50 +
>  src/cpu/cpu_map_x86_Opteron_G5.xml   |   53 +
>  src/cpu/cpu_map_x86_Penryn.xml   |   35 +
>  src/cpu/cpu_map_x86_SandyBridge-IBRS.xml |   45 +
>  src/cpu/cpu_map_x86_SandyBridge.xml  |   44 +
>  src/cpu/cpu_map_x86_Skylake-Client-IBRS.xml  |   70 +
>  src/cpu/cpu_map_x86_Skylake-Client.xml   |   69 +
>  src/cpu/cpu_map_x86_Skylake-Server-IBRS.xml  |   77 +
>  src/cpu/cpu_map_x86_Skylake-Server.xml   |   76 +
>  src/cpu/cpu_map_x86_Westmere-IBRS.xml|   39 +
>  src/cpu/cpu_map_x86_Westmere.xml |   38 +
>  src/cpu/cpu_map_x86_athlon.xml   |   28 +
>  src/cpu/cpu_map_x86_core2duo.xml |   33 +
>  src/cpu/cpu_map_x86_coreduo.xml  |   29 +
>  src/cpu/cpu_map_x86_cpu64-rhel5.xml  |   29 +
>  src/cpu/cpu_map_x86_cpu64-rhel6.xml  |   31 +
>  src/cpu/cpu_map_x86_features.xml |  440 
>  src/cpu/cpu_map_x86_kvm32.xml|   26 +
>  src/cpu/cpu_map_x86_kvm64.xml|   30 +
>  src/cpu/cpu_map_x86_n270.xml |   30 +
>  src/cpu/cpu_map_x86_pentium.xml  |   13 +
>  src/cpu/cpu_map_x86_pentium2.xml |   22 +
>  src/cpu/cpu_map_x86_pentium3.xml |   23 +
>  src/cpu/cpu_map_x86_pentiumpro.xml   |   21 +
>  src/cpu/cpu_map_x86_phenom.xml   |   36 +
>  src/cpu/cpu_map_x86_qemu32.xml   |   22 +
>  src/cpu/cpu_map_x86_qemu64.xml   |   40 +
>  src/cpu/cpu_map_x86_vendors.xml  |4 +
>  48 files changed, 2427 insertions(+), 2327 deletions(-)
>  create mode 100644 src/cpu/cpu_map_x86_486.xml
>  create mode 100644 src/cpu/cpu_map_x86_Broadwell-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_Broadwell-noTSX-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_Broadwell-noTSX.xml
>  create mode 100644 src/cpu/cpu_map_x86_Broadwell.xml
>  create mode 100644 src/cpu/cpu_map_x86_Conroe.xml
>  create mode 100644 src/cpu/cpu_map_x86_EPYC-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_EPYC.xml
>  create mode 100644 src/cpu/cpu_map_x86_Haswell-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_Haswell-noTSX-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_Haswell-noTSX.xml
>  create mode 100644 src/cpu/cpu_map_x86_Haswell.xml
>  create mode 100644 src/cpu/cpu_map_x86_IvyBridge-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_IvyBridge.xml
>  create mode 100644 src/cpu/cpu_map_x86_Nehalem-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_Nehalem.xml
>  create mode 100644 src/cpu/cpu_map_x86_Opteron_G1.xml
>  create mode 100644 src/cpu/cpu_map_x86_Opteron_G2.xml
>  create mode 100644 src/cpu/cpu_map_x86_Opteron_G3.xml
>  create mode 100644 src/cpu/cpu_map_x86_Opteron_G4.xml
>  create mode 100644 src/cpu/cpu_map_x86_Opteron_G5.xml
>  create mode 100644 src/cpu/cpu_map_x86_Penryn.xml
>  create mode 100644 src/cpu/cpu_map_x86_SandyBridge-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_SandyBridge.xml
>  create mode 100644 src/cpu/cpu_map_x86_Skylake-Client-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_Skylake-Client.xml
>  create mode 100644 src/cpu/cpu_map_x86_Skylake-Server-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_Skylake-Server.xml
>  create mode 100644 src/cpu/cpu_map_x86_Westmere-IBRS.xml
>  create mode 100644 src/cpu/cpu_map_x86_Westmere.xml
>  create mode 100644 src/cpu/cpu_map_x86_athlon.xml
>  create mode 100644 src/cpu/cpu_map_x86_core2duo.xml
>  create mode 100644 src/cpu/cpu_map_x86_coreduo.xml
>  create mode 100644 src/cpu/cpu_map_x86_cpu64-rhel5.xml
>  create mode 100644 src/cpu/cpu_map_x86_cpu64-rhel6.xml
>  create mode 100644 src/cpu/cpu_map_x86_features.xml
>  

Re: [libvirt] [PATCH 3/4] cpu: split PPC64 map data into separate files

[John Ferlan]

On 08/01/2018 01:02 PM, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/cpu/cpu_map.xml | 41 +
>  src/cpu/cpu_map_ppc64_POWER6.xml|  6 
>  src/cpu/cpu_map_ppc64_POWER7.xml|  7 +
>  src/cpu/cpu_map_ppc64_POWER8.xml|  8 +
>  src/cpu/cpu_map_ppc64_POWER9.xml|  6 
>  src/cpu/cpu_map_ppc64_POWERPC_e5500.xml |  6 
>  src/cpu/cpu_map_ppc64_POWERPC_e6500.xml |  6 
>  src/cpu/cpu_map_ppc64_vendors.xml   |  4 +++
>  8 files changed, 50 insertions(+), 34 deletions(-)
>  create mode 100644 src/cpu/cpu_map_ppc64_POWER6.xml
>  create mode 100644 src/cpu/cpu_map_ppc64_POWER7.xml
>  create mode 100644 src/cpu/cpu_map_ppc64_POWER8.xml
>  create mode 100644 src/cpu/cpu_map_ppc64_POWER9.xml
>  create mode 100644 src/cpu/cpu_map_ppc64_POWERPC_e5500.xml
>  create mode 100644 src/cpu/cpu_map_ppc64_POWERPC_e6500.xml
>  create mode 100644 src/cpu/cpu_map_ppc64_vendors.xml
> 

Reviewed-by: John Ferlan 

John

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

Re: [libvirt] [PATCH 2/4] cpu: push more parsing logic into common code

[John Ferlan]

On 08/01/2018 01:02 PM, Daniel P. Berrangé wrote:
> The x86 and ppc impls both duplicate some logic when parsing CPU
> features. Change the callback signature so that this duplication can be
> pushed up a level to common code.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/cpu/cpu_map.c   | 106 +++-
>  src/cpu/cpu_map.h   |  22 ++---
>  src/cpu/cpu_ppc64.c | 112 ++---
>  src/cpu/cpu_x86.c   | 196 +---
>  4 files changed, 155 insertions(+), 281 deletions(-)
> 
> diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c
> index 9e090919ed..17ed53fda6 100644
> --- a/src/cpu/cpu_map.c
> +++ b/src/cpu/cpu_map.c
> @@ -35,31 +35,51 @@
>  
>  VIR_LOG_INIT("cpu.cpu_map");
>  
> -VIR_ENUM_IMPL(cpuMapElement, CPU_MAP_ELEMENT_LAST,
> -"vendor",
> -"feature",
> -"model")
> -
> -
> -static int load(xmlXPathContextPtr ctxt,
> -cpuMapElement element,
> -cpuMapLoadCallback callback,
> -void *data)
> +static int
> +loadData(const char *mapfile,
> + xmlXPathContextPtr ctxt,
> + const char *xpath,
> + cpuMapLoadCallback callback,
> + void *data)
>  {
>  int ret = -1;
>  xmlNodePtr ctxt_node;
>  xmlNodePtr *nodes = NULL;
>  int n;
> +size_t i;
> +int rv;
>  
>  ctxt_node = ctxt->node;
>  
> -n = virXPathNodeSet(cpuMapElementTypeToString(element), ctxt, );
> -if (n < 0)
> +n = virXPathNodeSet(xpath, ctxt, );
> +if (n < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("cannot find '%s' in CPU map '%s'"), xpath, 
> mapfile);
>  goto cleanup;
> +}
>  
> -if (n > 0 &&
> -callback(element, ctxt, nodes, n, data) < 0)
> +if (n > 0 && !callback) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Unexpected %s in CPU map '%s'"), xpath, mapfile);

How about "Unexpected element %s..."

to be fair it's a callback function isn't provided for a specific arch,
but adding that level of detail would be a bit more painful for the low
level of benefit at least.

>  goto cleanup;
> +}
> +

[...]

>  
>  int cpuMapLoad(const char *arch,
> -   cpuMapLoadCallback cb,
> +   cpuMapLoadCallback vendorCB,
> +   cpuMapLoadCallback featureCB,
> +   cpuMapLoadCallback modelCB,
> void *data)
>  {
>  xmlDocPtr xml = NULL;
> @@ -157,7 +183,6 @@ int cpuMapLoad(const char *arch,
>  virBuffer buf = VIR_BUFFER_INITIALIZER;
>  char *xpath = NULL;
>  int ret = -1;
> -int element;
>  char *mapfile;
>  
>  if (!(mapfile = virFileFindResource("cpu_map.xml",
> @@ -173,9 +198,15 @@ int cpuMapLoad(const char *arch,
>  goto cleanup;
>  }
>  
> -if (cb == NULL) {
> +if (vendorCB == NULL) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   "%s", _("no vendor callback provided"));
> +goto cleanup;
> +}

To be fair, loadData does check "if (n > 0 && !callback)", so these
checks perhaps aren't necessary as if they were NULL we'd fail a bit
later on (if I'm reading things properly ;-)).

I suppose it doesn't matter if they stay or not until someone, some day
wonders why featureCB isn't checked.

> +
> +if (modelCB == NULL) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> -   "%s", _("no callback provided"));
> +   "%s", _("no model callback provided"));
>  goto cleanup;
>  }
>  

[...]

> diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
> index d562677fa3..75da5b77d8 100644
> --- a/src/cpu/cpu_ppc64.c
> +++ b/src/cpu/cpu_ppc64.c
> @@ -281,21 +281,19 @@ ppc64MapFree(struct ppc64_map *map)
>  VIR_FREE(map);
>  }
>  

[...]

>  ppc64ModelParse(xmlXPathContextPtr ctxt,
> -struct ppc64_map *map)
> +const char *name,
> +void *data)
>  {
> +struct ppc64_map *map = data;
>  struct ppc64_model *model;
>  xmlNodePtr *nodes = NULL;
>  char *vendor = NULL;
>  unsigned long pvr;
>  size_t i;
>  int n;
> +int ret = -1;
>  
>  if (VIR_ALLOC(model) < 0)
>  goto error;
>  
> -model->name = virXPathString("string(@name)", ctxt);
> -if (!model->name) {
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> -   "%s", _("Missing CPU model name"));
> +if (VIR_STRDUP(model->name, name) < 0)
>  goto error;
> -}
>  
>  if (ppc64ModelFind(map, model->name)) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -410,63 +387,22 @@ ppc64ModelParse(xmlXPathContextPtr ctxt,
>  model->data.pvr[i].mask = pvr;
>  }
>  
> +if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0)
> +goto error;
> +

Since VIR_APPEND_ELEMENT would clear @model on success, we don't
necessarily need the error and cleanup labels. More 

Re: [libvirt] [PATCH v3 00/12] PCI passthrough support on s390

[Yi Min Zhao]

在 2018/8/13 下午2:59, Cornelia Huck 写道:

On Mon, 13 Aug 2018 12:46:16 +0800
Yi Min Zhao  wrote:


Is there any comment? I expect comments from all of you.

Well, I don't have any objections from my side, but you need the
libvirt folks' opinion on this.

Thanks for your response!




在 2018/8/7 下午5:10, Yi Min Zhao 写道:

Abstract

The PCI representation in QEMU has recently been extended for S390
allowing configuration of zPCI attributes like uid (user-defined
identifier) and fid (PCI function identifier).
The details can be found here:
https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07262.html

To support the new zPCI feature of the S390 platform, two new XML
attributes, @uid and @fid, are introduced for device addresses of type
'pci', i.e.:

  
  

  
  


uid and fid are optional attributes. If they are defined by the user,
unique values within the guest domain must be used. If they are not
specified and the architecture requires them, they are automatically
generated with non-conflicting values.

Current implementation is the most seamless one for the user as it
unites the address specific data of a PCI device on one XML element.
It could accommodate both specifying our special parameters (uid and fid)
and re-using standard statements (domain, bus, slot and function) for
PCI devices. User can still specify bus/slot/function for the virtualized
PCI devices in the XML.

Thus uid/fid act as an extension to the PCI address and are stored in
a new structure 'virZPCIDeviceAddress' which is a member of common PCI
Address structure. Additionally, two hashtables are used for assignment
and reservation of uid/fid.

In support of extending the PCI address, a new PCI address extension flag is
introduced. This extension flag allows is not only dedicated for the S390
platform but also other architectures needing certain extensions to PCI
address space.

Code Base
=
commit in master:
087de2f5a3: docs: formatdomain: fix spacing before parentheses

Change Log
==
v2->v3:
1. Revise code style.
2. Update test cases.
3. Introduce qemuDomainCollectPCIAddressExtension() to collect PCI
 extension addresses.
4. Introduce virDeviceInfoPCIAddressExtensionPresent() to check if zPCI
 address exists.
5. Optimize zPCI address check logic.
6. Optimize passed parameters of zPCI addr alloc/release/reserve functions.
7. Report enum range error in qemuDomainDeviceSupportZPCI().
8. Update commit messages.

v1->v2:
1. Separate test commit and merge testcases into corresponding commits that
 introduce the functionalities firstly.
2. Spare some checks for zpci device.
3. Add vsock and controller support.
4. Add uin32 type schema.
5. Rename zpciuid and zpcifid to zpci_uid and zpci_fid.
6. Always return multibus support on S390.

Yi Min Zhao (12):
conf: Add definitions for 'uid' and 'fid' PCI address attributes
qemu: Introduce zPCI capability
conf: Introduce a new PCI address extension flag
qemu: Enable PCI multi bus for S390 guests
qemu: Auto add pci-root for s390/s390x guests
conf: Introduce address caching for PCI extensions
conf: Introduce parser, formatter for uid and fid
conf: Allocate/release 'uid' and 'fid' in PCI address
qemu: Generate and use zPCI device in QEMU command line
qemu: Add hotpluging support for PCI devices on S390 guests
docs: Add 'uid' and 'fid' information
news: Update news for PCI address extension attributes

   docs/formatdomain.html.in  |   9 +-
   docs/news.xml  |  11 +
   docs/schemas/basictypes.rng|  23 ++
   docs/schemas/domaincommon.rng  |   1 +
   src/conf/device_conf.c |  78 +
   src/conf/device_conf.h |   8 +
   src/conf/domain_addr.c | 379 
+
   src/conf/domain_addr.h |  29 ++
   src/conf/domain_conf.c |   6 +
   src/libvirt_private.syms   |   4 +
   src/qemu/qemu_capabilities.c   |   6 +
   src/qemu/qemu_capabilities.h   |   1 +
   src/qemu/qemu_command.c| 114 +++
   src/qemu/qemu_command.h|   4 +
   src/qemu/qemu_domain.c |   1 +
   src/qemu/qemu_domain_address.c | 204 ++-
   src/qemu/qemu_hotplug.c| 155 -
   src/util/virpci.h  |  13 +
   tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml   |   1 +
   tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   |   1 +
   tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   |   1 +
   tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml|   1 +
   tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml|   1 +
   

Re: [libvirt] [PATCH v2 RESEND 00/17] Introduce RDT memory bandwidth allocation support

[bing.niu]

On 2018年08月14日 02:33, John Ferlan wrote:



On 07/30/2018 11:54 PM, bing.niu wrote:



On 2018年07月31日 06:14, John Ferlan wrote:



On 07/29/2018 11:12 PM, bing@intel.com wrote:

From: Bing Niu 

This series is to introduce RDT memory bandwidth allocation support
by extending
current virresctrl implementation.


[]

Bing Niu (17):
    util: Rename some functions of virresctrl
    util: Refactor virResctrlGetInfo in virresctrl
    util: Refactor virResctrlAllocFormat of virresctrl
    util: Add MBA capability information query to resctrl
    util: Add MBA check to virResctrlInfoGetCache
    util: Add MBA allocation to virresctrl
    util: Add MBA schemata parse and format methods
    util: Add support to calculate MBA utilization
    util: Introduce virResctrlAllocForeachMemory
    util: Introduce virResctrlAllocSetMemoryBandwidth
    conf: Rename cachetune to resctrl
    conf: Factor out vcpus parsing part from virDomainCachetuneDefParse
    conf: Factor out vcpus overlapping from virDomainCachetuneDefParse
    conf: Factor out virDomainResctrlDef update from
  virDomainCachetuneDefParse
    conf: Add support for memorytune XML processing for resctrl MBA
    conf: Add return value check to virResctrlAllocForeachCache
    conf: Add memory bandwidth allocation capability of host

   docs/formatdomain.html.in  |  39 +-
   docs/schemas/capability.rng    |  33 ++
   docs/schemas/domaincommon.rng  |  17 +
   src/conf/capabilities.c    | 107 
   src/conf/capabilities.h    |  11 +
   src/conf/domain_conf.c | 428
---
   src/conf/domain_conf.h |  10 +-
   src/libvirt_private.syms   |   6 +-
   src/qemu/qemu_domain.c |   2 +-
   src/qemu/qemu_process.c    |  18 +-
   src/util/virresctrl.c  | 611
+++--
   src/util/virresctrl.h  |  55 +-
   .../memorytune-colliding-allocs.xml    |  30 +
   .../memorytune-colliding-cachetune.xml |  32 ++
   tests/genericxml2xmlindata/memorytune.xml  |  33 ++
   tests/genericxml2xmltest.c |   5 +
   .../linux-resctrl/resctrl/info/MB/bandwidth_gran   |   1 +
   .../linux-resctrl/resctrl/info/MB/min_bandwidth    |   1 +
   .../linux-resctrl/resctrl/info/MB/num_closids  |   1 +
   tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml   |   8 +
   tests/virresctrldata/resctrl.schemata  |   1 +
   21 files changed, 1280 insertions(+), 169 deletions(-)
   create mode 100644
tests/genericxml2xmlindata/memorytune-colliding-allocs.xml
   create mode 100644
tests/genericxml2xmlindata/memorytune-colliding-cachetune.xml
   create mode 100644 tests/genericxml2xmlindata/memorytune.xml
   create mode 100644
tests/vircaps2xmldata/linux-resctrl/resctrl/info/MB/bandwidth_gran
   create mode 100644
tests/vircaps2xmldata/linux-resctrl/resctrl/info/MB/min_bandwidth
   create mode 100644
tests/vircaps2xmldata/linux-resctrl/resctrl/info/MB/num_closids



Reviewed-by: John Ferlan 
(series)

I'll push once the tree is open for 4.7.0 commits unless someone else
chimes in with other major issues that need to be addressed.





Tree re-opened after I left for a week away from the virtual world...
Now that I'm back and digging out of email, figured I'd sync this series
up with current top and push.

Please post a followup docs/news.xml article describing the change.


Thanks for this. I will cook a patch for this. :)


Tks -

John



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

[libvirt] [PATCH] news: Add support for MBA (Memory Bandwidth Allocation)

[bing . niu]

From: Bing Niu 

Signed-off-by: Bing Niu 
---
 docs/news.xml | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 2f0c010..c6d03f5 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -44,6 +44,15 @@
   iscsiadm. It support basic pool operations: checkPool and 
refreshPool.
 
   
+  
+
+  Add support for MBA (Memory Bandwidth Allocation technology)
+
+
+  Domain vCPU threads can now have allocated some parts of host memory
+  bandwidth by using the memorytune element in 
cputune.
+
+  
 
 
   
-- 
2.7.4

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

Re: [libvirt] [PATCH v3 00/12] PCI passthrough support on s390

[Cornelia Huck]

On Mon, 13 Aug 2018 12:46:16 +0800
Yi Min Zhao  wrote:

> Is there any comment? I expect comments from all of you.

Well, I don't have any objections from my side, but you need the
libvirt folks' opinion on this.

> 
> 
> 在 2018/8/7 下午5:10, Yi Min Zhao 写道:
> > Abstract
> > 
> > The PCI representation in QEMU has recently been extended for S390
> > allowing configuration of zPCI attributes like uid (user-defined
> > identifier) and fid (PCI function identifier).
> > The details can be found here:
> > https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07262.html
> >
> > To support the new zPCI feature of the S390 platform, two new XML
> > attributes, @uid and @fid, are introduced for device addresses of type
> > 'pci', i.e.:
> >
> >  
> >  
> >
> >  
> >   > function='0x0'
> >uid='0x0003' fid='0x0027'/>
> >
> >
> > uid and fid are optional attributes. If they are defined by the user,
> > unique values within the guest domain must be used. If they are not
> > specified and the architecture requires them, they are automatically
> > generated with non-conflicting values.
> >
> > Current implementation is the most seamless one for the user as it
> > unites the address specific data of a PCI device on one XML element.
> > It could accommodate both specifying our special parameters (uid and fid)
> > and re-using standard statements (domain, bus, slot and function) for
> > PCI devices. User can still specify bus/slot/function for the virtualized
> > PCI devices in the XML.
> >
> > Thus uid/fid act as an extension to the PCI address and are stored in
> > a new structure 'virZPCIDeviceAddress' which is a member of common PCI
> > Address structure. Additionally, two hashtables are used for assignment
> > and reservation of uid/fid.
> >
> > In support of extending the PCI address, a new PCI address extension flag is
> > introduced. This extension flag allows is not only dedicated for the S390
> > platform but also other architectures needing certain extensions to PCI
> > address space.
> >
> > Code Base
> > =
> > commit in master:
> > 087de2f5a3: docs: formatdomain: fix spacing before parentheses
> >
> > Change Log
> > ==
> > v2->v3:
> > 1. Revise code style.
> > 2. Update test cases.
> > 3. Introduce qemuDomainCollectPCIAddressExtension() to collect PCI
> > extension addresses.
> > 4. Introduce virDeviceInfoPCIAddressExtensionPresent() to check if zPCI
> > address exists.
> > 5. Optimize zPCI address check logic.
> > 6. Optimize passed parameters of zPCI addr alloc/release/reserve functions.
> > 7. Report enum range error in qemuDomainDeviceSupportZPCI().
> > 8. Update commit messages.
> >
> > v1->v2:
> > 1. Separate test commit and merge testcases into corresponding commits that
> > introduce the functionalities firstly.
> > 2. Spare some checks for zpci device.
> > 3. Add vsock and controller support.
> > 4. Add uin32 type schema.
> > 5. Rename zpciuid and zpcifid to zpci_uid and zpci_fid.
> > 6. Always return multibus support on S390.
> >
> > Yi Min Zhao (12):
> >conf: Add definitions for 'uid' and 'fid' PCI address attributes
> >qemu: Introduce zPCI capability
> >conf: Introduce a new PCI address extension flag
> >qemu: Enable PCI multi bus for S390 guests
> >qemu: Auto add pci-root for s390/s390x guests
> >conf: Introduce address caching for PCI extensions
> >conf: Introduce parser, formatter for uid and fid
> >conf: Allocate/release 'uid' and 'fid' in PCI address
> >qemu: Generate and use zPCI device in QEMU command line
> >qemu: Add hotpluging support for PCI devices on S390 guests
> >docs: Add 'uid' and 'fid' information
> >news: Update news for PCI address extension attributes
> >
> >   docs/formatdomain.html.in  |   9 +-
> >   docs/news.xml  |  11 +
> >   docs/schemas/basictypes.rng|  23 ++
> >   docs/schemas/domaincommon.rng  |   1 +
> >   src/conf/device_conf.c |  78 +
> >   src/conf/device_conf.h |   8 +
> >   src/conf/domain_addr.c | 379 
> > +
> >   src/conf/domain_addr.h |  29 ++
> >   src/conf/domain_conf.c |   6 +
> >   src/libvirt_private.syms   |   4 +
> >   src/qemu/qemu_capabilities.c   |   6 +
> >   src/qemu/qemu_capabilities.h   |   1 +
> >   src/qemu/qemu_command.c| 114 +++
> >   src/qemu/qemu_command.h|   4 +
> >   src/qemu/qemu_domain.c |   1 +
> >   src/qemu/qemu_domain_address.c | 204 ++-
> >   src/qemu/qemu_hotplug.c| 155 -
> >   src/util/virpci.h  | 

[libvirt] [RFC 2/2] introduce timeout mode in virKModLoad

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/Makefile.am   |  4 +++-
 src/util/virkmod.c| 43 ++-
 src/virt-command.conf | 10 ++
 3 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 src/virt-command.conf

diff --git a/src/Makefile.am b/src/Makefile.am
index 61876cf..e0af476 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -150,7 +150,7 @@ lib_LTLIBRARIES = libvirt.la libvirt-qemu.la libvirt-lxc.la
 moddir = $(libdir)/libvirt/connection-driver
 
 confdir = $(sysconfdir)/libvirt
-conf_DATA += libvirt.conf libvirt-admin.conf
+conf_DATA += libvirt.conf libvirt-admin.conf virt-command.conf
 
 augeasdir = $(datadir)/augeas/lenses
 
@@ -952,6 +952,8 @@ libvirt_nss_la_SOURCES = \
util/virbuffer.h \
util/vircommand.c \
util/vircommand.h \
+   util/virconf.c \
+   util/virconf.h \
util/virerror.c \
util/virerror.h \
util/virfile.c \
diff --git a/src/util/virkmod.c b/src/util/virkmod.c
index 9d0375b..99cce4c 100644
--- a/src/util/virkmod.c
+++ b/src/util/virkmod.c
@@ -24,12 +24,45 @@
 #include "virkmod.h"
 #include "vircommand.h"
 #include "virstring.h"
+#include "virthread.h"
+#include "virconf.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+/*
+ * Kernel module load timeout (milliseconds)
+ * >0, the number of milliseconds before loading expires
+ * =0, waiting indefinitely
+ */
+static int virKModLoadTimeout = 3*1000;
+
+static int virKModOnceInit(void)
+{
+int timeout = 0;
+virConfPtr conf = NULL;
+if (virConfLoadConfig(, "virt-command.conf") < 0)
+goto cleanup;
+if (virConfGetValueInt(conf, "kmod_load_timeout", ) <= 0)
+goto cleanup;
+
+if (timeout >= 0)
+virKModLoadTimeout = timeout;
+
+ cleanup:
+virConfFree(conf);
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virKMod)
 
 static int
 doModprobe(const char *opts, const char *module, char **outbuf, char **errbuf)
 {
 VIR_AUTOPTR(virCommand) cmd = NULL;
 
+if (virKModInitialize() < 0)
+return -1;
+
 cmd = virCommandNew(MODPROBE);
 if (opts)
 virCommandAddArg(cmd, opts);
@@ -40,8 +73,16 @@ doModprobe(const char *opts, const char *module, char 
**outbuf, char **errbuf)
 if (errbuf)
 virCommandSetErrorBuffer(cmd, errbuf);
 
-if (virCommandRun(cmd, NULL) < 0)
+if (virKModLoadTimeout > 0)
+virCommandSetTimeout(cmd, virKModLoadTimeout);
+
+if (virCommandRun(cmd, NULL) < 0) {
+if (virCommandGetErr(cmd) == ETIME)
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("%s loading timeout. Check kmod_load_timeout from 
virt-command.conf"),
+   module);
 return -1;
+}
 
 return 0;
 }
diff --git a/src/virt-command.conf b/src/virt-command.conf
new file mode 100644
index 000..d88d24a
--- /dev/null
+++ b/src/virt-command.conf
@@ -0,0 +1,10 @@
+#
+# This can be used to set timeout for kernel module loading internally
+#
+# >0, the number of milliseconds for loading timeout
+# =0, waiting indefinitely until loading finished, i.e. no timeout.
+#
+# The default 3 seconds is adequate since most kernel module loading
+# with microsecond latency or millisecond latency
+#
+# kmod_load_timeout = 3000
-- 
2.7.4

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

[libvirt] [PATCH 13/16] Revert "Remove functions using yajl"

[Ján Tomko]

This reverts commit bf114decb34f21cd225ead6dc4d929d35a8c5fe5.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 src/util/virjson.c | 529 -
 1 file changed, 528 insertions(+), 1 deletion(-)

diff --git a/src/util/virjson.c b/src/util/virjson.c
index 80274bc6c5..608ba85d67 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -29,6 +29,22 @@
 #include "virstring.h"
 #include "virutil.h"
 
+#if WITH_YAJL
+# include 
+# include 
+
+# ifdef WITH_YAJL2
+#  define yajl_size_t size_t
+#  define VIR_YAJL_STATUS_OK(status) ((status) == yajl_status_ok)
+# else
+#  define yajl_size_t unsigned int
+#  define yajl_complete_parse yajl_parse_complete
+#  define VIR_YAJL_STATUS_OK(status) \
+((status) == yajl_status_ok || (status) == yajl_status_insufficient_data)
+# endif
+
+#endif
+
 /* XXX fixme */
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -72,6 +88,23 @@ struct _virJSONValue {
 };
 
 
+typedef struct _virJSONParserState virJSONParserState;
+typedef virJSONParserState *virJSONParserStatePtr;
+struct _virJSONParserState {
+virJSONValuePtr value;
+char *key;
+};
+
+typedef struct _virJSONParser virJSONParser;
+typedef virJSONParser *virJSONParserPtr;
+struct _virJSONParser {
+virJSONValuePtr head;
+virJSONParserStatePtr state;
+size_t nstate;
+int wrap;
+};
+
+
 virJSONType
 virJSONValueGetType(const virJSONValue *value)
 {
@@ -1458,7 +1491,501 @@ virJSONValueCopy(const virJSONValue *in)
 }
 
 
-#if WITH_JANSSON
+#if WITH_YAJL
+static int
+virJSONParserInsertValue(virJSONParserPtr parser,
+ virJSONValuePtr value)
+{
+if (!parser->head) {
+parser->head = value;
+} else {
+virJSONParserStatePtr state;
+if (!parser->nstate) {
+VIR_DEBUG("got a value to insert without a container");
+return -1;
+}
+
+state = >state[parser->nstate-1];
+
+switch (state->value->type) {
+case VIR_JSON_TYPE_OBJECT: {
+if (!state->key) {
+VIR_DEBUG("missing key when inserting object value");
+return -1;
+}
+
+if (virJSONValueObjectAppend(state->value,
+ state->key,
+ value) < 0)
+return -1;
+
+VIR_FREE(state->key);
+}   break;
+
+case VIR_JSON_TYPE_ARRAY: {
+if (state->key) {
+VIR_DEBUG("unexpected key when inserting array value");
+return -1;
+}
+
+if (virJSONValueArrayAppend(state->value,
+value) < 0)
+return -1;
+}   break;
+
+default:
+VIR_DEBUG("unexpected value type, not a container");
+return -1;
+}
+}
+
+return 0;
+}
+
+
+static int
+virJSONParserHandleNull(void *ctx)
+{
+virJSONParserPtr parser = ctx;
+virJSONValuePtr value = virJSONValueNewNull();
+
+VIR_DEBUG("parser=%p", parser);
+
+if (!value)
+return 0;
+
+if (virJSONParserInsertValue(parser, value) < 0) {
+virJSONValueFree(value);
+return 0;
+}
+
+return 1;
+}
+
+
+static int
+virJSONParserHandleBoolean(void *ctx,
+   int boolean_)
+{
+virJSONParserPtr parser = ctx;
+virJSONValuePtr value = virJSONValueNewBoolean(boolean_);
+
+VIR_DEBUG("parser=%p boolean=%d", parser, boolean_);
+
+if (!value)
+return 0;
+
+if (virJSONParserInsertValue(parser, value) < 0) {
+virJSONValueFree(value);
+return 0;
+}
+
+return 1;
+}
+
+
+static int
+virJSONParserHandleNumber(void *ctx,
+  const char *s,
+  yajl_size_t l)
+{
+virJSONParserPtr parser = ctx;
+char *str;
+virJSONValuePtr value;
+
+if (VIR_STRNDUP(str, s, l) < 0)
+return -1;
+value = virJSONValueNewNumber(str);
+VIR_FREE(str);
+
+VIR_DEBUG("parser=%p str=%s", parser, str);
+
+if (!value)
+return 0;
+
+if (virJSONParserInsertValue(parser, value) < 0) {
+virJSONValueFree(value);
+return 0;
+}
+
+return 1;
+}
+
+
+static int
+virJSONParserHandleString(void *ctx,
+  const unsigned char *stringVal,
+  yajl_size_t stringLen)
+{
+virJSONParserPtr parser = ctx;
+virJSONValuePtr value = virJSONValueNewStringLen((const char *)stringVal,
+ stringLen);
+
+VIR_DEBUG("parser=%p str=%p", parser, (const char *)stringVal);
+
+if (!value)
+return 0;
+
+if (virJSONParserInsertValue(parser, value) < 0) {
+virJSONValueFree(value);
+return 0;
+}
+
+return 1;
+}
+
+
+static int
+virJSONParserHandleMapKey(void 

[libvirt] [PATCH 14/16] Revert "Switch from yajl to Jansson"

[Ján Tomko]

This reverts commit 9cf38263d05ca7f27dbbd9b1a0b48d338d9280e2.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 libvirt.spec.in  |   4 +-
 m4/virt-nss.m4   |   4 +-
 m4/virt-yajl.m4  |  27 +++-
 src/Makefile.am  |   8 +-
 src/qemu/qemu_driver.c   |   2 +-
 src/util/Makefile.inc.am |   4 +-
 src/util/virjson.c   | 211 ---
 tests/Makefile.am|  12 +-
 tests/cputest.c  |  16 +--
 tests/libxlxml2domconfigtest.c   |   4 +-
 tests/qemuagenttest.c|   2 +-
 tests/qemublocktest.c|   1 -
 tests/qemucapabilitiestest.c |   2 +-
 tests/qemucaps2xmltest.c |   2 +-
 tests/qemucommandutiltest.c  |   2 +-
 tests/qemuhotplugtest.c  |   2 +-
 tests/qemumigparamsdata/empty.json   |   4 +-
 tests/qemumigparamsdata/unsupported.json |   4 +-
 tests/qemumigparamstest.c|   2 +-
 tests/qemumonitorjsontest.c  |   2 +-
 tests/virmacmaptestdata/empty.json   |   4 +-
 tests/virmocklibxl.c |   4 +-
 tests/virnetdaemontest.c |   2 +-
 tests/virstoragetest.c   |   4 +-
 24 files changed, 72 insertions(+), 257 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 51f3feef8a..2ef844f69a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -292,7 +292,7 @@ BuildRequires: libblkid-devel >= 2.17
 BuildRequires: augeas
 BuildRequires: systemd-devel >= 185
 BuildRequires: libpciaccess-devel >= 0.10.9
-BuildRequires: jansson-devel
+BuildRequires: yajl-devel
 %if %{with_sanlock}
 BuildRequires: sanlock-devel >= 2.4
 %endif
@@ -1190,7 +1190,7 @@ rm -f po/stamp-po
--without-apparmor \
--without-hal \
--with-udev \
-   --with-jansson \
+   --with-yajl \
%{?arg_sanlock} \
--with-libpcap \
--with-macvtap \
diff --git a/m4/virt-nss.m4 b/m4/virt-nss.m4
index 082b7b14f6..951a74e835 100644
--- a/m4/virt-nss.m4
+++ b/m4/virt-nss.m4
@@ -27,9 +27,9 @@ AC_DEFUN([LIBVIRT_CHECK_NSS],[
   bsd_nss=no
   fail=0
   if test "x$with_nss_plugin" != "xno" ; then
-if test "x$with_jansson" != "xyes" ; then
+if test "x$with_yajl" != "xyes" ; then
   if test "x$with_nss_plugin" = "xyes" ; then
-AC_MSG_ERROR([Can't build nss plugin without JSON support])
+AC_MSG_ERROR([Can't build nss plugin without yajl])
   else
 with_nss_plugin=no
   fi
diff --git a/m4/virt-yajl.m4 b/m4/virt-yajl.m4
index 8d4c43a6b2..c4ea0102a3 100644
--- a/m4/virt-yajl.m4
+++ b/m4/virt-yajl.m4
@@ -23,10 +23,31 @@ AC_DEFUN([LIBVIRT_ARG_YAJL],[
 
 AC_DEFUN([LIBVIRT_CHECK_YAJL],[
   dnl YAJL JSON library http://lloyd.github.com/yajl/
-  if test "$with_yajl" = yes; then
-AC_MSG_ERROR([Compilation with YAJL is no longer supported])
+  if test "$with_qemu:$with_yajl" = yes:check; then
+dnl Some versions of qemu require the use of yajl; try to detect them
+dnl here, although we do not require qemu to exist in order to compile.
+dnl This check mirrors src/qemu/qemu_capabilities.c
+AC_PATH_PROGS([QEMU], [qemu-kvm qemu kvm qemu-system-x86_64],
+  [], [$PATH:/usr/bin:/usr/libexec])
+if test -x "$QEMU"; then
+  if $QEMU -help 2>/dev/null | grep -q libvirt; then
+with_yajl=yes
+  else
+[qemu_version_sed='s/.*ersion \([0-9.,]*\).*/\1/']
+qemu_version=`$QEMU -version | sed "$qemu_version_sed"`
+case $qemu_version in
+  [[1-9]].* | 0.15.* ) with_yajl=yes ;;
+  0.* | '' ) ;;
+  *) AC_MSG_ERROR([Unexpected qemu version string]) ;;
+esac
+  fi
+fi
   fi
-  with_yajl=no
+
+  LIBVIRT_CHECK_LIB_ALT([YAJL], [yajl],
+[yajl_parse_complete], [yajl/yajl_common.h],
+[YAJL2], [yajl],
+[yajl_tree_parse], [yajl/yajl_common.h])
 ])
 
 AC_DEFUN([LIBVIRT_RESULT_YAJL],[
diff --git a/src/Makefile.am b/src/Makefile.am
index 83263e69e5..db8c8ebd1a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -544,7 +544,7 @@ libvirt_admin_la_CFLAGS = \
 libvirt_admin_la_CFLAGS += \
$(XDR_CFLAGS) \
$(CAPNG_CFLAGS) \
-   $(JANSSON_CFLAGS) \
+   $(YAJL_CFLAGS) \
$(SSH2_CFLAGS) \
$(SASL_CFLAGS) \
$(GNUTLS_CFLAGS) \
@@ -552,7 +552,7 @@ libvirt_admin_la_CFLAGS += \
 
 libvirt_admin_la_LIBADD += \
$(CAPNG_LIBS) \
-   $(JANSSON_LIBS) \
+   $(YAJL_LIBS) \
$(DEVMAPPER_LIBS) \
$(LIBXML_LIBS) \
$(SSH2_LIBS) \
@@ -994,14 +994,14 @@ 

[libvirt] [PATCH 07/16] Revert "tests: also skip qemuagenttest with old jansson"

[Ján Tomko]

This reverts commit c31146685f5c8558ff88d52d03a68533c9220feb.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 tests/qemuagenttest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index b3d737d8a8..232b34f9cd 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -907,8 +907,8 @@ mymain(void)
 {
 int ret = 0;
 
-#if !WITH_STABLE_ORDERING_JANSSON
-fputs("libvirt not compiled with recent enough Jansson, skipping this 
test\n", stderr);
+#if !WITH_JANSSON
+fputs("libvirt not compiled with JSON support, skipping this test\n", 
stderr);
 return EXIT_AM_SKIP;
 #endif
 
-- 
2.16.1

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

[libvirt] [PATCH 15/16] Revert "build: undef WITH_JANSSON for SETUID_RPC_CLIENT"

[Ján Tomko]

This reverts commit 93fdc9e0b0cbb2eec32745a868ac4633f0912ad5.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 config-post.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config-post.h b/config-post.h
index 24117bbbd4..063e30fa37 100644
--- a/config-post.h
+++ b/config-post.h
@@ -36,7 +36,6 @@
 # undef WITH_DEVMAPPER
 # undef WITH_DTRACE_PROBES
 # undef WITH_GNUTLS
-# undef WITH_JANSSON
 # undef WITH_LIBSSH
 # undef WITH_MACVTAP
 # undef WITH_NUMACTL
-- 
2.16.1

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

[libvirt] [PATCH 06/16] Revert "util: avoid symbol clash between json libraries"

[Ján Tomko]

This reverts commit ce3c6ef6843f98d81be5423ece11fad79eaab920.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 libvirt.spec.in  |   2 -
 src/Makefile.am  |   8 +-
 src/util/Makefile.inc.am |   3 +-
 src/util/virjson.c   |   9 +-
 src/util/virjsoncompat.c | 274 ---
 src/util/virjsoncompat.h |  88 ---
 6 files changed, 7 insertions(+), 377 deletions(-)
 delete mode 100644 src/util/virjsoncompat.c
 delete mode 100644 src/util/virjsoncompat.h

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 3edb60d2d2..51f3feef8a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -898,8 +898,6 @@ Requires: ncurses
 Requires: gettext
 # Needed by virt-pki-validate script.
 Requires: gnutls-utils
-# We dlopen(libjansson.so.4), so need an explicit dep
-Requires: jansson
 %if %{with_bash_completion}
 Requires: %{name}-bash-completion = %{version}-%{release}
 %endif
diff --git a/src/Makefile.am b/src/Makefile.am
index a4f213480e..83263e69e5 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -552,6 +552,7 @@ libvirt_admin_la_CFLAGS += \
 
 libvirt_admin_la_LIBADD += \
$(CAPNG_LIBS) \
+   $(JANSSON_LIBS) \
$(DEVMAPPER_LIBS) \
$(LIBXML_LIBS) \
$(SSH2_LIBS) \
@@ -689,7 +690,6 @@ libvirt_setuid_rpc_client_la_SOURCES = \
util/virhashcode.c \
util/virhostcpu.c \
util/virjson.c \
-   util/virjsoncompat.c \
util/virlog.c \
util/virobject.c \
util/virpidfile.c \
@@ -961,8 +961,6 @@ libvirt_nss_la_SOURCES = \
util/virhashcode.h \
util/virjson.c \
util/virjson.h \
-   util/virjsoncompat.c \
-   util/virjsoncompat.h \
util/virkmod.c \
util/virkmod.h \
util/virlease.c \
@@ -1001,6 +999,10 @@ libvirt_nss_la_CFLAGS = \
 libvirt_nss_la_LDFLAGS = \
$(AM_LDFLAGS) \
$(NULL)
+
+libvirt_nss_la_LIBADD = \
+   $(JANSSON_LIBS) \
+   $(NULL)
 endif WITH_NSS
 
 
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index 8ef9ee1dfa..71b2b93c2d 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -86,8 +86,6 @@ UTIL_SOURCES = \
util/viriscsi.h \
util/virjson.c \
util/virjson.h \
-   util/virjsoncompat.c \
-   util/virjsoncompat.h \
util/virkeycode.c \
util/virkeycode.h \
util/virkeyfile.c \
@@ -266,6 +264,7 @@ libvirt_util_la_CFLAGS = \
$(NULL)
 libvirt_util_la_LIBADD = \
$(CAPNG_LIBS) \
+   $(JANSSON_LIBS) \
$(LIBNL_LIBS) \
$(THREAD_LIBS) \
$(AUDIT_LIBS) \
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 5bab662cd3..01a387b2f7 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1437,8 +1437,7 @@ virJSONValueCopy(const virJSONValue *in)
 
 
 #if WITH_JANSSON
-
-# include "virjsoncompat.h"
+# include 
 
 static virJSONValuePtr
 virJSONValueFromJansson(json_t *json)
@@ -1525,9 +1524,6 @@ virJSONValueFromString(const char *jsonstring)
 size_t flags = JSON_REJECT_DUPLICATES |
JSON_DECODE_ANY;
 
-if (virJSONInitialize() < 0)
-return NULL;
-
 if (!(json = json_loads(jsonstring, flags, ))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to parse JSON %d:%d: %s"),
@@ -1634,9 +1630,6 @@ virJSONValueToString(virJSONValuePtr object,
 json_t *json;
 char *str = NULL;
 
-if (virJSONInitialize() < 0)
-return NULL;
-
 if (pretty)
 flags |= JSON_INDENT(2);
 else
diff --git a/src/util/virjsoncompat.c b/src/util/virjsoncompat.c
deleted file mode 100644
index 6c853e9bb5..00
--- a/src/util/virjsoncompat.c
+++ /dev/null
@@ -1,274 +0,0 @@
-/*
- * virjsoncompat.c: JSON object parsing/formatting
- *
- * Copyright (C) 2018 Red Hat, Inc.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library.  If not, see
- * .
- *
- */
-
-#include 
-
-#include "virthread.h"
-#include "virerror.h"
-#define VIR_JSON_COMPAT_IMPL
-#include "virjsoncompat.h"
-
-#define VIR_FROM_THIS VIR_FROM_NONE
-
-#if 

[libvirt] [PATCH 09/16] Revert "build: require Jansson if QEMU driver is enabled"

[Ján Tomko]

This reverts commit 01ce04375c3348fd683475e5aa5231149ef6a78a.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 m4/virt-driver-qemu.m4 | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
index 2d9992d0dc..ddb2834705 100644
--- a/m4/virt-driver-qemu.m4
+++ b/m4/virt-driver-qemu.m4
@@ -27,9 +27,6 @@ AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
 
 AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
   AC_REQUIRE([LIBVIRT_CHECK_JANSSON])
-  if test "$with_qemu:$with_jansson" = "yes:no"; then
-AC_MSG_ERROR([Jansson >= 2.5 is required to build QEMU driver])
-  fi
   if test "$with_qemu" = "check"; then
 with_qemu=$with_jansson
   fi
-- 
2.16.1

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

[libvirt] [dbus PATCH 1/2] configure: Fix quoting

[Andrea Bolognani]

Macro arguments are supposed to be quoted.

Signed-off-by: Andrea Bolognani 
---
 configure.ac | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/configure.ac b/configure.ac
index 8917b37..22d80dd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,6 +1,6 @@
 AC_INIT([libvirt-dbus], [1.3.0], [libvir-list@redhat.com], [], 
[https://libvirt.org])
 
-AC_CONFIG_SRCDIR(src/main.c)
+AC_CONFIG_SRCDIR([src/main.c])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_MACRO_DIR([m4])
@@ -37,10 +37,10 @@ AM_PROG_CC_C_O
 AC_PROG_CC_STDC
 AC_PROG_LIBTOOL
 
-PKG_CHECK_MODULES(GIO2, gio-unix-2.0 >= GLIB2_REQUIRED)
-PKG_CHECK_MODULES(GLIB2, glib-2.0 >= GLIB2_REQUIRED)
-PKG_CHECK_MODULES(LIBVIRT, libvirt >= $LIBVIRT_REQUIRED)
-PKG_CHECK_MODULES(LIBVIRT_GLIB, libvirt-glib-1.0 >= LIBVIRT_GLIB_REQUIRED)
+PKG_CHECK_MODULES([GIO2], [gio-unix-2.0 >= GLIB2_REQUIRED])
+PKG_CHECK_MODULES([GLIB2], [glib-2.0 >= GLIB2_REQUIRED])
+PKG_CHECK_MODULES([LIBVIRT], [libvirt >= $LIBVIRT_REQUIRED])
+PKG_CHECK_MODULES([LIBVIRT_GLIB], [libvirt-glib-1.0 >= LIBVIRT_GLIB_REQUIRED])
 
 LIBVIRT_COMPILE_WARNINGS
 LIBVIRT_LINKER_RELRO
@@ -49,17 +49,17 @@ LIBVIRT_COMPILE_PIE
 LIBVIRT_ARG_WITH([DBUS_SERVICES], [where D-Bus session services direcotry is],
  ['$(datadir)/dbus-1/services'])
 DBUS_SERVICES_DIR="$with_dbus_services"
-AC_SUBST(DBUS_SERVICES_DIR)
+AC_SUBST([DBUS_SERVICES_DIR])
 
 LIBVIRT_ARG_WITH([DBUS_SYSTEM_SERVICES], [where D-Bus system services 
directory is],
  ['$(datadir)/dbus-1/system-services'])
 DBUS_SYSTEM_SERVICES_DIR="$with_dbus_system_services"
-AC_SUBST(DBUS_SYSTEM_SERVICES_DIR)
+AC_SUBST([DBUS_SYSTEM_SERVICES_DIR])
 
 LIBVIRT_ARG_WITH([DBUS_SYSTEM_POLICIES], [where D-Bus system policies 
directory is],
  ['$(datadir)/dbus-1/system.d'])
 DBUS_SYSTEM_POLICIES_DIR="$with_dbus_system_policies"
-AC_SUBST(DBUS_SYSTEM_POLICIES_DIR)
+AC_SUBST([DBUS_SYSTEM_POLICIES_DIR])
 
 LIBVIRT_ARG_WITH([DBUS_INTERFACES], [where D-Bus interfaces directory is],
  ['$(datadir)/dbus-1/interfaces'])
@@ -69,7 +69,7 @@ AC_SUBST([DBUS_INTERFACES_DIR])
 LIBVIRT_ARG_WITH([POLKIT_RULES], [directory for polkit rules],
  ['$(datadir)/polkit-1/rules.d'])
 POLKIT_RULES_DIR="$with_polkit_rules"
-AC_SUBST(POLKIT_RULES_DIR)
+AC_SUBST([POLKIT_RULES_DIR])
 
 LIBVIRT_ARG_WITH([SYSTEM_USER], [username to run system instance as],
  ['libvirtdbus'])
@@ -78,9 +78,9 @@ AC_SUBST([SYSTEM_USER])
 
 AC_CONFIG_FILES([run],
 [chmod +x,-w run])
-AC_OUTPUT(Makefile
-  data/Makefile
-  docs/Makefile
-  src/Makefile
-  tests/Makefile
-  libvirt-dbus.spec)
+AC_OUTPUT([Makefile
+   data/Makefile
+   docs/Makefile
+   src/Makefile
+   tests/Makefile
+   libvirt-dbus.spec])
-- 
2.17.1

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

[libvirt] [dbus PATCH 2/2] configure: Call PKG_CHECK_MODULES() correctly

[Andrea Bolognani]

Most of the calls where missing the dollar sign when
referencing variables, which caused the corresponding
version checks to be skipped.

Signed-off-by: Andrea Bolognani 
---
 configure.ac | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 22d80dd..1c0e246 100644
--- a/configure.ac
+++ b/configure.ac
@@ -37,10 +37,10 @@ AM_PROG_CC_C_O
 AC_PROG_CC_STDC
 AC_PROG_LIBTOOL
 
-PKG_CHECK_MODULES([GIO2], [gio-unix-2.0 >= GLIB2_REQUIRED])
-PKG_CHECK_MODULES([GLIB2], [glib-2.0 >= GLIB2_REQUIRED])
+PKG_CHECK_MODULES([GIO2], [gio-unix-2.0 >= $GLIB2_REQUIRED])
+PKG_CHECK_MODULES([GLIB2], [glib-2.0 >= $GLIB2_REQUIRED])
 PKG_CHECK_MODULES([LIBVIRT], [libvirt >= $LIBVIRT_REQUIRED])
-PKG_CHECK_MODULES([LIBVIRT_GLIB], [libvirt-glib-1.0 >= LIBVIRT_GLIB_REQUIRED])
+PKG_CHECK_MODULES([LIBVIRT_GLIB], [libvirt-glib-1.0 >= $LIBVIRT_GLIB_REQUIRED])
 
 LIBVIRT_COMPILE_WARNINGS
 LIBVIRT_LINKER_RELRO
-- 
2.17.1

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

[libvirt] [PATCH 11/16] Revert "Remove virJSONValueNewStringLen"

[Ján Tomko]

This reverts commit 8f802c6d8659beb9eb3cab96ba2553e251728337.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 src/libvirt_private.syms |  1 +
 src/util/virjson.c   | 22 ++
 src/util/virjson.h   |  1 +
 3 files changed, 24 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 496de11168..f555b1d5b1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2096,6 +2096,7 @@ virJSONValueNewNumberUint;
 virJSONValueNewNumberUlong;
 virJSONValueNewObject;
 virJSONValueNewString;
+virJSONValueNewStringLen;
 virJSONValueObjectAdd;
 virJSONValueObjectAddVArgs;
 virJSONValueObjectAppend;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 01a387b2f7..80274bc6c5 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -420,6 +420,28 @@ virJSONValueNewString(const char *data)
 }
 
 
+virJSONValuePtr
+virJSONValueNewStringLen(const char *data,
+ size_t length)
+{
+virJSONValuePtr val;
+
+if (!data)
+return virJSONValueNewNull();
+
+if (VIR_ALLOC(val) < 0)
+return NULL;
+
+val->type = VIR_JSON_TYPE_STRING;
+if (VIR_STRNDUP(val->data.string, data, length) < 0) {
+VIR_FREE(val);
+return NULL;
+}
+
+return val;
+}
+
+
 static virJSONValuePtr
 virJSONValueNewNumber(const char *data)
 {
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 0d5a7ef753..75f7f17b44 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -59,6 +59,7 @@ int virJSONValueObjectAddVArgs(virJSONValuePtr obj, va_list 
args)
 
 
 virJSONValuePtr virJSONValueNewString(const char *data);
+virJSONValuePtr virJSONValueNewStringLen(const char *data, size_t length);
 virJSONValuePtr virJSONValueNewNumberInt(int data);
 virJSONValuePtr virJSONValueNewNumberUint(unsigned int data);
 virJSONValuePtr virJSONValueNewNumberLong(long long data);
-- 
2.16.1

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

[libvirt] [PATCH 10/16] Revert "build: switch --with-qemu default from yes to check"

[Ján Tomko]

This reverts commit c5ae8e0c2b4b6bb3c667cfadaf65a66c3f4f3d85.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 m4/virt-driver-qemu.m4 | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
index ddb2834705..80e1d3ad46 100644
--- a/m4/virt-driver-qemu.m4
+++ b/m4/virt-driver-qemu.m4
@@ -18,7 +18,7 @@ dnl .
 dnl
 
 AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
-  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [check])
+  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [yes])
   LIBVIRT_ARG_WITH([QEMU_USER], [username to run QEMU system instance as],
['platform dependent'])
   LIBVIRT_ARG_WITH([QEMU_GROUP], [groupname to run QEMU system instance as],
@@ -26,10 +26,6 @@ AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
 ])
 
 AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
-  AC_REQUIRE([LIBVIRT_CHECK_JANSSON])
-  if test "$with_qemu" = "check"; then
-with_qemu=$with_jansson
-  fi
   if test "$with_qemu" = "yes" ; then
 AC_DEFINE_UNQUOTED([WITH_QEMU], 1, [whether QEMU driver is enabled])
   fi
-- 
2.16.1

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

[libvirt] [PATCH 12/16] Revert "build: remove references to WITH_YAJL for SETUID_RPC_CLIENT"

[Ján Tomko]

This reverts commit 1caf8441604b58e4a89aa2c09975b8346928c52a.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 config-post.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config-post.h b/config-post.h
index 05672121f0..24117bbbd4 100644
--- a/config-post.h
+++ b/config-post.h
@@ -44,6 +44,8 @@
 # undef WITH_SSH2
 # undef WITH_SYSTEMD_DAEMON
 # undef WITH_VIRTUALPORT
+# undef WITH_YAJL
+# undef WITH_YAJL2
 #endif
 
 /*
-- 
2.16.1

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

[libvirt] [PATCH 16/16] Revert "build: add --with-jansson"

[Ján Tomko]

This reverts commit 12b34f094e2f1c7f414f4bb8f880a9d65c8fcd85.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Conflicts:
  configure.ac:
Commit 8aa85e0b introduced LIBVIRT_*_LIBISCSI macros.

Signed-off-by: Ján Tomko 
---
 configure.ac   |  3 ---
 m4/virt-jansson.m4 | 29 -
 2 files changed, 32 deletions(-)
 delete mode 100644 m4/virt-jansson.m4

diff --git a/configure.ac b/configure.ac
index 5e49b8483f..da940e34df 100644
--- a/configure.ac
+++ b/configure.ac
@@ -250,7 +250,6 @@ LIBVIRT_ARG_FIREWALLD
 LIBVIRT_ARG_FUSE
 LIBVIRT_ARG_GLUSTER
 LIBVIRT_ARG_HAL
-LIBVIRT_ARG_JANSSON
 LIBVIRT_ARG_LIBISCSI
 LIBVIRT_ARG_LIBPCAP
 LIBVIRT_ARG_LIBSSH
@@ -292,7 +291,6 @@ LIBVIRT_CHECK_FUSE
 LIBVIRT_CHECK_GLUSTER
 LIBVIRT_CHECK_GNUTLS
 LIBVIRT_CHECK_HAL
-LIBVIRT_CHECK_JANSSON
 LIBVIRT_CHECK_LIBISCSI
 LIBVIRT_CHECK_LIBNL
 LIBVIRT_CHECK_LIBPARTED
@@ -978,7 +976,6 @@ LIBVIRT_RESULT_FUSE
 LIBVIRT_RESULT_GLUSTER
 LIBVIRT_RESULT_GNUTLS
 LIBVIRT_RESULT_HAL
-LIBVIRT_RESULT_JANSSON
 LIBVIRT_RESULT_LIBISCSI
 LIBVIRT_RESULT_LIBNL
 LIBVIRT_RESULT_LIBPCAP
diff --git a/m4/virt-jansson.m4 b/m4/virt-jansson.m4
deleted file mode 100644
index 206d6a5ced..00
--- a/m4/virt-jansson.m4
+++ /dev/null
@@ -1,29 +0,0 @@
-dnl The jansson library
-dnl
-dnl This library is free software; you can redistribute it and/or
-dnl modify it under the terms of the GNU Lesser General Public
-dnl License as published by the Free Software Foundation; either
-dnl version 2.1 of the License, or (at your option) any later version.
-dnl
-dnl This library is distributed in the hope that it will be useful,
-dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
-dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-dnl Lesser General Public License for more details.
-dnl
-dnl You should have received a copy of the GNU Lesser General Public
-dnl License along with this library.  If not, see
-dnl .
-dnl
-
-AC_DEFUN([LIBVIRT_ARG_JANSSON],[
-  LIBVIRT_ARG_WITH_FEATURE([JANSSON], [jansson], [check])
-])
-
-AC_DEFUN([LIBVIRT_CHECK_JANSSON],[
-  dnl Jansson http://www.digip.org/jansson/
-  LIBVIRT_CHECK_PKG([JANSSON], [jansson], [2.5])
-])
-
-AC_DEFUN([LIBVIRT_RESULT_JANSSON],[
-  LIBVIRT_RESULT_LIB([JANSSON])
-])
-- 
2.16.1

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

[libvirt] [PATCH 08/16] Revert "m4: Introduce STABLE_ORDERING_JANSSON"

[Ján Tomko]

This reverts commit 4dd60540007042bfc0087a67f57f3e9f3311a84a.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 m4/virt-jansson.m4   | 3 ---
 tests/qemublocktest.c| 5 -
 tests/qemucapabilitiestest.c | 5 -
 tests/qemucommandutiltest.c  | 5 -
 tests/qemuhotplugtest.c  | 5 -
 tests/qemumigparamstest.c| 5 -
 tests/qemumonitorjsontest.c  | 5 -
 tests/virjsontest.c  | 5 -
 tests/virmacmaptest.c| 5 -
 tests/virnetdaemontest.c | 5 -
 10 files changed, 48 deletions(-)

diff --git a/m4/virt-jansson.m4 b/m4/virt-jansson.m4
index ab4c020f62..206d6a5ced 100644
--- a/m4/virt-jansson.m4
+++ b/m4/virt-jansson.m4
@@ -22,9 +22,6 @@ AC_DEFUN([LIBVIRT_ARG_JANSSON],[
 AC_DEFUN([LIBVIRT_CHECK_JANSSON],[
   dnl Jansson http://www.digip.org/jansson/
   LIBVIRT_CHECK_PKG([JANSSON], [jansson], [2.5])
-  dnl Older versions of Jansson did not preserve the order of object keys
-  dnl use this check to guard the tests that are sensitive to this
-  LIBVIRT_CHECK_PKG([STABLE_ORDERING_JANSSON], [jansson], [2.8], [true])
 ])
 
 AC_DEFUN([LIBVIRT_RESULT_JANSSON],[
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index d22b4b754e..9a387cf063 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -337,11 +337,6 @@ mymain(void)
 char *capslatest_x86_64 = NULL;
 virQEMUCapsPtr caps_x86_64 = NULL;
 
-#if !WITH_STABLE_ORDERING_JANSSON
-fputs("libvirt not compiled with recent enough Jansson, skipping this 
test\n", stderr);
-return EXIT_AM_SKIP;
-#endif
-
 if (qemuTestDriverInit() < 0)
 return EXIT_FAILURE;
 
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
index 28f903a88c..641ec4f597 100644
--- a/tests/qemucapabilitiestest.c
+++ b/tests/qemucapabilitiestest.c
@@ -141,11 +141,6 @@ mymain(void)
 int ret = 0;
 testQemuData data;
 
-#if !WITH_STABLE_ORDERING_JANSSON
-fputs("libvirt not compiled with recent enough Jansson, skipping this 
test\n", stderr);
-return EXIT_AM_SKIP;
-#endif
-
 #if !WITH_JANSSON
 fputs("libvirt not compiled with JSON support, skipping this test\n", 
stderr);
 return EXIT_AM_SKIP;
diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c
index 9b13dde63f..8e57a1b79d 100644
--- a/tests/qemucommandutiltest.c
+++ b/tests/qemucommandutiltest.c
@@ -76,11 +76,6 @@ mymain(void)
 int ret = 0;
 testQemuCommandBuildObjectFromJSONData data1;
 
-#if !WITH_STABLE_ORDERING_JANSSON
-fputs("libvirt not compiled with recent enough Jansson, skipping this 
test\n", stderr);
-return EXIT_AM_SKIP;
-#endif
-
 #if !WITH_JANSSON
 fputs("libvirt not compiled with JSON support, skipping this test\n", 
stderr);
 return EXIT_AM_SKIP;
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 3c0eecb079..2fb96c6158 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -593,11 +593,6 @@ mymain(void)
 struct qemuHotplugTestData data = {0};
 struct testQemuHotplugCpuParams cpudata;
 
-#if !WITH_STABLE_ORDERING_JANSSON
-fputs("libvirt not compiled with recent enough Jansson, skipping this 
test\n", stderr);
-return EXIT_AM_SKIP;
-#endif
-
 #if !WITH_JANSSON
 fputs("libvirt not compiled with JSON support, skipping this test\n", 
stderr);
 return EXIT_AM_SKIP;
diff --git a/tests/qemumigparamstest.c b/tests/qemumigparamstest.c
index 5e12430991..b8af68211b 100644
--- a/tests/qemumigparamstest.c
+++ b/tests/qemumigparamstest.c
@@ -203,11 +203,6 @@ mymain(void)
 virQEMUDriver driver;
 int ret = 0;
 
-#if !WITH_STABLE_ORDERING_JANSSON
-fputs("libvirt not compiled with recent enough Jansson, skipping this 
test\n", stderr);
-return EXIT_AM_SKIP;
-#endif
-
 #if !WITH_JANSSON
 fputs("libvirt not compiled with JSON support, skipping this test\n", 
stderr);
 return EXIT_AM_SKIP;
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 1826c4f297..c11615f7ac 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2863,11 +2863,6 @@ mymain(void)
 virJSONValuePtr metaschema = NULL;
 char *metaschemastr = NULL;
 
-#if !WITH_STABLE_ORDERING_JANSSON
-fputs("libvirt not compiled with recent enough Jansson, skipping this 
test\n", stderr);
-return EXIT_AM_SKIP;
-#endif
-
 #if !WITH_JANSSON
 fputs("libvirt not compiled with JSON support, skipping this test\n", 
stderr);
 return EXIT_AM_SKIP;
diff --git a/tests/virjsontest.c b/tests/virjsontest.c
index d352d370fd..d42413d11d 100644
--- a/tests/virjsontest.c
+++ b/tests/virjsontest.c
@@ -479,11 +479,6 @@ mymain(void)
 {
 int ret = 0;
 
-#if !WITH_STABLE_ORDERING_JANSSON
-fputs("libvirt not compiled with recent enough Jansson, skipping this 
test\n", stderr);
-return EXIT_AM_SKIP;
-#endif
-
 #define DO_TEST_FULL(name, cmd, doc, expect, pass) \
 do { \
 struct 

[libvirt] [dbus PATCH 0/2] configure: Various fixes

[Andrea Bolognani]

Andrea Bolognani (2):
  configure: Fix quoting
  configure: Call PKG_CHECK_MODULES() correctly

 configure.ac | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

-- 
2.17.1

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

Re: [libvirt] [PATCH 1/2] virnetdevip: Free data.devices in virNetDevIPCheckIPv6Forwarding() too

[Erik Skultety]

On Mon, Aug 13, 2018 at 11:21:44AM +0200, Michal Privoznik wrote:
> We are freeing the individual strings (which were filled by
> virNetDevIPCheckIPv6ForwardingCallback()) but not the array
> itself.
>
> Signed-off-by: Michal Privoznik 
> ---
>  src/util/virnetdevip.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
> index c6d6175627..866ddcc213 100644
> --- a/src/util/virnetdevip.c
> +++ b/src/util/virnetdevip.c
> @@ -651,8 +651,7 @@ virNetDevIPCheckIPv6Forwarding(void)
>
>   cleanup:
>  nlmsg_free(nlmsg);
> -for (i = 0; i < data.ndevices; i++)
> -VIR_FREE(data.devices[i]);
> +virStringListFreeCount(data.devices, data.ndevices);
>  return valid;

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 2/2] networkStartNetworkVirtual: Don't leak macmap object

[Erik Skultety]

On Mon, Aug 13, 2018 at 11:21:45AM +0200, Michal Privoznik wrote:
> When starting network a macmap object is created (which stores
> MAC -> domain name mappings). However, if something goes wrong
> (e.g. virNetDevIPCheckIPv6Forwarding() fails) then the object is
> leaked.
>
> Signed-off-by: Michal Privoznik 
> ---
>  src/network/bridge_driver.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index f92cc61e47..588b0d147d 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2453,6 +2453,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr 
> driver,
>  goto err1;
>
>  virNetworkObjSetMacMap(obj, macmap);
> +macmap = NULL;
>
>  /* Set bridge options */
>
> @@ -2590,6 +2591,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr 
> driver,
>  ignore_value(virNetDevTapDelete(macTapIfName, NULL));
>  VIR_FREE(macTapIfName);
>  }
> +virNetworkObjUnrefMacMap(obj);

Hopefully there's no occurrence of plain virObjectUnref for obj->macmap.

Reviewed-by: Erik Skultety 

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

[libvirt] [PATCH] storage: Don't hold storage pool locked during wipeVol

[Michal Privoznik]

Currently the way virStorageVolWipe() works is it looks up
pool containing given volume and hold it locked throughout while
API execution. This is suboptimal because wiping a volume means
writing data to it which can take ages. And if the whole pool is
locked during that operation no other API can be issued (nor
pool-list).

Signed-off-by: Michal Privoznik 
---
 src/storage/storage_backend_iscsi_direct.c | 5 +
 src/storage/storage_backend_rbd.c  | 7 ++-
 src/storage/storage_util.c | 8 +++-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_iscsi_direct.c 
b/src/storage/storage_backend_iscsi_direct.c
index 1624066e9c..58d25557f1 100644
--- a/src/storage/storage_backend_iscsi_direct.c
+++ b/src/storage/storage_backend_iscsi_direct.c
@@ -691,6 +691,9 @@ virStorageBackenISCSIDirectWipeVol(virStoragePoolObjPtr 
pool,
 if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, NULL)))
 return -1;
 
+vol->in_use++;
+virObjectUnlock(pool);
+
 switch ((virStorageVolWipeAlgorithm) algorithm) {
 case VIR_STORAGE_VOL_WIPE_ALG_ZERO:
 if (virStorageBackendISCSIDirectVolWipeZero(vol, iscsi) < 0) {
@@ -719,6 +722,8 @@ virStorageBackenISCSIDirectWipeVol(virStoragePoolObjPtr 
pool,
  cleanup:
 virISCSIDirectDisconnect(iscsi);
 iscsi_destroy_context(iscsi);
+virObjectLock(pool);
+vol->in_use--;
 return ret;
 }
 
diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 642cacb673..30c94c109b 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -1212,7 +1212,10 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool,
 VIR_DEBUG("Wiping RBD image %s/%s", def->source.name, vol->name);
 
 if (!(ptr = virStorageBackendRBDNewState(pool)))
-goto cleanup;
+return -1;
+
+vol->in_use++;
+virObjectUnlock(pool);
 
 if ((r = rbd_open(ptr->ioctx, vol->name, , NULL)) < 0) {
 virReportSystemError(-r, _("failed to open the RBD image %s"),
@@ -1271,6 +1274,8 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool,
 rbd_close(image);
 
 virStorageBackendRBDFreeState();
+virObjectLock(pool);
+vol->in_use--;
 
 return ret;
 }
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 42a9b6abf0..5c1fb7b7d3 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2750,7 +2750,7 @@ storageBackendVolWipePloop(virStorageVolDefPtr vol,
 
 
 int
-virStorageBackendVolWipeLocal(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+virStorageBackendVolWipeLocal(virStoragePoolObjPtr pool,
   virStorageVolDefPtr vol,
   unsigned int algorithm,
   unsigned int flags)
@@ -2759,6 +2759,9 @@ virStorageBackendVolWipeLocal(virStoragePoolObjPtr pool 
ATTRIBUTE_UNUSED,
 
 virCheckFlags(0, -1);
 
+vol->in_use++;
+virObjectUnlock(pool);
+
 VIR_DEBUG("Wiping volume with path '%s' and algorithm %u",
   vol->target.path, algorithm);
 
@@ -2769,6 +2772,9 @@ virStorageBackendVolWipeLocal(virStoragePoolObjPtr pool 
ATTRIBUTE_UNUSED,
  vol->target.allocation, false);
 }
 
+virObjectLock(pool);
+vol->in_use--;
+
 return ret;
 }
 
-- 
2.16.4

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

Re: [libvirt] [PATCH v3 07/11] util: netdev: use VIR_AUTOFREE instead of VIR_FREE for scalar types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:15AM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOFREE macro for declaring scalar variables, majority
> of the VIR_FREE calls can be dropped, which in turn leads to
> getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
>  src/util/virnetdev.c | 343 
> +++
>  1 file changed, 125 insertions(+), 218 deletions(-)
>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 8eac419..edb7393 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -535,18 +535,17 @@ int virNetDevSetMTUFromDevice(const char *ifname,
>   */
>  int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
>  {
> -int ret = -1;
> -char *pid = NULL;
> -char *phy = NULL;
> -char *phy_path = NULL;
>  int len;
> +VIR_AUTOFREE(char *) pid = NULL;
> +VIR_AUTOFREE(char *) phy = NULL;
> +VIR_AUTOFREE(char *) phy_path = NULL;
>
>  if (virAsprintf(, "%lld", (long long) pidInNs) == -1)
>  return -1;
>
>  /* The 802.11 wireless devices only move together with their PHY. */
>  if (virNetDevSysfsFile(_path, ifname, "phy80211/name") < 0)
> -goto cleanup;
> +return -1;
>
>  if ((len = virFileReadAllQuiet(phy_path, 1024, )) <= 0) {
>  /* Not a wireless device. */
> @@ -556,7 +555,7 @@ int virNetDevSetNamespace(const char *ifname, pid_t 
> pidInNs)
>
>  argv[5] = pid;
>  if (virRun(argv, NULL) < 0)
> -goto cleanup;
> +return -1;
>
>  } else {
>  const char *argv[] = {
> @@ -569,15 +568,10 @@ int virNetDevSetNamespace(const char *ifname, pid_t 
> pidInNs)
>  argv[2] = phy;
>  argv[5] = pid;
>  if (virRun(argv, NULL) < 0)
> -goto cleanup;
> +return -1;
>  }
>
> -ret = 0;
> - cleanup:
> -VIR_FREE(phy_path);
> -VIR_FREE(phy);
> -VIR_FREE(pid);
> -return ret;
> +return 0;
>  }
>
>  #if defined(SIOCSIFNAME) && defined(HAVE_STRUCT_IFREQ)
> @@ -969,25 +963,21 @@ int virNetDevGetIndex(const char *ifname 
> ATTRIBUTE_UNUSED,
>  int
>  virNetDevGetMaster(const char *ifname, char **master)
>  {
> -int ret = -1;
> -void *nlData = NULL;
>  struct nlattr *tb[IFLA_MAX + 1] = {NULL, };
> +VIR_AUTOFREE(void *) nlData = NULL;
>
>  *master = NULL;
>
>  if (virNetlinkDumpLink(ifname, -1, , tb, 0, 0) < 0)
> -goto cleanup;
> +return -1;
>
>  if (tb[IFLA_MASTER]) {
>  if (!(*master = virNetDevGetName(*(int *)RTA_DATA(tb[IFLA_MASTER]
> -goto cleanup;
> +return -1;
>  }
>
>  VIR_DEBUG("IFLA_MASTER for %s is %s", ifname, *master ? *master : 
> "(none)");
> -ret = 0;
> - cleanup:
> -VIR_FREE(nlData);
> -return ret;
> +return 0;
>  }
>
>
> @@ -1168,39 +1158,33 @@ virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, 
> const char *ifname,
>  static bool
>  virNetDevIsPCIDevice(const char *devpath)
>  {
> -char *subsys_link = NULL;
> -char *abs_path = NULL;
>  char *subsys = NULL;
> -bool ret = false;
> +VIR_AUTOFREE(char *) subsys_link = NULL;
> +VIR_AUTOFREE(char *) abs_path = NULL;
>
>  if (virAsprintf(_link, "%s/subsystem", devpath) < 0)
>  return false;
>
>  if (!virFileExists(subsys_link))
> -goto cleanup;
> +return false;
>
>  if (virFileResolveLink(subsys_link, _path) < 0) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Unable to resolve device subsystem symlink %s"),
> subsys_link);
> -goto cleanup;
> +return false;
>  }
>
>  subsys = last_component(abs_path);
> -ret = STRPREFIX(subsys, "pci");
> -
> - cleanup:
> -VIR_FREE(subsys_link);
> -VIR_FREE(abs_path);
> -return ret;
> +return STRPREFIX(subsys, "pci");
>  }
>
>  static virPCIDevicePtr
>  virNetDevGetPCIDevice(const char *devName)
>  {
> -char *vfSysfsDevicePath = NULL;
>  virPCIDeviceAddressPtr vfPCIAddr = NULL;
>  virPCIDevicePtr vfPCIDevice = NULL;
> +VIR_AUTOFREE(char *) vfSysfsDevicePath = NULL;
>
>  if (virNetDevSysfsFile(, devName, "device") < 0)
>  goto cleanup;
> @@ -1216,7 +1200,6 @@ virNetDevGetPCIDevice(const char *devName)
>vfPCIAddr->slot, vfPCIAddr->function);
>
>   cleanup:
> -VIR_FREE(vfSysfsDevicePath);
>  VIR_FREE(vfPCIAddr);
>
>  return vfPCIDevice;
> @@ -1241,25 +1224,20 @@ int
>  virNetDevGetPhysPortID(const char *ifname,
> char **physPortID)
>  {
> -int ret = -1;
> -char *physPortIDFile = NULL;
> +VIR_AUTOFREE(char *) physPortIDFile = NULL;
>
>  *physPortID = NULL;
>
>  if (virNetDevSysfsFile(, ifname, "phys_port_id") < 0)
> -goto cleanup;
> +return -1;
>
>  /* a failure to read just means the driver 

Re: [libvirt] [PATCH v3 10/11] util: netdevopenvswitch: use VIR_AUTOPTR for aggregate types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:18AM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH v3 02/11] util: netlink: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:10AM +0530, Sukrit Bhatnagar wrote:
> Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
> src/util/viralloc.h, define a new wrapper around an existing
> cleanup function which will be called when a variable declared
> with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant
> viralloc.h include, since that has moved from the source module into
> the header.
>
> This commit also typedefs virNlMsg to struct nl_msg type for use
> with the cleanup macros.
>
> When a variable of type virNlMsg * is declared using VIR_AUTOPTR,
> the function nlmsg_free will be run automatically on it when it
> goes out of scope.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
>  src/util/virnetlink.c | 1 -
>  src/util/virnetlink.h | 5 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index 162efe6..ecf62c9 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -38,7 +38,6 @@
>  #include "virnetlink.h"
>  #include "virnetdev.h"
>  #include "virlog.h"
> -#include "viralloc.h"
>  #include "virthread.h"
>  #include "virmacaddr.h"
>  #include "virerror.h"
> diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
> index 2a9de0a..8ebeab8 100644
> --- a/src/util/virnetlink.h
> +++ b/src/util/virnetlink.h
> @@ -22,6 +22,7 @@
>
>  # include "internal.h"
>  # include "virmacaddr.h"
> +# include "viralloc.h"
>
>  # if defined(__linux__) && defined(HAVE_LIBNL)
>
> @@ -44,6 +45,8 @@ struct nlmsghdr;
>
>  # endif /* __linux__ */
>
> +typedef struct nl_msg virNlMsg;

Since the name of the module is virNetlink, I'll rename this to virNetlinkMsg
and tweak all the affected places across the whole series.

Reviewed-by: Erik Skultety 

> +
>  int virNetlinkStartup(void);
>  void virNetlinkShutdown(void);
>
> @@ -123,4 +126,6 @@ int 
> virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB,
>  int virNetlinkEventRemoveClient(int watch, const virMacAddr *macaddr,
>  unsigned int protocol);
>
> +VIR_DEFINE_AUTOPTR_FUNC(virNlMsg, nlmsg_free)
> +
>  #endif /* __VIR_NETLINK_H__ */
> --
> 1.8.3.1
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

Re: [libvirt] [PATCH 1/2] Add API for printing tables.

[Michal Prívozník]

On 08/13/2018 11:46 AM, Simon Kobyda wrote:
> On Fri, 2018-08-10 at 15:40 +0200, Michal Privoznik wrote:
>> On 08/10/2018 11:11 AM, Simon Kobyda wrote:


>>> +static vshTableRowPtr
>>> +vshTableRowNew(size_t size, const char *arg, va_list ap)
>>
>> I know we discussed this in person, but now that I see this code and
>> think about it more I think this @size argument should be dropped
>> from
>> this function and the corresponding check should be moved to [1]
> That way I will implement identical loop also there [1], to count
> number of variable arguments to make that check (if there is better way
> to count number of variable arguments, I'm open to suggestions :) ).
> Sure I can drop it there, but what are pros of that? I'm curious :).

I'm not sure why you would need the second loop? The way your code works
right now is that vshTableRowNew must have the sentinel (= args have to
end with NULL). What I am suggesting is for vshTableRowNew() to just
consume all the args (like it is doing now) until the sentinel. And only
after that check at [1]  whether row->ncells is expected value.

>>
>>> +{
>>> +vshTableRowPtr row = NULL;
>>> +char *tmp = NULL;
>>> +
>>> +if (!arg) {
>>> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> +_("Table row cannot be empty"));
>>> +goto error;
>>> +}
>>> +
>>> +if (VIR_ALLOC(row) < 0)
>>> +goto error;
>>> +
>>> +while (arg) {
>>> +if (VIR_STRDUP(tmp, arg) < 0)
>>> +goto error;
>>> +
>>> +if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) < 0)
>>> +goto error;
>>> +
>>> +arg = va_arg(ap, const char *);
>>> +}
>>> +
>>> +if (size && row->ncells != size) {
>>> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> +_("Incorrect number of cells in a table
>>> row"));
>>> +goto error;
>>> +}
>>> +
>>> +return row;
>>> +
>>> + error:
>>> +vshTableRowFree(row);
>>> +return NULL;
>>> +}
>>> +


>>> +
>>> +/**
>>> + * vshTableRowPrint:
>>> + * @ctl virtshell control structure
>>> + * @row: table to append to
>>> + * @maxwidths: maximum count of characters for each columns
>>> + * @widths: count of character for each cell in this row
>>> + * @printCB function for priting table
>>> + * @buf: buffer to store table (only if @toStdout == true)
>>> + * @toStdout: whetever print table to Stdout or return in buffer
>>> + */
>>> +static void
>>> +vshTableRowPrint(vshControl *ctl,
>>> + vshTableRowPtr row,
>>> + size_t *maxwidths,
>>> + size_t *widths,
>>> + vshPrintCB printCB,
>>> + virBufferPtr buf,
>>> + bool toStdout)
>>> +{
>>> +size_t i;
>>> +size_t j;
>>> +
>>> +if (toStdout) {
>>> +for (i = 0; i < row->ncells; i++) {
>>> +printCB(ctl, " %s", row->cells[i]);
>>> +
>>> +for (j = 0; j < maxwidths[i] - widths[i] + 2; j++)
>>> +printCB(ctl, " ");
>>> +}
>>> +printCB(ctl, "\n");
>>> +} else {
>>> +for (i = 0; i < row->ncells; i++) {
>>> +virBufferAsprintf(buf, " %s", row->cells[i]);
>>> +
>>> +for (j = 0; j < maxwidths[i] - widths[i] + 2; j++)
>>> +virBufferAddStr(buf, " ");
>>> +}
>>> +virBufferAddStr(buf, "\n");
>>> +}
>>
>>
>> How about inverting these ifs and fors? I mean:
>>
>> for () {
>>   if (toStdout)
>> printCB()
>>   else
>> virBufferAsprintf.
>> }
>>
>> This suggestion applies to vshTablePrint too.
> 
> I had it the way you are suggesting before, but I inverted it because I
> think the current state of code is more readable, compared to its
> inverted state. 
> The length of code would be very similar, but perhaps it would be more
> optimized? (we would have only 2 'for' loops instead of 4, however more
> if-else statements) If that's the reason why are you suggesting to
> invert it.

Well, the reason is that if we ever need to change the way we're
iterating (e.g. call some function at the end of each loop) we can do it
in one change:

 for () {
   if (toStdout)
 printCB()
   else
 virBufferAsprintf()

+  newFunction();
 }


Michal
> 
>>
>>> +
>>> +}
>>> +
>>> +/**
>>> + * vshTablePrint:
>>> + * @ctl virtshell control structure
>>> + * @table: table to print
>>> + * @toStdout: whetever to print to stdout (true) or return in
>>> string (false)
>>> + *
>>> + * Prints table. To get an alignment of columns right, function
>>> + * fills 2d array @widths with count of characters in each cell
>>> and
>>> + * array @maxwidths maximum count of character in each column.
>>> + * Function then prints tables header and content.
>>> + *
>>> + * Returns string containing table, or NULL if table was printed
>>> to
>>> + * stdout
>>> + */
>>> +static char *
>>> +vshTablePrint(vshControl *ctl, vshTablePtr table, bool toStdout)
>>> +{
>>> +size_t i;
>>> +

Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable

[Ivan Mishonov]

On 08/13/2018 11:00 AM, Daniel P. Berrangé wrote:


On Sun, Aug 12, 2018 at 08:22:08PM +0400, Roman Bogorodskiy wrote:

   Ivan Mishonov wrote:


Yes, that makes sense. I'll try to find some time next week to redo my
code and send another patch. Since my time for working on libvirt is
very limited can you confirm that the LPC configuration should look like
this:

     
     
     

This looks reasonable to me. However, it adds some corner cases we need to
handle:

1. I'm wondering if we should still default to 31 if this entry is not 
specified?
We can generate this entry when post-processing XML, but I'm not sure
what's the best way to handle upgrades for the existing domains...

It depends if the BHyve driver is at a point where you consider stable
upgrades important or not. It could be valid for you to just change the
default to 31 if you think its better and upgrade stability is not
required yet.
I decided to check what vm-bhyve does. 
https://github.com/churchers/vm-bhyve/blob/master/lib/vm-run#L367. They 
seem to always place LPC at slot 31 so I guess it's safe to move it



2. According to bhyve(8) manual page, lpc is only supported on bus 0, so
need to add 'isa-bridge' specific validation to check that.

If its only supported in 1 address, then arguably you don't need to add
this at all - just fix the historically mistaken use of 31 in the code
and leave it out of XML.

Yes, we might not need that option at all


Regards,
Daniel


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

Re: [libvirt] [PATCH v3 08/11] util: netdev: use VIR_AUTOPTR for aggregate types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:16AM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH v3 09/11] util: netdevip: use VIR_AUTOPTR for aggregate types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:17AM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
>  src/util/virnetdevip.c | 130 
> +++--
>  1 file changed, 51 insertions(+), 79 deletions(-)
>
> diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
> index c6d6175..4a38a54 100644
> --- a/src/util/virnetdevip.c
> +++ b/src/util/virnetdevip.c
> @@ -168,10 +168,9 @@ virNetDevIPAddrAdd(const char *ifname,
> virSocketAddr *peer,
> unsigned int prefix)
>  {
> -virSocketAddr *broadcast = NULL;
> -int ret = -1;
> -struct nl_msg *nlmsg = NULL;
>  unsigned int recvbuflen;
> +VIR_AUTOPTR(virNlMsg) nlmsg = NULL;
> +VIR_AUTOPTR(virSocketAddr) broadcast = NULL;
>  VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
>  VIR_AUTOFREE(char *) ipStr = NULL;
>  VIR_AUTOFREE(char *) peerStr = NULL;
> @@ -186,13 +185,13 @@ virNetDevIPAddrAdd(const char *ifname,
>  !(peer && VIR_SOCKET_ADDR_VALID(peer))) {
>  /* compute a broadcast address if this is IPv4 */
>  if (VIR_ALLOC(broadcast) < 0)
> -goto cleanup;
> +return -1;
>
>  if (virSocketAddrBroadcastByPrefix(addr, prefix, broadcast) < 0) {
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> -   _("Failed to determine broadcast address for 
> '%s/%d'"),
> -   ipStr, prefix);
> -goto cleanup;
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed to determine broadcast address for 
> '%s/%d'"),
> +   ipStr, prefix);
> +return -1;
>  }
>  bcastStr = virSocketAddrFormat(broadcast);
>  }
> @@ -206,11 +205,11 @@ virNetDevIPAddrAdd(const char *ifname,
>  if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_NEWADDR, ifname,
> addr, prefix,
> broadcast, peer)))
> -goto cleanup;
> +return -1;
>
>  if (virNetlinkCommand(nlmsg, , ,
>0, 0, NETLINK_ROUTE, 0) < 0)
> -goto cleanup;
> +return -1;
>
>
>  if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) {
> @@ -220,14 +219,10 @@ virNetDevIPAddrAdd(const char *ifname,
> peerStr ? " peer " : "", peerStr ? peerStr : "",
> bcastStr ? " bcast " : "", bcastStr ? bcastStr : "",
> ifname);
> -goto cleanup;
> +return -1;
>  }
>
> -ret = 0;
> - cleanup:
> -nlmsg_free(nlmsg);
> -VIR_FREE(broadcast);
> -return ret;
> +return 0;
>  }
>
>
> @@ -246,30 +241,26 @@ virNetDevIPAddrDel(const char *ifname,
> virSocketAddr *addr,
> unsigned int prefix)
>  {
> -int ret = -1;
> -struct nl_msg *nlmsg = NULL;
>  unsigned int recvbuflen;
> +VIR_AUTOPTR(virNlMsg) nlmsg = NULL;
>  VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
>
>  if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_DELADDR, ifname,
> addr, prefix,
> NULL, NULL)))
> -goto cleanup;
> +return -1;
>
>  if (virNetlinkCommand(nlmsg, , , 0, 0,
>NETLINK_ROUTE, 0) < 0)
> -goto cleanup;
> +return -1;
>
>  if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) {
>  virReportError(VIR_ERR_SYSTEM_ERROR,
> _("Error removing IP address from %s"), ifname);
> -goto cleanup;
> +return -1;
>  }
>
> -ret = 0;
> - cleanup:
> -nlmsg_free(nlmsg);
> -return ret;
> +return 0;
>  }
>
>
> @@ -292,8 +283,6 @@ virNetDevIPRouteAdd(const char *ifname,
>  virSocketAddrPtr gateway,
>  unsigned int metric)
>  {
> -int ret = -1;
> -struct nl_msg *nlmsg = NULL;
>  struct nlmsghdr *resp = NULL;
>  unsigned int recvbuflen;
>  unsigned int ifindex;
> @@ -304,6 +293,7 @@ virNetDevIPRouteAdd(const char *ifname,
>  int errCode;
>  virSocketAddr defaultAddr;
>  virSocketAddrPtr actualAddr;
> +VIR_AUTOPTR(virNlMsg) nlmsg = NULL;
>  VIR_AUTOFREE(char *) toStr = NULL;
>  VIR_AUTOFREE(char *) viaStr = NULL;
>
> @@ -315,10 +305,10 @@ virNetDevIPRouteAdd(const char *ifname,
>  int family = VIR_SOCKET_ADDR_FAMILY(gateway);
>  if (family == AF_INET) {
>  if (virSocketAddrParseIPv4(, 
> VIR_SOCKET_ADDR_IPV4_ALL) < 0)
> -goto cleanup;
> +return 

Re: [libvirt] [RFC 0/2] util: introduce timeout mode in virCommandRun/Async and virKModLoad

[shilei.massclo...@gmx.com]

On Monday, August 13, 2018 at 10:02 AM, Daniel P. Berrangé wrote:
>On Mon, Aug 13, 2018 at 02:32:17PM +0800, Shi Lei wrote:
>> Hi, everyone!
>> It's possible that the running-time of a command is long than its caller
>> expected. Perhaps, it's useful to provide timeout mode for virCommandRun or
>> virCommandRunAsync + virCommandWait. And The caller can restrict the
>> running time of the command if necessary. I introduce a new function
>> virCommandSetTimeout for that.
>>
>> And then, virKModLoad will get a default timeout of 3 seconds. I hope it
>> could solve the problem of "strange delay" as John mentioned.
>
>I'm not really see what this is hoping to solve. If something in libvirt
>is running virKModLoad, I cannot see how it will work better by not waiting
>for it to complete. 

Thanks for your quick reply! I hope I could explain my idea more clear:)

The virKModLoad is used for loading external kernel modules which are not under
the control of Libvirt. We cannot make sure they're always solid and strong
(especially for various pci drivers). If the INIT process of a module is blocked
for some reasons, the caller of the virKModLoad will be blocked itself and
cannot known what happened internally and have no chance to handle it.
Most modules need only dozens of milliseconds to complete loading.
On my slow PC, module 'nbd' needs about 2 milliseconds to load and '8021q' needs
about 30 milliseconds. And I will try to test other pci drivers.
If loading time is longer than a few seconds, we can presume there are some 
exceptions
or errors in the loading process of the module. With timeout mode, virKModLoad 
can
report it to the caller and the caller could decide how to handle it.
Timeout mode for virKModLoad is just an insurance which could avoid blocking 
indefinitely.

It's more complicated to call virCommandRun/Async. So timeout mode for
virCommandRun/Async is just an optional mode and the developers can decide
wether they need to use it according to the command.

With virCommandSetTimeout, commands can be set timeout in milliseconds.
It can be used under one of these conditions:

1) In synchronous mode, using virCommandRun NOT in daemon mode.
By default, virCommandRun waits for the command to complete & exit and then 
checks
its exit status. If the caller uses virCommandSetTimeout before it, the command 
will
be aborted internally when timeout and there is no need to use virCommandAbort 
to
reap the child process. For example:

int timeout = 3000; /* in milliseconds */
virCommandSetTimeout(cmd, timeout);
if (virCommandRun(cmd, NULL) < 0) {
    if (virCommandGetErr(cmd) == ETIME)
        ... do something for timeout, e.g. report error ...

    return -1;
}

The argument timeout of the virCommandSetTimeout accepts a positive value. When 
timeout,
virCommandRun return -1 and the caller can use virCommandGetErr to check 
errorcode.
And ETIME means command timeout.
Note:virCommandSetTimeout cannot mix with virCommandDaemonize. Or virCommandRun 
fails directly.

2) In asynchronous mode, the caller can also call virCommandSetTimeout before
virCommandRunAsync to limit the running time of the command. The caller uses 
virCommandWait to
wait for the child process to complete & exit. virCommandWait returns -1 if 
timeout and
the caller can use virCommandGetErr to check the reason. For example:

char *outbuf = NULL;
char *errbuf = NULL;
int timeout = 3000; /* in milliseconds */
virCommandSetTimeout(cmd, timeout);

virCommandSetOutputBuffer(cmd, );
virCommandSetErrorBuffer(cmd, );
virCommandDoAsyncIO(cmd);
if (virCommandRunAsync(cmd, NULL) < 0)
   return -1;

... do something ...

if (virCommandWait(cmd, NULL) < 0) {
    if (virCommandGetErr(cmd) == ETIME)
        ... do something for timeout, e.g. report error ...

    return -1;
}

Regards,
Shilei

>
>Regards,
>Daniel
>--
>|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org -o- https://fstop138.berrange.com :|
>|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v3 01/11] util: iscsi: use VIR_AUTOFREE instead of VIR_FREE for scalar types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:09AM +0530, Sukrit Bhatnagar wrote:
> Add another usage for VIR_AUTOFREE macro which was left in the
> commit ec3e878, thereby dropping a VIR_FREE call and and a cleanup
> section.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
>  src/util/viriscsi.c | 22 ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
> index c805ffc..cf07968 100644
> --- a/src/util/viriscsi.c
> +++ b/src/util/viriscsi.c
> @@ -208,9 +208,10 @@ static int
>  virStorageBackendCreateIfaceIQN(const char *initiatoriqn,
>  char **ifacename)
>  {
> -int ret = -1, exitstatus = -1;
> +int exitstatus = -1;
> +VIR_AUTOPTR(virCommand) cmd = NULL;
> +VIR_AUTOFREE(char *) iface_name = NULL;
>  VIR_AUTOFREE(char *) temp_ifacename = NULL;
> -VIR_AUTOPTR(virCommand) cmd = NULL;

^This @cmd movement is unjustified, I'll drop it before merging.

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [RFC 0/2] util: introduce timeout mode in virCommandRun/Async and virKModLoad

[Daniel P . Berrangé]

On Mon, Aug 13, 2018 at 07:37:33PM +0800, shilei.massclo...@gmx.com wrote:
> On Monday, August 13, 2018 at 10:02 AM, Daniel P. Berrangé wrote:
> >On Mon, Aug 13, 2018 at 02:32:17PM +0800, Shi Lei wrote:
> >> Hi, everyone!
> >> It's possible that the running-time of a command is long than its caller
> >> expected. Perhaps, it's useful to provide timeout mode for virCommandRun or
> >> virCommandRunAsync + virCommandWait. And The caller can restrict the
> >> running time of the command if necessary. I introduce a new function
> >> virCommandSetTimeout for that.
> >>
> >> And then, virKModLoad will get a default timeout of 3 seconds. I hope it
> >> could solve the problem of "strange delay" as John mentioned.
> >
> >I'm not really see what this is hoping to solve. If something in libvirt
> >is running virKModLoad, I cannot see how it will work better by not waiting
> >for it to complete. 
> 
> Thanks for your quick reply! I hope I could explain my idea more clear:)
> 
> The virKModLoad is used for loading external kernel modules which are not 
> under
> the control of Libvirt. We cannot make sure they're always solid and strong
> (especially for various pci drivers). If the INIT process of a module is 
> blocked
> for some reasons, the caller of the virKModLoad will be blocked itself and
> cannot known what happened internally and have no chance to handle it.
> Most modules need only dozens of milliseconds to complete loading.
> On my slow PC, module 'nbd' needs about 2 milliseconds to load and '8021q' 
> needs
> about 30 milliseconds. And I will try to test other pci drivers.
> If loading time is longer than a few seconds, we can presume there are some 
> exceptions
> or errors in the loading process of the module. With timeout mode, 
> virKModLoad can
> report it to the caller and the caller could decide how to handle it.
> Timeout mode for virKModLoad is just an insurance which could avoid blocking 
> indefinitely.

virKModLoad is only used in two places, one to load the nbd module, and one
place to load either pci-stub or pciback or vfio-pci. It is not used to load
arbitrary external device specific kernel modules. So this still doesn't
explain why a timeout is required here.


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 00/16] Revert Jansson usage

[Ján Tomko]

For QEMU, we need a JSON parser that is able to handle its non-compliant
JSON usage:
https://bugzilla.redhat.com/show_bug.cgi?id=1614569

Unfortunately, this does not seem to be possible with Jansson.

Revert back to using yajl, which also lets us get rid of the 'dlopen'
hacks and their bugfixes.

(The QEMU default changes are not strictly required, but the patches
 specifically use JANSSON)

Also available on:
git://repo.or.cz/libvirt/jtomko.git revert

Ján Tomko (16):
  Revert "src: Move DLOPEN_LIBS to libraries introducing the dependency"
  Revert "Fix link errors in tools/nss and tests"
  Revert "remote: daemon: Make sure that JSON symbols are properly
loaded at startup"
  Revert "util: jsoncompat: Stub out virJSONInitialize when compiling
without jansson"
  Revert "tests: qemucapsprobe: Fix output after switching to jansson"
  Revert "util: avoid symbol clash between json libraries"
  Revert "tests: also skip qemuagenttest with old jansson"
  Revert "m4: Introduce STABLE_ORDERING_JANSSON"
  Revert "build: require Jansson if QEMU driver is enabled"
  Revert "build: switch --with-qemu default from yes to check"
  Revert "Remove virJSONValueNewStringLen"
  Revert "build: remove references to WITH_YAJL for SETUID_RPC_CLIENT"
  Revert "Remove functions using yajl"
  Revert "Switch from yajl to Jansson"
  Revert "build: undef WITH_JANSSON for SETUID_RPC_CLIENT"
  Revert "build: add --with-jansson"

 config-post.h|   3 +-
 configure.ac |   3 -
 libvirt.spec.in  |   6 +-
 m4/virt-driver-qemu.m4   |   9 +-
 m4/virt-jansson.m4   |  32 --
 m4/virt-nss.m4   |   4 +-
 m4/virt-yajl.m4  |  27 +-
 src/Makefile.am  |  14 +-
 src/libvirt_private.syms |   5 +-
 src/qemu/qemu_driver.c   |   2 +-
 src/remote/remote_daemon.c   |   4 -
 src/util/Makefile.inc.am |   6 +-
 src/util/virjson.c   | 617 ---
 src/util/virjson.h   |   1 +
 src/util/virjsoncompat.c | 285 --
 src/util/virjsoncompat.h |  88 -
 tests/Makefile.am|  12 +-
 tests/cputest.c  |  16 +-
 tests/libxlxml2domconfigtest.c   |   4 +-
 tests/qemuagenttest.c|   4 +-
 tests/qemublocktest.c|   6 -
 tests/qemucapabilitiestest.c |   7 +-
 tests/qemucaps2xmltest.c |   2 +-
 tests/qemucapsprobemock.c|   2 -
 tests/qemucommandutiltest.c  |   7 +-
 tests/qemuhotplugtest.c  |   7 +-
 tests/qemumigparamsdata/empty.json   |   4 +-
 tests/qemumigparamsdata/unsupported.json |   4 +-
 tests/qemumigparamstest.c|   7 +-
 tests/qemumonitorjsontest.c  |   7 +-
 tests/virjsontest.c  |   5 -
 tests/virmacmaptest.c|   5 -
 tests/virmacmaptestdata/empty.json   |   4 +-
 tests/virmocklibxl.c |   4 +-
 tests/virnetdaemontest.c |   7 +-
 tests/virstoragetest.c   |   4 +-
 tools/Makefile.am|   6 +-
 37 files changed, 557 insertions(+), 673 deletions(-)
 delete mode 100644 m4/virt-jansson.m4
 delete mode 100644 src/util/virjsoncompat.c
 delete mode 100644 src/util/virjsoncompat.h

-- 
2.16.1

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

[libvirt] [PATCH 01/16] Revert "src: Move DLOPEN_LIBS to libraries introducing the dependency"

[Ján Tomko]

This reverts commit 5d40272ea67c74049600e120095d1b42287ed2d2.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 src/Makefile.am  | 2 --
 src/util/Makefile.inc.am | 1 -
 tools/Makefile.am| 4 ++--
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 61876cf382..a4f213480e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -752,7 +752,6 @@ libvirt_setuid_rpc_client_la_CFLAGS = \
$(AM_CFLAGS) \
$(SECDRIVER_CFLAGS) \
$(XDR_CFLAGS) \
-   $(DLOPEN_LIBS) \
$(NULL)
 endif WITH_SETUID_RPC_CLIENT
 
@@ -1001,7 +1000,6 @@ libvirt_nss_la_CFLAGS = \
$(NULL)
 libvirt_nss_la_LDFLAGS = \
$(AM_LDFLAGS) \
-   $(DLOPEN_LIBS) \
$(NULL)
 endif WITH_NSS
 
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index c5c50f1844..8ef9ee1dfa 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -278,7 +278,6 @@ libvirt_util_la_LIBADD = \
$(NUMACTL_LIBS) \
$(ACL_LIBS) \
$(GNUTLS_LIBS) \
-   $(DLOPEN_LIBS) \
$(NULL)
 
 
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 3e129c04c4..26c887649e 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -528,7 +528,7 @@ nss_libnss_libvirt_impl_la_CFLAGS = \
 nss_libnss_libvirt_impl_la_LIBADD = \
../gnulib/lib/libgnu.la \
../src/libvirt-nss.la \
-   $(NULL)
+   $(DLOPEN_LIBS)
 
 nss_libnss_libvirt_la_SOURCES =
 nss_libnss_libvirt_la_LDFLAGS = \
@@ -556,7 +556,7 @@ nss_libnss_libvirt_guest_impl_la_CFLAGS = \
 nss_libnss_libvirt_guest_impl_la_LIBADD = \
../gnulib/lib/libgnu.la \
../src/libvirt-nss.la \
-   $(NULL)
+   $(DLOPEN_LIBS)
 
 nss_libnss_libvirt_guest_la_SOURCES =
 nss_libnss_libvirt_guest_la_LDFLAGS = \
-- 
2.16.1

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

[libvirt] [PATCH 04/16] Revert "util: jsoncompat: Stub out virJSONInitialize when compiling without jansson"

[Ján Tomko]

This reverts commit 9e44c2db8ad94d3c20acc1d081538c280af198b4.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 src/util/virjsoncompat.c | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/src/util/virjsoncompat.c b/src/util/virjsoncompat.c
index ffbeb5488c..6c853e9bb5 100644
--- a/src/util/virjsoncompat.c
+++ b/src/util/virjsoncompat.c
@@ -271,15 +271,4 @@ json_true_impl(void)
 return json_true_ptr();
 }
 
-
-#else /* !WITH_JANSSON */
-
-
-int
-virJSONInitialize(void)
-{
-return 0;
-}
-
-
-#endif /* !WITH_JANSSON */
+#endif /* WITH_JANSSON */
-- 
2.16.1

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

[libvirt] [PATCH 02/16] Revert "Fix link errors in tools/nss and tests"

[Ján Tomko]

This reverts commit b3d9b08ef797e569b14cfa42d3dceba23c2a5b14.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 tools/Makefile.am | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 26c887649e..1452d984a0 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -527,8 +527,7 @@ nss_libnss_libvirt_impl_la_CFLAGS = \
 
 nss_libnss_libvirt_impl_la_LIBADD = \
../gnulib/lib/libgnu.la \
-   ../src/libvirt-nss.la \
-   $(DLOPEN_LIBS)
+   ../src/libvirt-nss.la
 
 nss_libnss_libvirt_la_SOURCES =
 nss_libnss_libvirt_la_LDFLAGS = \
@@ -555,8 +554,7 @@ nss_libnss_libvirt_guest_impl_la_CFLAGS = \
 
 nss_libnss_libvirt_guest_impl_la_LIBADD = \
../gnulib/lib/libgnu.la \
-   ../src/libvirt-nss.la \
-   $(DLOPEN_LIBS)
+   ../src/libvirt-nss.la
 
 nss_libnss_libvirt_guest_la_SOURCES =
 nss_libnss_libvirt_guest_la_LDFLAGS = \
-- 
2.16.1

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

[libvirt] [PATCH 05/16] Revert "tests: qemucapsprobe: Fix output after switching to jansson"

[Ján Tomko]

This reverts commit 397447f80588438545994a86883792a5999cad15.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 tests/qemucapsprobemock.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/qemucapsprobemock.c b/tests/qemucapsprobemock.c
index 049b16273a..5936975505 100644
--- a/tests/qemucapsprobemock.c
+++ b/tests/qemucapsprobemock.c
@@ -76,7 +76,6 @@ qemuMonitorSend(qemuMonitorPtr mon,
 printLineSkipEmpty("\n", stdout);
 
 printLineSkipEmpty(reformatted, stdout);
-printLineSkipEmpty("\n", stdout);
 VIR_FREE(reformatted);
 
 return realQemuMonitorSend(mon, msg);
@@ -117,7 +116,6 @@ qemuMonitorJSONIOProcessLine(qemuMonitorPtr mon,
 printLineSkipEmpty("\n", stdout);
 
 printLineSkipEmpty(json, stdout);
-printLineSkipEmpty("\n", stdout);
 }
 
  cleanup:
-- 
2.16.1

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

[libvirt] [PATCH 03/16] Revert "remote: daemon: Make sure that JSON symbols are properly loaded at startup"

[Ján Tomko]

This reverts commit 3251fc9c9b9639c3fec3181530599415523d671a.

Jansson cannot parse QEMU's quirky JSON.
Revert back to yajl.

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

Signed-off-by: Ján Tomko 
---
 src/libvirt_private.syms   | 4 
 src/remote/remote_daemon.c | 4 
 2 files changed, 8 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ee0dca6129..496de11168 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2133,10 +2133,6 @@ virJSONValueObjectStealObject;
 virJSONValueToString;
 
 
-# util/virjsoncompat.h
-virJSONInitialize;
-
-
 # util/virkeycode.h
 virKeycodeSetTypeFromString;
 virKeycodeSetTypeToString;
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 8bbc3818bb..9f3a5f38ad 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -59,7 +59,6 @@
 #include "virutil.h"
 #include "virgettext.h"
 #include "util/virnetdevopenvswitch.h"
-#include "virjsoncompat.h"
 
 #include "driver.h"
 
@@ -1184,9 +1183,6 @@ int main(int argc, char **argv) {
 exit(EXIT_FAILURE);
 }
 
-if (virJSONInitialize() < 0)
-exit(EXIT_FAILURE);
-
 daemonSetupNetDevOpenvswitch(config);
 
 if (daemonSetupAccessManager(config) < 0) {
-- 
2.16.1

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

[libvirt] [PATCH] qemu-doc: mark ppc/prep machine as deprecated

[Hervé Poussineau]

40p machine type should be used instead.

Signed-off-by: Hervé Poussineau 
---
 qemu-deprecated.texi | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 9920a85adc..9c7ff3fe2d 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -218,6 +218,12 @@ support page sizes < 4096 any longer.
 These machine types are very old and likely can not be used for live migration
 from old QEMU versions anymore. A newer machine type should be used instead.
 
+@subsection prep (PowerPC) (since 3.1)
+
+This machine type uses an unmaintained firmware, broken in lots of ways,
+and unable to start post-2004 operating systems. 40p machine type should be
+used instead.
+
 @section Device options
 
 @subsection Block device options
-- 
2.11.0

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

Re: [libvirt] [PATCH 0/3] conf: qemu: support new Hyper-V enlightenments

[Roman Kagan]

On Thu, Aug 09, 2018 at 03:14:18PM +0200, Vitaly Kuznetsov wrote:
> Several new Hyper-V enlightenments were recently added to Qemu:
> - hv-frequencies
> - hv-reenlightenment
> - hv-tlbflush
> 
> Support these in libvirt.
> 
> Vitaly Kuznetsov (3):
>   conf: qemu: add support for Hyper-V frequency MSRs
>   conf: qemu: add support for Hyper-V reenlightenment notifications
>   conf: qemu: add support for Hyper-V PV TLB flush
> 
>  docs/formatdomain.html.in   | 21 +
>  docs/schemas/domaincommon.rng   | 15 +++
>  src/conf/domain_conf.c  | 14 +-
>  src/conf/domain_conf.h  |  3 +++
>  src/cpu/cpu_x86.c   |  9 +
>  src/cpu/cpu_x86_data.h  |  3 +++
>  src/qemu/qemu_command.c |  3 +++
>  src/qemu/qemu_parse_command.c   |  3 +++
>  src/qemu/qemu_process.c |  3 +++
>  tests/qemuxml2argvdata/hyperv-off.xml   |  3 +++
>  tests/qemuxml2argvdata/hyperv.args  |  3 ++-
>  tests/qemuxml2argvdata/hyperv.xml   |  3 +++
>  tests/qemuxml2xmloutdata/hyperv-off.xml |  3 +++
>  tests/qemuxml2xmloutdata/hyperv.xml |  3 +++
>  14 files changed, 87 insertions(+), 2 deletions(-)

Bringing this to the attention of our libvirt guru...

Roman.

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

Re: [libvirt] [PATCH] esx: Fix build when libcurl debug is enabled

[Marcos Paulo de Souza]

On Mon, Aug 13, 2018 at 03:51:51PM +0200, Michal Prívozník wrote:
> On 08/11/2018 04:39 PM, Marcos Paulo de Souza wrote:
> > When building libvirt with libcurl debug enabled (with
> > ESX_VI__CURL__ENABLE_DEBUG_OUTPUT set), the message bellow pops up:
> > 
> > make[3]: Entering directory '/mnt/data/gitroot/libvirt/src'
> >   CC   esx/libvirt_driver_esx_la-esx_vi.lo
> > esx/esx_vi.c: In function 'esxVI_CURL_Debug':
> > esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_SSL_DATA_IN' not 
> > handled in switch [-Werror=switch-enum]
> >  switch (type) {
> >  ^~
> > esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_SSL_DATA_OUT' not 
> > handled in switch [-Werror=switch-enum]
> > esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_END' not handled in 
> > switch [-Werror=switch-enum]
> > 
> > Our build requires at least libcurl 7.18.0, which is pretty stable since
> > it was release in 2008. Fix this problem by handling the mentioned enums
> > in the code.
> > 
> > Signed-off-by: Marcos Paulo de Souza 
> > ---
> >  src/esx/esx_vi.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> > index a816c3a4f9..588670e137 100644
> > --- a/src/esx/esx_vi.c
> > +++ b/src/esx/esx_vi.c
> > @@ -163,7 +163,7 @@ esxVI_CURL_WriteBuffer(char *data, size_t size, size_t 
> > nmemb, void *userdata)
> >  return 0;
> >  }
> >  
> > -#define ESX_VI__CURL__ENABLE_DEBUG_OUTPUT 0
> > +#define ESX_VI__CURL__ENABLE_DEBUG_OUTPUT 1
> 
> The part below is fine. However, I'm not sure about this one ^^. This
> turns curl debugging on by default. I'm not sure that is what we want.

You are right, this part was included by mistake. Would you want me to send a
new version with this part removed?

> 
> >  
> >  #if ESX_VI__CURL__ENABLE_DEBUG_OUTPUT
> >  static int
> > @@ -205,13 +205,19 @@ esxVI_CURL_Debug(CURL *curl ATTRIBUTE_UNUSED, 
> > curl_infotype type,
> >  break;
> >  
> >case CURLINFO_DATA_IN:
> > +  case CURLINFO_SSL_DATA_IN:
> >  VIR_DEBUG("CURLINFO_DATA_IN %s", buffer);
> >  break;
> >  
> >case CURLINFO_DATA_OUT:
> > +  case CURLINFO_SSL_DATA_OUT:
> >  VIR_DEBUG("CURLINFO_DATA_OUT %s", buffer);
> >  break;
> >  
> > +  case CURLINFO_END:
> > +VIR_DEBUG("CURLINFO_END %s", buffer);
> > +break;
> > +
> >default:
> >  VIR_DEBUG("unknown");
> >  break;
> > 
> 
> 
> Michal

-- 
Thanks,
Marcos

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

Re: [libvirt] [PATCH v3 3/3] storage: add wipeVol to iscsi-direct storage backend

[Michal Prívozník]

On 08/13/2018 12:26 AM, c...@lse.epita.fr wrote:
> From: Clementine Hayat 
> 
> Signed-off-by: Clementine Hayat 
> ---
>  src/storage/storage_backend_iscsi_direct.c | 121 -
>  1 file changed, 120 insertions(+), 1 deletion(-)
> 
> diff --git a/src/storage/storage_backend_iscsi_direct.c 
> b/src/storage/storage_backend_iscsi_direct.c
> index 7bdd39582b..5a7d70f24d 100644
> --- a/src/storage/storage_backend_iscsi_direct.c
> +++ b/src/storage/storage_backend_iscsi_direct.c
> @@ -41,6 +41,8 @@
>  
>  #define ISCSI_DEFAULT_TARGET_PORT 3260
>  #define VIR_ISCSI_TEST_UNIT_TIMEOUT 30 * 1000
> +#define BLOCK_PER_PACKET 128
> +#define VOL_NAME_PREFIX "unit:0:0:"
>  
>  VIR_LOG_INIT("storage.storage_backend_iscsi_direct");
>  
> @@ -225,7 +227,7 @@ virISCSIDirectSetVolumeAttributes(virStoragePoolObjPtr 
> pool,
>  {
>  virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
>  
> -if (virAsprintf(>name, "unit:0:0:%u", lun) < 0)
> +if (virAsprintf(>name, "%s%u", VOL_NAME_PREFIX, lun) < 0)
>  return -1;
>  if (virAsprintf(>key, "ip-%s-iscsi-%s-lun-%u", portal,
>  def->source.devices[0].path, lun) < 0)
> @@ -601,12 +603,129 @@ 
> virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool)
>  return ret;
>  }
>  
> +static int
> +virStorageBackendISCSIDirectGetLun(virStorageVolDefPtr vol,
> +   int *lun)
> +{
> +const char *name = vol->name;
> +int ret = -1;
> +
> +if (!STRPREFIX(name, VOL_NAME_PREFIX)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Invalid volume name %s"), name);
> +goto cleanup;
> +}
> +
> +name += strlen(VOL_NAME_PREFIX);
> +if (virStrToLong_i(name, NULL, 10, lun) < 0)
> +goto cleanup;
> +
> +ret = 0;
> + cleanup:
> +return ret;
> +}
> +
> +static int
> +virStorageBackendISCSIDirectVolWipeZero(virStorageVolDefPtr vol,
> +struct iscsi_context *iscsi)
> +{
> +uint32_t lba = 0;
> +uint32_t block_size;
> +uint32_t nb_block;
> +struct scsi_task *task = NULL;
> +int lun = 0;
> +int ret = -1;
> +unsigned char *data;
> +
> +if (virStorageBackendISCSIDirectGetLun(vol, ) < 0)
> +return ret;
> +if (virISCSIDirectTestUnitReady(iscsi, lun) < 0)
> +return ret;
> +if (virISCSIDirectGetVolumeCapacity(iscsi, lun, _size, _block))
> +return ret;
> +if (VIR_ALLOC_N(data, block_size * BLOCK_PER_PACKET))
> +return ret;
> +
> +while (lba < nb_block) {
> +if (nb_block - lba > block_size * BLOCK_PER_PACKET) {
> +
> +if (!(task = iscsi_write10_sync(iscsi, lun, lba, data,
> +block_size * BLOCK_PER_PACKET,
> +block_size, 0, 0, 0, 0, 0)))
> +goto cleanup;
> +scsi_free_scsi_task(task);
> +lba += BLOCK_PER_PACKET;
> +} else {
> +if (!(task = iscsi_write10_sync(iscsi, lun, lba, data, 
> block_size,
> +block_size, 0, 0, 0, 0, 0)))
> +goto cleanup;
> +scsi_free_scsi_task(task);
> +lba++;
> +}
> +}
> +
> +ret = 0;
> + cleanup:
> +VIR_FREE(data);
> +return ret;
> +}
> +
> +static int
> +virStorageBackenISCSIDirectWipeVol(virStoragePoolObjPtr pool,
> +   virStorageVolDefPtr vol,
> +   unsigned int algorithm,
> +   unsigned int flags)
> +{
> +struct iscsi_context *iscsi = NULL;
> +char *portal = NULL;

After my changes in 2/3 this variable is not needed.

> +int ret = -1;
> +
> +virCheckFlags(0, -1);
> +
> +if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, )))
> +goto cleanup;
> +
> +switch ((virStorageVolWipeAlgorithm) algorithm) {
> +case VIR_STORAGE_VOL_WIPE_ALG_ZERO:
> +if (virStorageBackendISCSIDirectVolWipeZero(vol, iscsi) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to wipe volume %s"),
> + vol->name);
> +goto disconnect;
> +}
> +break;
> +case VIR_STORAGE_VOL_WIPE_ALG_TRIM:
> +case VIR_STORAGE_VOL_WIPE_ALG_NNSA:
> +case VIR_STORAGE_VOL_WIPE_ALG_DOD:
> +case VIR_STORAGE_VOL_WIPE_ALG_BSI:
> +case VIR_STORAGE_VOL_WIPE_ALG_GUTMANN:
> +case VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER:
> +case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7:
> +case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33:
> +case VIR_STORAGE_VOL_WIPE_ALG_RANDOM:
> +case VIR_STORAGE_VOL_WIPE_ALG_LAST:
> +virReportError(VIR_ERR_INVALID_ARG, _("unsupported algorithm %d"),
> +   algorithm);
> +goto disconnect;

It's a pity we have to connect even though we know whether requested

Re: [libvirt] [PATCH v3 2/3] storage: add SetConnection to iscsi-direct backend

[Michal Prívozník]

On 08/13/2018 12:26 AM, c...@lse.epita.fr wrote:
> From: Clementine Hayat 
> 
> The code to set the connection and connect using libiscsi will always be
> the same. Add function to avoid code duplication.
> 
> Signed-off-by: Clementine Hayat 
> ---
>  src/storage/storage_backend_iscsi_direct.c | 38 +++---
>  1 file changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/src/storage/storage_backend_iscsi_direct.c 
> b/src/storage/storage_backend_iscsi_direct.c
> index 62b7e0d8bc..7bdd39582b 100644
> --- a/src/storage/storage_backend_iscsi_direct.c
> +++ b/src/storage/storage_backend_iscsi_direct.c
> @@ -557,23 +557,37 @@ virStorageBackendISCSIDirectFindPoolSources(const char 
> *srcSpec,
>  return ret;
>  }
>  
> +struct iscsi_context *
> +virStorageBackendISCSIDirectSetConnection(virStoragePoolObjPtr pool,
> +  char **portal)

The function needs to be static. You're not exporting it, only using it
within this file.
Also, right now the only caller of the function needs to know the
portal, but that will not always be the case. So I think we can have the
function accepting NULL for *portal meaning caller doesn't care what the
portal is.

> +{
> +virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
> +struct iscsi_context *iscsi = NULL;
> +
> +if (!(*iscsi = virISCSIDirectCreateContext(def->source.initiator.iqn)))
> +return iscsi;

This doesn't look right. We don't know what iscsi_context look like, we
shouldn't dereference it. Have you tried to compile this patch? ;-)

> +if (!(*portal = virStorageBackendISCSIDirectPortal(>source)))
> +goto error ;
> +if (virStorageBackendISCSIDirectSetAuth(*iscsi, >source) < 0)
> +goto error ;
> +if (virISCSIDirectSetContext(*iscsi, def->source.devices[0].path, 
> ISCSI_SESSION_NORMAL) < 0)
> +goto error ;
> +if (virISCSIDirectConnect(*iscsi, *portal) < 0)
> +goto error ;
> +return iscsi;
> +
> + error:
> +iscsi_destroy_context(iscsi);
> +return iscsi;
> +}
> +
>  static int
>  virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool)
>  {
> -virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
>  struct iscsi_context *iscsi = NULL;
>  char *portal = NULL;
>  int ret = -1;
> -
> -if (!(iscsi = virISCSIDirectCreateContext(def->source.initiator.iqn)))
> -goto cleanup;
> -if (!(portal = virStorageBackendISCSIDirectPortal(>source)))
> -goto cleanup;
> -if (virStorageBackendISCSIDirectSetAuth(iscsi, >source) < 0)
> -goto cleanup;
> -if (virISCSIDirectSetContext(iscsi, def->source.devices[0].path, 
> ISCSI_SESSION_NORMAL) < 0)
> -goto cleanup;
> -if (virISCSIDirectConnect(iscsi, portal) < 0)
> +if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, )))
>  goto cleanup;
>  if (virISCSIDirectReportLuns(pool, iscsi, portal) < 0)
>  goto disconect;
> @@ -581,9 +595,9 @@ 
> virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool)
>  ret = 0;
>   disconect:
>  virISCSIDirectDisconnect(iscsi);
> +iscsi_destroy_context(iscsi);
>   cleanup:
>  VIR_FREE(portal);
> -iscsi_destroy_context(iscsi);
>  return ret;
>  }
>  
> 

Michal

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

Re: [libvirt] [PATCH v3 0/3] add wipeVol to iscsi-direct

[Michal Prívozník]

On 08/13/2018 12:26 AM, c...@lse.epita.fr wrote:
> From: Clementine Hayat 
> 
> Hello,
> 
> This series of patch is the follow up of:
> v1:https://www.redhat.com/archives/libvir-list/2018-August/msg00557.html
> v2:https://www.redhat.com/archives/libvir-list/2018-August/msg00570.html
> 
> Best Regards,
> 

I'm fixing all the issues I've found, ACKing and pushing.

Michal

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

Re: [libvirt] [PATCH] qemu:Fix snapshot creating with vm xml persistent

[Peter Krempa]

On Thu, Aug 09, 2018 at 17:01:49 +0200, Michal Privoznik wrote:
> On 08/07/2018 01:46 PM, Bobo Du wrote:
> > the vm xml will be existed when the vm is undefined after started.
> > blockcommit interface also has the bug with above.
> > Step1:prepare a vm，eg:test1,start it and undefined
> > Step2: virsh snapshot-create-as test1 --disk-only --no-metadata
> > Step3:ls /etc/libvirt/qemu/test1.xml,then it will be exist here
> > 
> > Signed-off-by: Bobo Du  > ---
> >  src/qemu/qemu_driver.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index fb0d4a8..d977922 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -7570,6 +7570,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
> >  if (!virDomainObjIsActive(vm))
> >  qemuDomainRemoveInactive(driver, vm);
> >  
> > +virDomainDefFree(vm->newDef);
> >  ret = 0;
> >   endjob:
> >  qemuDomainObjEndJob(driver, vm);
> > 
> 
> This doesn't feel right. Firstly, vm->newDef becomes a stale pointer

This is obviously true ...

> after this patch. But more importantly, if snapshot-create-as saves the
> xml it is clearly not testing vm->persistent flag and the fix should
> focus on that.

That was the previous approach which is wrong in my opinion. If
vm->newDef exists we should modify it. The snapshot code should not care
if the VM is persistent or not. We need to modify the definition and
afterwards it's saved.

If the vm->newDef gets used in a different way we'd need to change a lot
of code just because of that.

The real bug is that for transient vms currently there should be no
vm->newDef as this is a fact in many places.

Also the commit message is obviously misleading as it's pointing out the
symptom but not the real bug.

If vm->newDef gets cleared and the commit message points out to the real
problem this should be the preferred solution.


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

Re: [libvirt] [RFC 0/2] util: introduce timeout mode in virCommandRun/Async and virKModLoad

[Daniel P . Berrangé]

On Mon, Aug 13, 2018 at 02:32:17PM +0800, Shi Lei wrote:
> Hi, everyone!
> It's possible that the running-time of a command is long than its caller
> expected. Perhaps, it's useful to provide timeout mode for virCommandRun or
> virCommandRunAsync + virCommandWait. And The caller can restrict the
> running time of the command if necessary. I introduce a new function
> virCommandSetTimeout for that.
> 
> And then, virKModLoad will get a default timeout of 3 seconds. I hope it
> could solve the problem of "strange delay" as John mentioned.

I'm not really see what this is hoping to solve. If something in libvirt
is running virKModLoad, I cannot see how it will work better by not waiting
for it to complete.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 1/2] Add API for printing tables.

[Simon Kobyda]

On Fri, 2018-08-10 at 15:40 +0200, Michal Privoznik wrote:
> On 08/10/2018 11:11 AM, Simon Kobyda wrote:
> > It solves problems with alignment of columns. Width of each column
> > is calculated by its biggest cell. Should solve unicode bug.
> > In future, it may be implemented in virsh, virt-admin...
> > 
> > This API has 5 public functions:
> > - vshTableNew - adds new table and defines its header
> > - vshTableRowAppend - appends new row (for same number of columns
> > as in
> > header)
> > - vshTablePrintToStdout
> > - vshTablePrintToString
> > - vshTableFree
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1574624
> > https://bugzilla.redhat.com/show_bug.cgi?id=1584630
> > 
> > Signed-off-by: Simon Kobyda 
> > ---
> >  tools/Makefile.am |   4 +-
> >  tools/vsh-table.c | 367
> > ++
> >  tools/vsh-table.h |  42 ++
> >  3 files changed, 412 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/vsh-table.c
> >  create mode 100644 tools/vsh-table.h
> > 
> 
> Missing cover letter. When sending more than one patch it's
> necessary.
> Also the patch should have "vsh: " prefix so that it's obvious what
> part
> of the ever growing source it's touching.
> 
> > diff --git a/tools/Makefile.am b/tools/Makefile.am
> > index 26c887649e..ed23575aa7 100644
> > --- a/tools/Makefile.am
> > +++ b/tools/Makefile.am
> > @@ -144,7 +144,9 @@ libvirt_shell_la_LIBADD = \
> > $(READLINE_LIBS) \
> > ../gnulib/lib/libgnu.la \
> > $(NULL)
> > -libvirt_shell_la_SOURCES = vsh.c vsh.h
> > +libvirt_shell_la_SOURCES = \
> > +   vsh.c vsh.h \
> > +   vsh-table.c vsh-table.h
> >  
> >  virt_host_validate_SOURCES = \
> > virt-host-validate.c \
> > diff --git a/tools/vsh-table.c b/tools/vsh-table.c
> > new file mode 100644
> > index 00..11dc807ba9
> > --- /dev/null
> > +++ b/tools/vsh-table.c
> > @@ -0,0 +1,367 @@
> > +/*
> > + * vsh-table.c: table printing helper
> > + *
> > + * Copyright (C) 2018 Red Hat, Inc.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later
> > version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General
> > Public
> > + * License along with this library.  If not, see
> > + * .
> > + *
> > + * Authors:
> > + *   Simon Kobyda 
> > + *
> > + */
> > +
> > +#include 
> > +#include "vsh-table.h"
> > +#include "virsh-util.h"
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "viralloc.h"
> > +#include "virbuffer.h"
> > +#include "virstring.h"
> 
> Nit pick, we tend to include system headers first and only after that
> the local ones.
> 
> > +
> > +typedef void (*vshPrintCB)(vshControl *ctl, const char *fmt, ...);
> > +
> > +struct _vshTableRow {
> > +char **cells;
> > +size_t ncells;
> > +};
> > +
> > +struct _vshTable {
> > +vshTableRowPtr *rows;
> > +size_t nrows;
> > +};
> > +
> > +static void
> > +vshTableRowFree(vshTableRowPtr row)
> > +{
> > +size_t i;
> > +
> > +if (!row)
> > +return;
> > +
> > +for (i = 0; i < row->ncells; i++)
> > +VIR_FREE(row->cells[i]);
> > +
> > +VIR_FREE(row->cells);
> > +VIR_FREE(row);
> > +}
> > +
> > +void
> > +vshTableFree(vshTablePtr table)
> > +{
> > +size_t i;
> > +
> > +if (!table)
> > +return;
> > +
> > +for (i = 0; i < table->nrows; i++)
> > +vshTableRowFree(table->rows[i]);
> > +VIR_FREE(table->rows);
> > +VIR_FREE(table);
> > +}
> > +
> > +/**
> > + * vshTableRowNew:
> > + * @size: expected number of cells in row.
> > + * @arg: the first argument.
> > + * @ap: list of variadic arguments
> > + *
> > + * Creates a new row in the table. Each argument passed
> > + * represents a cell in the row. If the number of cells is not
> > + * equal to @size, then NULL is returned.  However, if @size is
> > + * equal to 0, the aforementioned check is suppressed.
> > + *
> > + * Returns: pointer to vshTableRowPtr row or NULL.
> > + */
> > +static vshTableRowPtr
> > +vshTableRowNew(size_t size, const char *arg, va_list ap)
> 
> I know we discussed this in person, but now that I see this code and
> think about it more I think this @size argument should be dropped
> from
> this function and the corresponding check should be moved to [1]
That way I will implement identical loop also there [1], to count
number of variable arguments to make that check (if there is better way
to count number of variable arguments, 

Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable

[Daniel P . Berrangé]

On Sun, Aug 12, 2018 at 08:22:08PM +0400, Roman Bogorodskiy wrote:
>   Ivan Mishonov wrote:
> 
> > Yes, that makes sense. I'll try to find some time next week to redo my 
> > code and send another patch. Since my time for working on libvirt is 
> > very limited can you confirm that the LPC configuration should look like 
> > this:
> > 
> >     
> >     
> >     
> 
> This looks reasonable to me. However, it adds some corner cases we need to
> handle:
> 
> 1. I'm wondering if we should still default to 31 if this entry is not 
> specified?
> We can generate this entry when post-processing XML, but I'm not sure
> what's the best way to handle upgrades for the existing domains...

It depends if the BHyve driver is at a point where you consider stable
upgrades important or not. It could be valid for you to just change the
default to 31 if you think its better and upgrade stability is not
required yet. 

> 2. According to bhyve(8) manual page, lpc is only supported on bus 0, so
> need to add 'isa-bridge' specific validation to check that.

If its only supported in 1 address, then arguably you don't need to add
this at all - just fix the historically mistaken use of 31 in the code
and leave it out of XML.

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] storage: Properly terminate secrets

[Michal Privoznik]

The virSecretGetSecretString() helper looks up a secret for given
pool and returns its value in @secret_value and its length in
@secret_value_size. However, the trailing '\0' is not included in
either of the variables. This is because usually the value of the
secret is passed to some encoder (usually base64 encoder) where
the trailing zero must not be accounted for.

However, in two places we actually want the string as we don't
process is any further.

Signed-off-by: Michal Privoznik 
---

I wonder if putting this realloc into virSecretGetSecretString() would
be a better fix or not. I mean, without changing @secret_size. Opinions?

 src/storage/storage_backend_iscsi.c| 5 +
 src/storage/storage_backend_iscsi_direct.c | 5 +
 2 files changed, 10 insertions(+)

diff --git a/src/storage/storage_backend_iscsi.c 
b/src/storage/storage_backend_iscsi.c
index 6242cd0fac..55fe47f5e1 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -303,6 +303,11 @@ virStorageBackendISCSISetAuth(const char *portal,
  _value, _size) < 0)
 goto cleanup;
 
+if (VIR_REALLOC_N(secret_value, secret_size + 1) < 0)
+goto cleanup;
+
+secret_value[secret_size] = '\0';
+
 if (virISCSINodeUpdate(portal,
source->devices[0].path,
"node.session.auth.authmethod",
diff --git a/src/storage/storage_backend_iscsi_direct.c 
b/src/storage/storage_backend_iscsi_direct.c
index 1624066e9c..0d7d6ba9c3 100644
--- a/src/storage/storage_backend_iscsi_direct.c
+++ b/src/storage/storage_backend_iscsi_direct.c
@@ -115,6 +115,11 @@ virStorageBackendISCSIDirectSetAuth(struct iscsi_context 
*iscsi,
  _value, _size) < 0)
 goto cleanup;
 
+if (VIR_REALLOC_N(secret_value, secret_size + 1) < 0)
+goto cleanup;
+
+secret_value[secret_size] = '\0';
+
 if (iscsi_set_initiator_username_pwd(iscsi,
  authdef->username,
  (const char *)secret_value) < 0) {
-- 
2.16.4

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

[libvirt] [PATCH 2/2] networkStartNetworkVirtual: Don't leak macmap object

[Michal Privoznik]

When starting network a macmap object is created (which stores
MAC -> domain name mappings). However, if something goes wrong
(e.g. virNetDevIPCheckIPv6Forwarding() fails) then the object is
leaked.

Signed-off-by: Michal Privoznik 
---
 src/network/bridge_driver.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index f92cc61e47..588b0d147d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2453,6 +2453,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr 
driver,
 goto err1;
 
 virNetworkObjSetMacMap(obj, macmap);
+macmap = NULL;
 
 /* Set bridge options */
 
@@ -2590,6 +2591,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr 
driver,
 ignore_value(virNetDevTapDelete(macTapIfName, NULL));
 VIR_FREE(macTapIfName);
 }
+virNetworkObjUnrefMacMap(obj);
 VIR_FREE(macMapFile);
 
  err0:
-- 
2.16.4

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

[libvirt] [PATCH 0/2] Couple of almost trivial memleak fixes

[Michal Privoznik]

*** NOT PUSHED. PLEASE REVIEW ***

Michal Prívozník (2):
  virnetdevip: Free data.devices in virNetDevIPCheckIPv6Forwarding() too
  networkStartNetworkVirtual: Don't leak macmap object

 src/network/bridge_driver.c | 2 ++
 src/util/virnetdevip.c  | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.16.4

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

[libvirt] [PATCH 1/2] virnetdevip: Free data.devices in virNetDevIPCheckIPv6Forwarding() too

[Michal Privoznik]

We are freeing the individual strings (which were filled by
virNetDevIPCheckIPv6ForwardingCallback()) but not the array
itself.

Signed-off-by: Michal Privoznik 
---
 src/util/virnetdevip.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index c6d6175627..866ddcc213 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -651,8 +651,7 @@ virNetDevIPCheckIPv6Forwarding(void)
 
  cleanup:
 nlmsg_free(nlmsg);
-for (i = 0; i < data.ndevices; i++)
-VIR_FREE(data.devices[i]);
+virStringListFreeCount(data.devices, data.ndevices);
 return valid;
 }
 
-- 
2.16.4

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

[libvirt] [jenkins-ci PATCH 0/2] Don't build libvirt-dbus on Debian 8

[Andrea Bolognani]

I thought that it would work, but I was clearly mistaken :/

Andrea Bolognani (2):
  projects: Don't build libvirt-dbus on Debian 8
  guests: Don't prepare Debian 8 for libvirt-dbus

 guests/host_vars/libvirt-debian-8/main.yml | 1 -
 projects/libvirt-dbus.yaml | 1 -
 2 files changed, 2 deletions(-)

-- 
2.17.1

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

Re: [libvirt] [dbus PATCH 1/4] tests: Drop pytest.mark.usefixtures from test_nodedev

[Ján Tomko]

On Fri, Jul 27, 2018 at 02:37:38PM +0200, Andrea Bolognani wrote:

We're already using node_device_create explicitly for all
test cases in order to retrieve the result of the fixture,
so using pytest.mark.usefixtures as well is pointless.

Additionally, we're soon going to add some test cases
where we need the fixture *not* to run automatically.

Signed-off-by: Andrea Bolognani 
---
tests/test_nodedev.py | 1 -
1 file changed, 1 deletion(-)

diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py
index 22c8d60..082cf0b 100755
--- a/tests/test_nodedev.py
+++ b/tests/test_nodedev.py
@@ -5,7 +5,6 @@ import libvirttest
import pytest


-@pytest.mark.usefixtures("node_device_create")
class TestNodeDevice(libvirttest.BaseTestClass):
""" Tests for methods and properties of the NodeDevice interface
"""


This patch broke syntax-check:
./tests/test_nodedev.py:5:1: F401 'pytest' imported but unused
import pytest
^

Jano


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

Re: [libvirt] [PATCH v3 03/11] util: netlink: use VIR_AUTOFREE instead of VIR_FREE for scalar types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:11AM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOFREE macro for declaring scalar variables, majority
> of the VIR_FREE calls can be dropped, which in turn leads to
> getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
>  src/util/virnetlink.c | 43 ---
>  1 file changed, 20 insertions(+), 23 deletions(-)
>
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index ecf62c9..fcdc09d 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -342,10 +342,8 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg,
>unsigned int protocol, unsigned int groups,
>void *opaque)
>  {
> -int ret = -1;
>  bool end = false;
>  int len = 0;
> -struct nlmsghdr *resp = NULL;
>  struct nlmsghdr *msg = NULL;
>
>  struct sockaddr_nl nladdr = {
> @@ -357,9 +355,11 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg,
>
>  if (!(nlhandle = virNetlinkSendRequest(nl_msg, src_pid, nladdr,
> protocol, groups)))
> -goto cleanup;
> +return -1;
>
>  while (!end) {
> +VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
> +
>  len = nl_recv(nlhandle, , (unsigned char **), NULL);
>  VIR_WARNINGS_NO_CAST_ALIGN
>  for (msg = resp; NLMSG_OK(msg, len); msg = NLMSG_NEXT(msg, len)) {
> @@ -368,19 +368,14 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg,
>  end = true;
>
>  if (virNetlinkGetErrorCode(msg, len) < 0)
> -goto cleanup;
> +return -1;
>
>  if (callback(msg, opaque) < 0)
> -goto cleanup;
> +return -1;
>  }
> -VIR_FREE(resp);
>  }
>
> -ret = 0;
> -
> - cleanup:
> -VIR_FREE(resp);
> -return ret;
> +return 0;
>  }
>
>  /**
> @@ -408,7 +403,6 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
> uint32_t src_pid, uint32_t dst_pid)
>  {
>  int rc = -1;
> -struct nlmsghdr *resp = NULL;
>  struct nlmsgerr *err;
>  struct ifinfomsg ifinfo = {
>  .ifi_family = AF_UNSPEC,
> @@ -416,6 +410,9 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
>  };
>  unsigned int recvbuflen;
>  struct nl_msg *nl_msg;
> +VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
> +
> +*nlData = NULL;

unnecessary...

>
>  if (ifname && ifindex <= 0 && virNetDevGetIndex(ifname, ) < 0)
>  return -1;
> @@ -483,12 +480,12 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
>  default:
>  goto malformed_resp;
>  }
> +
> +VIR_STEAL_PTR(*nlData, resp);
>  rc = 0;
> +
>   cleanup:
>  nlmsg_free(nl_msg);
> -if (rc < 0)
> -   VIR_FREE(resp);
> -*nlData = resp;
>  return rc;
>
>   malformed_resp:
> @@ -522,11 +519,11 @@ int
>  virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback)
>  {
>  int rc = -1;
> -struct nlmsghdr *resp = NULL;
>  struct nlmsgerr *err;
>  struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
>  unsigned int recvbuflen;
>  struct nl_msg *nl_msg;
> +VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
>
>  nl_msg = nlmsg_alloc_simple(RTM_DELLINK,
>  NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
> @@ -577,7 +574,6 @@ virNetlinkDelLink(const char *ifname, 
> virNetlinkDelLinkFallback fallback)
>  rc = 0;
>   cleanup:
>  nlmsg_free(nl_msg);
> -VIR_FREE(resp);
>  return rc;
>
>   malformed_resp:
> @@ -610,13 +606,15 @@ int
>  virNetlinkGetNeighbor(void **nlData, uint32_t src_pid, uint32_t dst_pid)
>  {
>  int rc = -1;
> -struct nlmsghdr *resp = NULL;
>  struct nlmsgerr *err;
>  struct ndmsg ndinfo = {
>  .ndm_family = AF_UNSPEC,
>  };
>  unsigned int recvbuflen;
>  struct nl_msg *nl_msg;
> +VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
> +
> +*nlData = NULL;

unnecessary...

The rest is fine, I'll adjust before merging.
Reviewed-by: Erik Skultety 

>
>  nl_msg = nlmsg_alloc_simple(RTM_GETNEIGH, NLM_F_DUMP | NLM_F_REQUEST);
>  if (!nl_msg) {
> @@ -654,13 +652,12 @@ virNetlinkGetNeighbor(void **nlData, uint32_t src_pid, 
> uint32_t dst_pid)
>  default:
>  goto malformed_resp;
>  }
> +
> +VIR_STEAL_PTR(*nlData, resp);
>  rc = recvbuflen;
>
>   cleanup:
>  nlmsg_free(nl_msg);
> -if (rc < 0)
> -   VIR_FREE(resp);
> -*nlData = resp;
>  return rc;
>
>   malformed_resp:
> @@ -766,12 +763,12 @@ virNetlinkEventCallback(int watch,
>  void *opaque)
>  {
>  virNetlinkEventSrvPrivatePtr srv = opaque;
> -struct nlmsghdr *msg;
>  struct sockaddr_nl peer;
>  struct ucred *creds = NULL;
>  size_t i;
>  int length;
>  bool handled = false;
> +

Re: [libvirt] [PATCH v3 05/11] util: netdevbridge: use VIR_AUTOFREE instead of VIR_FREE for scalar types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:13AM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOFREE macro for declaring scalar variables, majority
> of the VIR_FREE calls can be dropped, which in turn leads to
> getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 1/2] Add API for printing tables.

[Simon Kobyda]

On Mon, 2018-08-13 at 12:01 +0200, Michal Prívozník wrote:
> On 08/13/2018 11:46 AM, Simon Kobyda wrote:
> > On Fri, 2018-08-10 at 15:40 +0200, Michal Privoznik wrote:
> > > On 08/10/2018 11:11 AM, Simon Kobyda wrote:
> 
> 
> > > > +static vshTableRowPtr
> > > > +vshTableRowNew(size_t size, const char *arg, va_list ap)
> > > 
> > > I know we discussed this in person, but now that I see this code
> > > and
> > > think about it more I think this @size argument should be dropped
> > > from
> > > this function and the corresponding check should be moved to [1]
> > 
> > That way I will implement identical loop also there [1], to count
> > number of variable arguments to make that check (if there is better
> > way
> > to count number of variable arguments, I'm open to suggestions :)
> > ).
> > Sure I can drop it there, but what are pros of that? I'm curious
> > :).
> 
> I'm not sure why you would need the second loop? The way your code
> works
> right now is that vshTableRowNew must have the sentinel (= args have
> to
> end with NULL). What I am suggesting is for vshTableRowNew() to just
> consume all the args (like it is doing now) until the sentinel. And
> only
> after that check at [1]  whether row->ncells is expected value.
> 
I see, I misunderstood. That's also an option.
> > > 
> > > > +{
> > > > +vshTableRowPtr row = NULL;
> > > > +char *tmp = NULL;
> > > > +
> > > > +if (!arg) {
> > > > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > +_("Table row cannot be empty"));
> > > > +goto error;
> > > > +}
> > > > +
> > > > +if (VIR_ALLOC(row) < 0)
> > > > +goto error;
> > > > +
> > > > +while (arg) {
> > > > +if (VIR_STRDUP(tmp, arg) < 0)
> > > > +goto error;
> > > > +
> > > > +if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) <
> > > > 0)
> > > > +goto error;
> > > > +
> > > > +arg = va_arg(ap, const char *);
> > > > +}
> > > > +
> > > > +if (size && row->ncells != size) {
> > > > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > +_("Incorrect number of cells in a
> > > > table
> > > > row"));
> > > > +goto error;
> > > > +}
> > > > +
> > > > +return row;
> > > > +
> > > > + error:
> > > > +vshTableRowFree(row);
> > > > +return NULL;
> > > > +}
> > > > +
> 
> 
> > > > +
> > > > +/**
> > > > + * vshTableRowPrint:
> > > > + * @ctl virtshell control structure
> > > > + * @row: table to append to
> > > > + * @maxwidths: maximum count of characters for each columns
> > > > + * @widths: count of character for each cell in this row
> > > > + * @printCB function for priting table
> > > > + * @buf: buffer to store table (only if @toStdout == true)
> > > > + * @toStdout: whetever print table to Stdout or return in
> > > > buffer
> > > > + */
> > > > +static void
> > > > +vshTableRowPrint(vshControl *ctl,
> > > > + vshTableRowPtr row,
> > > > + size_t *maxwidths,
> > > > + size_t *widths,
> > > > + vshPrintCB printCB,
> > > > + virBufferPtr buf,
> > > > + bool toStdout)
> > > > +{
> > > > +size_t i;
> > > > +size_t j;
> > > > +
> > > > +if (toStdout) {
> > > > +for (i = 0; i < row->ncells; i++) {
> > > > +printCB(ctl, " %s", row->cells[i]);
> > > > +
> > > > +for (j = 0; j < maxwidths[i] - widths[i] + 2; j++)
> > > > +printCB(ctl, " ");
> > > > +}
> > > > +printCB(ctl, "\n");
> > > > +} else {
> > > > +for (i = 0; i < row->ncells; i++) {
> > > > +virBufferAsprintf(buf, " %s", row->cells[i]);
> > > > +
> > > > +for (j = 0; j < maxwidths[i] - widths[i] + 2; j++)
> > > > +virBufferAddStr(buf, " ");
> > > > +}
> > > > +virBufferAddStr(buf, "\n");
> > > > +}
> > > 
> > > 
> > > How about inverting these ifs and fors? I mean:
> > > 
> > > for () {
> > >   if (toStdout)
> > > printCB()
> > >   else
> > > virBufferAsprintf.
> > > }
> > > 
> > > This suggestion applies to vshTablePrint too.
> > 
> > I had it the way you are suggesting before, but I inverted it
> > because I
> > think the current state of code is more readable, compared to its
> > inverted state. 
> > The length of code would be very similar, but perhaps it would be
> > more
> > optimized? (we would have only 2 'for' loops instead of 4, however
> > more
> > if-else statements) If that's the reason why are you suggesting to
> > invert it.
> 
> Well, the reason is that if we ever need to change the way we're
> iterating (e.g. call some function at the end of each loop) we can do
> it
> in one change:
> 
>  for () {
>if (toStdout)
>  printCB()
>else
>  virBufferAsprintf()
> 
> +  newFunction();
>  }
> 
> 
> Michal
> > 
> > > 
> > > > +
> > > > +}
> > > > +
> > > > +/**
> > > > + * vshTablePrint:
> > > > + * 

Re: [libvirt] [Qemu-devel] [PATCH] qemu-doc: mark ppc/prep machine as deprecated

[Mark Cave-Ayland]

On 10/08/18 23:46, Hervé Poussineau wrote:

> 40p machine type should be used instead.
> 
> Signed-off-by: Hervé Poussineau 
> ---
>  qemu-deprecated.texi | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 9920a85adc..9c7ff3fe2d 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -218,6 +218,12 @@ support page sizes < 4096 any longer.
>  These machine types are very old and likely can not be used for live 
> migration
>  from old QEMU versions anymore. A newer machine type should be used instead.
>  
> +@subsection prep (PowerPC) (since 3.1)
> +
> +This machine type uses an unmaintained firmware, broken in lots of ways,
> +and unable to start post-2004 operating systems. 40p machine type should be
> +used instead.
> +
>  @section Device options
>  
>  @subsection Block device options

Acked-by: Mark Cave-Ayland 


ATB,

Mark.

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

[libvirt] [PATCH] virsh: Add missed fields to pool-define-as item entry

[John Ferlan]

Commit id d45bee449 updated the pool-define-as qualifier descriptions
to add some new fields, but neglected to modify the command item list
in order to add those fields as well.

Signed-off-by: John Ferlan 
---

 Discovered by QE while validating bz1601377.

 tools/virsh.pod | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index a5e02c62cd..4e118851f8 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -3831,7 +3831,9 @@ just I<--build> is provided, then B is called 
with no flags.
 [I<--auth-type authtype> I<--auth-username username>
 [I<--secret-usage usage> | I<--secret-uuid uuid>]]
 [[I<--adapter-name name>] | [I<--adapter-wwnn> wwnn I<--adapter-wwpn> wwpn]
-[I<--adapter-parent parent>]]
+[I<--adapter-parent parent> |
+ I<--adapter-parent-wwnn parent_wwnn> I |
+ I<--adapter-parent-fabric-wwn parent_fabric_wwn>]]
 [I<--build>] [[I<--overwrite>] | [I<--no-overwrite>]] [I<--print-xml>]
 
 
-- 
2.17.1

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

Re: [libvirt] [PATCH v3 03/11] util: netlink: use VIR_AUTOFREE instead of VIR_FREE for scalar types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:11AM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOFREE macro for declaring scalar variables, majority
> of the VIR_FREE calls can be dropped, which in turn leads to
> getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
>  src/util/virnetlink.c | 43 ---
>  1 file changed, 20 insertions(+), 23 deletions(-)
>

Actually, I squashed the following into this patch:

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index ecf62c9e72..cb8072ff94 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -297,13 +297,13 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
   uint32_t src_pid, uint32_t dst_pid,
   unsigned int protocol, unsigned int groups)
 {
-int ret = -1;
 struct sockaddr_nl nladdr = {
 .nl_family = AF_NETLINK,
 .nl_pid= dst_pid,
 .nl_groups = 0,
 };
 struct pollfd fds[1];
+VIR_AUTOFREE(struct nlmsghdr *) temp_resp = NULL;
 VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL;
 int len = 0;

@@ -311,28 +311,22 @@ int virNetlinkCommand(struct nl_msg *nl_msg,

 if (!(nlhandle = virNetlinkSendRequest(nl_msg, src_pid, nladdr,
protocol, groups)))
-goto cleanup;
+return -1;

-len = nl_recv(nlhandle, , (unsigned char **)resp, NULL);
+len = nl_recv(nlhandle, , (unsigned char **)_resp, NULL);
 if (len == 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("nl_recv failed - returned 0 bytes"));
-goto cleanup;
+return -1;
 }
 if (len < 0) {
 virReportSystemError(errno, "%s", _("nl_recv failed"));
-goto cleanup;
+return -1;
 }

-ret = 0;
+VIR_STEAL_PTR(*resp, temp_resp);
 *respbuflen = len;
- cleanup:
-if (ret < 0) {
-*resp = NULL;
-*respbuflen = 0;
-}
-
-return ret;
+return 0;
 }

 Erik

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

Re: [libvirt] [PATCH v3 06/11] util: netdevbridge: use VIR_AUTOPTR for aggregate types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:14AM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 00/16] Revert Jansson usage

[Ján Tomko]

On Mon, Aug 13, 2018 at 01:55:10PM +0200, Ján Tomko wrote:

For QEMU, we need a JSON parser that is able to handle its non-compliant
JSON usage:
https://bugzilla.redhat.com/show_bug.cgi?id=1614569

Unfortunately, this does not seem to be possible with Jansson.

Revert back to using yajl, which also lets us get rid of the 'dlopen'
hacks and their bugfixes.

(The QEMU default changes are not strictly required, but the patches
specifically use JANSSON)

Also available on:
git://repo.or.cz/libvirt/jtomko.git revert

Ján Tomko (16):
 Revert "src: Move DLOPEN_LIBS to libraries introducing the dependency"


Note that this patch broke the build with clang by adding DLOPEN_LIBS to
libvirt_setuid_rpc_client_la_CFLAGS instead of LDFLAGS:
https://www.redhat.com/archives/libvir-list/2018-August/msg00604.html
So technically this series is a build breaker fix ;)


 Revert "Fix link errors in tools/nss and tests"
 Revert "remote: daemon: Make sure that JSON symbols are properly
   loaded at startup"
 Revert "util: jsoncompat: Stub out virJSONInitialize when compiling
   without jansson"
 Revert "tests: qemucapsprobe: Fix output after switching to jansson"
 Revert "util: avoid symbol clash between json libraries"
 Revert "tests: also skip qemuagenttest with old jansson"
 Revert "m4: Introduce STABLE_ORDERING_JANSSON"
 Revert "build: require Jansson if QEMU driver is enabled"
 Revert "build: switch --with-qemu default from yes to check"
 Revert "Remove virJSONValueNewStringLen"
 Revert "build: remove references to WITH_YAJL for SETUID_RPC_CLIENT"
 Revert "Remove functions using yajl"
 Revert "Switch from yajl to Jansson"
 Revert "build: undef WITH_JANSSON for SETUID_RPC_CLIENT"



 Revert "build: add --with-jansson"


And this patch was the only one that did not revert cleanly.

Jano





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

Re: [libvirt] [PATCH 00/16] Revert Jansson usage

[Daniel P . Berrangé]

On Mon, Aug 13, 2018 at 02:54:00PM +0200, Ján Tomko wrote:
> On Mon, Aug 13, 2018 at 01:55:10PM +0200, Ján Tomko wrote:
> > For QEMU, we need a JSON parser that is able to handle its non-compliant
> > JSON usage:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1614569
> > 
> > Unfortunately, this does not seem to be possible with Jansson.
> > 
> > Revert back to using yajl, which also lets us get rid of the 'dlopen'
> > hacks and their bugfixes.
> > 
> > (The QEMU default changes are not strictly required, but the patches
> > specifically use JANSSON)
> > 
> > Also available on:
> > git://repo.or.cz/libvirt/jtomko.git revert
> > 
> > Ján Tomko (16):
> >  Revert "src: Move DLOPEN_LIBS to libraries introducing the dependency"
> 
> Note that this patch broke the build with clang by adding DLOPEN_LIBS to
> libvirt_setuid_rpc_client_la_CFLAGS instead of LDFLAGS:
> https://www.redhat.com/archives/libvir-list/2018-August/msg00604.html
> So technically this series is a build breaker fix ;)
> 
> >  Revert "Fix link errors in tools/nss and tests"
> >  Revert "remote: daemon: Make sure that JSON symbols are properly
> >loaded at startup"
> >  Revert "util: jsoncompat: Stub out virJSONInitialize when compiling
> >without jansson"
> >  Revert "tests: qemucapsprobe: Fix output after switching to jansson"
> >  Revert "util: avoid symbol clash between json libraries"
> >  Revert "tests: also skip qemuagenttest with old jansson"
> >  Revert "m4: Introduce STABLE_ORDERING_JANSSON"
> >  Revert "build: require Jansson if QEMU driver is enabled"
> >  Revert "build: switch --with-qemu default from yes to check"
> >  Revert "Remove virJSONValueNewStringLen"
> >  Revert "build: remove references to WITH_YAJL for SETUID_RPC_CLIENT"
> >  Revert "Remove functions using yajl"
> >  Revert "Switch from yajl to Jansson"
> >  Revert "build: undef WITH_JANSSON for SETUID_RPC_CLIENT"
> 
> >  Revert "build: add --with-jansson"
> 
> And this patch was the only one that did not revert cleanly.

Great, on the basis that these are all clean reverts, and the last
looks sane:

  Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [jenkins-ci PATCH 1/2] projects: Don't build libvirt-dbus on Debian 8

[Andrea Bolognani]

Turns out Debian 8 includes GLib 2.42 whereas libvirt-dbus
requires at least GLib 2.44 to build.

I had accidentally installed GLib 2.48 from backports on
my local Debian 8 guests, which explains how the build
could succeed during testing.

This reverts commit d9acf6fe3f22b2acf2d849890d1fcfdcc09ef63a.

Signed-off-by: Andrea Bolognani 
---
 projects/libvirt-dbus.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/projects/libvirt-dbus.yaml b/projects/libvirt-dbus.yaml
index 3212465..c12a769 100644
--- a/projects/libvirt-dbus.yaml
+++ b/projects/libvirt-dbus.yaml
@@ -9,7 +9,6 @@
   parent_jobs: 'libvirt-glib-master-build'
   machines:
 - libvirt-centos-7
-- libvirt-debian-8
 - libvirt-debian-9
 - libvirt-fedora-27
 - libvirt-fedora-28
-- 
2.17.1

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

[libvirt] [jenkins-ci PATCH 2/2] guests: Don't prepare Debian 8 for libvirt-dbus

[Andrea Bolognani]

We're no longer attempting the build, which was failing
anyway, so no point in installing the dependencies.

This reverts commit ef2b446ecb77cefa2f041e9ad03124bbc13d22c3.

Signed-off-by: Andrea Bolognani 
---
 guests/host_vars/libvirt-debian-8/main.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/guests/host_vars/libvirt-debian-8/main.yml 
b/guests/host_vars/libvirt-debian-8/main.yml
index aca0663..fb37205 100644
--- a/guests/host_vars/libvirt-debian-8/main.yml
+++ b/guests/host_vars/libvirt-debian-8/main.yml
@@ -2,7 +2,6 @@
 projects:
   - libosinfo
   - libvirt
-  - libvirt-dbus
   - libvirt-glib
   - libvirt-go
   - libvirt-go-xml
-- 
2.17.1

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

Re: [libvirt] [jenkins-ci PATCH 0/2] Don't build libvirt-dbus on Debian 8

[Daniel P . Berrangé]

On Mon, Aug 13, 2018 at 02:08:30PM +0200, Andrea Bolognani wrote:
> I thought that it would work, but I was clearly mistaken :/
> 
> Andrea Bolognani (2):
>   projects: Don't build libvirt-dbus on Debian 8
>   guests: Don't prepare Debian 8 for libvirt-dbus

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v3 04/11] util: netlink: use VIR_AUTOPTR for aggregate types

[Erik Skultety]

On Thu, Aug 09, 2018 at 09:42:12AM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
>  src/util/virnetlink.c | 72 
> ---
>  1 file changed, 28 insertions(+), 44 deletions(-)
>
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index fcdc09d..66e80e2 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -297,15 +297,16 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
>uint32_t src_pid, uint32_t dst_pid,
>unsigned int protocol, unsigned int groups)
>  {
> -int ret = -1;
>  struct sockaddr_nl nladdr = {
>  .nl_family = AF_NETLINK,
>  .nl_pid= dst_pid,
>  .nl_groups = 0,
>  };
>  struct pollfd fds[1];
> -VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL;
>  int len = 0;
> +VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL;

unjustified code movement...

> +
> +*respbuflen = 0;

unnecessary initialization..

>
>  memset(fds, 0, sizeof(fds));
>
> @@ -324,15 +325,12 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
>  goto cleanup;
>  }
>
> -ret = 0;
>  *respbuflen = len;
> - cleanup:
> -if (ret < 0) {
> -*resp = NULL;
> -*respbuflen = 0;
> -}
> +return 0;
>
> -return ret;
> + cleanup:
> +*resp = NULL;
> +return -1;

I moved ^this hunk into the previous patch as I converted 1 more var into
VIR_AUTOFREE.

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH] rpm: simplify applying of patches

[no-reply]

Hi,

This series was run against 'syntax-check' test by patchew.org, which failed, 
please find the details below:

Type: series
Message-id: 20180803093532.16922-1-berra...@redhat.com
Subject: [libvirt] [PATCH] rpm: simplify applying of patches

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
time bash -c './autogen.sh && make syntax-check'
=== TEST SCRIPT END ===

Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01
Switched to a new branch 'test'
fatal: Not a valid branch point: '9eae8398edde9446ecc99f4f393bea94652fb6a2'.
Traceback (most recent call last):
  File "patchew-tester/src/patchew-cli", line 523, in test_one
cwd=clone, stdout=logf, stderr=logf)
  File "/usr/lib64/python3.6/subprocess.py", line 291, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'branch', 'base', 
'9eae8398edde9446ecc99f4f393bea94652fb6a2']' returned non-zero exit status 128.


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

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

Re: [libvirt] [PATCH] qemu_migration: Avoid writing to freed memory

[no-reply]

Hi,

This series was run against 'syntax-check' test by patchew.org, which failed, 
please find the details below:

Type: series
Message-id: 
7824152ee221199b352c0753db3eefaa5ec9dbb7.1533222453.git.jdene...@redhat.com
Subject: [libvirt] [PATCH] qemu_migration: Avoid writing to freed memory

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
time bash -c './autogen.sh && make syntax-check'
=== TEST SCRIPT END ===

Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01
>From https://github.com/patchew-project/libvirt
 t [tag update]patchew/20180803093532.16922-1-berra...@redhat.com 
-> patchew/20180803093532.16922-1-berra...@redhat.com
 t [tag update]
patchew/7824152ee221199b352c0753db3eefaa5ec9dbb7.1533222453.git.jdene...@redhat.com
 -> 
patchew/7824152ee221199b352c0753db3eefaa5ec9dbb7.1533222453.git.jdene...@redhat.com
Switched to a new branch 'test'
fatal: Not a valid branch point: '9eae8398edde9446ecc99f4f393bea94652fb6a2'.
Traceback (most recent call last):
  File "patchew-tester/src/patchew-cli", line 523, in test_one
cwd=clone, stdout=logf, stderr=logf)
  File "/usr/lib64/python3.6/subprocess.py", line 291, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'branch', 'base', 
'9eae8398edde9446ecc99f4f393bea94652fb6a2']' returned non-zero exit status 128.


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

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

Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable

[Roman Bogorodskiy]

  Ivan Mishonov wrote:

> On 08/13/2018 11:00 AM, Daniel P. Berrangé wrote:
> 
> > On Sun, Aug 12, 2018 at 08:22:08PM +0400, Roman Bogorodskiy wrote:
> >>Ivan Mishonov wrote:
> >>
> >>> Yes, that makes sense. I'll try to find some time next week to redo my
> >>> code and send another patch. Since my time for working on libvirt is
> >>> very limited can you confirm that the LPC configuration should look like
> >>> this:
> >>>
> >>>      
> >>>      
> >>>      
> >> This looks reasonable to me. However, it adds some corner cases we need to
> >> handle:
> >>
> >> 1. I'm wondering if we should still default to 31 if this entry is not 
> >> specified?
> >> We can generate this entry when post-processing XML, but I'm not sure
> >> what's the best way to handle upgrades for the existing domains...
> > It depends if the BHyve driver is at a point where you consider stable
> > upgrades important or not. It could be valid for you to just change the
> > default to 31 if you think its better and upgrade stability is not
> > required yet.

My general view on upgrades is that it's best-effort based at this point. In 
this
specific case, I hope it won't affect many users as we can't run guests
that a picky about LPC, and other guests should be fine with the changed
LPC slot... Though I'll test if my existing guests will handle that
fine.

I guess when bhyve supports migrations (there are some WIP patches for
that), we'll have less flexibility in moving devices/slots around...

> I decided to check what vm-bhyve does. 
> https://github.com/churchers/vm-bhyve/blob/master/lib/vm-run#L367. They 
> seem to always place LPC at slot 31 so I guess it's safe to move it
> >
> >> 2. According to bhyve(8) manual page, lpc is only supported on bus 0, so
> >> need to add 'isa-bridge' specific validation to check that.
> > If its only supported in 1 address, then arguably you don't need to add
> > this at all - just fix the historically mistaken use of 31 in the code
> > and leave it out of XML.
> Yes, we might not need that option at all

Good, so let's stick to the simpler approach for now, and we can always
make things configurable if there's a demand for that.

> > Regards,
> > Daniel
> 

Roman Bogorodskiy


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

[libvirt] [PATCHv2 02/62] tests: qemumonitorjson: Simplify debugging of 'blockInfo' test

[Peter Krempa]

Print the differences in case when the expected data does not match.

Signed-off-by: Peter Krempa 
---
 tests/qemumonitorjsontest.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index e9b2632655..3a0490bd26 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1585,12 +1585,29 @@ testQemuMonitorJSONqemuMonitorJSONGetVirtType(const 
void *data)
 return ret;
 }

+
+static void
+testQemuMonitorJSONGetBlockInfoPrint(const struct qemuDomainDiskInfo *d)
+{
+VIR_TEST_VERBOSE("removable: %d, tray: %d, tray_open: %d, empty: %d, "
+ "io_status: %d, nodename: '%s'\n",
+ d->removable, d->tray, d->tray_open, d->empty,
+ d->io_status, NULLSTR(d->nodename));
+}
+
+
 static int
 testHashEqualQemuDomainDiskInfo(const void *value1, const void *value2)
 {
 const struct qemuDomainDiskInfo *info1 = value1, *info2 = value2;
+int ret;
+
+if ((ret = memcmp(info1, info2, sizeof(*info1))) != 0) {
+testQemuMonitorJSONGetBlockInfoPrint(info1);
+testQemuMonitorJSONGetBlockInfoPrint(info2);
+}

-return memcmp(info1, info2, sizeof(*info1));
+return ret;
 }

 static int
-- 
2.16.2

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

[libvirt] [PATCHv2 10/62] qemu: monitor: Reuse qemuMonitorJSONQueryBlock in qemuMonitorJSONBlockIoThrottleInfo

[Peter Krempa]

The wrapper executes the command and does error detection so there's no
need to open-code all of those things.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_monitor_json.c | 37 +
 1 file changed, 5 insertions(+), 32 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e20851eeb2..48439ccae1 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4817,21 +4817,14 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon,
 goto cleanup; \
 }
 static int
-qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result,
+qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle,
const char *device,
virDomainBlockIoTuneInfoPtr reply)
 {
-virJSONValuePtr io_throttle;
 int ret = -1;
 size_t i;
 bool found = false;

-if (!(io_throttle = virJSONValueObjectGetArray(result, "return"))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _(" block_io_throttle reply was missing device list"));
-goto cleanup;
-}
-
 for (i = 0; i < virJSONValueArraySize(io_throttle); i++) {
 virJSONValuePtr temp_dev = virJSONValueArrayGet(io_throttle, i);
 virJSONValuePtr inserted;
@@ -5001,33 +4994,13 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr 
mon,
   virDomainBlockIoTuneInfoPtr reply)
 {
 int ret = -1;
-virJSONValuePtr cmd = NULL;
-virJSONValuePtr result = NULL;
+virJSONValuePtr devices = NULL;

-cmd = qemuMonitorJSONMakeCommand("query-block", NULL);
-if (!cmd)
+if (!(devices = qemuMonitorJSONQueryBlock(mon)))
 return -1;

-if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
-goto cleanup;
-
-if (virJSONValueObjectHasKey(result, "error")) {
-if (qemuMonitorJSONHasError(result, "DeviceNotActive"))
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _("No active operation on device: %s"), device);
-else if (qemuMonitorJSONHasError(result, "NotSupported"))
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _("Operation is not supported for device: %s"), 
device);
-else
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Unexpected error"));
-goto cleanup;
-}
-
-ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply);
- cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(result);
+ret = qemuMonitorJSONBlockIoThrottleInfo(devices, device, reply);
+virJSONValueFree(devices);
 return ret;
 }

-- 
2.16.2

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

[libvirt] [PATCHv2 15/62] tests: qemuxml2argv: Fork CAPS_LATEST test cases for 'blockdev'

[Peter Krempa]

The blockdev support will change existing approach to add disks to VMs
so all tests using the DO_TEST_CAPS_LATEST approach which have any disks
need to be forked so that the changes can be applied.

Signed-off-by: Peter Krempa 
---
 tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args | 37 +
 .../qemuxml2argvdata/disk-cache.x86_64-2.12.0.args | 50 +
 .../disk-cdrom-network.x86_64-2.12.0.args  | 41 ++
 .../disk-cdrom-tray.x86_64-2.12.0.args | 39 ++
 .../qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args | 35 
 .../disk-copy_on_read.x86_64-2.12.0.args   | 41 ++
 .../disk-detect-zeroes.x86_64-2.12.0.args  | 37 +
 .../disk-error-policy.x86_64-2.12.0.args   | 41 ++
 .../disk-floppy-q35-2_11.x86_64-2.12.0.args| 35 
 .../disk-floppy-q35-2_9.x86_64-2.12.0.args | 35 
 .../disk-floppy.x86_64-2.12.0.args | 35 
 .../disk-network-gluster.x86_64-2.12.0.args| 44 +++
 .../disk-network-iscsi.x86_64-2.12.0.args  | 63 ++
 .../disk-network-nbd.x86_64-2.12.0.args| 46 
 .../disk-network-rbd.x86_64-2.12.0.args| 61 +
 .../disk-network-sheepdog.x86_64-2.12.0.args   | 35 
 .../disk-network-source-auth.x86_64-2.12.0.args| 47 
 .../disk-network-tlsx509.x86_64-2.12.0.args| 59 
 .../disk-readonly-disk.x86_64-2.12.0.args  | 34 
 .../disk-shared.x86_64-2.12.0.args | 37 +
 ...isk-virtio-scsi-reservations.x86_64-2.12.0.args | 43 +++
 .../floppy-drive-fat.x86_64-2.12.0.args| 33 
 tests/qemuxml2argvtest.c   | 22 
 23 files changed, 950 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-cache.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-cdrom-network.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-cdrom-tray.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-copy_on_read.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-detect-zeroes.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-error-policy.x86_64-2.12.0.args
 create mode 100644 
tests/qemuxml2argvdata/disk-floppy-q35-2_11.x86_64-2.12.0.args
 create mode 100644 
tests/qemuxml2argvdata/disk-floppy-q35-2_9.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-floppy.x86_64-2.12.0.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-gluster.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-nbd.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd.x86_64-2.12.0.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-sheepdog.x86_64-2.12.0.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-source-auth.x86_64-2.12.0.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-readonly-disk.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/disk-shared.x86_64-2.12.0.args
 create mode 100644 
tests/qemuxml2argvdata/disk-virtio-scsi-reservations.x86_64-2.12.0.args
 create mode 100644 tests/qemuxml2argvdata/floppy-drive-fat.x86_64-2.12.0.args

diff --git a/tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args 
b/tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args
new file mode 100644
index 00..1dfade0882
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args
@@ -0,0 +1,37 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
+cache=none,aio=native \
+-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1,\
+write-cache=on \
+-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-1-0,\
+readonly=on,aio=threads \
+-device 

[libvirt] [PATCHv2 11/62] qemu: monitor: Allow using 'id' instead of 'device' for 'block_set_io_throttle'

[Peter Krempa]

The 'device' argument matches only the legacy drive alias. For blockdev
we need to set the throttling for a QOM id and thus we'll need to use
the 'id' field.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c   |  2 +-
 src/qemu/qemu_monitor.c  |  8 +---
 src/qemu/qemu_monitor.h  |  3 ++-
 src/qemu/qemu_monitor_json.c | 14 ++
 src/qemu/qemu_monitor_json.h |  3 ++-
 tests/qemumonitorjsontest.c  |  2 +-
 6 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 15ec92bf10..36ec0a2346 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18474,7 +18474,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
   * via the JSON error code from the block_set_io_throttle call */

 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorSetBlockIoThrottle(priv->mon, device,
+ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, NULL,
 , supportMaxOptions,
 set_fields & 
QEMU_BLOCK_IOTUNE_SET_GROUP_NAME,
 supportMaxLengthOptions);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 4f0bbc147d..2902c3b246 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3444,17 +3444,19 @@ qemuMonitorGetBlockJobInfo(qemuMonitorPtr mon,

 int
 qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
-  const char *device,
+  const char *drivealias,
+  const char *qomid,
   virDomainBlockIoTuneInfoPtr info,
   bool supportMaxOptions,
   bool supportGroupNameOption,
   bool supportMaxLengthOptions)
 {
-VIR_DEBUG("device=%p, info=%p", device, info);
+VIR_DEBUG("drivealias=%s, qomid=%s, info=%p",
+  NULLSTR(drivealias), NULLSTR(qomid), info);

 QEMU_CHECK_MONITOR(mon);

-return qemuMonitorJSONSetBlockIoThrottle(mon, device, info,
+return qemuMonitorJSONSetBlockIoThrottle(mon, drivealias, qomid, info,
  supportMaxOptions,
  supportGroupNameOption,
  supportMaxLengthOptions);
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index b8e3ca2ce1..16fc75819f 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -932,7 +932,8 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon,
 bool skipauth);

 int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
-  const char *device,
+  const char *drivealias,
+  const char *qomid,
   virDomainBlockIoTuneInfoPtr info,
   bool supportMaxOptions,
   bool supportGroupNameOption,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 48439ccae1..3a1dfb8563 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4897,7 +4897,8 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr 
io_throttle,
 #undef GET_THROTTLE_STATS_OPTIONAL

 int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon,
-  const char *device,
+  const char *drivealias,
+  const char *qomid,
   virDomainBlockIoTuneInfoPtr info,
   bool supportMaxOptions,
   bool supportGroupNameOption,
@@ -4907,12 +4908,17 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr 
mon,
 virJSONValuePtr cmd = NULL;
 virJSONValuePtr result = NULL;
 virJSONValuePtr args = NULL;
+const char *errdev = drivealias;
+
+if (!errdev)
+errdev = qomid;

 if (!(cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", NULL)))
 return -1;

 if (virJSONValueObjectCreate(,
- "s:device", device,
+ "S:device", drivealias,
+ "S:id", qomid,
  "U:bps", info->total_bytes_sec,
  "U:bps_rd", info->read_bytes_sec,
  "U:bps_wr", info->write_bytes_sec,
@@ -4967,10 +4973,10 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr 
mon,
 if (virJSONValueObjectHasKey(result, "error")) {
 if (qemuMonitorJSONHasError(result, "DeviceNotActive")) {
 virReportError(VIR_ERR_OPERATION_INVALID,
-   _("No active operation on device: %s"), device);
+   

[libvirt] [PATCHv2 16/62] tests: qemu: Add test data for backing chains and indexes

[Peter Krempa]

Add test data for nested backing chains with/without indexes (used in
status XMLs) which will excercise blockdev and the related work.

Signed-off-by: Peter Krempa 
---
 .../disk-backing-chains-index.x86_64-2.12.0.args   |   1 +
 .../disk-backing-chains-index.x86_64-latest.args   |   1 +
 .../qemuxml2argvdata/disk-backing-chains-index.xml | 145 +
 .../disk-backing-chains-noindex.x86_64-2.12.0.args |  58 +
 .../disk-backing-chains-noindex.x86_64-latest.args |  58 +
 .../disk-backing-chains-noindex.xml| 145 +
 tests/qemuxml2argvtest.c   |   4 +
 .../disk-backing-chains-index-active.xml   |  76 +++
 .../disk-backing-chains-index-inactive.xml |  76 +++
 .../disk-backing-chains-noindex-active.xml |  76 +++
 .../disk-backing-chains-noindex-inactive.xml   |  76 +++
 tests/qemuxml2xmltest.c|   2 +
 12 files changed, 718 insertions(+)
 create mode 12 
tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args
 create mode 12 
tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-index.xml
 create mode 100644 
tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-2.12.0.args
 create mode 100644 
tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-noindex.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-backing-chains-index-inactive.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-backing-chains-noindex-active.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-backing-chains-noindex-inactive.xml

diff --git 
a/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args 
b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args
new file mode 12
index 00..3f4cd9040d
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args
@@ -0,0 +1 @@
+disk-backing-chains-noindex.x86_64-2.12.0.args
\ No newline at end of file
diff --git 
a/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args 
b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args
new file mode 12
index 00..549eb65512
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args
@@ -0,0 +1 @@
+disk-backing-chains-noindex.x86_64-latest.args
\ No newline at end of file
diff --git a/tests/qemuxml2argvdata/disk-backing-chains-index.xml 
b/tests/qemuxml2argvdata/disk-backing-chains-index.xml
new file mode 100644
index 00..95b8a64cf8
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-backing-chains-index.xml
@@ -0,0 +1,145 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-i686
+
+  
+  
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+  
+  
+
+
+
+  
+  
+  
+
+
+
+  
+  
+  
+  
+
+
+
+  
+  
+  
+
+  
+
+  
+
+  
+  
+
+
+  
+  
+  
+
+  
+  
+
+
+  
+  
+
+  
+  
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+  
+  
+  
+
+
+  
+  
+  
+
+
+
+  
+  
+  
+
+
+
+  
+  
+  
+
+
+
+  
+  
+  
+
+
+
+  
+  
+  
+
+
+
+  
+  
+  
+
+
+
+  
+
+  
+
+  
+
+  
+
+  
+
+  
+  
+
+
+
+
+  
+
diff --git 
a/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-2.12.0.args 
b/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-2.12.0.args
new file mode 100644
index 00..dea109b13a
--- /dev/null
+++ 

[libvirt] [PATCHv2 18/62] util: virqemu: Simplify debugging if building QOM object with missing args

[Peter Krempa]

Print the values so it's simpler to debug.

Signed-off-by: Peter Krempa 
---
 src/util/virqemu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index bc788538af..13ad7410b4 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -235,8 +235,9 @@ virQEMUBuildObjectCommandlineFromJSONInternal(virBufferPtr 
buf,
   virJSONValuePtr props)
 {
 if (!type || !alias) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("missing 'type' or 'alias' field of QOM 'object'"));
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("missing 'type'(%s) or 'alias'(%s) field of QOM 
'object'"),
+   NULLSTR(type), NULLSTR(alias));
 return -1;
 }

-- 
2.16.2

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

[libvirt] [PATCHv2 05/62] qemu: monitor: Remove useless 'locked' property from struct qemuDomainDiskInfo

[Peter Krempa]

We don't use it for anything useful so it does not make much sense to
extract it.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.h   | 1 -
 src/qemu/qemu_monitor_json.c | 7 ---
 tests/qemumonitorjsontest.c  | 1 -
 3 files changed, 9 deletions(-)

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index bff293fc0a..7b79d77257 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -438,7 +438,6 @@ struct _qemuDomainVcpuPrivate {

 struct qemuDomainDiskInfo {
 bool removable;
-bool locked;
 bool tray;
 bool tray_open;
 bool empty;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 0695ec8d2c..2389ebb9aa 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2243,13 +2243,6 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
 goto cleanup;
 }

-if (virJSONValueObjectGetBoolean(dev, "locked", >locked) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cannot read %s value"),
-   "locked");
-goto cleanup;
-}
-
 /* 'tray_open' is present only if the device has a tray */
 if (virJSONValueObjectGetBoolean(dev, "tray_open", >tray_open) 
== 0)
 info->tray = true;
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 3a0490bd26..955892b1f6 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1647,7 +1647,6 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void 
*data)
 if (VIR_ALLOC(info) < 0)
 goto cleanup;

-info->locked = true;
 info->removable = true;
 info->tray = true;

-- 
2.16.2

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

[libvirt] [PATCHv2 06/62] tests: qemucapabilities: Update capability data for qemu 3.0.0

[Peter Krempa]

The diff contains changes from the change of the JSON library
reformatting as well as dropping of the preconfig state and adding of
the 'qdev' field to output of 'query-blockstats'.

Signed-off-by: Peter Krempa 
---
 .../qemucapabilitiesdata/caps_3.0.0.x86_64.replies | 591 +++--
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |   8 +-
 2 files changed, 88 insertions(+), 511 deletions(-)

diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.replies 
b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.replies
index b2f8377248..714fac5a6f 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.replies
@@ -4,8 +4,7 @@
 }

 {
-  "return": {
-  },
+  "return": {},
   "id": "libvirt-1"
 }

@@ -17,11 +16,11 @@
 {
   "return": {
 "qemu": {
-  "micro": 90,
+  "micro": 93,
   "minor": 12,
   "major": 2
 },
-"package": "v3.0.0-rc0-31-g633e824037"
+"package": "v3.0.0-rc3-17-g09b94ac0f2"
   },
   "id": "libvirt-2"
 }
@@ -175,7 +174,7 @@
   "name": "system_wakeup"
 },
 {
-  "name": "exit-preconfig"
+  "name": "x-exit-preconfig"
 },
 {
   "name": "cont"
@@ -5346,40 +5345,35 @@
 {
   "name": "max",
   "typename": "max-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": false
 },
 {
   "name": "host",
   "typename": "host-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": false
 },
 {
   "name": "base",
   "typename": "base-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": true,
   "migration-safe": true
 },
 {
   "name": "qemu64",
   "typename": "qemu64-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "qemu32",
   "typename": "qemu32-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
@@ -5400,64 +5394,56 @@
 {
   "name": "pentium3",
   "typename": "pentium3-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "pentium2",
   "typename": "pentium2-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "pentium",
   "typename": "pentium-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "n270",
   "typename": "n270-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "kvm64",
   "typename": "kvm64-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "kvm32",
   "typename": "kvm32-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "coreduo",
   "typename": "coreduo-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "core2duo",
   "typename": "core2duo-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
@@ -5475,16 +5461,14 @@
 {
   "name": "Westmere-IBRS",
   "typename": "Westmere-IBRS-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "Westmere",
   "typename": "Westmere-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
@@ -5525,40 +5509,35 @@
 {
   "name": "Skylake-Client-IBRS",
   "typename": "Skylake-Client-IBRS-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "Skylake-Client",
   "typename": "Skylake-Client-x86_64-cpu",
-  "unavailable-features": [
-  ],
+  "unavailable-features": [],
   "static": false,
   "migration-safe": true
 },
 {
   "name": "SandyBridge-IBRS",
   

[libvirt] [PATCHv2 08/62] qemu: hotplug: consolidate media change code paths

[Peter Krempa]

Use qemuDomainAttachDeviceDiskLive to change the media in
qemuDomainChangeDiskLive as the former function already does all the
necessary steps to prepare the new medium.

This also allows us to turn qemuDomainChangeEjectableMedia static.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c  | 18 ++
 src/qemu/qemu_hotplug.c | 18 +++---
 src/qemu/qemu_hotplug.h |  9 ++---
 tests/qemuhotplugtest.c |  2 +-
 4 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4fc1e358fa..15ec92bf10 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7599,7 +7599,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 switch ((virDomainDeviceType)dev->type) {
 case VIR_DOMAIN_DEVICE_DISK:
 qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL);
-ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev);
+ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev, false);
 if (!ret) {
 alias = dev->data.disk->info.alias;
 dev->data.disk = NULL;
@@ -7850,12 +7850,6 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
 virDomainDeviceDef oldDev = { .type = dev->type };
 int ret = -1;

-if (virDomainDiskTranslateSourcePool(disk) < 0)
-goto cleanup;
-
-if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0)
-goto cleanup;
-
 if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
disk->bus, disk->dst))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -7884,16 +7878,8 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
 goto cleanup;
 }

-/* Add the new disk src into shared disk hash table */
-if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
-goto cleanup;
-
-if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
-   dev->data.disk->src, force) < 0) {
-ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
-  vm->def->name));
+if (qemuDomainAttachDeviceDiskLive(driver, vm, dev, force) < 0)
 goto cleanup;
-}

 dev->data.disk->src = NULL;
 }
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5b52fe9edc..c46f27f4b4 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -586,7 +586,7 @@ qemuHotplugDiskSourceRemove(qemuMonitorPtr mon,
  *
  * Returns 0 on success, -1 on error and reports libvirt error
  */
-int
+static int
 qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
@@ -909,10 +909,22 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr 
driver,
 }


+/**
+ * qemuDomainAttachDeviceDiskLive:
+ * @driver: qemu driver struct
+ * @vm: domain object
+ * @dev: device to attach (expected type is DISK)
+ * @forceMediaChange: Forcibly open the drive if changing media
+ *
+ * Attach a new disk or in case of cdroms/floppies change the media in the 
drive.
+ * This function handles all the necessary steps to attach a new storage source
+ * to the VM. If @forceMediaChange is true the drive is opened forcibly.
+ */
 int
 qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
virDomainObjPtr vm,
-   virDomainDeviceDefPtr dev)
+   virDomainDeviceDefPtr dev,
+   bool forceMediaChange)
 {
 size_t i;
 virDomainDiskDefPtr disk = dev->data.disk;
@@ -946,7 +958,7 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
 }

 if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
-   disk->src, false) < 0)
+   disk->src, forceMediaChange) < 0)
 goto cleanup;

 disk->src = NULL;
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 0297e42a98..c085c45082 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -28,12 +28,6 @@
 # include "qemu_domain.h"
 # include "domain_conf.h"

-int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
-   virDomainObjPtr vm,
-   virDomainDiskDefPtr disk,
-   virStorageSourcePtr newsrc,
-   bool force);
-
 void qemuDomainDelTLSObjects(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  qemuDomainAsyncJob asyncJob,
@@ -60,7 +54,8 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
  virDomainControllerDefPtr controller);
 int qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
  

[libvirt] [PATCHv2 14/62] tests: qemu: Drop disk from hostdev-mdev tests

[Peter Krempa]

The disk is not necessary to test the mdevs.

Signed-off-by: Peter Krempa 
---
 tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml| 6 --
 .../hostdev-mdev-display-spice-egl-headless.x86_64-latest.args  | 2 --
 tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml  | 6 --
 .../hostdev-mdev-display-spice-opengl.x86_64-latest.args| 2 --
 tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml| 6 --
 .../hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args| 2 --
 tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.xml| 6 --
 tests/qemuxml2argvdata/hostdev-mdev-display-vnc.x86_64-latest.args  | 2 --
 tests/qemuxml2argvdata/hostdev-mdev-display-vnc.xml | 6 --
 tests/qemuxml2argvdata/hostdev-mdev-display.xml | 6 --
 tests/qemuxml2argvdata/hostdev-mdev-invalid-target-address.xml  | 5 -
 tests/qemuxml2argvdata/hostdev-mdev-precreated.args | 3 ---
 tests/qemuxml2argvdata/hostdev-mdev-precreated.xml  | 6 --
 tests/qemuxml2argvdata/hostdev-mdev-src-address-invalid.xml | 6 --
 tests/qemuxml2xmloutdata/hostdev-mdev-display.xml   | 6 --
 tests/qemuxml2xmloutdata/hostdev-mdev-precreated.xml| 6 --
 16 files changed, 76 deletions(-)

diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml 
b/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml
index ea559a6444..55b60ba133 100644
--- a/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml
+++ b/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml
@@ -14,12 +14,6 @@
   destroy
   
 /usr/bin/qemu-system-i686
-
-  
-  
-  
-  
-
 
 
 
diff --git 
a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args
 
b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args
index 0ac90c81d2..b84869264e 100644
--- 
a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args
+++ 
b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args
@@ -23,8 +23,6 @@ file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \
 -no-acpi \
 -boot strict=on \
 -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
--drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
--device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
 -spice port=0,seamless-migration=on \
 -display egl-headless \
 -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,\
diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml 
b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml
index c8f10c2f3a..3a686ad2bf 100644
--- a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml
+++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml
@@ -14,12 +14,6 @@
   destroy
   
 /usr/bin/qemu-system-i686
-
-  
-  
-  
-  
-
 
 
 
diff --git 
a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args 
b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args
index 1fd9fdaa16..80c56abfb9 100644
--- 
a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args
+++ 
b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args
@@ -23,8 +23,6 @@ file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \
 -no-acpi \
 -boot strict=on \
 -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
--drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
--device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
 -spice port=0,gl=on,rendernode=/dev/dri/foo,seamless-migration=on \
 -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,\
 vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2 \
diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml 
b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml
index 18c9817608..a632e58a41 100644
--- a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml
+++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml
@@ -14,12 +14,6 @@
   destroy
   
 /usr/bin/qemu-system-i686
-
-  
-  
-  
-  
-
 
 
 
diff --git 
a/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args
 
b/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args
index cdf545d0e0..91708d7663 100644
--- 
a/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args
+++ 
b/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args
@@ -23,8 +23,6 @@ file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \
 -no-acpi \
 -boot strict=on \
 -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
--drive 

[libvirt] [PATCHv2 20/62] qemu: process: clear QEMU_CAPS_BLOCKDEV for VMs with SD card

[Peter Krempa]

SD cards are currently passed by using -drive only which would not be
compatible with using -blockdev fully.

Clear QEMU_CAPS_BLOCKDEV if the VM has such devices.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_process.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c1a8dfda29..38e88404aa 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5882,6 +5882,15 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,

 qemuProcessPrepareAllowReboot(vm);

+/* clear the 'blockdev' capability for VMs which have disks that need
+ * -drive or which have floppies where we can't reliably get the QOM path 
*/
+for (i = 0; i < vm->def->ndisks; i++) {
+if (qemuDiskBusNeedsDriveArg(vm->def->disks[i]->bus)) {
+virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
+break;
+}
+}
+
 /*
  * Normally PCI addresses are assigned in the virDomainCreate
  * or virDomainDefine methods. We might still need to assign
-- 
2.16.2

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

[libvirt] [PATCHv2 07/62] Revert "qemu: monitor: Add the 'query-nodes' argument for query-blockstats"

[Peter Krempa]

Turns out that 'query-nodes' is not what we want and the
'query-blockstats' command was in fact buggy. Revert the new field since
it's not needed.

This reverts commit 50edca1331298bfcb2622e8fe588d493aff9ab68.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_block.c| 2 +-
 src/qemu/qemu_monitor.c  | 8 ++--
 src/qemu/qemu_monitor.h  | 3 +--
 src/qemu/qemu_monitor_json.c | 9 +++--
 src/qemu/qemu_monitor_json.h | 3 +--
 5 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index be120b30f0..ccdd334367 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -358,7 +358,7 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
 return -1;

 data = qemuMonitorQueryNamedBlockNodes(qemuDomainGetMonitor(vm));
-blockstats = qemuMonitorQueryBlockstats(qemuDomainGetMonitor(vm), false);
+blockstats = qemuMonitorQueryBlockstats(qemuDomainGetMonitor(vm));

 if (qemuDomainObjExitMonitor(driver, vm) < 0 || !data || !blockstats)
 goto cleanup;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 6e0644221b..4f0bbc147d 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2243,19 +2243,15 @@ qemuMonitorGetBlockInfo(qemuMonitorPtr mon)
 /**
  * qemuMonitorQueryBlockstats:
  * @mon: monitor object
- * @nodenames: include backing chain nodes with explicitly specified name
  *
  * Returns data from a call to 'query-blockstats'.
  */
 virJSONValuePtr
-qemuMonitorQueryBlockstats(qemuMonitorPtr mon,
-   bool nodenames)
+qemuMonitorQueryBlockstats(qemuMonitorPtr mon)
 {
 QEMU_CHECK_MONITOR_NULL(mon);

-VIR_DEBUG("nodenames: %d", nodenames);
-
-return qemuMonitorJSONQueryBlockstats(mon, nodenames);
+return qemuMonitorJSONQueryBlockstats(mon);
 }


diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2fa8d5b51d..b8e3ca2ce1 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -563,8 +563,7 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
 int qemuMonitorBlockIOStatusToError(const char *status);
 virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon);

-virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon,
-   bool nodenames);
+virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon);

 typedef struct _qemuBlockStats qemuBlockStats;
 typedef qemuBlockStats *qemuBlockStatsPtr;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2389ebb9aa..e20851eeb2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2362,16 +2362,13 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev,


 virJSONValuePtr
-qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon,
-   bool nodenames)
+qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon)
 {
 virJSONValuePtr cmd;
 virJSONValuePtr reply = NULL;
 virJSONValuePtr ret = NULL;

-if (!(cmd = qemuMonitorJSONMakeCommand("query-blockstats",
-   "B:query-nodes", nodenames,
-   NULL)))
+if (!(cmd = qemuMonitorJSONMakeCommand("query-blockstats", NULL)))
 return NULL;

 if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
@@ -2400,7 +2397,7 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
 size_t i;
 virJSONValuePtr devices;

-if (!(devices = qemuMonitorJSONQueryBlockstats(mon, false)))
+if (!(devices = qemuMonitorJSONQueryBlockstats(mon)))
 return -1;

 for (i = 0; i < virJSONValueArraySize(devices); i++) {
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 2408ab0c5b..0458d81c0d 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -86,8 +86,7 @@ int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
 int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
 virHashTablePtr table);

-virJSONValuePtr qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon,
-   bool nodenames);
+virJSONValuePtr qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon);
 int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
 virHashTablePtr hash,
 bool backingChain);
-- 
2.16.2

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

[libvirt] [PATCHv2 01/62] qemu: process: Fix alias for disk-tray-moved event

[Peter Krempa]

Currently we'd report the alias of the drive which is backing the cdrom
rather than the device itself:

 $ virsh event ds tray-change --loop
 event 'tray-change' for domain ds disk drive-ide0-0-1: opened
 event 'tray-change' for domain ds disk drive-ide0-0-1: closed

Report the disk device alias as we document in the API docs.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_process.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c4e33723d1..c1a8dfda29 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1053,9 +1053,7 @@ qemuProcessHandleTrayChange(qemuMonitorPtr mon 
ATTRIBUTE_UNUSED,
 disk = qemuProcessFindDomainDiskByAlias(vm, devAlias);

 if (disk) {
-event = virDomainEventTrayChangeNewFromObj(vm,
-   devAlias,
-   reason);
+event = virDomainEventTrayChangeNewFromObj(vm, disk->info.alias, 
reason);
 /* Update disk tray status */
 if (reason == VIR_DOMAIN_EVENT_TRAY_CHANGE_OPEN)
 disk->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN;
-- 
2.16.2

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

[libvirt] [PATCHv2 03/62] qemu: Improve errors in qemuDomainBlockResize

[Peter Krempa]

Remove the pointless "empty path" check and use a better error message
if the disk was not found.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d4a2379e48..4fc1e358fa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10963,12 +10963,6 @@ qemuDomainBlockResize(virDomainPtr dom,

 virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES, -1);

-if (path[0] == '\0') {
-virReportError(VIR_ERR_INVALID_ARG,
-   "%s", _("empty path"));
-return -1;
-}
-
 /* We prefer operating on bytes.  */
 if ((flags & VIR_DOMAIN_BLOCK_RESIZE_BYTES) == 0) {
 if (size > ULLONG_MAX / 1024) {
@@ -10996,7 +10990,7 @@ qemuDomainBlockResize(virDomainPtr dom,

 if (!(disk = virDomainDiskByName(vm->def, path, false))) {
 virReportError(VIR_ERR_INVALID_ARG,
-   _("invalid path: %s"), path);
+   _("disk '%s' was not found in the domain config"), 
path);
 goto endjob;
 }

-- 
2.16.2

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

[libvirt] [PATCHv2 09/62] qemu: hotplug: Prepare disk source in qemuDomainAttachDeviceDiskLive

[Peter Krempa]

Move the preparation steps from qemuDomainAttachDiskGeneric up into
qemuDomainAttachDeviceDiskLive so that also media changing can use the
prepared file.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_hotplug.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c46f27f4b4..87bc63e5e1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -641,7 +641,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 qemuHotplugDiskSourceDataPtr diskdata = NULL;
 char *devstr = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);

 if (qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, false) < 0)
 goto cleanup;
@@ -649,9 +648,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
 if (qemuAssignDeviceDiskAlias(vm->def, disk) < 0)
 goto error;

-if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
-goto error;
-
 if (!(diskdata = qemuHotplugDiskSourceAttachPrepare(disk, priv->qemuCaps)))
 goto error;

@@ -686,7 +682,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
 qemuHotplugDiskSourceDataFree(diskdata);
 qemuDomainSecretDiskDestroy(disk);
 VIR_FREE(devstr);
-virObjectUnref(cfg);
 return ret;

  exit_monitor:
@@ -927,6 +922,8 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
bool forceMediaChange)
 {
 size_t i;
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+qemuDomainObjPrivatePtr priv = vm->privateData;
 virDomainDiskDefPtr disk = dev->data.disk;
 virDomainDiskDefPtr orig_disk = NULL;
 int ret = -1;
@@ -943,6 +940,9 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
 if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0)
 goto cleanup;

+if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
+goto cleanup;
+
 switch ((virDomainDiskDevice) disk->device)  {
 case VIR_DOMAIN_DISK_DEVICE_CDROM:
 case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
@@ -1013,6 +1013,7 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
  cleanup:
 if (ret != 0)
 ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
+virObjectUnref(cfg);
 return ret;
 }

-- 
2.16.2

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