Re: [libvirt] [PATCH] vz: fix build for virNetDev* changes

[Laine Stump]

On 06/27/2016 02:23 PM, Olga Krishtal wrote:

Patch fixes vz build after changes in IP-related netdev functions(cf0568b0, 
fbc1843d).


Sorry about that. I *thought* I had searched in the drivers I couldn't 
build. (BTW, I tried downloading and installing the parallels-sdk on my 
Fedora system, but configure still says it's not there. Are there more 
verbose instructions somewhere so that I can just always build with 
--with-vz in the future?)


(BTW, although some of the patches I mistakenly pushed yesterday have 
been reverted, the patches that cause this build failure for the vz 
driver haven't.)




Signed-off-by: Olga Krishtal 
---
  src/vz/vz_sdk.c | 49 +
  1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 1203ed6..7e75e44 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -773,13 +773,13 @@ prlsdkAddDomainOpticalDisksInfo(vzDriverPtr driver, 
PRL_HANDLE sdkdom, virDomain
  return -1;
  }
  
-static virDomainNetIpDefPtr

+static virNetDevIPAddrPtr
  prlsdkParseNetAddress(char *addr)
  {
  char *maskstr = NULL;
  int nbits;
  virSocketAddr mask;
-virDomainNetIpDefPtr ip = NULL, ret = NULL;
+virNetDevIPAddrPtr ip = NULL, ret = NULL;
  
  if (!(maskstr = strchr(addr, '/')))

  goto cleanup;
@@ -829,7 +829,7 @@ prlsdkGetNetAddresses(PRL_HANDLE sdknet, virDomainNetDefPtr 
net)
  prlsdkCheckRetGoto(pret, cleanup);
  
  for (i = 0; i < num; ++i) {

-virDomainNetIpDefPtr ip = NULL;
+   virNetDevIPAddrPtr ip = NULL;


Indentation was off. I fixed it.


  PRL_UINT32 buflen = 0;
  char *addr;
  
@@ -845,7 +845,7 @@ prlsdkGetNetAddresses(PRL_HANDLE sdknet, virDomainNetDefPtr net)

  if (!(ip = prlsdkParseNetAddress(addr)))
  continue;
  
-if (VIR_APPEND_ELEMENT(net->ips, net->nips, ip) < 0) {

+if (VIR_APPEND_ELEMENT(net->guestIP.ips, net->guestIP.nips, ip) < 0) {
  VIR_FREE(ip);
  goto cleanup;
  }
@@ -864,7 +864,7 @@ prlsdkGetRoutes(PRL_HANDLE sdknet, virDomainNetDefPtr net)
  int ret = -1;
  char *gw = NULL;
  char *gw6 = NULL;
-virNetworkRouteDefPtr route = NULL;
+virNetDevIPRoutePtr route = NULL;
  
  if (!(gw = prlsdkGetStringParamVar(PrlVmDevNet_GetDefaultGateway, sdknet)))

  goto cleanup;
@@ -873,29 +873,30 @@ prlsdkGetRoutes(PRL_HANDLE sdknet, virDomainNetDefPtr net)
  goto cleanup;
  
  if (*gw != '\0') {

-if (!(route = virNetworkRouteDefCreate(_("Domain interface"),
+
+if (!(route = virNetDevIPRouteCreate(_("Domain interface"),
 "ipv4", 
VIR_SOCKET_ADDR_IPV4_ALL,
 NULL, gw, 0, true, 0, false)))


Indentation of the lines following the change was off.


  goto cleanup;
  
-if (VIR_APPEND_ELEMENT(net->routes, net->nroutes, route) < 0)

+if (VIR_APPEND_ELEMENT(net->guestIP.routes, net->guestIP.nroutes, route) 
< 0)
  goto cleanup;
  }
  
  if (*gw6 != '\0') {

-if (!(route = virNetworkRouteDefCreate(_("Domain interface"),
+if (!(route = virNetDevIPRouteCreate(_("Domain interface"),
 "ipv6", 
VIR_SOCKET_ADDR_IPV6_ALL,
 NULL, gw6, 0, true, 0, false)))


Again with indentation.


  goto cleanup;
  
-if (VIR_APPEND_ELEMENT(net->routes, net->nroutes, route) < 0)

+if (VIR_APPEND_ELEMENT(net->guestIP.routes, net->guestIP.nroutes, route) 
< 0)
  goto cleanup;
  }
  
  ret = 0;
  
   cleanup:

-VIR_FREE(route);
+virNetDevIPRouteFree(route);


This actually fixes a memory leak - even before the changes a route had 
a char* allocated for family, so you needed to call 
virNetworkRouteDefFree().


I'm unable to build with --with-vz, but assume that you've done this 
(since that was the entire reason for the patch :-), so ACK. I've pushed 
the patch so that the next RC of 2.0 will build properly.


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

Re: [libvirt] [PATCH] qemuDomainDeviceDefValidate: Drop unused qemuCaps

[John Ferlan]

On 06/27/2016 11:50 AM, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_domain.c | 4 
>  1 file changed, 4 deletions(-)
> 

ACK - I'd think this is safe too

John

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

[libvirt] [PATCH] virt-admin: Call virInitialize to fix startup crash

[Cole Robinson]

Similar to what virsh and virt-login-shell do

https://bugzilla.redhat.com/show_bug.cgi?id=1350315
---
I can't actually reproduce the bug, the backtrace is similar to
97973ebb7 which added the same fix for virt-login-shell, and
that commit also mentions the randomness of reproducing...

 tools/virt-admin.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 4acac65..7bff5c3 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -1371,6 +1371,11 @@ main(int argc, char **argv)
 return EXIT_FAILURE;
 }
 
+if (virInitialize() < 0) {
+vshError(ctl, "%s", _("Failed to initialize libvirt"));
+return EXIT_FAILURE;
+}
+
 virFileActivateDirOverride(argv[0]);
 
 if (!vshInit(ctl, cmdGroups, NULL))
-- 
2.7.4

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

[libvirt] [PATCH 0/1] vz: fix build for virNetDev* changes

[Olga Krishtal]

Although this changes have been reverted, it useful to
have patch, that makes this changes in vz driver.

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

[libvirt] [PATCH] vz: fix build for virNetDev* changes

[Olga Krishtal]

Patch fixes vz build after changes in IP-related netdev functions(cf0568b0, 
fbc1843d).

Signed-off-by: Olga Krishtal 
---
 src/vz/vz_sdk.c | 49 +
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 1203ed6..7e75e44 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -773,13 +773,13 @@ prlsdkAddDomainOpticalDisksInfo(vzDriverPtr driver, 
PRL_HANDLE sdkdom, virDomain
 return -1;
 }
 
-static virDomainNetIpDefPtr
+static virNetDevIPAddrPtr
 prlsdkParseNetAddress(char *addr)
 {
 char *maskstr = NULL;
 int nbits;
 virSocketAddr mask;
-virDomainNetIpDefPtr ip = NULL, ret = NULL;
+virNetDevIPAddrPtr ip = NULL, ret = NULL;
 
 if (!(maskstr = strchr(addr, '/')))
 goto cleanup;
@@ -829,7 +829,7 @@ prlsdkGetNetAddresses(PRL_HANDLE sdknet, virDomainNetDefPtr 
net)
 prlsdkCheckRetGoto(pret, cleanup);
 
 for (i = 0; i < num; ++i) {
-virDomainNetIpDefPtr ip = NULL;
+   virNetDevIPAddrPtr ip = NULL;
 PRL_UINT32 buflen = 0;
 char *addr;
 
@@ -845,7 +845,7 @@ prlsdkGetNetAddresses(PRL_HANDLE sdknet, virDomainNetDefPtr 
net)
 if (!(ip = prlsdkParseNetAddress(addr)))
 continue;
 
-if (VIR_APPEND_ELEMENT(net->ips, net->nips, ip) < 0) {
+if (VIR_APPEND_ELEMENT(net->guestIP.ips, net->guestIP.nips, ip) < 0) {
 VIR_FREE(ip);
 goto cleanup;
 }
@@ -864,7 +864,7 @@ prlsdkGetRoutes(PRL_HANDLE sdknet, virDomainNetDefPtr net)
 int ret = -1;
 char *gw = NULL;
 char *gw6 = NULL;
-virNetworkRouteDefPtr route = NULL;
+virNetDevIPRoutePtr route = NULL;
 
 if (!(gw = prlsdkGetStringParamVar(PrlVmDevNet_GetDefaultGateway, sdknet)))
 goto cleanup;
@@ -873,29 +873,30 @@ prlsdkGetRoutes(PRL_HANDLE sdknet, virDomainNetDefPtr net)
 goto cleanup;
 
 if (*gw != '\0') {
-if (!(route = virNetworkRouteDefCreate(_("Domain interface"),
+
+if (!(route = virNetDevIPRouteCreate(_("Domain interface"),
"ipv4", 
VIR_SOCKET_ADDR_IPV4_ALL,
NULL, gw, 0, true, 0, false)))
 goto cleanup;
 
-if (VIR_APPEND_ELEMENT(net->routes, net->nroutes, route) < 0)
+if (VIR_APPEND_ELEMENT(net->guestIP.routes, net->guestIP.nroutes, 
route) < 0)
 goto cleanup;
 }
 
 if (*gw6 != '\0') {
-if (!(route = virNetworkRouteDefCreate(_("Domain interface"),
+if (!(route = virNetDevIPRouteCreate(_("Domain interface"),
"ipv6", 
VIR_SOCKET_ADDR_IPV6_ALL,
NULL, gw6, 0, true, 0, false)))
 goto cleanup;
 
-if (VIR_APPEND_ELEMENT(net->routes, net->nroutes, route) < 0)
+if (VIR_APPEND_ELEMENT(net->guestIP.routes, net->guestIP.nroutes, 
route) < 0)
 goto cleanup;
 }
 
 ret = 0;
 
  cleanup:
-VIR_FREE(route);
+virNetDevIPRouteFree(route);
 VIR_FREE(gw);
 VIR_FREE(gw6);
 
@@ -2654,7 +2655,7 @@ static int 
prlsdkCheckNetUnsupportedParams(virDomainNetDefPtr net)
 return -1;
 }
 
-if (net->guestIf.name) {
+if (net->ifname_guest) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Setting guest interface name is not "
  "supported by vz driver."));
@@ -2886,16 +2887,16 @@ static int prlsdkConfigureGateways(PRL_HANDLE sdknet, 
virDomainNetDefPtr net)
 {
 int ret = -1;
 size_t i;
-virNetworkRouteDefPtr route4 = NULL, route6 = NULL;
+virNetDevIPRoutePtr route4 = NULL, route6 = NULL;
 char *gw4 = NULL, *gw6 = NULL;
 PRL_RESULT pret;
 
-for (i = 0; i < net->nroutes; i++) {
+for (i = 0; i < net->guestIP.nroutes; i++) {
 virSocketAddrPtr addrdst, gateway;
 virSocketAddr zero;
 
-addrdst = virNetworkRouteDefGetAddress(net->routes[i]);
-gateway = virNetworkRouteDefGetGateway(net->routes[i]);
+addrdst = virNetDevIPRouteGetAddress(net->guestIP.routes[i]);
+gateway = virNetDevIPRouteGetGateway(net->guestIP.routes[i]);
 
 ignore_value(virSocketAddrParse(&zero,
 (VIR_SOCKET_ADDR_IS_FAMILY(addrdst, AF_INET)
@@ -2917,7 +2918,7 @@ static int prlsdkConfigureGateways(PRL_HANDLE sdknet, 
virDomainNetDefPtr net)
 return -1;
 }
 
-route4 = net->routes[i];
+route4 = net->guestIP.routes[i];
 
 break;
 case AF_INET6:
@@ -2927,7 +2928,7 @@ static int prlsdkConfigureGateways(PRL_HANDLE sdknet, 
virDomainNetDefPtr net)
 return -1;
 }
 
-route6 = net->routes[i];
+route6 = net->guestIP.routes[i];
 
 break;
 default:
@@ -2941,14 +2942,14 @@ static int prlsdkConfigureGateways(PRL_HANDLE sd

Re: [libvirt] [PATCH RFC 11/48] Introduce virStreamSparseSendAll

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:28PM +0200, Michal Privoznik wrote:
> This is just a wrapper over new function that have been just
> introduced: virStreamSkip() . It's very similar to
> virStreamSendAll() except it handles sparse streams well.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  include/libvirt/libvirt-stream.h |  43 ++--
>  src/libvirt-stream.c | 104 
> +++
>  src/libvirt_public.syms  |   1 +
>  3 files changed, 145 insertions(+), 3 deletions(-)

ACK


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH RFC 10/48] Introduce virStreamSparseRecvAll

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:27PM +0200, Michal Privoznik wrote:
> This is just a wrapper over new functions that have been just
> introduced: virStreamRecvFlags(), virStreamHoleSize(). It's very
> similar to virStreamRecvAll() except it handles sparse streams
> well.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  include/libvirt/libvirt-stream.h |  28 -
>  src/libvirt-stream.c | 120 
> +++
>  src/libvirt_public.syms  |   1 +
>  3 files changed, 146 insertions(+), 3 deletions(-)

ACK

> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 0439434..4e8db51 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -741,6 +741,7 @@ LIBVIRT_2.0.0 {
>  virStreamHoleSize;
>  virStreamRecvFlags;
>  virStreamSkip;
> +virStreamSparseRecvAll;
>  } LIBVIRT_1.3.3;

2.1.0

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH RFC 09/48] Introduce VIR_STREAM_RECV_STOP_AT_HOLE flag

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:26PM +0200, Michal Privoznik wrote:
> This flag is for virStreamRecvFlags API. Its purpose is to stop
> reading from the stream if a hole occurred as holes are to be
> threated separately.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  include/libvirt/libvirt-stream.h |  4 
>  src/libvirt-stream.c | 30 +-
>  2 files changed, 33 insertions(+), 1 deletion(-)

ACK


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH RFC 08/48] Introduce virStreamHoleSize

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:25PM +0200, Michal Privoznik wrote:
> This function is basically a counterpart for virStreamSkip. If
> one side of a stream called virStreamSkip() the other should call
> virStreamHoleSize() to get the size of the hole.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  include/libvirt/libvirt-stream.h |  3 +++
>  src/driver-stream.h  |  5 +
>  src/libvirt-stream.c | 42 
> 
>  src/libvirt_public.syms  |  1 +
>  4 files changed, 51 insertions(+)
> 
> diff --git a/include/libvirt/libvirt-stream.h 
> b/include/libvirt/libvirt-stream.h
> index 4e0a599..2ebda74 100644
> --- a/include/libvirt/libvirt-stream.h
> +++ b/include/libvirt/libvirt-stream.h
> @@ -53,6 +53,9 @@ int virStreamRecvFlags(virStreamPtr st,
>  int virStreamSkip(virStreamPtr st,
>unsigned long long length);
>  
> +int virStreamHoleSize(virStreamPtr,
> +  unsigned long long *length);
> +
>  
>  /**
>   * virStreamSourceFunc:
> diff --git a/src/driver-stream.h b/src/driver-stream.h
> index 20ea13f..e196b6d 100644
> --- a/src/driver-stream.h
> +++ b/src/driver-stream.h
> @@ -46,6 +46,10 @@ typedef int
>  unsigned long long length);
>  
>  typedef int
> +(*virDrvStreamHoleSize)(virStreamPtr st,
> +unsigned long long *length);
> +
> +typedef int
>  (*virDrvStreamEventAddCallback)(virStreamPtr stream,
>  int events,
>  virStreamEventCallback cb,
> @@ -73,6 +77,7 @@ struct _virStreamDriver {
>  virDrvStreamRecv streamRecv;
>  virDrvStreamRecvFlags streamRecvFlags;
>  virDrvStreamSkip streamSkip;
> +virDrvStreamHoleSize streamHoleSize;
>  virDrvStreamEventAddCallback streamEventAddCallback;
>  virDrvStreamEventUpdateCallback streamEventUpdateCallback;
>  virDrvStreamEventRemoveCallback streamEventRemoveCallback;
> diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
> index 55f3ef5..3ac9e0d 100644
> --- a/src/libvirt-stream.c
> +++ b/src/libvirt-stream.c
> @@ -403,6 +403,48 @@ virStreamSkip(virStreamPtr stream,
>  
>  
>  /**
> + * virStreamHoleSize:
> + * @stream: pointer to the stream object
> + * @length: number of bytes to skip
> + *
> + * This function is a counterpart to virStreamSkip(). That is, if
> + * one side of a stream has called virStreamSkip() the other side
> + * of the stream should call virStreamHoleSize() to retrieve the
> + * size of hole. If there's currently no hole in the stream, -1
> + * is returned.
> + *
> + * Returns 0 on success,
> + *-1 on error
> + */
> +int
> +virStreamHoleSize(virStreamPtr stream,
> +  unsigned long long *length)
> +{
> +VIR_DEBUG("stream=%p, length=%p", stream, length);
> +
> +virResetLastError();
> +
> +virCheckStreamReturn(stream, -1);
> +virCheckNonNullArgReturn(length, -1);
> +
> +if (stream->driver &&
> +stream->driver->streamHoleSize) {
> +int ret;
> +ret = (stream->driver->streamHoleSize)(stream, length);
> +if (ret < 0)
> +goto error;
> +return ret;
> +}
> +
> +virReportUnsupportedError();
> +
> + error:
> +virDispatchError(stream->conn);
> +return -1;
> +}
> +
> +
> +/**
>   * virStreamSendAll:
>   * @stream: pointer to the stream object
>   * @handler: source callback for reading data from application
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 5b536eb..0439434 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -738,6 +738,7 @@ LIBVIRT_2.0.0 {
>  virDomainGetGuestVcpus;
>  virDomainSetGuestVcpus;
>  virConnectStoragePoolEventRegisterAny;
> +virStreamHoleSize;
>  virStreamRecvFlags;
>  virStreamSkip;
>  } LIBVIRT_1.3.3;

Use 2.1.0 now

ACK

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH RFC 07/48] Introduce virStreamSkip

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:24PM +0200, Michal Privoznik wrote:
> This API can be used to tell the other side of the stream to skip
> some bytes in the stream. This can be used to create a sparse
> file on the receiving side of a stream.
> 
> It takes just one argument @length, which says how big the hole
> is. Since our streams are not rewindable like regular files, we
> don't need @whence argument like seek(2) has.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  include/libvirt/libvirt-stream.h |  3 +++
>  src/driver-stream.h  |  5 
>  src/libvirt-stream.c | 57 
> 
>  src/libvirt_public.syms  |  1 +
>  4 files changed, 66 insertions(+)
> 
> diff --git a/include/libvirt/libvirt-stream.h 
> b/include/libvirt/libvirt-stream.h
> index bee2516..4e0a599 100644
> --- a/include/libvirt/libvirt-stream.h
> +++ b/include/libvirt/libvirt-stream.h
> @@ -50,6 +50,9 @@ int virStreamRecvFlags(virStreamPtr st,
> size_t nbytes,
> unsigned int flags);
>  
> +int virStreamSkip(virStreamPtr st,
> +  unsigned long long length);
> +
>  
>  /**
>   * virStreamSourceFunc:
> diff --git a/src/driver-stream.h b/src/driver-stream.h
> index d4b0480..20ea13f 100644
> --- a/src/driver-stream.h
> +++ b/src/driver-stream.h
> @@ -42,6 +42,10 @@ typedef int
>   unsigned int flags);
>  
>  typedef int
> +(*virDrvStreamSkip)(virStreamPtr st,
> +unsigned long long length);
> +
> +typedef int
>  (*virDrvStreamEventAddCallback)(virStreamPtr stream,
>  int events,
>  virStreamEventCallback cb,
> @@ -68,6 +72,7 @@ struct _virStreamDriver {
>  virDrvStreamSend streamSend;
>  virDrvStreamRecv streamRecv;
>  virDrvStreamRecvFlags streamRecvFlags;
> +virDrvStreamSkip streamSkip;
>  virDrvStreamEventAddCallback streamEventAddCallback;
>  virDrvStreamEventUpdateCallback streamEventUpdateCallback;
>  virDrvStreamEventRemoveCallback streamEventRemoveCallback;
> diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
> index 80b2d47..55f3ef5 100644
> --- a/src/libvirt-stream.c
> +++ b/src/libvirt-stream.c
> @@ -346,6 +346,63 @@ virStreamRecvFlags(virStreamPtr stream,
>  
>  
>  /**
> + * virStreamSkip:
> + * @stream: pointer to the stream object
> + * @length: number of bytes to skip
> + *
> + * Skip @length bytes in the stream. This is useful when there's
> + * no actual data in the stream, just a hole. If that's the case,
> + * this API can be used to skip the hole properly instead of
> + * transmitting zeroes to the other side.
> + *
> + * An example using this with a hypothetical file upload API
> + * looks like:
> + *
> + *   virStream st;
> + *
> + *   while (1) {
> + * char buf[4096];
> + * size_t len;
> + * if (..in hole...) {
> + *   ..get hole size...
> + *   virStreamSkip(st, len);
> + * } else {
> + *   ...read len bytes...
> + *   virStreamSend(st, buf, len);
> + * }
> + *   }
> + *
> + * Returns 0 on success,
> + *-1 error
> + */
> +int
> +virStreamSkip(virStreamPtr stream,
> +  unsigned long long length)
> +{
> +VIR_DEBUG("stream=%p, length=%llu", stream, length);
> +
> +virResetLastError();
> +
> +virCheckStreamReturn(stream, -1);
> +
> +if (stream->driver &&
> +stream->driver->streamSkip) {
> +int ret;
> +ret = (stream->driver->streamSkip)(stream, length);
> +if (ret < 0)
> +goto error;
> +return ret;
> +}
> +
> +virReportUnsupportedError();
> +
> + error:
> +virDispatchError(stream->conn);
> +return -1;
> +}
> +
> +
> +/**
>   * virStreamSendAll:
>   * @stream: pointer to the stream object
>   * @handler: source callback for reading data from application
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index d013d9f..5b536eb 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -739,6 +739,7 @@ LIBVIRT_2.0.0 {
>  virDomainSetGuestVcpus;
>  virConnectStoragePoolEventRegisterAny;
>  virStreamRecvFlags;
> +virStreamSkip;
>  } LIBVIRT_1.3.3;

We can put this in a 2.1.0 block now


ACK

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH RFC 06/48] Implement virStreamRecvFlags to some drivers

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:23PM +0200, Michal Privoznik wrote:
> We have three virStreamDriver-s currently in our tree.
> virFDStream, remote driver and ESX driver.f or now, support for
> remote driver and ESX driver is sufficient, because
> implementation for virFDStream is going to be supplied later as
> it needs to be slightly different.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/esx/esx_stream.c   | 16 +++-
>  src/remote/remote_driver.c | 21 +
>  2 files changed, 32 insertions(+), 5 deletions(-)

ACK


>  
> +static int
> +remoteStreamRecv(virStreamPtr st,
> + char *data,
> + size_t nbytes)
> +{
> +return remoteStreamRecv(st, data, nbytes);

Err, infinite loop ?

> +}
> +


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH RFC 05/48] Introduce virStreamRecvFlags

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:22PM +0200, Michal Privoznik wrote:
> Although we already have virStreamRecv, just like some other
> older APIs it is missing @flags argument. This means, the
> function is not that flexible and therefore we need
> virStreamRecvFlags. The latter is going to be needed when we will
> want it to stop at a hole in stream.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  include/libvirt/libvirt-stream.h |  5 
>  src/driver-stream.h  |  7 +
>  src/libvirt-stream.c | 60 
> 
>  src/libvirt_public.syms  |  4 ++-
>  4 files changed, 75 insertions(+), 1 deletion(-)

ACK


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH RFC 04/48] virFDStreamData: Turn into virObjectLockable

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:21PM +0200, Michal Privoznik wrote:
> While this is no functional change, it makes the code look a bit
> nicer. Moreover, it prepares ground for future work.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/fdstream.c | 97 
> ++
>  1 file changed, 57 insertions(+), 40 deletions(-)

ACK

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH RFC 03/48] fdstream: s/struct virFDStreamData */virFDStreamDataPtr/

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:20PM +0200, Michal Privoznik wrote:
> There is really no reason why we should have to have 'struct'
> everywhere.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/fdstream.c | 24 +---
>  1 file changed, 13 insertions(+), 11 deletions(-)

ACK


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH RFC 02/48] src: Move iohelper out from utils/ to a separate dir

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:19PM +0200, Michal Privoznik wrote:
> This helper binary will grow in the future (e.g. it will gain
> a so library which will be shared with daemon too) therefore its
> sources should live somewhere else than utils/. I've chosen
> src/iohelper.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  po/POTFILES.in| 2 +-
>  src/Makefile.am   | 6 +++---
>  src/{util => iohelper}/iohelper.c | 0
>  3 files changed, 4 insertions(+), 4 deletions(-)
>  rename src/{util => iohelper}/iohelper.c (100%)

ACK


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH RFC 01/48] util: Introduce virFileInData

[Daniel P. Berrange]

On Wed, Jun 22, 2016 at 04:43:18PM +0200, Michal Privoznik wrote:
> This function takes a FD and determines whether the current
> position is in data section or in a hole. In addition to that,
> it also determines how much bytes are there remaining till the
> current section ends.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virfile.c   | 70 
> 
>  src/util/virfile.h   |  4 +++
>  3 files changed, 75 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 501c23e..f476eae 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1513,6 +1513,7 @@ virFileGetHugepageSize;
>  virFileGetMountReverseSubtree;
>  virFileGetMountSubtree;
>  virFileHasSuffix;
> +virFileInData;
>  virFileIsAbsPath;
>  virFileIsDir;
>  virFileIsExecutable;
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index f47bf39..05b709a 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -3443,3 +3443,73 @@ int virFileIsSharedFS(const char *path)
>   VIR_FILE_SHFS_SMB |
>   VIR_FILE_SHFS_CIFS);
>  }
> +
> +
> +int virFileInData(int fd,
> +  int *inData,
> +  unsigned long long *length)
> +{
> +int ret = -1;
> +off_t cur, data, hole;
> +
> +/* Get current position */
> +cur = lseek(fd, 0, SEEK_CUR);
> +if (cur == (off_t) -1) {
> +virReportSystemError(errno, "%s",
> + _("Unable to get current position in file"));
> +goto cleanup;
> +}
> +
> +/* Now try to get data and hole offsets */
> +data = lseek(fd, cur, SEEK_DATA);
> +
> +/* There are four options:
> + * 1) data == cur;  @cur is in data
> + * 2) data > cur; @cur is in a hole, next data at @data
> + * 3) data < 0, errno = ENXIO; either @cur is in trailing hole, or @cur 
> is beyond EOF.
> + * 4) data < 0, errno != ENXIO; we learned nothing
> + */
> +
> +if (data == (off_t) -1) {
> +/* cases 3 and 4 */
> +if (errno != ENXIO) {
> +virReportSystemError(errno, "%s",
> + _("Unable to seek to data"));
> +goto cleanup;
> +}
> +*inData = 0;
> +*length = 0;
> +} else if (data > cur) {
> +/* case 2 */
> +*inData = 0;
> +*length = data - cur;
> +} else {
> +/* case 1 */
> +*inData = 1;
> +
> +/* We don't know where does the next hole start. Let's
> + * find out. Here we get the same 4 possibilities as
> + * described above.*/
> +hole = lseek(fd, data, SEEK_HOLE);
> +if (hole == (off_t) -1 || hole == data) {
> +/* cases 1, 3 and 4 */
> +/* Wait a second. The reason why we are here is
> + * because we are in data. But at the same time we
> + * are in a trailing hole? Wut!? Do the best what we
> + * can do here. */
> +virReportSystemError(errno, "%s",
> + _("unable to seek to hole"));
> +goto cleanup;
> +} else {
> +/* case 2 */
> +*length = (hole - data);
> +}
> +}
> +
> +ret = 0;
> + cleanup:
> +/* At any rate, reposition back to where we started. */
> +if (cur != (off_t) -1)
> +ignore_value(lseek(fd, cur, SEEK_SET));

Is it really safe to ignore the value here ?  IIUC, callers of this
function would be justified in thinking it would *not* have a side
effect on file position. IOW, I think we'd probably want to treat
this error as fatal too.

I think it'd be desirable to have a unit test written explicitly
for this function, since there's a few fun edge cases to worry
about here.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

[Ian Jackson]

Jim Fehlig writes ("Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver 
breakage -- where to define LIBXL_API_VERSION?"):
> On 06/27/2016 10:12 AM, Ian Jackson wrote:
> > Does libvirt have stable release branches ?  One approach would be to
> > have osstest track a suitable libvirt stable release branche for each
> > Xen stable release branch.
> 
> I see Daniel already answered this question.
> 
> >
> > That would involve setting up a push gate for each of the chosen
> > libvirt stable branches.  That would be worthwhile if we expect those
> > stable branches to acquire commits which break Xen, and which we could
> > like to be told about.  But I'm not sure that's the case.
> 
> I occasionally backport Xen bug fixes to -maint branches. Cole has
> also grabbed some Xen bug fixes when making a stable release of a
> -maint branch. But such backports should be trivial and obvious bug
> fixes that shouldn't cause build or runtime breakage with Xen.

OK.  Thanks for the feedback.  I'll go ahead with my plan with the
git commit ids named in my earlier email.

Thanks,
Ian.

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

Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

[Jim Fehlig]

On 06/27/2016 10:12 AM, Ian Jackson wrote:

Daniel P. Berrange writes ("Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver 
breakage -- where to define LIBXL_API_VERSION?"):

On Mon, Jun 27, 2016 at 04:54:35PM +0100, Ian Jackson wrote:

Created the following branch refs on xenbits in the toplevel
libvirt.git:

  osstest/frozen/xen-4.3-testing 9a0c7f5f834185db9017c34aabc03ad99cf37bed
  osstest/frozen/xen-4.4-testing 33fb8ff185846a7b4974105d2c9400690a6f95cf
  osstest/frozen/xen-4.5-testing cda1cc170f07b45911b3dad03e42c8ebfc210fa1
  osstest/frozen/xen-4.6-testing eac167e2610d3e59b32f7ec7ba78cbc8c420a425
  osstest/frozen/xen-4.7-testing 1a41ed5af5e1704dd9b0bdca40df5c9cacbdabfc

How did you pick those hashes ? Would it make more sense to pick the
nearest libvirt release tag ? eg v1.3.2 instead of 33fb8ff18584 ?


These were those tested by the following `tolerable' osstest push gate
flights for the corresponding Xen tree:

  xen-4.3-testing 9a0c7f5f8341 86673
  xen-4.4-testing 33fb8ff18584 85031
  xen-4.5-testing cda1cc170f07 83135
  xen-4.6-testing eac167e2610d 96031
  xen-4.7-testing 1a41ed5af5e1 95728

I picked them by searching my mail archives for osstest `tolerable'
push gate flights - ie, passes in our CI system.

That minimises the risk that the selected versions are themselves
troublesome for some reason, needing another round of adjustment.

It might indeed be better to convert them to nearby release tags.
However:

mariner:libvirt> git-describe 9a0c7f5f834185db9017c34aabc03ad99cf37bed
v1.3.2-202-g9a0c7f5
mariner:libvirt> git-describe 33fb8ff185846a7b4974105d2c9400690a6f95cf
v1.3.2-rc2-1-g33fb8ff
mariner:libvirt> git-describe cda1cc170f07b45911b3dad03e42c8ebfc210fa1
v1.3.1-262-gcda1cc1


It seems odd that Xen 4.5 would use an older libvirt release than Xen 4.3.


mariner:libvirt> git-describe eac167e2610d3e59b32f7ec7ba78cbc8c420a425
v1.3.5-318-geac167e
mariner:libvirt> git-describe 1a41ed5af5e1704dd9b0bdca40df5c9cacbdabfc
v1.3.5-129-g1a41ed5
mariner:libvirt>

So in most cases these hashes are well away from a release tag.

Does libvirt have stable release branches ?  One approach would be to
have osstest track a suitable libvirt stable release branche for each
Xen stable release branch.


I see Daniel already answered this question.



That would involve setting up a push gate for each of the chosen
libvirt stable branches.  That would be worthwhile if we expect those
stable branches to acquire commits which break Xen, and which we could
like to be told about.  But I'm not sure that's the case.


I occasionally backport Xen bug fixes to -maint branches. Cole has also grabbed 
some Xen bug fixes when making a stable release of a -maint branch. But such 
backports should be trivial and obvious bug fixes that shouldn't cause build or 
runtime breakage with Xen.


Regards,
Jim

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

Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

[Daniel P. Berrange]

On Mon, Jun 27, 2016 at 05:12:52PM +0100, Ian Jackson wrote:
> Daniel P. Berrange writes ("Re: [libvirt] [Xen-devel] Fixing libvirt's libxl 
> driver breakage -- where to define LIBXL_API_VERSION?"):
> > On Mon, Jun 27, 2016 at 04:54:35PM +0100, Ian Jackson wrote:
> > > Created the following branch refs on xenbits in the toplevel
> > > libvirt.git:
> > > 
> > >  osstest/frozen/xen-4.3-testing 9a0c7f5f834185db9017c34aabc03ad99cf37bed
> > >  osstest/frozen/xen-4.4-testing 33fb8ff185846a7b4974105d2c9400690a6f95cf
> > >  osstest/frozen/xen-4.5-testing cda1cc170f07b45911b3dad03e42c8ebfc210fa1
> > >  osstest/frozen/xen-4.6-testing eac167e2610d3e59b32f7ec7ba78cbc8c420a425
> > >  osstest/frozen/xen-4.7-testing 1a41ed5af5e1704dd9b0bdca40df5c9cacbdabfc
> > 
> > How did you pick those hashes ? Would it make more sense to pick the
> > nearest libvirt release tag ? eg v1.3.2 instead of 33fb8ff18584 ?
> > 
> > > These were those tested by the following `tolerable' osstest push gate
> > > flights for the corresponding Xen tree:
> > > 
> > >  xen-4.3-testing 9a0c7f5f8341 86673
> > >  xen-4.4-testing 33fb8ff18584 85031
> > >  xen-4.5-testing cda1cc170f07 83135
> > >  xen-4.6-testing eac167e2610d 96031
> > >  xen-4.7-testing 1a41ed5af5e1 95728
> 
> I picked them by searching my mail archives for osstest `tolerable'
> push gate flights - ie, passes in our CI system.
> 
> That minimises the risk that the selected versions are themselves
> troublesome for some reason, needing another round of adjustment.
> 
> It might indeed be better to convert them to nearby release tags.
> However:
> 
> mariner:libvirt> git-describe 9a0c7f5f834185db9017c34aabc03ad99cf37bed
> v1.3.2-202-g9a0c7f5
> mariner:libvirt> git-describe 33fb8ff185846a7b4974105d2c9400690a6f95cf
> v1.3.2-rc2-1-g33fb8ff
> mariner:libvirt> git-describe cda1cc170f07b45911b3dad03e42c8ebfc210fa1
> v1.3.1-262-gcda1cc1
> mariner:libvirt> git-describe eac167e2610d3e59b32f7ec7ba78cbc8c420a425
> v1.3.5-318-geac167e
> mariner:libvirt> git-describe 1a41ed5af5e1704dd9b0bdca40df5c9cacbdabfc
> v1.3.5-129-g1a41ed5
> mariner:libvirt> 
> 
> So in most cases these hashes are well away from a release tag.
> 
> Does libvirt have stable release branches ?  One approach would be to
> have osstest track a suitable libvirt stable release branche for each
> Xen stable release branch.

Yep, there is a vN.N.N-maint branch for every release

NB, with our new numbering that'll be changing nto vN.N-maint instead.

> That would involve setting up a push gate for each of the chosen
> libvirt stable branches.  That would be worthwhile if we expect those
> stable branches to acquire commits which break Xen, and which we could
> like to be told about.  But I'm not sure that's the case.

Stuff goes onto the stable branches on an as-needed basis - mostly coming
from the distro maintainers response to bug fixes from their users. There's
probably not a whole lot that's touching xen on a regular basis and we're
quite strict in what we accept for stable.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

[Ian Jackson]

Daniel P. Berrange writes ("Re: [libvirt] [Xen-devel] Fixing libvirt's libxl 
driver breakage -- where to define LIBXL_API_VERSION?"):
> On Mon, Jun 27, 2016 at 04:54:35PM +0100, Ian Jackson wrote:
> > Created the following branch refs on xenbits in the toplevel
> > libvirt.git:
> > 
> >  osstest/frozen/xen-4.3-testing 9a0c7f5f834185db9017c34aabc03ad99cf37bed
> >  osstest/frozen/xen-4.4-testing 33fb8ff185846a7b4974105d2c9400690a6f95cf
> >  osstest/frozen/xen-4.5-testing cda1cc170f07b45911b3dad03e42c8ebfc210fa1
> >  osstest/frozen/xen-4.6-testing eac167e2610d3e59b32f7ec7ba78cbc8c420a425
> >  osstest/frozen/xen-4.7-testing 1a41ed5af5e1704dd9b0bdca40df5c9cacbdabfc
> 
> How did you pick those hashes ? Would it make more sense to pick the
> nearest libvirt release tag ? eg v1.3.2 instead of 33fb8ff18584 ?
> 
> > These were those tested by the following `tolerable' osstest push gate
> > flights for the corresponding Xen tree:
> > 
> >  xen-4.3-testing 9a0c7f5f8341 86673
> >  xen-4.4-testing 33fb8ff18584 85031
> >  xen-4.5-testing cda1cc170f07 83135
> >  xen-4.6-testing eac167e2610d 96031
> >  xen-4.7-testing 1a41ed5af5e1 95728

I picked them by searching my mail archives for osstest `tolerable'
push gate flights - ie, passes in our CI system.

That minimises the risk that the selected versions are themselves
troublesome for some reason, needing another round of adjustment.

It might indeed be better to convert them to nearby release tags.
However:

mariner:libvirt> git-describe 9a0c7f5f834185db9017c34aabc03ad99cf37bed
v1.3.2-202-g9a0c7f5
mariner:libvirt> git-describe 33fb8ff185846a7b4974105d2c9400690a6f95cf
v1.3.2-rc2-1-g33fb8ff
mariner:libvirt> git-describe cda1cc170f07b45911b3dad03e42c8ebfc210fa1
v1.3.1-262-gcda1cc1
mariner:libvirt> git-describe eac167e2610d3e59b32f7ec7ba78cbc8c420a425
v1.3.5-318-geac167e
mariner:libvirt> git-describe 1a41ed5af5e1704dd9b0bdca40df5c9cacbdabfc
v1.3.5-129-g1a41ed5
mariner:libvirt> 

So in most cases these hashes are well away from a release tag.

Does libvirt have stable release branches ?  One approach would be to
have osstest track a suitable libvirt stable release branche for each
Xen stable release branch.

That would involve setting up a push gate for each of the chosen
libvirt stable branches.  That would be worthwhile if we expect those
stable branches to acquire commits which break Xen, and which we could
like to be told about.  But I'm not sure that's the case.

Ian.

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

Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

[Daniel P. Berrange]

On Mon, Jun 27, 2016 at 04:54:35PM +0100, Ian Jackson wrote:
> (Adding Jan Beulich)
> 
> Ian Jackson writes ("Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver 
> breakage -- where to define LIBXL_API_VERSION?"):
> > It seems that the libvirt LIBXL_API_VERSION is now rather higher, at
> > 0x040400, since libvirt#fccf27253ced
> >   libxl: switch to using libxl_domain_create_restore from v4.4 API
> > 
> > One unfortunate effect of this is to break the osstest tests of the
> > Xen 4.3 stable branch, which for the moment is still allegedly in
> > security support.
> > 
> > I can't really see a way that this kind of problem could be avoided
> > in principle, without
> >   - providing a more sophisticated way for libxl callers to set the
> > requested version
> >   - providing more compatibility code in libvirt, too, and retaining
> > it for some time
> > 
> > I think instead that it would probably be better for osstest to
> > "freeze" the version of libvirt that it is using, every time we branch
> > Xen.
> > 
> > So Xen 4.4 would be tested with whatever libvirt we were using when
> > the stable branch for Xen 4.4 was made, and so on.
> > 
> > Does that sound sensible ?
> 
> In the assumption that it is, I have:
> 
> Created the following branch refs on xenbits in the toplevel
> libvirt.git:
> 
>  osstest/frozen/xen-4.3-testing 9a0c7f5f834185db9017c34aabc03ad99cf37bed
>  osstest/frozen/xen-4.4-testing 33fb8ff185846a7b4974105d2c9400690a6f95cf
>  osstest/frozen/xen-4.5-testing cda1cc170f07b45911b3dad03e42c8ebfc210fa1
>  osstest/frozen/xen-4.6-testing eac167e2610d3e59b32f7ec7ba78cbc8c420a425
>  osstest/frozen/xen-4.7-testing 1a41ed5af5e1704dd9b0bdca40df5c9cacbdabfc

How did you pick those hashes ? Would it make more sense to pick the
nearest libvirt release tag ? eg v1.3.2 instead of 33fb8ff18584 ?

> 
> These were those tested by the following `tolerable' osstest push gate
> flights for the corresponding Xen tree:
> 
>  xen-4.3-testing 9a0c7f5f8341 86673
>  xen-4.4-testing 33fb8ff18584 85031
>  xen-4.5-testing cda1cc170f07 83135
>  xen-4.6-testing eac167e2610d 96031
>  xen-4.7-testing 1a41ed5af5e1 95728
> 
> And I have prepared the patch below, which (together with a
> prerequisite, in my tree) will implement this in osstest.
> 
> Ian.
> 
> From 5d1c91d3c53b580305e96d62f8ca84f85f8d3011 Mon Sep 17 00:00:00 2001
> From: Ian Jackson 
> Date: Mon, 27 Jun 2016 16:49:52 +0100
> Subject: [OSSTEST PATCH] cr-daily-branch: libvirt: use frozen version on
>  stable branches
> 
> libvirt master might increase its LIBXL_API_VERSION.  When this feeds
> through osstest it can cause the push gates of Xen stable branches to
> break.
> 
> So for stable Xen branches do not track libvirt upstream.  Instead,
> use a frozen revision.  (Only for main push gate tests of stable Xen
> branches.)
> 
> The frozen branch is never going to be updated so it is not suitable
> for other kinds of uses.  In particular it won't get security fixes.
> So we call the refs   osstest/frozen/xen-K.L-testing  to discourage
> users from using them.
> 
> Deployment note: The Xen release checklist needs a new item "add this
> frozen libvirt branch".
> 
> Signed-off-by: Ian Jackson 
> ---
>  cr-daily-branch | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/cr-daily-branch b/cr-daily-branch
> index 8b7c789..21780b8 100755
> --- a/cr-daily-branch
> +++ b/cr-daily-branch
> @@ -186,6 +186,12 @@ if [ "x$REVISION_OVMF" = x ]; then
>  fi
>  fi
>  if [ "x$REVISION_LIBVIRT" = x ]; then
> + case "$xenbranch" in
> + xen-[0-9]*-testing)
> + BASE_TAG_LIBVIRT=osstest/frozen/$xenbranch
> + export BASE_TAG_LIBVIRT
> + ;;
> + esac
>   determine_version REVISION_LIBVIRT libvirt LIBVIRT
>   export REVISION_LIBVIRT
>  fi

Overall I think your approach makes sense.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH] Drop virStorageBackendLogicalMatchPoolSource

[Ján Tomko]

On Mon, Jun 27, 2016 at 01:56:58PM +0200, Peter Krempa wrote:

On Fri, Jun 24, 2016 at 18:09:01 +0200, Ján Tomko wrote:

On Wed, Jun 22, 2016 at 07:03:46AM -0400, John Ferlan wrote:
>On 06/17/2016 11:16 AM, Ján Tomko wrote:
>> On Thu, Jun 16, 2016 at 07:44:05AM -0400, John Ferlan wrote:
>>> On 06/16/2016 06:03 AM, Ján Tomko wrote:
 The whole point of LVM is abstraction from the lower layers so we
 shouldn't ever be checking this.

>>>
>>> So it's OK to activate/start a pool where the source device path is
>>> incorrect?
>>
>> For LVM, the important path is in .
>>
>
>But isn't there a 1-to-1 relationship between the VG and the PV?  A PV
>cannot be in more than one VG, so if you move your PV, then you have to
>update your XML to let libvirt know.
>

Why would it need to know? Apart from pool building, libvirt does not
need the value for anything.


It is needed when deleting the pool, where libvirt attempts to pvremove
the devices. Fortunately even in the current broken state it's not
possible to remove PVs that are member of a different VG which this
would allow.



The problem here is:
[A] on pool deletion, we call pvremove on all the devices listed in the
persistent XML config (since deletion only works on stopped pools)

With the persistent config out of sync, currently we either:
* call pvremove on PVs that are no longer in the VG
 This errors out if there is something useful on the PV (e.g. a new
 filesystem or it's used by a VG)
* we fail to call it on a PV that was added to the VG behind our back
 This leaves PV metadata on the device, so it shows up in pvs/pvdisplay
 output and some mkfs tools will not want to create a filesystem on it
 without --force. In our fs backend, we only run mkfs -t xfs with
 --force. On the other hand, pvcreate happily runs on such volume.


The broken part of the check is that it doesn't enforce that ALL PVs as
listed in the XML are member of the given VG and no other is. If just
one PV matches then the code happily accepts it.



[B] The PVs listed in the XML do not match the on-disk config.
[C] The pool fails to start after vgextend/vgreduce.


In the current state the check is pointless since it doesn't properly
enforce the configuration and still still allows other wrong
configurations.


It also slows down startup for pools which do not have any PVs
in the persistent config, but that can be easily solved by skipping the
whole check.
It also unnecessarily calls vgscan if we have an active VG.



So the options are:



The current state does nothing to address [A], fixes a subset of [B]
while breaking [C].


1) Remove it:
  - If the admin messes with the state we will happily ignore the
difference. Deletion will not work properly unless updated.
Pros: the storage pool will work if the volume group is found
Cons: deletion may break



Partially breaks [B], fixes [C].


2) Enforce it properly:
  - Fix the check so that the data provided in the XML must match the
state in the system.
Pros: the state will be always right
Cons: existing setups may stop to work



Fixes [B], breaks [C]. Possibly fixes [A] if we enforce it on delete.


3) Update the state in the XML
  - Re-detect the PV paths on pool startup.
Pros: state will be in sync, existing configs will work
Cons: less robust in some cases, weird semantics with persistent
  config



3a) Do it only for the live XML:
Fixes [B] and [C], does nothing for [A]

3b) Also update persistent config
Fixes [A], [B], [C], but with weird semantics.


4) 2 + 3. Update it via a flag when starting.


This fixes all three, giving the user an option to update the persistent
config.


I think [C] is more important than [B], which is why I proposed reverting
the check. It would be nice to get it into the release even without
addressing [B] at all. This does not make [A] any worse.

As for [B], it would be nice to update the live XML, but (as I stated in
an earlier mail) should not block fixing [C]. Rejecting such pools is
just wrong.


For [A], we can either:
1) Refuse to delete the pool if the PVs do not match the config.
 Requiring manual intervention, or an option to re-detect them in
 libvirt.
2) Automatically update the persistent config.
 This feels strange.
3) Make the issue not matter by not doing pvremove.
 This will not make them reusable for some tools, e.g. mkfs -t ext4,
 but since we already use --force for mkfs -t xfs, maybe we should do
 it here too.
4) Do nothing.
 Rely on pvremove to do the right thing.

Considering the user is capable of adjusting the VGs manually, I think
running PoolDelete on such pool is even more unlikely than creating the
pool via libvirt and adjusting it behind its back, so the chosen option
won't matter much.
My order of preference is [4] [1] (requiring manual intervention), then
[4] [4] [4] [3] [2].

Jan

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

Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

[Ian Jackson]

(Adding Jan Beulich)

Ian Jackson writes ("Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver 
breakage -- where to define LIBXL_API_VERSION?"):
> It seems that the libvirt LIBXL_API_VERSION is now rather higher, at
> 0x040400, since libvirt#fccf27253ced
>   libxl: switch to using libxl_domain_create_restore from v4.4 API
> 
> One unfortunate effect of this is to break the osstest tests of the
> Xen 4.3 stable branch, which for the moment is still allegedly in
> security support.
> 
> I can't really see a way that this kind of problem could be avoided
> in principle, without
>   - providing a more sophisticated way for libxl callers to set the
> requested version
>   - providing more compatibility code in libvirt, too, and retaining
> it for some time
> 
> I think instead that it would probably be better for osstest to
> "freeze" the version of libvirt that it is using, every time we branch
> Xen.
> 
> So Xen 4.4 would be tested with whatever libvirt we were using when
> the stable branch for Xen 4.4 was made, and so on.
> 
> Does that sound sensible ?

In the assumption that it is, I have:

Created the following branch refs on xenbits in the toplevel
libvirt.git:

 osstest/frozen/xen-4.3-testing 9a0c7f5f834185db9017c34aabc03ad99cf37bed
 osstest/frozen/xen-4.4-testing 33fb8ff185846a7b4974105d2c9400690a6f95cf
 osstest/frozen/xen-4.5-testing cda1cc170f07b45911b3dad03e42c8ebfc210fa1
 osstest/frozen/xen-4.6-testing eac167e2610d3e59b32f7ec7ba78cbc8c420a425
 osstest/frozen/xen-4.7-testing 1a41ed5af5e1704dd9b0bdca40df5c9cacbdabfc

These were those tested by the following `tolerable' osstest push gate
flights for the corresponding Xen tree:

 xen-4.3-testing 9a0c7f5f8341 86673
 xen-4.4-testing 33fb8ff18584 85031
 xen-4.5-testing cda1cc170f07 83135
 xen-4.6-testing eac167e2610d 96031
 xen-4.7-testing 1a41ed5af5e1 95728

And I have prepared the patch below, which (together with a
prerequisite, in my tree) will implement this in osstest.

Ian.

>From 5d1c91d3c53b580305e96d62f8ca84f85f8d3011 Mon Sep 17 00:00:00 2001
From: Ian Jackson 
Date: Mon, 27 Jun 2016 16:49:52 +0100
Subject: [OSSTEST PATCH] cr-daily-branch: libvirt: use frozen version on
 stable branches

libvirt master might increase its LIBXL_API_VERSION.  When this feeds
through osstest it can cause the push gates of Xen stable branches to
break.

So for stable Xen branches do not track libvirt upstream.  Instead,
use a frozen revision.  (Only for main push gate tests of stable Xen
branches.)

The frozen branch is never going to be updated so it is not suitable
for other kinds of uses.  In particular it won't get security fixes.
So we call the refs   osstest/frozen/xen-K.L-testing  to discourage
users from using them.

Deployment note: The Xen release checklist needs a new item "add this
frozen libvirt branch".

Signed-off-by: Ian Jackson 
---
 cr-daily-branch | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/cr-daily-branch b/cr-daily-branch
index 8b7c789..21780b8 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -186,6 +186,12 @@ if [ "x$REVISION_OVMF" = x ]; then
 fi
 fi
 if [ "x$REVISION_LIBVIRT" = x ]; then
+   case "$xenbranch" in
+   xen-[0-9]*-testing)
+   BASE_TAG_LIBVIRT=osstest/frozen/$xenbranch
+   export BASE_TAG_LIBVIRT
+   ;;
+   esac
determine_version REVISION_LIBVIRT libvirt LIBVIRT
export REVISION_LIBVIRT
 fi
-- 
2.1.4

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

[libvirt] [PATCH] qemuDomainDeviceDefValidate: Drop unused qemuCaps

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_domain.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0c107aa..fd25669 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2196,12 +2196,9 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef 
*dev,
 void *opaque)
 {
 virQEMUDriverPtr driver = opaque;
-virQEMUCapsPtr qemuCaps = NULL;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 int ret = -1;
 
-qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
-
 if (dev->type == VIR_DOMAIN_DEVICE_NET) {
 const virDomainNetDef *net = dev->data.net;
 
@@ -2216,7 +2213,6 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
 
 ret = 0;
  cleanup:
-virObjectUnref(qemuCaps);
 virObjectUnref(cfg);
 return ret;
 }
-- 
2.9.0

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

[libvirt] [PATCH v2] storage: dir: adapts .wipeVol for ploop volumes

[Olga Krishtal]

The modification of .volWipe callback wipes ploop volume using one of
given wiping algorithm: dod, nnsa, etc.
However, in case of ploop volume we need to reinitialize root.hds and 
DiskDescriptor.xml.

v2:
- added check on ploop tools presens
- virCommandAddArgFormat changed to virCommandAddArg

Signed-off-by: Olga Krishtal 
---
 src/storage/storage_backend.c | 68 +++
 1 file changed, 62 insertions(+), 6 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 5adf1fd..14b3a64 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -2195,6 +2195,55 @@ virStorageBackendWipeLocal(virStorageVolDefPtr vol,
 return ret;
 }
 
+static int
+virStorageBackendVolWipePloop(virStorageVolDefPtr vol)
+{
+virCommandPtr cmd = NULL;
+char *target_path = NULL;
+char *disk_desc = NULL;
+char *create_tool = NULL;
+
+int ret = -1;
+
+create_tool = virFindFileInPath("ploop");
+if (!create_tool) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("unable to find ploop tools, please install them"));
+return -1;
+}
+
+if (virAsprintf(&target_path, "%s/root.hds", vol->target.path) < 0)
+goto cleanup;
+
+if (virAsprintf(&disk_desc, "%s/DiskDescriptor.xml", vol->target.path) < 0)
+goto cleanup;
+
+if (virFileRemove(disk_desc, 0, 0) < 0) {
+virReportError(errno, _("Failed to delete DiskDescriptor.xml of volume 
'%s'"),
+   vol->target.path);
+goto cleanup;
+}
+if (virFileRemove(target_path, 0, 0) < 0) {
+virReportError(errno, _("failed to delete root.hds of volume '%s'"),
+   vol->target.path);
+goto cleanup;
+}
+
+cmd = virCommandNewArgList(create_tool, "init", "-s", NULL);
+
+virCommandAddArgFormat(cmd, "%lluM", VIR_DIV_UP(vol->target.capacity,
+(1024 * 1024)));
+virCommandAddArgList(cmd, "-t", "ext4", NULL);
+virCommandAddArg(cmd, target_path);
+ret = virCommandRun(cmd, NULL);
+
+ cleanup:
+VIR_FREE(disk_desc);
+VIR_FREE(target_path);
+VIR_FREE(create_tool);
+virCommandFree(cmd);
+return ret;
+}
 
 int
 virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -2207,6 +2256,8 @@ virStorageBackendVolWipeLocal(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 const char *alg_char = NULL;
 struct stat st;
 virCommandPtr cmd = NULL;
+char *path = NULL;
+char *target_path = vol->target.path;
 
 virCheckFlags(0, -1);
 
@@ -2214,12 +2265,12 @@ virStorageBackendVolWipeLocal(virConnectPtr conn 
ATTRIBUTE_UNUSED,
   vol->target.path, algorithm);
 
 if (vol->target.format == VIR_STORAGE_FILE_PLOOP) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("wiping for ploop volumes is not supported"));
-goto cleanup;
+if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 0)
+goto cleanup;
+target_path = path;
 }
 
-fd = open(vol->target.path, O_RDWR);
+fd = open(target_path, O_RDWR);
 if (fd == -1) {
 virReportSystemError(errno,
  _("Failed to open storage volume with path '%s'"),
@@ -2276,13 +2327,12 @@ virStorageBackendVolWipeLocal(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 if (algorithm != VIR_STORAGE_VOL_WIPE_ALG_ZERO) {
 cmd = virCommandNew(SCRUB);
 virCommandAddArgList(cmd, "-f", "-p", alg_char,
- vol->target.path, NULL);
+ target_path, NULL);
 
 if (virCommandRun(cmd, NULL) < 0)
 goto cleanup;
 
 ret = 0;
-goto cleanup;
 } else {
 if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) {
 ret = virStorageBackendVolZeroSparseFileLocal(vol, st.st_size, fd);
@@ -2292,10 +2342,16 @@ virStorageBackendVolWipeLocal(virConnectPtr conn 
ATTRIBUTE_UNUSED,
  vol->target.allocation,
  st.st_blksize);
 }
+if (ret < 0)
+goto cleanup;
 }
 
+if (vol->target.format == VIR_STORAGE_FILE_PLOOP)
+ret = virStorageBackendVolWipePloop(vol);
+
  cleanup:
 virCommandFree(cmd);
+VIR_FREE(path);
 VIR_FORCE_CLOSE(fd);
 return ret;
 }
-- 
1.8.3.1

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

Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

[Ian Jackson]

Jim Fehlig writes ("Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver 
breakage -- where to define LIBXL_API_VERSION?"):
> I think that is a good idea too. I've sent a patch setting
> LIBXL_API_VERSION to 0x040200
> 
> https://www.redhat.com/archives/libvir-list/2016-April/msg00771.html

It seems that the libvirt LIBXL_API_VERSION is now rather higher, at
0x040400, since libvirt#fccf27253ced
  libxl: switch to using libxl_domain_create_restore from v4.4 API

One unfortunate effect of this is to break the osstest tests of the
Xen 4.3 stable branch, which for the moment is still allegedly in
security support.

I can't really see a way that this kind of problem could be avoided
in principle, without
  - providing a more sophisticated way for libxl callers to set the
requested version
  - providing more compatibility code in libvirt, too, and retaining
it for some time

I think instead that it would probably be better for osstest to
"freeze" the version of libvirt that it is using, every time we branch
Xen.

So Xen 4.4 would be tested with whatever libvirt we were using when
the stable branch for Xen 4.4 was made, and so on.

Does that sound sensible ?

Ian.

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

Re: [libvirt] [PATCH] logging: fixing log level initialization from cmdline

[Jaroslav Suchanek]

On Fri, Jun 24, 2016 at 02:29:57PM +0100, Daniel P. Berrange wrote:
> On Fri, Jun 24, 2016 at 02:25:27PM +0200, Jaroslav Suchanek wrote:
> > Reorder code for setting default log level from cmdline prior
> > initialization of log outputs. Thus the --verbose option is reflected.
> > 
> > This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1325072
> > ---
> >  src/logging/log_daemon.c | 12 ++--
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
> > index 8f1ccc2..9f71ca9 100644
> > --- a/src/logging/log_daemon.c
> > +++ b/src/logging/log_daemon.c
> > @@ -405,6 +405,12 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config,
> >  virLogParseOutputs(config->log_outputs);
> > 
> >  /*
> > + * Command line override for --verbose
> > + */
> > +if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO))
> > +virLogSetDefaultPriority(VIR_LOG_INFO);
> > +
> > +/*
> >   * If no defined outputs, and either running
> >   * as daemon or not on a tty, then first try
> >   * to direct it to the systemd journal
> > @@ -464,12 +470,6 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config,
> >  VIR_FREE(tmp);
> >  }
> > 
> > -/*
> > - * Command line override for --verbose
> > - */
> > -if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO))
> > -virLogSetDefaultPriority(VIR_LOG_INFO);
> > -
> >  return 0;
> > 
> >   error:
> 
> The same pattern is present in lock_daemon.c too. Libvirtd seems to
> already use the pattern you're adding.

Thank you Daniel for reminding. I checked only the libvirtd part. I will
post v2 later.

J.

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

Re: [libvirt] [PATCH 2/2] domaincapstest: Don't read data from host

[Andrea Bolognani]

On Mon, 2016-06-27 at 16:53 +0200, Jiri Denemark wrote:
> On Mon, Jun 27, 2016 at 16:41:08 +0200, Andrea Bolognani wrote:
> > On Mon, 2016-06-27 at 15:50 +0200, Jiri Denemark wrote:
> > > virQEMUCapsFillDomainCaps would use virHostCPUGetKVMMaxVCPUs for KVM
> > > domains.
> > >  
> > > diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
> > > new file mode 100644
> > > index 000..5266b73
> > > --- /dev/null
> > > +++ b/tests/domaincapsmock.c
> > > @@ -0,0 +1,26 @@
> > > +/*
> > 
> > Missing copyright statement.
> 
> That's deliberate.

The "How to Apply These Terms to Your New Libraries" section
of the LGPL explicitly states

> [...] each file should have at least the "copyright" line
> and a pointer to where the full notice is found.

so I don't think you can just add the license text without a
corresponding copyright statement.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 2/2] domaincapstest: Don't read data from host

[Jiri Denemark]

On Mon, Jun 27, 2016 at 16:41:08 +0200, Andrea Bolognani wrote:
> On Mon, 2016-06-27 at 15:50 +0200, Jiri Denemark wrote:
> > virQEMUCapsFillDomainCaps would use virHostCPUGetKVMMaxVCPUs for KVM
> > domains.
> > 
> > diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
> > new file mode 100644
> > index 000..5266b73
> > --- /dev/null
> > +++ b/tests/domaincapsmock.c
> > @@ -0,0 +1,26 @@
> > +/*
> 
> Missing copyright statement.

That's deliberate.

...
> We should return a positive number instead, either something
> like 160 (so we can check that limiting vCPUs based on
> MAX_VCPUS works) or INT_MAX (so we know it'll never be
> smaller than the QEMU limit).

INT_MAX is better. In the future we can expand this to be configurable
per test case so that we can check more combinations.

Jirka

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

[libvirt] [PATCH 1/3] qemu: hot-plug: Fix broken SCSI disk hot-plug

[Marc Hartmayer]

The commit "qemu: hot-plug: Assume support for -device in
qemuDomainAttachSCSIDisk" dropped the code for the automatic SCSI
controller creation used in SCSI disk hot-plugging. If we are
hot-plugging a SCSI disk to a domain and there is no proper SCSI
controller defined, it results in an "error: internal error: Could not
find scsi controller with index X required for device" error.

For that reason reverting a hunk of the commit
d4d32005d6e8b2cc0a2f26b483ca1de10171db6d.

This patch also adds an extra comment to the code to clarify the
loop.

Reviewed-by: Boris Fiuczynski 
Reviewed-by: Bjoern Walk 
Signed-off-by: Marc Hartmayer 
---
 src/qemu/qemu_hotplug.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e0b8230..5e6a8cb 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -544,7 +544,9 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
  virDomainObjPtr vm,
  virDomainDiskDefPtr disk)
 {
+size_t i;
 qemuDomainObjPrivatePtr priv = vm->privateData;
+virDomainControllerDefPtr cont = NULL;
 char *drivestr = NULL;
 char *devstr = NULL;
 int ret = -1;
@@ -561,6 +563,24 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
 goto error;
 }
 
+/* Let's make sure the disk has a controller defined and loaded before
+ * trying to add it. The controller used by the disk must exist before a
+ * qemu command line string is generated.
+ *
+ * Ensure that the given controller and all controllers with a smaller 
index
+ * exist; there must not be any missing index in between.
+ */
+for (i = 0; i <= disk->info.addr.drive.controller; i++) {
+cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i);
+if (!cont)
+goto error;
+}
+
+/* Tell clang that "cont" is non-NULL.
+   This is because disk->info.addr.driver.controller is unsigned,
+   and hence the above loop must iterate at least once. */
+sa_assert(cont);
+
 if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
 goto error;
 
-- 
2.5.5

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

[libvirt] [PATCH 2/3] qemu: SCSI hostdev hot-plug: Fix automatic creation of SCSI controllers

[Marc Hartmayer]

Ensure that the given controller and all controllers with a smaller
index exist; there must not be any missing index in between.

Reviewed-by: Boris Fiuczynski 
Reviewed-by: Bjoern Walk 
Signed-off-by: Marc Hartmayer 
---
 src/qemu/qemu_hotplug.c | 21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5e6a8cb..037e601 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1873,6 +1873,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
virDomainObjPtr vm,
virDomainHostdevDefPtr hostdev)
 {
+size_t i;
 int ret = -1;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virErrorPtr orig_err;
@@ -1888,9 +1889,23 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
 return -1;
 }
 
-cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, 
hostdev->info->addr.drive.controller);
-if (!cont)
-return -1;
+/* Let's make sure the disk has a controller defined and loaded before
+ * trying to add it. The controller used by the disk must exist before a
+ * qemu command line string is generated.
+ *
+ * Ensure that the given controller and all controllers with a smaller 
index
+ * exist; there must not be any missing index in between.
+ */
+for (i = 0; i <= hostdev->info->addr.drive.controller; i++) {
+cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i);
+if (!cont)
+return -1;
+}
+
+/* Tell clang that "cont" is non-NULL.
+   This is because disk->info.addr.driver.controller is unsigned,
+   and hence the above loop must iterate at least once. */
+sa_assert(cont);
 
 if (qemuHostdevPrepareSCSIDevices(driver, vm->def->name,
   &hostdev, 1)) {
-- 
2.5.5

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

[libvirt] [PATCH 3/3] tests: Add test cases for SCSI disk hot-plug with QEMU

[Marc Hartmayer]

Verify that SCSI controllers get created automatically when a SCSI disk
is hot-plugged to a domain that doesn't have a matching SCSI controller
defined already.

Reviewed-by: Boris Fiuczynski 
Signed-off-by: Marc Hartmayer 
---
 tests/qemuhotplugtest.c| 29 ++
 .../qemuhotplug-disk-scsi-2.xml|  8 +++
 ...-base-with-scsi-controller-live+disk-scsi-2.xml | 51 +
 ...se-without-scsi-controller-live+disk-scsi-2.xml | 66 ++
 ...argv-hotplug-base-with-scsi-controller-live.xml | 56 ++
 ...v-hotplug-base-without-scsi-controller-live.xml | 40 +
 6 files changed, 250 insertions(+)
 create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml
 create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml
 create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-with-scsi-controller-live.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-without-scsi-controller-live.xml

diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 91bf331..ae57c0a 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -487,6 +487,35 @@ mymain(void)
"device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK,
"human-monitor-command", HMP(""));
 
+DO_TEST_ATTACH("hotplug-base-without-scsi-controller-live", "disk-scsi-2", 
false, true,
+   /* Four controllers added */
+   "device_add", QMP_OK,
+   "device_add", QMP_OK,
+   "device_add", QMP_OK,
+   "device_add", QMP_OK,
+   "human-monitor-command", HMP("OK\\r\\n"),
+   /* Disk added */
+   "device_add", QMP_OK);
+DO_TEST_DETACH("hotplug-base-with-scsi-controller-live", "disk-scsi-2", 
false, false,
+   "device_del", QMP_OK,
+   "human-monitor-command", HMP(""));
+
+DO_TEST_ATTACH_EVENT("hotplug-base-without-scsi-controller-live", 
"disk-scsi-2", false, true,
+ /* Four controllers added */
+ "device_add", QMP_OK,
+ "device_add", QMP_OK,
+ "device_add", QMP_OK,
+ "device_add", QMP_OK,
+ "human-monitor-command", HMP("OK\\r\\n"),
+ /* Disk added */
+ "device_add", QMP_OK);
+DO_TEST_DETACH("hotplug-base-with-scsi-controller-live", "disk-scsi-2", 
true, true,
+   "device_del", QMP_OK,
+   "human-monitor-command", HMP(""));
+DO_TEST_DETACH("hotplug-base-with-scsi-controller-live", "disk-scsi-2", 
false, false,
+   "device_del", QMP_DEVICE_DELETED("scsi3-0-5-7") QMP_OK,
+   "human-monitor-command", HMP(""));
+
 DO_TEST_ATTACH("hotplug-base-live", "qemu-agent", false, true,
"chardev-add", QMP_OK,
"device_add", QMP_OK);
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml 
b/tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml
new file mode 100644
index 000..3a847fb
--- /dev/null
+++ b/tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml
@@ -0,0 +1,8 @@
+
+  
+  
+  
+  
+  
+  
+
diff --git 
a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml
 
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml
new file mode 100644
index 000..b2c9a07
--- /dev/null
+++ 
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml
@@ -0,0 +1,51 @@
+
+  hotplug
+  d091ea82-29e6-2e34-3005-f02617b36e87
+  4194304
+  4194304
+  4
+  
+hvm
+
+  
+  
+
+
+
+  
+  
+  destroy
+  restart
+  restart
+  
+/usr/libexec/qemu-kvm
+
+  
+  
+  
+  
+  
+  
+
+
+  
+
+
+
+  
+
+
+  
+
+
+  
+
+
+  
+
+
+
+
+  
+  
+
diff --git 
a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml
 
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml
new file mode 100644
index 000..2b11f21
--- /dev/null
+++ 
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml
@@ -0,0 +1,66 @@
+
+  hotplug
+  d091ea82-29e6-2e34-3005-f02617b36e87
+  4194304
+  4194304
+  4
+  
+hvm
+
+  
+  
+
+
+
+  
+  
+  destroy
+  restart
+  restart
+  
+/usr/libexec/qemu-kvm
+
+  
+  
+  
+  
+  
+  
+  
+  
+
+
+  
+  
+
+
+  

[libvirt] [PATCH 0/3] qemu: Fix auto-generation of SCSI controllers for hot-plugged SCSI disks and add tests

[Marc Hartmayer]

This patch series fixes the auto-generation of SCSI controllers for hot-plugged
SCSI disks and hot-plugged SCSI hostdevs.

Additionally this patch series adds test cases for the auto-generation of SCSI
controllers to the test suite.

Marc Hartmayer (3):
  qemu: hot-plug: Fix broken SCSI disk hot-plug
  qemu: SCSI hostdev hot-plug: Fix automatic creation of SCSI
controllers
  tests: Add test cases for SCSI disk hot-plug with QEMU

 src/qemu/qemu_hotplug.c| 41 +-
 tests/qemuhotplugtest.c| 29 ++
 .../qemuhotplug-disk-scsi-2.xml|  8 +++
 ...-base-with-scsi-controller-live+disk-scsi-2.xml | 51 +
 ...se-without-scsi-controller-live+disk-scsi-2.xml | 66 ++
 ...argv-hotplug-base-with-scsi-controller-live.xml | 56 ++
 ...v-hotplug-base-without-scsi-controller-live.xml | 40 +
 7 files changed, 288 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml
 create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml
 create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-with-scsi-controller-live.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-without-scsi-controller-live.xml

-- 
2.5.5

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

Re: [libvirt] [PATCH 2/2] domaincapstest: Don't read data from host

[Jiri Denemark]

On Mon, Jun 27, 2016 at 15:50:44 +0200, Jiri Denemark wrote:
> virQEMUCapsFillDomainCaps would use virHostCPUGetKVMMaxVCPUs for KVM
> domains.
> diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
> new file mode 100644
> index 000..5266b73
> --- /dev/null
> +++ b/tests/domaincapsmock.c
> @@ -0,0 +1,26 @@
> +#include 
> +
> +#include "virhostcpu.h"
> +
> +int
> +virHostCPUGetKVMMaxVCPUs(void)
> +{
> +return -1;
> +}

Actually, this should return INT_MAX rather than an error.

Jirka

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

Re: [libvirt] [PATCH 1/2] qemu: Remove redundant parameter in virQEMUCapsFillDomainCaps

[Andrea Bolognani]

On Mon, 2016-06-27 at 15:50 +0200, Jiri Denemark wrote:
> virttype is already included in domCaps, no need to pass it separately.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_capabilities.c | 5 ++---
>  src/qemu/qemu_capabilities.h | 3 +--
>  src/qemu/qemu_driver.c   | 2 +-
>  tests/domaincapstest.c   | 3 +--
>  4 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 28d5321..2c0b29d 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -4337,8 +4337,7 @@ int
>  virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
>virQEMUCapsPtr qemuCaps,
>virFirmwarePtr *firmwares,
> -  size_t nfirmwares,
> -  virDomainVirtType virttype)
> +  size_t nfirmwares)
>  {
>  virDomainCapsOSPtr os = &domCaps->os;
>  virDomainCapsDeviceDiskPtr disk = &domCaps->disk;
> @@ -4348,7 +4347,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
>  
>  domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps,
>   domCaps->machine);
> -if (virttype == VIR_DOMAIN_VIRT_KVM) {
> +if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM) {
>  int hostmaxvcpus = virHostCPUGetKVMMaxVCPUs();
>  if (hostmaxvcpus >= 0)
>  domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus);
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 9d891c8..affb639 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -492,7 +492,6 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
>  int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
>virQEMUCapsPtr qemuCaps,
>virFirmwarePtr *firmwares,
> -  size_t nfirmwares,
> -  virDomainVirtType virttype);
> +  size_t nfirmwares);
>  
>  #endif /* __QEMU_CAPABILITIES_H__*/
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 61d184b..749f8a1 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18411,7 +18411,7 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
>  goto cleanup;
>  
>  if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps,
> -  cfg->firmwares, cfg->nfirmwares, virttype) 
> < 0)
> +  cfg->firmwares, cfg->nfirmwares) < 0)
>  goto cleanup;
>  
>  ret = virDomainCapsFormat(domCaps);
> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
> index 01ebfcc..ae31146 100644
> --- a/tests/domaincapstest.c
> +++ b/tests/domaincapstest.c
> @@ -129,8 +129,7 @@ fillQemuCaps(virDomainCapsPtr domCaps,
>  
>  if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps,
>cfg->firmwares,
> -  cfg->nfirmwares,
> -  VIR_DOMAIN_VIRT_QEMU) < 0)
> +  cfg->nfirmwares) < 0)
>  goto cleanup;
>  
>  /* The function above tries to query host's KVM & VFIO capabilities by

This should be patch 2/2 in the series.

ACK with that changed.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 2/2] domaincapstest: Don't read data from host

[Andrea Bolognani]

On Mon, 2016-06-27 at 15:50 +0200, Jiri Denemark wrote:
> virQEMUCapsFillDomainCaps would use virHostCPUGetKVMMaxVCPUs for KVM
> domains.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  tests/Makefile.am  |  7 +++
>  tests/domaincapsmock.c | 26 ++
>  tests/domaincapstest.c |  2 +-
>  3 files changed, 34 insertions(+), 1 deletion(-)
>  create mode 100644 tests/domaincapsmock.c
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 444e0fd..1639540 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -404,6 +404,7 @@ test_libraries = libshunload.la \
>   virrandommock.la \
>   virhostcpumock.la \
>   nssmock.la \
> + domaincapsmock.la \
>   $(NULL)
>  if WITH_QEMU
>  test_libraries += libqemumonitortestutils.la \
> @@ -919,6 +920,12 @@ vircaps2xmltest_SOURCES = \
>   vircaps2xmltest.c testutils.h testutils.c
>  vircaps2xmltest_LDADD = $(LDADDS)
>  
> +
> +domaincapsmock_la_SOURCES = domaincapsmock.c
> +domaincapsmock_la_CFLAGS = $(AM_CFLAGS)
> +domaincapsmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
> +domaincapsmock_la_LIBADD = $(MOCKLIBS_LIBS)
> +
>  domaincapstest_SOURCES = \
>   domaincapstest.c testutils.h testutils.c
>  domaincapstest_LDADD = $(LDADDS)
> diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
> new file mode 100644
> index 000..5266b73
> --- /dev/null
> +++ b/tests/domaincapsmock.c
> @@ -0,0 +1,26 @@
> +/*

Missing copyright statement.

> + * 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
> + * .
> + *

Unneeded empty line.

> + */
> +
> +#include 
> +
> +#include "virhostcpu.h"
> +
> +int
> +virHostCPUGetKVMMaxVCPUs(void)
> +{
> +return -1;

As discussed offline, returning -1 signals the caller that
an error has occurred.

We should return a positive number instead, either something
like 160 (so we can check that limiting vCPUs based on
MAX_VCPUS works) or INT_MAX (so we know it'll never be
smaller than the QEMU limit).

> +}
> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
> index ae31146..5b7b7d0 100644
> --- a/tests/domaincapstest.c
> +++ b/tests/domaincapstest.c
> @@ -385,4 +385,4 @@ mymain(void)
>  return ret;
>  }
>  
> -VIRT_TEST_MAIN(mymain)
> +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/domaincapsmock.so")

ACK with that fixed.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 0/2] Fix FreeBSD build

[Andrea Bolognani]

On Mon, 2016-06-27 at 16:12 +0200, Martin Kletzander wrote:
> On Mon, Jun 27, 2016 at 12:43:47PM +0200, Andrea Bolognani wrote:
> > Pushed under the build-breaker rule.
> > 
> > Andrea Bolognani (2):
> >  util: netdevip: Include vircommand.h
> 
> I don't understand how it can work on other platforms without this patch
> anyways, but ACK series and safe for freeze

Because non of the code that uses virCommand is compiled when

  defined(__linux__) && defined(HAVE_LIBNL)

which is how it got through review in the first place :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 0/2] Fix FreeBSD build

[Martin Kletzander]

On Mon, Jun 27, 2016 at 04:12:22PM +0200, Martin Kletzander wrote:

On Mon, Jun 27, 2016 at 12:43:47PM +0200, Andrea Bolognani wrote:

Pushed under the build-breaker rule.

Andrea Bolognani (2):
 util: netdevip: Include vircommand.h


I don't understand how it can work on other platforms without this patch
anyways, but ACK series and safe for freeze



Never mind, I missed the other thread reverting the series...


 Clean up after virNetDevIP creation

src/util/virnetdev.h | 21 -
src/util/virnetdevip.c   |  5 +++--
tests/qemuxml2argvmock.c |  9 +
3 files changed, 8 insertions(+), 27 deletions(-)

--
2.7.4

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


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

Re: [libvirt] [PATCH 0/2] Fix FreeBSD build

[Martin Kletzander]

On Mon, Jun 27, 2016 at 12:43:47PM +0200, Andrea Bolognani wrote:

Pushed under the build-breaker rule.

Andrea Bolognani (2):
 util: netdevip: Include vircommand.h


I don't understand how it can work on other platforms without this patch
anyways, but ACK series and safe for freeze


 Clean up after virNetDevIP creation

src/util/virnetdev.h | 21 -
src/util/virnetdevip.c   |  5 +++--
tests/qemuxml2argvmock.c |  9 +
3 files changed, 8 insertions(+), 27 deletions(-)

--
2.7.4

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


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

Re: [libvirt] [Bug] make check fails when LIBVIRT_DEBUG is defined

[Martin Kletzander]

On Mon, Jun 27, 2016 at 01:23:46PM +0200, Michal Privoznik wrote:

On 27.06.2016 10:05, Daniel P. Berrange wrote:

On Sat, Jun 25, 2016 at 04:32:21PM -0600, Eric Blake wrote:

[moderator note: the mail server balked at 1.3 megabytes of attachments,
so I've stripped them in my reply]

On 12/31/1969 05:00 PM,  wrote:

Hello everybody,

I set up a clean libvirt repo with:
git clone
./autogen.sh
make -j5

and then executed tests with various values of LIBVIRT_DEBUG using
the following command:
LIBVIRT_DEBUG=0…5 VIR_TEST_DEBUG=1 make check

The logs are in this mail's attachments.


With LIBVIRT_DEBUG=0 (or =5 or probably any invalid value), the logs say:
"Ignoring invalid log level setting”
but still, 10 tests fail due to various reasons.

With LIBVIRT_DEBUG=1, there is a slight difference. The number of tests
that fail is also 10, but here, commandtest fails instead of eventtest.

With LIBVIRT_DEBUG=2, only 9 tests fail, because commandtest passes.

With LIBVIRT_DEBUG=3, only 1 test fails: qemuargv2xmltest. The
reason is the same as in the previous tests.

With LIBVIRT_DEBUG=4, it’s just qemuargv2xmltest that is failing,
but for a different reason than before:
"qemuParseCommandLineString should have logged a warning”.


Perhaps it’s okay that these tests fail, but still there's no harm
in letting people know.

Thank you rbogorodskiy for guidance on the mailing list :)

Have a nice day,
Tomasz



I don't know that anyone ever expected the testsuite to work with
LIBVIRT_DEBUG set to anything (because turning on debug can change
output of libvirt itself).  Things are different with VIR_TEST_DEBUG,


I'd certainly expect the test suite to pass 100% no matter what the
debug level is set to. Debug logging should not have a functional
effect on our code, besides outputting the log messages.


That's not 100% true. If we enable the debug messages, those are printed
to stdout. Now we have some tests that expect certain output or some
exact message to be logged. If we enable debug logs, the outputs are
poisoned and don't match the expected values.
I'd say this is a weakness of our test suite, not actual libvirt code.



I'd expect that not to work, OTOH I don't see why we couldn't make it
work.  Some tests must check the output, of course, but we can
prioritize the tests' logging choice above users'.  Having said that I
must mention that I wouldn't put fixing this above

Fixing the argv2xml is pretty easy (although during that I've found
bunch of old things that could use a good scrub as well) but the rest I
haven't looked at.

Martin


Michal

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


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

[libvirt] [PATCH v2 0/3] Small cleanups and improvements

[Andrea Bolognani]

Remove some obsolete code, and improve the behavior
of the 'virsh domcapabilities --virttype kvm' command when
running on non-Linux platform.


Andrea Bolognani (3):
  util: hostcpu: Add virHostCPUGetKVMMaxVCPUs() stub
  util: hostcpu: Drop obsolete compatibility code
  qemu: capabilities: Make virHostCPUGetKVMMaxVCPUs() errors fatal

 src/qemu/qemu_capabilities.c |  9 ++---
 src/util/virhostcpu.c| 25 -
 2 files changed, 18 insertions(+), 16 deletions(-)

-- 
2.7.4

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

[libvirt] [PATCH v2 1/3] util: hostcpu: Add virHostCPUGetKVMMaxVCPUs() stub

[Andrea Bolognani]

If we don't HAVE_LINUX_KVM_H, we can't query /dev/kvm to discover
the limits on the number of vCPUs, so we report an error and
return a negative value instead.
---
 src/util/virhostcpu.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 4ff4e72..a33932f 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1299,6 +1299,7 @@ virHostCPUGetThreadsPerSubcore(virArch arch 
ATTRIBUTE_UNUSED)
 
 #endif /* HAVE_LINUX_KVM_H && defined(KVM_CAP_PPC_SMT) */
 
+#if HAVE_LINUX_KVM_H
 int
 virHostCPUGetKVMMaxVCPUs(void)
 {
@@ -1310,11 +1311,11 @@ virHostCPUGetKVMMaxVCPUs(void)
 return -1;
 }
 
-#ifdef KVM_CAP_MAX_VCPUS
+# ifdef KVM_CAP_MAX_VCPUS
 /* at first try KVM_CAP_MAX_VCPUS to determine the maximum count */
 if ((ret = ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS)) > 0)
 goto cleanup;
-#endif /* KVM_CAP_MAX_VCPUS */
+# endif /* KVM_CAP_MAX_VCPUS */
 
 /* as a fallback get KVM_CAP_NR_VCPUS (the recommended maximum number of
  * vcpus). Note that on most machines this is set to 160. */
@@ -1329,3 +1330,12 @@ virHostCPUGetKVMMaxVCPUs(void)
 VIR_FORCE_CLOSE(fd);
 return ret;
 }
+#else
+int
+virHostCPUGetKVMMaxVCPUs(void)
+{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("KVM is not supported on this platform"));
+return -1;
+}
+#endif /* HAVE_LINUX_KVM_H */
-- 
2.7.4

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

Re: [libvirt] [Bug] make check fails when LIBVIRT_DEBUG is defined

[Daniel P. Berrange]

On Mon, Jun 27, 2016 at 01:23:46PM +0200, Michal Privoznik wrote:
> On 27.06.2016 10:05, Daniel P. Berrange wrote:
> > On Sat, Jun 25, 2016 at 04:32:21PM -0600, Eric Blake wrote:
> >> [moderator note: the mail server balked at 1.3 megabytes of attachments,
> >> so I've stripped them in my reply]
> >>
> >> On 12/31/1969 05:00 PM,  wrote:
> >>> Hello everybody,
> >>>
> >>> I set up a clean libvirt repo with:
> >>> git clone
> >>> ./autogen.sh
> >>> make -j5
> >>>
> >>> and then executed tests with various values of LIBVIRT_DEBUG using
> >>> the following command:
> >>> LIBVIRT_DEBUG=0…5 VIR_TEST_DEBUG=1 make check
> >>>
> >>> The logs are in this mail's attachments.
> >>>
> >>>
> >>> With LIBVIRT_DEBUG=0 (or =5 or probably any invalid value), the logs say:
> >>> "Ignoring invalid log level setting”
> >>> but still, 10 tests fail due to various reasons.
> >>>
> >>> With LIBVIRT_DEBUG=1, there is a slight difference. The number of tests
> >>> that fail is also 10, but here, commandtest fails instead of eventtest.
> >>>
> >>> With LIBVIRT_DEBUG=2, only 9 tests fail, because commandtest passes.
> >>>
> >>> With LIBVIRT_DEBUG=3, only 1 test fails: qemuargv2xmltest. The
> >>> reason is the same as in the previous tests.
> >>>
> >>> With LIBVIRT_DEBUG=4, it’s just qemuargv2xmltest that is failing,
> >>> but for a different reason than before:
> >>> "qemuParseCommandLineString should have logged a warning”.
> >>>
> >>>
> >>> Perhaps it’s okay that these tests fail, but still there's no harm
> >>> in letting people know.
> >>>
> >>> Thank you rbogorodskiy for guidance on the mailing list :)
> >>>
> >>> Have a nice day,
> >>> Tomasz
> >>>
> >>
> >> I don't know that anyone ever expected the testsuite to work with
> >> LIBVIRT_DEBUG set to anything (because turning on debug can change
> >> output of libvirt itself).  Things are different with VIR_TEST_DEBUG,
> > 
> > I'd certainly expect the test suite to pass 100% no matter what the
> > debug level is set to. Debug logging should not have a functional
> > effect on our code, besides outputting the log messages.
> 
> That's not 100% true. If we enable the debug messages, those are printed
> to stdout. Now we have some tests that expect certain output or some
> exact message to be logged. If we enable debug logs, the outputs are
> poisoned and don't match the expected values.
> I'd say this is a weakness of our test suite, not actual libvirt code.

Hmm, yes, unit tests really should not be expecting precise output on
stderr/stdout. Any that do are broken by design IMHO. At most they
should be doing a sub-string match if they want to look for something
and certainly not fail if extra output appears. Not least because I'll
often set LIBVIRT_DEBUG in order to debug the unit tests and would not
expect that to have side effects on them.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

[libvirt] [PATCH v2 3/3] qemu: capabilities: Make virHostCPUGetKVMMaxVCPUs() errors fatal

[Andrea Bolognani]

An error in virHostCPUGetKVMMaxVCPUs() means we've been unable
to access /dev/kvm, or we're running on a platform that doesn't
support KVM in the first place.

If that's the case, we shouldn't ignore the error and report
domcapabilities even though we know the user won't be able to
start any KVM guest.
---
 src/qemu/qemu_capabilities.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 28d5321..ae54c94 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4349,9 +4349,12 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
 domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps,
  domCaps->machine);
 if (virttype == VIR_DOMAIN_VIRT_KVM) {
-int hostmaxvcpus = virHostCPUGetKVMMaxVCPUs();
-if (hostmaxvcpus >= 0)
-domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus);
+int hostmaxvcpus;
+
+if ((hostmaxvcpus = virHostCPUGetKVMMaxVCPUs()) < 0)
+return -1;
+
+domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus);
 }
 
 if (virQEMUCapsFillDomainOSCaps(os, firmwares, nfirmwares) < 0 ||
-- 
2.7.4

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

[libvirt] [PATCH 2/2] domaincapstest: Don't read data from host

[Jiri Denemark]

virQEMUCapsFillDomainCaps would use virHostCPUGetKVMMaxVCPUs for KVM
domains.

Signed-off-by: Jiri Denemark 
---
 tests/Makefile.am  |  7 +++
 tests/domaincapsmock.c | 26 ++
 tests/domaincapstest.c |  2 +-
 3 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 tests/domaincapsmock.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 444e0fd..1639540 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -404,6 +404,7 @@ test_libraries = libshunload.la \
virrandommock.la \
virhostcpumock.la \
nssmock.la \
+   domaincapsmock.la \
$(NULL)
 if WITH_QEMU
 test_libraries += libqemumonitortestutils.la \
@@ -919,6 +920,12 @@ vircaps2xmltest_SOURCES = \
vircaps2xmltest.c testutils.h testutils.c
 vircaps2xmltest_LDADD = $(LDADDS)
 
+
+domaincapsmock_la_SOURCES = domaincapsmock.c
+domaincapsmock_la_CFLAGS = $(AM_CFLAGS)
+domaincapsmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
+domaincapsmock_la_LIBADD = $(MOCKLIBS_LIBS)
+
 domaincapstest_SOURCES = \
domaincapstest.c testutils.h testutils.c
 domaincapstest_LDADD = $(LDADDS)
diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
new file mode 100644
index 000..5266b73
--- /dev/null
+++ b/tests/domaincapsmock.c
@@ -0,0 +1,26 @@
+/*
+ * 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 "virhostcpu.h"
+
+int
+virHostCPUGetKVMMaxVCPUs(void)
+{
+return -1;
+}
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index ae31146..5b7b7d0 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -385,4 +385,4 @@ mymain(void)
 return ret;
 }
 
-VIRT_TEST_MAIN(mymain)
+VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/domaincapsmock.so")
-- 
2.9.0

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

[libvirt] [PATCH 1/2] qemu: Remove redundant parameter in virQEMUCapsFillDomainCaps

[Jiri Denemark]

virttype is already included in domCaps, no need to pass it separately.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_capabilities.c | 5 ++---
 src/qemu/qemu_capabilities.h | 3 +--
 src/qemu/qemu_driver.c   | 2 +-
 tests/domaincapstest.c   | 3 +--
 4 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 28d5321..2c0b29d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4337,8 +4337,7 @@ int
 virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
   virQEMUCapsPtr qemuCaps,
   virFirmwarePtr *firmwares,
-  size_t nfirmwares,
-  virDomainVirtType virttype)
+  size_t nfirmwares)
 {
 virDomainCapsOSPtr os = &domCaps->os;
 virDomainCapsDeviceDiskPtr disk = &domCaps->disk;
@@ -4348,7 +4347,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
 
 domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps,
  domCaps->machine);
-if (virttype == VIR_DOMAIN_VIRT_KVM) {
+if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM) {
 int hostmaxvcpus = virHostCPUGetKVMMaxVCPUs();
 if (hostmaxvcpus >= 0)
 domCaps->maxvcpus = MIN(domCaps->maxvcpus, hostmaxvcpus);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 9d891c8..affb639 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -492,7 +492,6 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
 int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
   virQEMUCapsPtr qemuCaps,
   virFirmwarePtr *firmwares,
-  size_t nfirmwares,
-  virDomainVirtType virttype);
+  size_t nfirmwares);
 
 #endif /* __QEMU_CAPABILITIES_H__*/
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 61d184b..749f8a1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18411,7 +18411,7 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
 goto cleanup;
 
 if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps,
-  cfg->firmwares, cfg->nfirmwares, virttype) < 
0)
+  cfg->firmwares, cfg->nfirmwares) < 0)
 goto cleanup;
 
 ret = virDomainCapsFormat(domCaps);
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 01ebfcc..ae31146 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -129,8 +129,7 @@ fillQemuCaps(virDomainCapsPtr domCaps,
 
 if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps,
   cfg->firmwares,
-  cfg->nfirmwares,
-  VIR_DOMAIN_VIRT_QEMU) < 0)
+  cfg->nfirmwares) < 0)
 goto cleanup;
 
 /* The function above tries to query host's KVM & VFIO capabilities by
-- 
2.9.0

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

[libvirt] [PATCH 0/2] virQEMUCapsFillDomainCaps: Drop redundant parameter

[Jiri Denemark]

Jiri Denemark (2):
  qemu: Remove redundant parameter in virQEMUCapsFillDomainCaps
  domaincapstest: Don't read data from host

 src/qemu/qemu_capabilities.c |  5 ++---
 src/qemu/qemu_capabilities.h |  3 +--
 src/qemu/qemu_driver.c   |  2 +-
 tests/Makefile.am|  7 +++
 tests/domaincapsmock.c   | 26 ++
 tests/domaincapstest.c   |  5 ++---
 6 files changed, 39 insertions(+), 9 deletions(-)
 create mode 100644 tests/domaincapsmock.c

-- 
2.9.0

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

Re: [libvirt] [PATCH 2/2] util: Only define /dev/kvm path once

[Andrea Bolognani]

On Mon, 2016-06-27 at 15:17 +0200, Ján Tomko wrote:
> > > > Remove the local kvmpath variable from
> > > > virHostCPUGetThreadsPerSubcore() and use the global KVM_DEVICE
> > > > define instead.
> > > > ---
> > > > src/util/virhostcpu.c | 7 +++
> > > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > > 
> > > ACK
> > 
> > ... and safe for freeze? :)
> 
> Safe? Yes.
> 
> But it does not fix anything so I don't see the point of pushing it
> during the freeze.

Fair enough, I'll push it after release. Thanks!

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH v2 2/3] util: hostcpu: Drop obsolete compatibility code

[Andrea Bolognani]

All Linux releases we support (RHEL6 era) include these
definitions.
---
 src/util/virhostcpu.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index a33932f..8a8bda8 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -65,17 +65,6 @@ VIR_LOG_INIT("util.hostcpu");
 
 #define KVM_DEVICE "/dev/kvm"
 
-/* add definitions missing in older linux/kvm.h */
-#ifndef KVMIO
-# define KVMIO 0xAE
-#endif
-#ifndef KVM_CHECK_EXTENSION
-# define KVM_CHECK_EXTENSION   _IO(KVMIO,   0x03)
-#endif
-#ifndef KVM_CAP_NR_VCPUS
-# define KVM_CAP_NR_VCPUS 9   /* returns max vcpus per vm */
-#endif
-
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 static int
-- 
2.7.4

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

Re: [libvirt] [PATCH 2/2] util: Only define /dev/kvm path once

[Ján Tomko]

On Mon, Jun 27, 2016 at 01:15:56PM +0200, Andrea Bolognani wrote:

On Sat, 2016-06-25 at 10:19 +0200, Ján Tomko wrote:

On Fri, Jun 24, 2016 at 07:34:38PM +0200, Andrea Bolognani wrote:
> Remove the local kvmpath variable from
> virHostCPUGetThreadsPerSubcore() and use the global KVM_DEVICE
> define instead.
> ---
> src/util/virhostcpu.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>

ACK


... and safe for freeze? :)



Safe? Yes.

But it does not fix anything so I don't see the point of pushing it
during the freeze.

Jan

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

Re: [libvirt] [PATCH 0/6] Revert: 'IP peer address and host-side address config support'

[Ján Tomko]

On Mon, Jun 27, 2016 at 08:22:30AM -0400, Laine Stump wrote:

On 06/27/2016 07:01 AM, Ján Tomko wrote:

This series was accidentally pushed in the feature freeze.

Revert the last six commits adding the new functionality.

The rest are either bugfixes or preparation for the bugfixes.


Sigh. Sorry about pushing these after the freeze. I was watching for the
freeze notice, but I guess not well enough, and the tag apparaently
scrolled by during a large pull Sunday morning.

ACK series


Thanks, pushed now.


(although truthfully the ones most likely to cause problems
(as evidenced by what Andrea found with the FreeBSD build) are the ones
setting up for these final patches, which are more straightforward in
comparison (in particular, the code is built on all platforms, except
for the final two, which are just a few lines outside of new unit tests).


I did not want to revert the bugfixes and resolving the conflicts caused
by trying to revert everything but bugfixes would probably be more risky.

Jan

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

Re: [libvirt] [PATCH v2 08/15] encryption: Add luks parsing for storageencryption

[Peter Krempa]

On Fri, Jun 24, 2016 at 11:32:43 -0400, John Ferlan wrote:
> 
> 
> On 06/24/2016 09:45 AM, Peter Krempa wrote:
> > On Thu, Jun 23, 2016 at 13:29:04 -0400, John Ferlan wrote:
> >> Add parse and format of the luks/passphrase secret including tests for
> >> volume XML parsing.
> >>
> >> Signed-off-by: John Ferlan 
> >> ---
> >>  docs/formatsecret.html.in  |  7 +++-
> >>  docs/formatstorageencryption.html.in   | 26 -
> >>  docs/schemas/storagecommon.rng |  2 +
> >>  src/qemu/qemu_process.c|  6 +++
> >>  src/storage/storage_backend.c  |  3 +-
> >>  src/storage/storage_backend_fs.c   |  7 +++-
> >>  src/storage/storage_backend_gluster.c  |  2 +
> >>  src/util/virstorageencryption.c|  2 +-
> >>  src/util/virstorageencryption.h|  1 +
> >>  tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml | 41 
> >> 
> >>  .../qemuxml2xmlout-luks-disks.xml  | 45 
> >> ++
> >>  tests/qemuxml2xmltest.c|  1 +
> >>  tests/storagevolxml2xmlin/vol-luks.xml | 21 ++
> >>  tests/storagevolxml2xmlout/vol-luks.xml| 21 ++
> >>  tests/storagevolxml2xmltest.c  |  1 +
> >>  15 files changed, 180 insertions(+), 6 deletions(-)
> >>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml
> >>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml
> >>  create mode 100644 tests/storagevolxml2xmlin/vol-luks.xml
> >>  create mode 100644 tests/storagevolxml2xmlout/vol-luks.xml
> > 
> > [...]
> > 
> >> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml 
> >> b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml
> >> new file mode 100644
> >> index 000..00399cf
> >> --- /dev/null
> >> +++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml
> >> @@ -0,0 +1,41 @@
> > 
> > [...]
> > 
> >> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml 
> >> b/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml
> >> new file mode 100644
> >> index 000..9ce15c0
> >> --- /dev/null
> >> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml
> > 
> > This is the diff of the above files.
> > 
> > $ diff -u tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml 
> > tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml
> > --- tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml  2016-06-24 
> > 15:37:13.215501639 +0200
> > +++ tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml  2016-06-24 
> > 15:37:13.215501639 +0200
> > @@ -32,14 +32,10 @@
> >
> > > function='0x0'/>
> >  
> > -
> > -   > function='0x2'/>
> > -
> > +
> >  
> >  
> >  
> > -
> > -   > function='0x0'/>
> > -
> > +
> >
> >  
> > 
> > Use of a separate output file doesn't make any sense.
> > 
> > ACK if you get rid of tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml
> > 
> 
> OK - I know this was a "more recent" change to not have extra output
> files, but it wasn't something I followed closely enough to make it part
> of my "routine" when adding new tests.

Sorry for that. Apparently this is no longer the case. Previously
(until 1.3.1 afaik) it was not necessary to provide the output file, but
it recently started to be. Sigh. More useless stuff in the repo.

Peter

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

Re: [libvirt] [PATCH 0/6] Revert: 'IP peer address and host-side address config support'

[Laine Stump]

On 06/27/2016 07:01 AM, Ján Tomko wrote:

This series was accidentally pushed in the feature freeze.

Revert the last six commits adding the new functionality.

The rest are either bugfixes or preparation for the bugfixes.


Sigh. Sorry about pushing these after the freeze. I was watching for the 
freeze notice, but I guess not well enough, and the tag apparaently 
scrolled by during a large pull Sunday morning.


ACK series (although truthfully the ones most likely to cause problems 
(as evidenced by what Andrea found with the FreeBSD build) are the ones 
setting up for these final patches, which are more straightforward in 
comparison (in particular, the code is built on all platforms, except 
for the final two, which are just a few lines outside of new unit tests).




Ján Tomko (6):
   Revert "qemu: support setting host-side IP addresses/routes"
   Revert "lxc: support setting host-side IP addresses/routes"
   Revert "util: support setting peer for virNetDevIPInfo addresses"
   Revert "conf: support host-side IP/route information in "
   Revert "conf: allow setting peer address in  element of
 "
   Revert "util: new function virNetDevIPInfoAddToDev"

  docs/formatdomain.html.in  |  60 +++
  docs/schemas/domaincommon.rng  |   8 +-
  src/conf/domain_conf.c | 117 -
  src/conf/domain_conf.h |   3 +-
  src/libvirt_private.syms   |   1 -
  src/lxc/lxc_container.c|  47 +++--
  src/lxc/lxc_process.c  |   8 --
  src/qemu/qemu_interface.c  |   6 +-
  src/util/virnetdevip.c |  60 ---
  src/util/virnetdevip.h |   7 +-
  tests/lxcxml2xmldata/lxc-ethernet-hostip.xml   |  44 
  tests/lxcxml2xmltest.c |   1 -
  .../qemuxml2argv-net-eth-hostip.args   |  23 
  .../qemuxml2argv-net-eth-hostip.xml|  39 ---
  tests/qemuxml2argvtest.c   |   1 -
  .../qemuxml2xmlout-net-eth-hostip.xml  |  44 
  tests/qemuxml2xmltest.c|   1 -
  17 files changed, 75 insertions(+), 395 deletions(-)
  delete mode 100644 tests/lxcxml2xmldata/lxc-ethernet-hostip.xml
  delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.args
  delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.xml
  delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-hostip.xml



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

Re: [libvirt] [PATCH] Drop virStorageBackendLogicalMatchPoolSource

[Peter Krempa]

On Fri, Jun 24, 2016 at 18:09:01 +0200, Ján Tomko wrote:
> On Wed, Jun 22, 2016 at 07:03:46AM -0400, John Ferlan wrote:
> >On 06/17/2016 11:16 AM, Ján Tomko wrote:
> >> On Thu, Jun 16, 2016 at 07:44:05AM -0400, John Ferlan wrote:
> >>> On 06/16/2016 06:03 AM, Ján Tomko wrote:
>  On Wed, Jun 15, 2016 at 06:45:59PM -0400, John Ferlan wrote:
> > On 06/15/2016 01:19 PM, Ján Tomko wrote:
> >> Regression introduced by commit 71b803a for [1] that prevents
> >> starting up
> >> a logical pool created with 
> >> after it has been moved to a different physical volume.

[...]

> >> 
> >>  vgname
> >>  
> >>
> >>vgname
> >
> >As I see things, this more or less lets libvirt "know" that "/dev/sda4"
> >and "vgname" are associated.
> >
> >>   
> >>   
> >>  /dev/vgname
> >>   
> >> 
> >>
> >> Then pvmove /dev/sda4 to /dev/sda5, libvirt will refuse that pool
> >> even though vgname is enough to uniquely identify a storage
> >> pool.
> >>
> >
> >As an admin you take this step to pvmove your data from /dev/sda4 to
> >/dev/sda5.

Note that this messes with the configuration without notifying libvirt
since we don't support such operation via the APIs.

> >But then you expect libvirt to know that? Do you expect libvirt to
> >automagically update the XML to /dev/sda5 during 'build' or 'start'?

That is one possibility ...

>  The whole point of LVM is abstraction from the lower layers so we
>  shouldn't ever be checking this.
> 
> >>>
> >>> So it's OK to activate/start a pool where the source device path is
> >>> incorrect?
> >>
> >> For LVM, the important path is in .
> >>
> >
> >But isn't there a 1-to-1 relationship between the VG and the PV?  A PV
> >cannot be in more than one VG, so if you move your PV, then you have to
> >update your XML to let libvirt know.
> >
> 
> Why would it need to know? Apart from pool building, libvirt does not
> need the value for anything.

It is needed when deleting the pool, where libvirt attempts to pvremove
the devices. Fortunately even in the current broken state it's not
possible to remove PVs that are member of a different VG which this
would allow.

The broken part of the check is that it doesn't enforce that ALL PVs as
listed in the XML are member of the given VG and no other is. If just
one PV matches then the code happily accepts it.

In the current state the check is pointless since it doesn't properly
enforce the configuration and still still allows other wrong
configurations.

So the options are:

1) Remove it:
   - If the admin messes with the state we will happily ignore the
 difference. Deletion will not work properly unless updated.
 Pros: the storage pool will work if the volume group is found
 Cons: deletion may break

2) Enforce it properly:
   - Fix the check so that the data provided in the XML must match the
 state in the system.
 Pros: the state will be always right
 Cons: existing setups may stop to work

3) Update the state in the XML
   - Re-detect the PV paths on pool startup.
 Pros: state will be in sync, existing configs will work
 Cons: less robust in some cases, weird semantics with persistent
   config

4) 2 + 3. Update it via a flag when starting.

Peter

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

Re: [libvirt] [PATCH 1/2] util: Drop obsolete compatibility code

[Andrea Bolognani]

On Sat, 2016-06-25 at 10:19 +0200, Ján Tomko wrote:
> On Fri, Jun 24, 2016 at 07:34:37PM +0200, Andrea Bolognani wrote:
> > All operating system releases we support include these
> > definitions.
> 
> Do we have a list of supported OSes somewhere? :)

RHEL6 and OSs released around that time. I don't think it's
been written down anywhere, but we dropped support for
anything older a while back.

> Also, it seems the virHostCPUGetKVMMaxVCPUs function is built
> unconditionally, so it might need to be wrapped in HAVE_LINUX_KVM_H.
> 
> Have you tried building it on non-Linux?

You're right, it doesn't. I'll fix it :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [Bug] make check fails when LIBVIRT_DEBUG is defined

[Michal Privoznik]

On 27.06.2016 10:05, Daniel P. Berrange wrote:
> On Sat, Jun 25, 2016 at 04:32:21PM -0600, Eric Blake wrote:
>> [moderator note: the mail server balked at 1.3 megabytes of attachments,
>> so I've stripped them in my reply]
>>
>> On 12/31/1969 05:00 PM,  wrote:
>>> Hello everybody,
>>>
>>> I set up a clean libvirt repo with:
>>> git clone
>>> ./autogen.sh
>>> make -j5
>>>
>>> and then executed tests with various values of LIBVIRT_DEBUG using
>>> the following command:
>>> LIBVIRT_DEBUG=0…5 VIR_TEST_DEBUG=1 make check
>>>
>>> The logs are in this mail's attachments.
>>>
>>>
>>> With LIBVIRT_DEBUG=0 (or =5 or probably any invalid value), the logs say:
>>> "Ignoring invalid log level setting”
>>> but still, 10 tests fail due to various reasons.
>>>
>>> With LIBVIRT_DEBUG=1, there is a slight difference. The number of tests
>>> that fail is also 10, but here, commandtest fails instead of eventtest.
>>>
>>> With LIBVIRT_DEBUG=2, only 9 tests fail, because commandtest passes.
>>>
>>> With LIBVIRT_DEBUG=3, only 1 test fails: qemuargv2xmltest. The
>>> reason is the same as in the previous tests.
>>>
>>> With LIBVIRT_DEBUG=4, it’s just qemuargv2xmltest that is failing,
>>> but for a different reason than before:
>>> "qemuParseCommandLineString should have logged a warning”.
>>>
>>>
>>> Perhaps it’s okay that these tests fail, but still there's no harm
>>> in letting people know.
>>>
>>> Thank you rbogorodskiy for guidance on the mailing list :)
>>>
>>> Have a nice day,
>>> Tomasz
>>>
>>
>> I don't know that anyone ever expected the testsuite to work with
>> LIBVIRT_DEBUG set to anything (because turning on debug can change
>> output of libvirt itself).  Things are different with VIR_TEST_DEBUG,
> 
> I'd certainly expect the test suite to pass 100% no matter what the
> debug level is set to. Debug logging should not have a functional
> effect on our code, besides outputting the log messages.

That's not 100% true. If we enable the debug messages, those are printed
to stdout. Now we have some tests that expect certain output or some
exact message to be logged. If we enable debug logs, the outputs are
poisoned and don't match the expected values.
I'd say this is a weakness of our test suite, not actual libvirt code.

Michal

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

Re: [libvirt] [PATCH 2/2] util: Only define /dev/kvm path once

[Andrea Bolognani]

On Sat, 2016-06-25 at 10:19 +0200, Ján Tomko wrote:
> On Fri, Jun 24, 2016 at 07:34:38PM +0200, Andrea Bolognani wrote:
> > Remove the local kvmpath variable from
> > virHostCPUGetThreadsPerSubcore() and use the global KVM_DEVICE
> > define instead.
> > ---
> > src/util/virhostcpu.c | 7 +++
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> 
> ACK

... and safe for freeze? :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCHv3 07/10] Reserve existing USB addresses

[Erik Skultety]

On 23/06/16 09:45, Ján Tomko wrote:
> Check if they fit on the USB controllers the domain has,
> and error out if two devices try to use the same address.
> ---
>  src/conf/domain_addr.c | 39 
> ++
>  src/conf/domain_addr.h |  4 +++
>  src/libvirt_private.syms   |  1 +
>  src/qemu/qemu_domain.h |  1 +
>  src/qemu/qemu_domain_address.c | 38 -
>  .../qemuxml2argv-usb-hub-conflict.xml  | 22 
>  tests/qemuxml2argvtest.c   |  3 ++
>  7 files changed, 107 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub-conflict.xml
> 
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 343ae0c..853f4ce 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -1558,3 +1558,42 @@ int 
> virDomainUSBAddressSetAddControllers(virDomainUSBAddressSetPtr addrs,
>  }
>  return 0;
>  }
> +
> +
> +int
> +virDomainUSBAddressReserve(virDomainDeviceInfoPtr info,
> +   void *data)
> +{
> +virDomainUSBAddressSetPtr addrs = data;
> +virDomainUSBAddressHubPtr targetHub = NULL;
> +char *portStr = NULL;
> +int ret = -1;
> +int targetPort;
> +
> +if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB)
> +return 0;
> +
> +portStr = virDomainUSBAddressPortFormat(info->addr.usb.port);

So when you leave out the *port* attribute, and attempt to define such
domain would result in an unknown error, yet defining such a domain
worked previously just fine (yes, there were/are other issues with
that). But you already realized this, since you posted a patch
(https://www.redhat.com/archives/libvir-list/2016-June/msg01973.html) to
make the port attribute mandatory (See my comment there).
Compared to the behaviour of PCI address assignment, one would assume
that if you omit an attribute that the docs does not specify as
mandatory, everything will turn out just fine for the USB addresses, as
it is the case with PCI addresses.

Erik

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

Re: [libvirt] [PATCH] Reject USB address without a port

[Erik Skultety]

On 25/06/16 18:11, Ján Tomko wrote:
> We would happily accept a usb address with a missing port attribute,
> then format it back as port='(null)', so the persistent domain
> would not be able to start (due to the XML sightseeing tour in
> virDomainObjSetDefTransient) or survive daemon restart.
> ---
>  src/conf/domain_conf.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 79d15c8..d0684eb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5132,7 +5132,11 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node,
>  
>  memset(addr, 0, sizeof(*addr));
>  
> -port = virXMLPropString(node, "port");
> +if (!(port = virXMLPropString(node, "port"))) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("USB address must have a port"));
> +goto cleanup;
> +}
>  bus = virXMLPropString(node, "bus");
>  
>  if (port && virDomainDeviceUSBAddressParsePort(port) < 0)
> 

We talked about this on IRC in private, so you already know that this
approach will still permit port='0' as a valid port specification. The
USB address reservation will again result in an unknown error. As you
proposed on IRC, it would be possible to have some sort of validation
(analogical to PCI addresses) that would take place during address
reservation. But since reservation would currently be done only for
QEMU, other drivers would be still broken. So I'd rather see the port
attribute still as optional, error out when detecting port=0 and
skipping it completely (i.e. success) if the attribute is missing during
address reservation. Lastly, I find it a little weird that we format
back a missing port as an empty string, that we cannot even parse after
ourselves, rather that leaving it out completely.

Erik

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

[libvirt] [PATCH 4/6] Revert "conf: support host-side IP/route information in "

[Ján Tomko]

This reverts commit fe6a77898a38f491403a70cc49925a584101daee.

This feature was accidentally pushed in the feature freeze.
---
 docs/formatdomain.html.in|  26 ---
 docs/schemas/domaincommon.rng|   3 +-
 src/conf/domain_conf.c   | 101 ++-
 src/conf/domain_conf.h   |   3 +-
 tests/lxcxml2xmldata/lxc-ethernet-hostip.xml |  44 
 tests/lxcxml2xmltest.c   |   1 -
 6 files changed, 23 insertions(+), 155 deletions(-)
 delete mode 100644 tests/lxcxml2xmldata/lxc-ethernet-hostip.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index bb1c079..2466df7 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5012,32 +5012,6 @@ qemu-kvm -net nic,model=? /dev/null
 definitions.  This is used by the LXC driver.
 
 
-
-  ...
-  
-
-  
-
-
-
-  
-  ...
-
-...
-  
-  ...
-
-
-
-  Since 2.0.0 network devices of type
-  "ethernet" can optionally be provided one or more IP addresses
-  and one or more routes to set on the host side of the
-  network device. These are configured as subelements of
-  the  element of the interface, and
-  have the same attributes as the similarly named elements used to
-  configure the guest side of the interface (described above).
-
-
 vhost-user interface
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 964ff92..2d12da9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2142,7 +2142,7 @@
   
 
   
-
+
   
 
 
@@ -2392,6 +2392,7 @@
   
 
   
+  
 
   
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 90d2eaa..3a81f7e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1798,7 +1798,6 @@ virDomainNetDefClear(virDomainNetDefPtr def)
 VIR_FREE(def->ifname_guest_actual);
 
 virNetDevIPInfoClear(&def->guestIP);
-virNetDevIPInfoClear(&def->hostIP);
 virDomainDeviceInfoClear(&def->info);
 
 VIR_FREE(def->filter);
@@ -4609,23 +4608,6 @@ virDomainRedirdevDefValidate(const virDomainDef *def,
 
 
 static int
-virDomainNetDefValidate(const virDomainNetDef *net)
-{
-if ((net->hostIP.nroutes || net->hostIP.nips) &&
-net->type != VIR_DOMAIN_NET_TYPE_ETHERNET) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Invalid attempt to set network interface "
- "host-side IP route and/or address info on "
- "interface of type '%s'. This is only supported "
- "on interfaces of type 'ethernet'"),
-   virDomainNetTypeToString(net->type));
-return -1;
-}
-return 0;
-}
-
-
-static int
 virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
const virDomainDef *def)
 {
@@ -4636,11 +4618,9 @@ virDomainDeviceDefValidateInternal(const 
virDomainDeviceDef *dev,
 case VIR_DOMAIN_DEVICE_REDIRDEV:
 return virDomainRedirdevDefValidate(def, dev->data.redirdev);
 
-case VIR_DOMAIN_DEVICE_NET:
-return virDomainNetDefValidate(dev->data.net);
-
 case VIR_DOMAIN_DEVICE_LEASE:
 case VIR_DOMAIN_DEVICE_FS:
+case VIR_DOMAIN_DEVICE_NET:
 case VIR_DOMAIN_DEVICE_INPUT:
 case VIR_DOMAIN_DEVICE_SOUND:
 case VIR_DOMAIN_DEVICE_VIDEO:
@@ -9009,15 +8989,6 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 cur = node->children;
 while (cur != NULL) {
 if (cur->type == XML_ELEMENT_NODE) {
-if (xmlStrEqual(cur->name, BAD_CAST "source")) {
-xmlNodePtr tmpnode = ctxt->node;
-
-ctxt->node = cur;
-if (virDomainNetIPInfoParseXML(_("interface host IP"),
-   ctxt, &def->hostIP) < 0)
-goto error;
-ctxt->node = tmpnode;
-}
 if (!macaddr && xmlStrEqual(cur->name, BAD_CAST "mac")) {
 macaddr = virXMLPropString(cur, "address");
 } else if (!network &&
@@ -20721,7 +20692,6 @@ virDomainNetDefFormat(virBufferPtr buf,
 {
 unsigned int actualType = virDomainNetGetActualType(def);
 bool publicActual = false;
-int sourceLines = 0;
 const char *typeStr;
 virDomainHostdevDefPtr hostdef = NULL;
 char macstr[VIR_MAC_STRING_BUFLEN];
@@ -20791,7 +20761,15 @@ virDomainNetDefFormat(virBufferPtr buf,

[libvirt] [PATCH 5/6] Revert "conf: allow setting peer address in element of "

[Ján Tomko]

This reverts commit 93135abf1454d8a1c8542e8c951ed615305ffa24.

This feature was accidentally pushed in the feature freeze.
---
 docs/formatdomain.html.in | 40 +++-
 docs/schemas/domaincommon.rng |  5 -
 src/conf/domain_conf.c| 16 +---
 src/util/virnetdevip.h|  5 ++---
 4 files changed, 18 insertions(+), 48 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 2466df7..f660aa6 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4967,7 +4967,6 @@ qemu-kvm -net nic,model=? /dev/null
   
   
   
-  
   
   
 
@@ -4986,30 +4985,21 @@ qemu-kvm -net nic,model=? /dev/null
 
 
 
-  Since 1.2.12 network devices and
-  hostdev devices with network capabilities can optionally be provided
-  one or more IP addresses to set on the network device in the
-  guest. Note that some hypervisors or network device types will
-  simply ignore them or only use the first one.
-  The family attribute can be set to
-  either ipv4 or ipv6, and the
-  address attribute contains the IP address. The
-  optional prefix is the number of 1 bits in the
-  netmask, and will be automatically set if not specified - for
-  IPv4 the default prefix is determined according to the network
-  "class" (A, B, or C - see RFC870), and for IPv6 the default
-  prefix is 64. The optional peer attribute holds the
-  IP address of the other end of a point-to-point network
-  device (since 2.0.0).
-
-
-
-Since 1.2.12 route elements can also be
-added to define IP routes to add in the guest.  The attributes of
-this element are described in the documentation for
-the route element
-in network
-definitions.  This is used by the LXC driver.
+Since 1.2.12 the network devices and host 
devices
+with network capabilities can be provided zero or more IP addresses to set
+on the target device. Note that some hypervisors or network device types
+will simply ignore them or only use the first one. The family
+attribute can be set to either ipv4 or ipv6, the
+address attribute holds the IP address. The 
prefix
+is not mandatory since some hypervisors do not handle it.
+
+
+
+Since 1.2.12 route elements can also be added
+to define the network routes to use for the network device. The attributes
+of this element are described in the documentation for the 
route
+element in network 
definitions.
+This is only used by the LXC driver.
 
 
 vhost-user interface
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 2d12da9..563cb3c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2629,11 +2629,6 @@
 
   
 
-
-  
-
-  
-
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3a81f7e..ef266af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6121,7 +6121,7 @@ virDomainNetIPParseXML(xmlNodePtr node)
 unsigned int prefixValue = 0;
 char *familyStr = NULL;
 int family = AF_UNSPEC;
-char *address = NULL, *peer = NULL;
+char *address = NULL;
 
 if (!(address = virXMLPropString(node, "address"))) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -6159,13 +6159,6 @@ virDomainNetIPParseXML(xmlNodePtr node)
 }
 ip->prefix = prefixValue;
 
-if ((peer = virXMLPropString(node, "peer")) != NULL &&
-virSocketAddrParse(&ip->peer, peer, family) < 0) {
-virReportError(VIR_ERR_INVALID_ARG,
-   _("Invalid peer '%s' in "), peer);
-goto cleanup;
-}
-
 ret = ip;
 ip = NULL;
 
@@ -6173,7 +6166,6 @@ virDomainNetIPParseXML(xmlNodePtr node)
 VIR_FREE(prefixStr);
 VIR_FREE(familyStr);
 VIR_FREE(address);
-VIR_FREE(peer);
 VIR_FREE(ip);
 return ret;
 }
@@ -20272,12 +20264,6 @@ virDomainNetIPInfoFormat(virBufferPtr buf,
 virBufferAsprintf(buf, " family='%s'", familyStr);
 if (def->ips[i]->prefix)
 virBufferAsprintf(buf, " prefix='%u'", def->ips[i]->prefix);
-if (VIR_SOCKET_ADDR_VALID(&def->ips[i]->peer)) {
-if (!(ipStr = virSocketAddrFormat(&def->ips[i]->peer)))
-return -1;
-virBufferAsprintf(buf, " peer='%s'", ipStr);
-VIR_FREE(ipStr);
-}
 virBufferAddLit(buf, "/>\n");
 }
 
diff --git a/src/util/virnetdevip.h b/src/util/virnetdevip.h
index 8277654..86fb77e 100644
--- a/src/util

[libvirt] [PATCH 6/6] Revert "util: new function virNetDevIPInfoAddToDev"

[Ján Tomko]

This reverts commit f1e0d0da11c473905470c28a6488bf57d9d0ae6e.

This feature was accidentally pushed in the feature freeze.
---
 src/libvirt_private.syms |  1 -
 src/lxc/lxc_container.c  | 47 -
 src/util/virnetdevip.c   | 60 
 src/util/virnetdevip.h   |  2 --
 4 files changed, 36 insertions(+), 74 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8579cfd..4617f5d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1931,7 +1931,6 @@ virNetDevBridgeSetVlanFiltering;
 virNetDevIPAddrAdd;
 virNetDevIPAddrDel;
 virNetDevIPAddrGet;
-virNetDevIPInfoAddToDev;
 virNetDevIPInfoClear;
 virNetDevIPRouteAdd;
 virNetDevIPRouteFree;
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 916a37b..0d5f16c 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -490,7 +490,7 @@ lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef,
   char **veths)
 {
 int ret = -1;
-size_t i;
+size_t i, j;
 const char *newname;
 virDomainNetDefPtr netDef;
 bool privNet = vmDef->features[VIR_DOMAIN_FEATURE_PRIVNET] ==
@@ -509,28 +509,53 @@ lxcContainerRenameAndEnableInterfaces(virDomainDefPtr 
vmDef,
 
 VIR_DEBUG("Renaming %s to %s", veths[i], newname);
 if (virNetDevSetName(veths[i], newname) < 0)
-goto cleanup;
+   goto cleanup;
+
+for (j = 0; j < netDef->guestIP.nips; j++) {
+virNetDevIPAddrPtr ip = netDef->guestIP.ips[j];
+int prefix;
+char *ipStr = virSocketAddrFormat(&ip->address);
+
+if ((prefix = virSocketAddrGetIPPrefix(&ip->address,
+   NULL, ip->prefix)) < 0) {
+ipStr = virSocketAddrFormat(&ip->address);
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to determine prefix for IP address 
'%s'"),
+   ipStr);
+VIR_FREE(ipStr);
+goto cleanup;
+}
+VIR_FREE(ipStr);
+
+if (virNetDevIPAddrAdd(newname, &ip->address, NULL, prefix) < 0)
+goto cleanup;
+}
 
-/* Only enable this device if there is a reason to do so (either
- * at least one IP was specified, or link state was set to up in
- * the config)
- */
 if (netDef->guestIP.nips ||
 netDef->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP) {
 VIR_DEBUG("Enabling %s", newname);
 if (virNetDevSetOnline(newname, true) < 0)
 goto cleanup;
-}
 
-/* set IP addresses and routes */
-if (virNetDevIPInfoAddToDev(newname, &netDef->guestIP) < 0)
-goto cleanup;
+/* Set the routes */
+for (j = 0; j < netDef->guestIP.nroutes; j++) {
+virNetDevIPRoutePtr route = netDef->guestIP.routes[j];
+
+if (virNetDevIPRouteAdd(newname,
+virNetDevIPRouteGetAddress(route),
+virNetDevIPRouteGetPrefix(route),
+virNetDevIPRouteGetGateway(route),
+virNetDevIPRouteGetMetric(route)) < 0) 
{
+goto cleanup;
+}
+}
+}
 }
 
 /* enable lo device only if there were other net devices */
 if ((veths || privNet) &&
 virNetDevSetOnline("lo", true) < 0)
-goto cleanup;
+   goto cleanup;
 
 ret = 0;
  cleanup:
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index cae566a..e5b88fe 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -887,63 +887,3 @@ virNetDevIPInfoClear(virNetDevIPInfoPtr ip)
 virNetDevIPRouteFree(ip->routes[i]);
 VIR_FREE(ip->routes);
 }
-
-
-/**
- * virNetDevIPInfoAddToDev:
- * @ifname: name of device to operate on
- * @ipInfo: list of routes and IP addresses to add to this device
- *
- * All IP routes and IP addresses in ipInfo are added to the named device.
- *
- * Returns: 0 on success, -1 (and error reported) on failure.
- */
-int
-virNetDevIPInfoAddToDev(const char *ifname,
-virNetDevIPInfo const *ipInfo)
-{
-int ret = -1;
-size_t i;
-char *ipStr = NULL;
-int prefix;
-
-/* add all IP addresses */
-for (i = 0; i < ipInfo->nips; i++) {
-virNetDevIPAddrPtr ip = ipInfo->ips[i];
-
-ipStr = virSocketAddrFormat(&ip->address);
-if ((prefix = virSocketAddrGetIPPrefix(&ip->address,
-   NULL, ip->prefix)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Failed to determine prefix for IP address '%s'"),
-   ipStr);
-g

[libvirt] [PATCH 1/6] Revert "qemu: support setting host-side IP addresses/routes"

[Ján Tomko]

This reverts commit 0b4645a7e061abc8a4be71fe89865cf248ce6e56.

This feature was accidentally pushed in the feature freeze.
---
 src/qemu/qemu_interface.c  |  6 +--
 .../qemuxml2argv-net-eth-hostip.args   | 23 ---
 .../qemuxml2argv-net-eth-hostip.xml| 39 ---
 tests/qemuxml2argvtest.c   |  1 -
 .../qemuxml2xmlout-net-eth-hostip.xml  | 44 --
 tests/qemuxml2xmltest.c|  1 -
 6 files changed, 1 insertion(+), 113 deletions(-)
 delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.args
 delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.xml
 delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-hostip.xml

diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index e637d21..b48ae50 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -196,12 +196,8 @@ qemuInterfaceStopDevice(virDomainNetDefPtr net)
 break;
 }
 
-case VIR_DOMAIN_NET_TYPE_ETHERNET:
-if (virNetDevIPInfoAddToDev(net->ifname, &net->hostIP) < 0)
-goto cleanup;
-break;
-
 case VIR_DOMAIN_NET_TYPE_USER:
+case VIR_DOMAIN_NET_TYPE_ETHERNET:
 case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
 case VIR_DOMAIN_NET_TYPE_SERVER:
 case VIR_DOMAIN_NET_TYPE_CLIENT:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.args 
b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.args
deleted file mode 100644
index b96c933..000
--- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.args
+++ /dev/null
@@ -1,23 +0,0 @@
-LC_ALL=C \
-PATH=/bin \
-HOME=/home/test \
-USER=test \
-LOGNAME=test \
-QEMU_AUDIO_DRV=none \
-/usr/bin/qemu \
--name QEMUGuest1 \
--S \
--M pc \
--m 214 \
--smp 1 \
--uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
--nographic \
--nodefaults \
--monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
--no-acpi \
--boot c \
--usb \
--drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
--device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--device rtl8139,vlan=0,id=net0,mac=00:11:22:33:44:55,bus=pci.0,addr=0x3 \
--net tap,fd=3,vlan=0,name=hostnet0
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.xml
deleted file mode 100644
index 6d08e82..000
--- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.xml
+++ /dev/null
@@ -1,39 +0,0 @@
-
-  QEMUGuest1
-  c7a5fdbd-edaf-9455-926a-d65c16db1809
-  219100
-  219100
-  1
-  
-hvm
-
-  
-  
-  destroy
-  restart
-  destroy
-  
-/usr/bin/qemu
-
-  
-  
-  
-  
-
-
-
-
-
-  
-  
-
-
-  
-  
-  
-
-
-
-
-  
-
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index bc0d805..a4b8bf4 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1012,7 +1012,6 @@ mymain(void)
 DO_TEST("net-eth", NONE);
 DO_TEST("net-eth-ifname", NONE);
 DO_TEST("net-eth-names", NONE);
-DO_TEST("net-eth-hostip", NONE);
 DO_TEST("net-client", NONE);
 DO_TEST("net-server", NONE);
 DO_TEST("net-mcast", NONE);
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-hostip.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-hostip.xml
deleted file mode 100644
index 856b35b..000
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-hostip.xml
+++ /dev/null
@@ -1,44 +0,0 @@
-
-  QEMUGuest1
-  c7a5fdbd-edaf-9455-926a-d65c16db1809
-  219100
-  219100
-  1
-  
-hvm
-
-  
-  
-  destroy
-  restart
-  destroy
-  
-/usr/bin/qemu
-
-  
-  
-  
-  
-
-
-  
-
-
-  
-
-
-
-  
-  
-
-
-  
-  
-  
-  
-
-
-
-
-  
-
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 29a29c5..7db9cb7 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -464,7 +464,6 @@ mymain(void)
 DO_TEST("net-virtio-disable-offloads");
 DO_TEST("net-eth");
 DO_TEST("net-eth-ifname");
-DO_TEST("net-eth-hostip");
 DO_TEST("net-virtio-network-portgroup");
 DO_TEST("net-hostdev");
 DO_TEST("net-hostdev-vfio");
-- 
2.7.3

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

[libvirt] [PATCH 3/6] Revert "util: support setting peer for virNetDevIPInfo addresses"

[Ján Tomko]

This reverts commit cb20f989df393ec97ba65afb06089d0ab87af484.

This feature was accidentally pushed in the feature freeze.
---
 src/util/virnetdevip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index f8b04e4..cae566a 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -919,7 +919,7 @@ virNetDevIPInfoAddToDev(const char *ifname,
ipStr);
 goto cleanup;
 }
-if (virNetDevIPAddrAdd(ifname, &ip->address, &ip->peer, prefix) < 0)
+if (virNetDevIPAddrAdd(ifname, &ip->address, NULL, prefix) < 0)
 goto cleanup;
 VIR_FREE(ipStr);
 }
-- 
2.7.3

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

[libvirt] [PATCH 0/6] Revert: 'IP peer address and host-side address config support'

[Ján Tomko]

This series was accidentally pushed in the feature freeze.

Revert the last six commits adding the new functionality.

The rest are either bugfixes or preparation for the bugfixes.

Ján Tomko (6):
  Revert "qemu: support setting host-side IP addresses/routes"
  Revert "lxc: support setting host-side IP addresses/routes"
  Revert "util: support setting peer for virNetDevIPInfo addresses"
  Revert "conf: support host-side IP/route information in "
  Revert "conf: allow setting peer address in  element of
"
  Revert "util: new function virNetDevIPInfoAddToDev"

 docs/formatdomain.html.in  |  60 +++
 docs/schemas/domaincommon.rng  |   8 +-
 src/conf/domain_conf.c | 117 -
 src/conf/domain_conf.h |   3 +-
 src/libvirt_private.syms   |   1 -
 src/lxc/lxc_container.c|  47 +++--
 src/lxc/lxc_process.c  |   8 --
 src/qemu/qemu_interface.c  |   6 +-
 src/util/virnetdevip.c |  60 ---
 src/util/virnetdevip.h |   7 +-
 tests/lxcxml2xmldata/lxc-ethernet-hostip.xml   |  44 
 tests/lxcxml2xmltest.c |   1 -
 .../qemuxml2argv-net-eth-hostip.args   |  23 
 .../qemuxml2argv-net-eth-hostip.xml|  39 ---
 tests/qemuxml2argvtest.c   |   1 -
 .../qemuxml2xmlout-net-eth-hostip.xml  |  44 
 tests/qemuxml2xmltest.c|   1 -
 17 files changed, 75 insertions(+), 395 deletions(-)
 delete mode 100644 tests/lxcxml2xmldata/lxc-ethernet-hostip.xml
 delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.args
 delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-eth-hostip.xml
 delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-hostip.xml

-- 
2.7.3

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

[libvirt] [PATCH 2/6] Revert "lxc: support setting host-side IP addresses/routes"

[Ján Tomko]

This reverts commit cd5c9f21ded4f8e6216eba02b8795f70503ab404.

This feature was accidentally pushed in the feature freeze.
---
 src/lxc/lxc_process.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 28313f0..07eb22a 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -304,14 +304,6 @@ virLXCProcessSetupInterfaceTap(virDomainDefPtr vm,
 if (virNetDevSetOnline(parentVeth, true) < 0)
 goto cleanup;
 
-if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_ETHERNET) {
-/* Set IP info for the host side, but only if the type is
- * 'ethernet'.
- */
-if (virNetDevIPInfoAddToDev(parentVeth, &net->hostIP) < 0)
-goto cleanup;
-}
-
 if (net->filter &&
 virDomainConfNWFilterInstantiate(vm->uuid, net) < 0)
 goto cleanup;
-- 
2.7.3

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

[libvirt] [PATCH 2/2] Clean up after virNetDevIP creation

[Andrea Bolognani]

Commit cf0568b0af4e moved a bunch of functions from virNetDev
to the more specific virNetDevIP; however, not all of the
existing uses were moved properly, causing build failures on
FreeBSD.

Complete the transition to the new names and drop the
obsolete declarations from the header file while at it.
---
 src/util/virnetdev.h | 21 -
 src/util/virnetdevip.c   |  4 ++--
 tests/qemuxml2argvmock.c |  9 +
 3 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 849f8e7..c3e8ffc 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -124,27 +124,6 @@ int virNetDevGetOnline(const char *ifname,
   bool *online)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 
-int virNetDevSetIPAddress(const char *ifname,
-  virSocketAddr *addr,
-  virSocketAddr *peer,
-  unsigned int prefix)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
-int virNetDevAddRoute(const char *ifname,
-  virSocketAddrPtr addr,
-  unsigned int prefix,
-  virSocketAddrPtr gateway,
-  unsigned int metric)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4)
-ATTRIBUTE_RETURN_CHECK;
-int virNetDevClearIPAddress(const char *ifname,
-virSocketAddr *addr,
-unsigned int prefix)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
-int virNetDevGetIPAddress(const char *ifname, virSocketAddrPtr addr)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
-int virNetDevWaitDadFinish(virSocketAddrPtr *addrs, size_t count)
-ATTRIBUTE_NONNULL(1);
-
 
 int virNetDevSetMAC(const char *ifname,
 const virMacAddr *macaddr)
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index 20f3ae1..f8b04e4 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -647,8 +647,8 @@ virNetDevIPRouteAdd(const char *ifname,
 
 /* return after DAD finishes for all known IPv6 addresses or an error */
 int
-virNetDevWaitDadFinish(virSocketAddrPtr *addrs ATTRIBUTE_UNUSED,
-   size_t count ATTRIBUTE_UNUSED)
+virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs ATTRIBUTE_UNUSED,
+ size_t count ATTRIBUTE_UNUSED)
 {
 virReportSystemError(ENOSYS, "%s",
  _("Unable to wait for IPv6 DAD on this platform"));
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index b99496a..e0ec2db 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -26,6 +26,7 @@
 #include "vircrypto.h"
 #include "virmock.h"
 #include "virnetdev.h"
+#include "virnetdevip.h"
 #include "virnetdevtap.h"
 #include "virnuma.h"
 #include "virrandom.h"
@@ -127,10 +128,10 @@ virNetDevSetMAC(const char *ifname ATTRIBUTE_UNUSED,
 return 0;
 }
 
-int virNetDevSetIPAddress(const char *ifname ATTRIBUTE_UNUSED,
-  virSocketAddr *addr ATTRIBUTE_UNUSED,
-  virSocketAddr *peer ATTRIBUTE_UNUSED,
-  unsigned int prefix ATTRIBUTE_UNUSED)
+int virNetDevIPAddrAdd(const char *ifname ATTRIBUTE_UNUSED,
+   virSocketAddr *addr ATTRIBUTE_UNUSED,
+   virSocketAddr *peer ATTRIBUTE_UNUSED,
+   unsigned int prefix ATTRIBUTE_UNUSED)
 {
 return 0;
 }
-- 
2.7.4

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

[libvirt] [PATCH 0/2] Fix FreeBSD build

[Andrea Bolognani]

Pushed under the build-breaker rule.

Andrea Bolognani (2):
  util: netdevip: Include vircommand.h
  Clean up after virNetDevIP creation

 src/util/virnetdev.h | 21 -
 src/util/virnetdevip.c   |  5 +++--
 tests/qemuxml2argvmock.c |  9 +
 3 files changed, 8 insertions(+), 27 deletions(-)

-- 
2.7.4

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

[libvirt] [PATCH 1/2] util: netdevip: Include vircommand.h

[Andrea Bolognani]

Not including the header causes

  util/virnetdevip.c:520:5: error:
  unknown type name 'virCommandPtr'; did you mean 'virCondPtr'?
virCommandPtr cmd = NULL;
^

and plenty more similar failures when compiling on FreeBSD.
---
 src/util/virnetdevip.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index 0663e14..20f3ae1 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -31,6 +31,7 @@
 #include "virlog.h"
 #include "virstring.h"
 #include "virutil.h"
+#include "vircommand.h"
 
 #if HAVE_GETIFADDRS
 # include 
-- 
2.7.4

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

Re: [libvirt] [PATCH] qemu: match controller index for LIVE+CONFIG when doing hotplug

[Martin Kletzander]

On Sat, Jun 25, 2016 at 03:42:38PM +0200, Tomasz Flendrich wrote:



On 24 Jun 2016, at 13:25, Martin Kletzander  wrote:

And what about the current bad behavior when you do this?

 virsh attach interface f24 --type network --source default --live
 virsh attach interface f24 --type network --source default --live
--config



This can be separated into two different issues.  If you do
attach-interface, we generate an XML without address, so you should be
able to do the above and have 2 more interfaces live, the second one
would be identical to the only one added to config.


What if we guaranteed that adding a device with both “—live —config” options
at once would always generate the same address? It could even leave some
holes (unassigned addresses) in one of {config, live}, but it doesn’t bother us,
does it? It would make the ABI stable.
If finding such address would be impossible, the user would be informed that
he/she can try adding the device separately using two calls without ABI 
stability.
This solution means that there are less surprises for the user.
Is there any reason it can’t be done, apart from complicating the code?



That's what we were talking about and that's what we need to do =)
There is no reason it can't be done.


Besides, how often do people run —live without —config? Perhaps we should
figure out what the most common use case is, make it work flawlessly
and have some undesired behavior in other cases as a compromise.



It's hard to guess.  However when concentrating on the guarantee of ABI
stability, it's pretty clear how it should behave from the user's POV.

- when both live and config are used, the devices plugged there must be
  identical, i.e. anything generated must be the same, all addresses, etc.

- when you can't plug identical devices to both config and live
  definitions, just error out.  Of course the more descriptive error
  message there is, the better.


Have a nice day,


U2


Tomasz


Martin


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

Re: [libvirt] [PATCH] libvirtd.conf: Fix invalid default of max_anonymous_clients

[Erik Skultety]

On 27/06/16 10:26, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1343442
> 
> When a client connects, it is placed into a queue. As soon as it
> authenticate, it is taken out of that queue and placed into a
> different one. Now, we have a setting in the daemon config file
> that allows users to control the length of the queue of yet not
> authenticated clients. By default, it has a value 20 but in the
> description to the config knob we clam it's zero.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  daemon/libvirtd.conf | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
> index 3b957e5..8466616 100644
> --- a/daemon/libvirtd.conf
> +++ b/daemon/libvirtd.conf
> @@ -283,8 +283,8 @@
>  #max_queued_clients = 1000
>  
>  # The maximum length of queue of accepted but not yet
> -# authenticated clients. The default value is zero, meaning
> -# the feature is disabled.
> +# authenticated clients. The default value is 20. Set this to
> +# zero to turn this feature off.
>  #max_anonymous_clients = 20
>  
>  # The minimum limit sets the number of workers to start up
> 

ACK, safe for freeze.

Erik

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

Re: [libvirt] 答复: how to make syntax-check on tar-tall released-version

[Andrea Bolognani]

On Sat, 2016-06-25 at 00:49 +, Zhangbo (Oscar) wrote:
> Here's my situation: I want to use libvirt v1.3.4 as my own git base, the 
> steps are:
> 1 Download libvirt v1.3.4 from http://libvirt.org/sources
> 2 add .gitignore inside the code tree
> 3 run "git init" to generate my own git tree
> 4 do some develop work
> 5 FAILED to do make syntax-check
> 
> If we want to have our own git repository, what's the right way to do? Is it 
> necessary to do "git fork" rather than "git init" to do development work?

Yeah, you'll want to clone the upstream git repository using

  git clone git://libvirt.org/libvirt.git

You'll then be able create your development branch based on
1.3.4 with

  git branch devel v1.3.4
  git checkout devel

Once you've done that, 'make syntax-check' will work.

Please note that, unless you have very specific needs, you
should base your development on the master branch instead
of the 1.3.4 release: among other things, that's a
requirement for having your changes merged upstream.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH] libvirtd.conf: Fix invalid default of max_anonymous_clients

[Michal Privoznik]

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

When a client connects, it is placed into a queue. As soon as it
authenticate, it is taken out of that queue and placed into a
different one. Now, we have a setting in the daemon config file
that allows users to control the length of the queue of yet not
authenticated clients. By default, it has a value 20 but in the
description to the config knob we clam it's zero.

Signed-off-by: Michal Privoznik 
---
 daemon/libvirtd.conf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index 3b957e5..8466616 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -283,8 +283,8 @@
 #max_queued_clients = 1000
 
 # The maximum length of queue of accepted but not yet
-# authenticated clients. The default value is zero, meaning
-# the feature is disabled.
+# authenticated clients. The default value is 20. Set this to
+# zero to turn this feature off.
 #max_anonymous_clients = 20
 
 # The minimum limit sets the number of workers to start up
-- 
2.8.4

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

Re: [libvirt] [Bug] make check fails when LIBVIRT_DEBUG is defined

[Daniel P. Berrange]

On Sat, Jun 25, 2016 at 04:32:21PM -0600, Eric Blake wrote:
> [moderator note: the mail server balked at 1.3 megabytes of attachments,
> so I've stripped them in my reply]
> 
> On 12/31/1969 05:00 PM,  wrote:
> > Hello everybody,
> > 
> > I set up a clean libvirt repo with:
> > git clone
> > ./autogen.sh
> > make -j5
> > 
> > and then executed tests with various values of LIBVIRT_DEBUG using
> > the following command:
> > LIBVIRT_DEBUG=0…5 VIR_TEST_DEBUG=1 make check
> > 
> > The logs are in this mail's attachments.
> > 
> > 
> > With LIBVIRT_DEBUG=0 (or =5 or probably any invalid value), the logs say:
> > "Ignoring invalid log level setting”
> > but still, 10 tests fail due to various reasons.
> > 
> > With LIBVIRT_DEBUG=1, there is a slight difference. The number of tests
> > that fail is also 10, but here, commandtest fails instead of eventtest.
> > 
> > With LIBVIRT_DEBUG=2, only 9 tests fail, because commandtest passes.
> > 
> > With LIBVIRT_DEBUG=3, only 1 test fails: qemuargv2xmltest. The
> > reason is the same as in the previous tests.
> > 
> > With LIBVIRT_DEBUG=4, it’s just qemuargv2xmltest that is failing,
> > but for a different reason than before:
> > "qemuParseCommandLineString should have logged a warning”.
> > 
> > 
> > Perhaps it’s okay that these tests fail, but still there's no harm
> > in letting people know.
> > 
> > Thank you rbogorodskiy for guidance on the mailing list :)
> > 
> > Have a nice day,
> > Tomasz
> > 
> 
> I don't know that anyone ever expected the testsuite to work with
> LIBVIRT_DEBUG set to anything (because turning on debug can change
> output of libvirt itself).  Things are different with VIR_TEST_DEBUG,

I'd certainly expect the test suite to pass 100% no matter what the
debug level is set to. Debug logging should not have a functional
effect on our code, besides outputting the log messages.

> which explicitly exists to change the verbosity of the testsuite itself,
> but not affect libvirt.  If the testsuite passes without LIBVIRT_DEBUG
> set, then that's the solution :)  But if you still get failures with
> LIBVIRT_DEBUG removed from the environment on your particular platform,
> then it is worth investigating how we can fix those failures (it may be
> a weakness in the testsuite, or an actual libvirt bug).

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] 答复: how to make syntax-check on tar-tall released-version

[Daniel P. Berrange]

On Sat, Jun 25, 2016 at 12:49:41AM +, Zhangbo (Oscar) wrote:
> >> Hi all:
> >> I downloaded libvirt v1.3.4 from http://libvirt.org/sources, and try to
> >make syntax-check inside the code tree.
> >> But I encounted with many problems, first, I need to have file 
> >> .gitignore
> >and directory .git, and then have gnulib, many problems still comes out after
> >that.
> >> I still have not fixed the problems until now.
> >> So what's the right way to make syntax-check on released-version 
> >> libvirt
> >project? Thanks in advance.
> >
> >You're not. The syntax-check rule is for developers to use before
> >merging code to git master. There's no reason to use it on released
> >versions
> >
> 
> Here's my situation: I want to use libvirt v1.3.4 as my own git base, the 
> steps are:
> 1 Download libvirt v1.3.4 from http://libvirt.org/sources
> 2 add .gitignore inside the code tree
> 3 run "git init" to generate my own git tree
> 4 do some develop work
> 5 FAILED to do make syntax-check
> 
> If we want to have our own git repository, what's the right way to do?
> Is it necessary to do "git fork" rather than "git init" to do development
> work?

Why on earth are you even doing this. Just clone the real git repository
in the normal manner. We always want all patches to be submitted against
git master, not released versions. Even if you did want to work on an
old release, you can just checkout the vN.N.N-maint branch from the main
git repo.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH v2 1/1] perf: add more perf events support

[Ren, Qiaowei]

Hi Peter,

According to your comment, I updated the XML documentation and post new version 
here. Do you have any more comments?

Thanks,
Qiaowei

> -Original Message-
> From: Ren, Qiaowei
> Sent: Tuesday, June 21, 2016 3:41 PM
> To: libvir-list@redhat.com
> Cc: Daniel P. Berrange ; Peter Krempa
> ; Ren, Qiaowei 
> Subject: [PATCH v2 1/1] perf: add more perf events support
> 
> With current perf framework, this patch adds support to more perf events,
> including cache missing, cache peference, cpu cycles, instrction, etc..
> 
> Signed-off-by: Qiaowei Ren 
> ---
>  docs/formatdomain.html.in| 24 +++
>  docs/schemas/domaincommon.rng|  4 +++
>  include/libvirt/libvirt-domain.h | 39 
>  src/libvirt-domain.c |  8 +
>  src/qemu/qemu_driver.c   | 23 +++---
>  src/util/virperf.c   | 65 
> +++-
>  src/util/virperf.h   |  4 +++
>  7 files changed, 154 insertions(+), 13 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index
> 7d34363..a4064ae 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1839,6 +1839,10 @@
>  
>  
>  
> +
> +
> +
> +
>
>...
>  
> @@ -1864,6 +1868,26 @@
>bandwidth of memory traffic for a memory controller
>perf.mbml
>  
> +
> +  cache_misses
> +  the amount of cache missing by applications running on the
> platform
> +  perf.cache_misses
> +
> +
> +  cache_peferences
> +  the amount of cache hit by applications running on the 
> platform
> +  perf.cache_peferences
> +
> +
> +  instructions
> +  the amount of instructions by applications running on the 
> platform
> +  perf.instructions
> +
> +
> +  cpu_cycles
> +  the amount of cycles one instruction needs
> +  perf.cpu_cycles
> +
>
> 
>  Devices
> diff --git a/docs/schemas/domaincommon.rng
> b/docs/schemas/domaincommon.rng index 162c2e0..01db999 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -414,6 +414,10 @@
>cmt
>mbmt
>mbml
> +  cache_misses
> +  cache_peferences
> +  instructions
> +  cpu_cycles
>  
>
>
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index cba4fa5..99c4c48 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1928,6 +1928,45 @@ void
> virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
>   */
>  # define VIR_PERF_PARAM_MBML "mbml"
> 
> +/**
> + * VIR_PERF_PARAM_CACHE_MISSES:
> + *
> + * Macro for typed parameter name that represents cache_misses perf
> + * event which can be used to measure the amount of cache missing by
> + * applications running on the platform. It corresponds to the
> + * "perf.cache_misses" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_CACHE_MISSES "cache_misses"
> +
> +/**
> + * VIR_PERF_PARAM_CACHE_REFERENCES:
> + *
> + * Macro for typed parameter name that represents cache_peferences
> + * perf event which can be used to measure the amount of cache hit
> + * by applications running on the platform. It corresponds to the
> + * "perf.cache_peferences" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_CACHE_REFERENCES "cache_peferences"
> +
> +/**
> + * VIR_PERF_PARAM_INSTRUCTIONS:
> + *
> + * Macro for typed parameter name that represents instructions perf
> + * event which can be used to measure the amount of instructions
> + * by applications running on the platform. It corresponds to the
> + * "perf.instructions" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_INSTRUCTIONS "instructions"
> +
> +/**
> + * VIR_PERF_PARAM_CPU_CYCLES:
> + *
> + * Macro for typed parameter name that represents cpu_cycles perf event
> + * which can be used to measure how many cycles one instruction needs.
> + * It corresponds to the "perf.cpu_cycles" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_CPU_CYCLES "cpu_cycles"
> +
>  int virDomainGetPerfEvents(virDomainPtr dom,
> virTypedParameterPtr *params,
> int *nparams, diff --git a/src/libvirt-domain.c 
> b/src/libvirt-
> domain.c index 73ae369..ef71b31 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11452,6 +11452,14 @@ virConnectGetDomainCapabilities(virConnectPtr
> conn,
>   * "perf.mbml" - the amount of data (bytes/s) sent through the memory
> controller
>   * 

Re: [libvirt] [PATCH 1/2] util: new function virXMLNodeSanitizeNamespaces()

[Peter Krempa]

On Fri, Jun 24, 2016 at 11:33:50 -0400, Laine Stump wrote:
> This is a generic version of virDomainDefMetadataSanitize() - the same
> functionality is now needed for network metadata.
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virxml.c| 52 
> 
>  src/util/virxml.h|  2 ++
>  3 files changed, 55 insertions(+)

[...]

> diff --git a/src/util/virxml.c b/src/util/virxml.c
> index aa97940..37f0817 100644
> --- a/src/util/virxml.c
> +++ b/src/util/virxml.c
> @@ -1083,6 +1083,58 @@ virXMLInjectNamespace(xmlNodePtr node,
>  return 0;
>  }
>  
> +/**
> + * virXMLNodeSanitizeNamespaces()
> + * @node: Sanitize the namespaces for this node
> + *
> + * This function removes subnodes in node that share the namespace.
> + * The first metadata entry of every duplicate namespace is kept. 
> Additionally

This is a little inconsistent since the sentence still hints to
'metadata'.

> + * nodes with no namespace are deleted.

Maybe you can state here that this is mostly used to sanitize metadata.

> + */
> +void
> +virXMLNodeSanitizeNamespaces(xmlNodePtr node)
> +{

ACK

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

Re: [libvirt] [PATCH 2/2] qemu: use virXMLNodeSanitizeNamespaces()

[Peter Krempa]

On Fri, Jun 24, 2016 at 11:33:51 -0400, Laine Stump wrote:
> Replace the virDomainDef-specific virDomainDefMetadataSanitize() with
> virXMLNodeSanitizeNamespaces().
> ---
>  src/conf/domain_conf.c | 56 
> ++
>  1 file changed, 2 insertions(+), 54 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 75ad03f..efc95f7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]

> @@ -4504,7 +4451,8 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
>  }
>  
>  /* clean up possibly duplicated metadata entries */
> -virDomainDefMetadataSanitize(def);
> +if (def->metadata)

This condition is integrated into the funciton so you can call it
directly

ACK if you squash this to the previous patch. Code movement patches are
generally okay together.

Peter

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