Re: [libvirt] [Dnsmasq-discuss] Chaining instances?

2013-02-25 Thread Laine Stump
On 02/25/2013 02:05 AM, Laine Stump wrote:
 This discussion should really be taking place on libvir-list - I'm
 Cc'ing it there.

Bah. I'm not sure what happened, but I ended up not actually Cc'ing...




 On 02/24/2013 04:11 PM, TJ wrote:
 On 24/02/13 19:19, Laine Stump wrote:
 On 02/24/2013 05:09 AM, TJ wrote:
 I wondered if maybe configuring the libvirtd dnsmasq instances to be dhcp 
 proxies for the LAN dnsmasq, and use multiple dhcp-range's with tags might 
 do it?

 My brain is a bit fried right now having had to fix several bugs in 
 vmbuilder and libvirt, plus add new functionality to libvirtd to allow its 
 dnsmasq instance to read a conf-file and use a separate log-facility, 
 What you're talking about doing sounds *very* useful to have supported
 directly in libvirt, but you may want to contact libvirt's upstream
 developers (at libvir-list@redhat.com, or on irc.oftc.net in #virt)
 prior to expending a lot of effort on libvirt code - in general a
 problem may already be solved in a different manner, or somebody else
 may already be working on it. Failing that, there may have already been
 considerable debate on a particular subject, and a path to a solution
 generally agreed on, but with nobody yet working on the code.
 It turns out that what I need(ed) to do was completely *disable* libvirt's 
 use of dnsmasq
 and instead use Simon's related dhcp-helper program which acts as a full 
 dhcp-relay using, f.e:

 /usr/sbin/dhcp-helper -u libvirt-dnsmasq -i virbr1 -b eth0

 This forwards all requests to the primary dnsmasq DHCP server on the LAN 
 which matches against the
 appropriate dhcp-range:

 # physical network leases
 dhcp-range=set:phys,10.254.251.50,10.254.251.199,255.255.255.0,1440m
 # subnet for VMs on server1
 dhcp-range=set:vmlan1,10.254.1.100,10.254.1.199,255.255.255.0,1440m
 # default route (gateway)
 dhcp-option=tag:phys,option:router,10.254.251.1
 dhcp-option=tag:vmlan1,option:router,10.254.1.1
 # DNS server
 dhcp-option=6,10.254.251.1


 To that end this evening I added two additional options to libvirt:

 dnsmasq enabled='true'/
 dhcphelper enabled='false'/
 libvirt's XML should remain implementation-agnostic. We don't want to
 have an option to disable the specific implementation of dns+dhcp called
 dnsmasq; rather we want to be able to enable/disable a network's dhcp
 server and/or dns server. (The reason for this is that we want to leave
 the door open for someone to implement a different backend using a
 different dhcp server and/or dns server and be able to use the same
 configuration.)

 We have actually previously discussed disabling dns and/or dhcp (see
 http://www.redhat.com/archives/libvir-list/2012-November/msg00861.html).
 It is already possible to completely disable dhcp - simply don't include
 a dhcp element in the network definition. As for dns, from the very
 beginning dns has been *always* enabled on all networks, so even though
 there is a dns element, simply having not having one is not a valid
 way to say no dns server (as it would cause backward compatibility
 problems with existing installations).

 Instead, the conclusion of the above-mentioned thread was that the
 proper way to handle this would be to add an enable element to the
 existing dns element, which would default to yes. To disable
 libvirt-supplied dns services for a subnet, you would just put:

 dns enable='no'/

 in the network definition. In the case that dns had enable='no' AND
 there was no dhcp element, dnsmasq simply wouldn't be started at all.
 That hasn't been implemented yet, but shouldn't be too complex to do.

 As for dhcp-helper, aside from the fact that again you've created an
 option that is specific to a particular implementation of what is
 needed, that command isn't universally available anyway (it's not in any
 package for Fedora/RHEL, for example), and I'm not sure that it's really
 necessary to have it started up by libvirt anyway - can it detect newly
 created/upped interfaces as dnsmasq can? If so, it could be started up
 by regular host system config even before libvirt was started.


 These are the default values when the options are *not* defined. They allow 
 the admin to disable dnsmasq entirely:

 dnsmasq enabled='false'/

 and to enable dhcp-helper:

 dhcphelper enabled='true'/

 Using two new functions:

 int networkBuildDhcphelperArgv(...)
 int networkBuildDhcpHelperCommandLine(...)

 the existing function:

 int networkStartDhcpDaemon(...)

 is able to launch either or both of dnsmasq and dhcp-helper with the correct 
 options.

 dhcp-helper's -i option is filled from network-def-bridge and its -b 
 option is taken from
 the first forward device declared in a forward dev='?'/ or interface 
 dev='?'/. If no forward
 device is declared it throws a VIR_ERR_INVALID_INTERFACE error with 
 appropriate explanatory text.


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


Re: [libvirt] [PATCHv2 2/8] doc: schema: Add basic documentation for the virtual RNG device support

2013-02-25 Thread Peter Krempa

On 02/22/13 19:20, Eric Blake wrote:

On 02/21/2013 07:47 AM, Peter Krempa wrote:

This patch documents XML elements used for (basic) support of virtual
RNG devices.

In the devices section in the domain XML users may specify:

For the default 'random' backend:
   devices
 rng model='virtio'
   backend model='random'/dev/urandom/backend
 /rng
   /devices

For the slightly more advanced EGD backend:
   devices
 rng model='virtio'
   backend model='egd' type='udp'
 !-- this is a definition of a character device --
 source mode='bind' service='1234'/
 source mode='connect' host='1.2.3.4' service='1234'/
 !-- or other valid character device configuration --


You don't really allow two source; maybe a better layout would be a
strategic comment, such as:

   backend model='egd' type='udp'
 !-- this is a definition of a character device --
 source mode='bind' service='1234'/
 !-- or other valid character device configuration, such as
 source mode='connect' host='1.2.3.4' service='1234'/
 --


   /backend
 /rng
   /devices

For the planned random daemon/pool:
   devices
 rng model='virtio'
   backend model='pool' pool='poolname'class/backend


Missing /rng


   /devices

to enable the RNG device for guests.
---

Notes:
 Version 2:
 - ACKed, no change, unfortunately doesn't make sense to push alone


Still some nits to fix before pushing:



+pre
+  ...
+  lt;devicesgt;
+lt;rng model='virtio'gt;
+  lt;backend model='random'gt;/dev/randomlt;/backendgt;
+  lt;!-- OR --gt;
+  lt;backend model='egd' type='udp'gt;
+lt;source mode='bind' service='1234'gt;
+lt;source mode='connect' host='1.2.3.4' service='1234'gt;
+  lt;/backendgt;
+lt;/rnggt;
+  lt;/devicesgt;
+  ...


Do we really want two source in a single backend in the example, or
would it be easier to show multiple rng devices, one for each type of
backend?


That actually is valid for the character device backends. The UDP 
backend has to use two separate sources for bi-directional 
communication. The definition of that source type is declared as a type 
in our RNG schema an I merely reused that.





+  dd
+p
+  The codebackend/code element specifies the source of entropy
+  to be used for the doimain. The source model is configured using the


s/doimain/domain/


+  codemodel/code attribute. Supported source models are:
+/p
+ul
+  li'random' mdash; /dev/random (default) or similar device as 
source/li
+  li'egd' mdash; a EGD protocol backend. /li


not consistent on whether your li end with '.'



Peter

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


Re: [libvirt] [PATCHv2 3/8] conf: Add support for RNG device configuration in XML

2013-02-25 Thread Peter Krempa

On 02/23/13 00:33, Eric Blake wrote:

On 02/21/2013 07:47 AM, Peter Krempa wrote:

This patch adds basic configuration support for the RNG device suporting


s/suporting/supporting/


the virtio model with the random and egd backend types as described
in the schema in the previous patch.
---

Notes:
 Version 2:
 - fix a ton of memory leaks (I assumed that virXMLGetProp returns static 
strings)
 - Add new device type to even more places
 - Fix error message cp error
 - Fix memleak in RNGDef free func



...

@@ -10601,6 +10734,22 @@ virDomainDefParseXML(virCapsPtr caps,
  }
  }

+/* Parse the RNG device */
+if ((n = virXPathNodeSet(./devices/rng, ctxt, nodes))  0)
+goto error;
+
+if (n  1) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(only a single RNG device is supported));


Is this an inherent limit of qemu?  For that matter, is it an inherent
limit, and no hypervisor can ever support more than one?  In the bare
metal case, can't you plug in multiple rng hardware dongles?


AFAIK qemu is able to support multiple RNG devices but I didn't find 
that particularly useful. I will follow up with a patch that will add 
support for more than one RNG device, although I don't think it will be 
used much.


Peter

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


Re: [libvirt] [PATCH 0/2] network: add force attribute for dhcp options

2013-02-25 Thread Michal Privoznik
On 25.02.2013 05:37, Laine Stump wrote:
 The firs patch just fixes unexpected behavior in
 virNetworkDHCPOptionDefParseXML found when testing the second patch.
 
 The second patch adds the force attribute I mentioned in the review
 of the patch that added support for specifying dhcp options in network
 config. While documenting this new attribute, I noticed that I'd
 neglected to see that original patch didn't include documentation, so
 I added full documentation for option in order to have a place to
 add the documentation for force='yes'.
 
 I'm also working on a way to allow specification of option names
 rather than numbers, but didn't know if I could get it in before the
 freeze, so I'm sending this patch now.
 
 Laine Stump (2):
   conf: use VIR_EXPAND instead of VIR_REALLOC in
 virNetworkDHCPDefParseXML
   network: add force attribute for dhcp options
 
  docs/formatnetwork.html.in | 21 
  src/conf/network_conf.c| 32 
 +-
  src/conf/network_conf.h|  3 ++-
  src/network/bridge_driver.c|  3 ++-
  tests/networkxml2confdata/nat-network.conf |  2 +-
  tests/networkxml2confdata/nat-network.xml  |  2 +-
  6 files changed, 50 insertions(+), 13 deletions(-)
 

ACK to both patches.

Michal

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


Re: [libvirt] [Dnsmasq-discuss] Chaining instances?

2013-02-25 Thread Laine Stump
This discussion should really be taking place on libvir-list - I'm
Cc'ing it there.


On 02/24/2013 04:11 PM, TJ wrote:
 On 24/02/13 19:19, Laine Stump wrote:
 On 02/24/2013 05:09 AM, TJ wrote:
 I wondered if maybe configuring the libvirtd dnsmasq instances to be dhcp 
 proxies for the LAN dnsmasq, and use multiple dhcp-range's with tags might 
 do it?

 My brain is a bit fried right now having had to fix several bugs in 
 vmbuilder and libvirt, plus add new functionality to libvirtd to allow its 
 dnsmasq instance to read a conf-file and use a separate log-facility, 
 What you're talking about doing sounds *very* useful to have supported
 directly in libvirt, but you may want to contact libvirt's upstream
 developers (at libvir-list@redhat.com, or on irc.oftc.net in #virt)
 prior to expending a lot of effort on libvirt code - in general a
 problem may already be solved in a different manner, or somebody else
 may already be working on it. Failing that, there may have already been
 considerable debate on a particular subject, and a path to a solution
 generally agreed on, but with nobody yet working on the code.
 It turns out that what I need(ed) to do was completely *disable* libvirt's 
 use of dnsmasq
 and instead use Simon's related dhcp-helper program which acts as a full 
 dhcp-relay using, f.e:

 /usr/sbin/dhcp-helper -u libvirt-dnsmasq -i virbr1 -b eth0

 This forwards all requests to the primary dnsmasq DHCP server on the LAN 
 which matches against the
 appropriate dhcp-range:

 # physical network leases
 dhcp-range=set:phys,10.254.251.50,10.254.251.199,255.255.255.0,1440m
 # subnet for VMs on server1
 dhcp-range=set:vmlan1,10.254.1.100,10.254.1.199,255.255.255.0,1440m
 # default route (gateway)
 dhcp-option=tag:phys,option:router,10.254.251.1
 dhcp-option=tag:vmlan1,option:router,10.254.1.1
 # DNS server
 dhcp-option=6,10.254.251.1


 To that end this evening I added two additional options to libvirt:

 dnsmasq enabled='true'/
 dhcphelper enabled='false'/

libvirt's XML should remain implementation-agnostic. We don't want to
have an option to disable the specific implementation of dns+dhcp called
dnsmasq; rather we want to be able to enable/disable a network's dhcp
server and/or dns server. (The reason for this is that we want to leave
the door open for someone to implement a different backend using a
different dhcp server and/or dns server and be able to use the same
configuration.)

We have actually previously discussed disabling dns and/or dhcp (see
http://www.redhat.com/archives/libvir-list/2012-November/msg00861.html).
It is already possible to completely disable dhcp - simply don't include
a dhcp element in the network definition. As for dns, from the very
beginning dns has been *always* enabled on all networks, so even though
there is a dns element, simply having not having one is not a valid
way to say no dns server (as it would cause backward compatibility
problems with existing installations).

Instead, the conclusion of the above-mentioned thread was that the
proper way to handle this would be to add an enable element to the
existing dns element, which would default to yes. To disable
libvirt-supplied dns services for a subnet, you would just put:

dns enable='no'/

in the network definition. In the case that dns had enable='no' AND
there was no dhcp element, dnsmasq simply wouldn't be started at all.
That hasn't been implemented yet, but shouldn't be too complex to do.

As for dhcp-helper, aside from the fact that again you've created an
option that is specific to a particular implementation of what is
needed, that command isn't universally available anyway (it's not in any
package for Fedora/RHEL, for example), and I'm not sure that it's really
necessary to have it started up by libvirt anyway - can it detect newly
created/upped interfaces as dnsmasq can? If so, it could be started up
by regular host system config even before libvirt was started.



 These are the default values when the options are *not* defined. They allow 
 the admin to disable dnsmasq entirely:

 dnsmasq enabled='false'/

 and to enable dhcp-helper:

 dhcphelper enabled='true'/

 Using two new functions:

 int networkBuildDhcphelperArgv(...)
 int networkBuildDhcpHelperCommandLine(...)

 the existing function:

 int networkStartDhcpDaemon(...)

 is able to launch either or both of dnsmasq and dhcp-helper with the correct 
 options.

 dhcp-helper's -i option is filled from network-def-bridge and its -b 
 option is taken from
 the first forward device declared in a forward dev='?'/ or interface 
 dev='?'/. If no forward
 device is declared it throws a VIR_ERR_INVALID_INTERFACE error with 
 appropriate explanatory text.




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


Re: [libvirt] [PATCH 3/5] tests: avoid segfault if json monitor not present

2013-02-25 Thread Michal Privoznik
On 23.02.2013 00:09, Eric Blake wrote:
 On a machine without json headers, I was seeing random segfaults
 from qemumonitorjsontest (about 90% of the runs on my particular
 machine).  The segfault was inside virClassIsDerivedFrom, which
 points to a case of a race leading to unreferencing a stale
 pointer to an object that had already been freed.  I also noticed
 that if I got the segfault, I was seeing messages such as:
 
 2013-02-22 16:12:37.504+: 19833: error : virNetSocketWriteWire:1361 : 
 Cannot write data: Bad file descriptor
 
 which is also evidence of deferencing a stale pointer.  I traced it
 to a race where qemuMonitorTestIO could execute late, after the
 main thread had already called qemuMonitorTestFree and called
 virNetSocketClose(test-client) but not clearing it out to NULL.
 Sure enough, after test-client has been closed, fd is -1, which
 causes an attempt to write to the socket to fail, which in turn
 triggers the error code of qemuMonitorTestIO that tries to re-close
 test-client.
 
 * tests/qemumonitortestutils.c (qemuMonitorTestIO): Don't attempt
 to free client again if test already quit.
 ---
  tests/qemumonitortestutils.c | 4 
  1 file changed, 4 insertions(+)
 
 diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
 index 1ed42ce..979623a 100644
 --- a/tests/qemumonitortestutils.c
 +++ b/tests/qemumonitortestutils.c
 @@ -214,6 +214,10 @@ static void qemuMonitorTestIO(virNetSocketPtr sock,
  bool err = false;
 
  virMutexLock(test-lock);
 +if (test-quit) {
 +virMutexUnlock(test-lock);
 +return;
 +}
  if (events  VIR_EVENT_HANDLE_WRITABLE) {
  ssize_t ret;
  if ((ret = virNetSocketWrite(sock,
 

ACK

Michal

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


Re: [libvirt] [PATCH 2/5] qemu: minor monitor lock cleanups

2013-02-25 Thread Michal Privoznik
On 23.02.2013 00:09, Eric Blake wrote:
 If virCondInit fails (okay, so that's unlikely), then we end up
 attempting a virObjectUnlock() on the cleanup path, even though
 we don't hold a lock.  This is not guaranteed to be safe.  While
 at it, I noticed a couple places where we were referencing mon-fd
 outside locks.
 
 * src/qemu/qemu_monitor.c (qemuMonitorOpenInternal): Minimize lock
 duration.  mon-watch doesn't need clean up on error.
 (qemuMonitorGetBlockExtent, qemuMonitorBlockResize): Don't
 dereference fd outside of lock.
 ---
  src/qemu/qemu_monitor.c | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)
 
 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 7f4a7a0..40eea75 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -717,7 +717,6 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
  if (json)
  mon-wait_greeting = 1;
  mon-cb = cb;
 -virObjectLock(mon);
 
  if (virSetCloseExec(mon-fd)  0) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 @@ -731,6 +730,7 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
  }
 
 
 +virObjectLock(mon);
  virObjectRef(mon);
  if ((mon-watch = virEventAddHandle(mon-fd,
  VIR_EVENT_HANDLE_HANGUP |
 @@ -740,6 +740,7 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
  mon,
  virObjectFreeCallback))  0) {
  virObjectUnref(mon);
 +virObjectUnlock(mon);

Heh, it took me while to realize this is good actually. I thought it
should be vice versa to prevent Unlock() being called on just freed
memory. But then I realized after the Unref() mon refcount == 1.

  virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 _(unable to register monitor events));
  goto cleanup;
 @@ -759,11 +760,8 @@ cleanup:
   * so kill the callbacks now.
   */
  mon-cb = NULL;
 -virObjectUnlock(mon);
  /* The caller owns 'fd' on failure */
  mon-fd = -1;
 -if (mon-watch)
 -virEventRemoveHandle(mon-watch);
  qemuMonitorClose(mon);
  return NULL;
  }
 @@ -1508,8 +1506,7 @@ int qemuMonitorGetBlockExtent(qemuMonitorPtr mon,
unsigned long long *extent)
  {
  int ret;
 -VIR_DEBUG(mon=%p, fd=%d, dev_name=%p,
 -  mon, mon-fd, dev_name);
 +VIR_DEBUG(mon=%p, dev_name=%p, mon, dev_name);
 
  if (mon-json)
  ret = qemuMonitorJSONGetBlockExtent(mon, dev_name, extent);
 @@ -1524,8 +1521,7 @@ int qemuMonitorBlockResize(qemuMonitorPtr mon,
 unsigned long long size)
  {
  int ret;
 -VIR_DEBUG(mon=%p, fd=%d, devname=%p size=%llu,
 -  mon, mon-fd, device, size);
 +VIR_DEBUG(mon=%p, devname=%p size=%llu, mon, device, size);
 
  if (mon-json)
  ret = qemuMonitorJSONBlockResize(mon, device, size);
 

ACK

Michal

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


Re: [libvirt] [PATCH 1/5] qemu: don't override earlier json error

2013-02-25 Thread Michal Privoznik
On 23.02.2013 00:09, Eric Blake wrote:
 I built without json support, and noticed a strange failure message
 in qemumonitorjsontest:
 
 2013-02-22 16:12:37.503+: 19812: error : virJSONValueToString:1119 : 
 internal error No JSON parser implementation is available
 2013-02-22 16:12:37.503+: 19812: error : qemuMonitorJSONCommandWithFd:253 
 : out of memory
 
 While a later patch will fix the test to skip when json is not present,
 this patch avoids overriding the more useful error message from
 virJSONValueToString returning NULL.
 
 * src/qemu/qemu_monitor_json.c (qemuMonitorJSONCommandWithFd):
 Don't override message.
 (qemuMonitorJSONCheckError): Don't print NULL.
 * src/qemu/qemu_agent.c (qemuAgentCommand): Don't override message.
 (qemuAgentCheckError): Don't print NULL.
 (qemuAgentArbitraryCommand): Properly fail on OOM.
 ---
  src/qemu/qemu_agent.c| 13 ++---
  src/qemu/qemu_monitor_json.c |  8 +++-
  2 files changed, 9 insertions(+), 12 deletions(-)

ACK

Michal

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


Re: [libvirt] [PATCH 4/5] tests: don't test json when not compiled in

2013-02-25 Thread Michal Privoznik
On 23.02.2013 00:09, Eric Blake wrote:
 Now that the segfault is solved, we can skip instead of fail
 the test when yajl is not present.
 
 * tests/qemumonitorjsontest.c (mymain): Skip if no yajl.
 ---
  tests/qemumonitorjsontest.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
 index 55032d6..e2f8cb1 100644
 --- a/tests/qemumonitorjsontest.c
 +++ b/tests/qemumonitorjsontest.c
 @@ -1,5 +1,5 @@
  /*
 - * Copyright (C) 2011-2012 Red Hat, Inc.
 + * Copyright (C) 2011-2013 Red Hat, Inc.
   *
   * This library is free software; you can redistribute it and/or
   * modify it under the terms of the GNU Lesser General Public
 @@ -444,2 +443,2 @@ mymain(void)
  int ret = 0;
  virCapsPtr caps;
 
 +#if !WITH_YAJL
 +fprintf(stderr, libvirt not compiled with yajl, skipping);
 +return EXIT_AM_SKIP;
 +#endif
 +
  if (virThreadInitialize()  0)
  exit(EXIT_FAILURE);
 

ACK

Michal

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


Re: [libvirt] [PATCH 5/5] tests: uniformly report test failures

2013-02-25 Thread Michal Privoznik
On 23.02.2013 00:09, Eric Blake wrote:
 testutils.c likes to print summaries after a test completes,
 including if it failed.  But if the test outright exit()s,
 this summary is skipped.  Enforce that we return instead of exit.
 
 * cfg.mk (sc_prohibit_exit_tests): New syntax check.
 * tests/commandhelper.c (main): Fix offenders.
 * tests/qemumonitorjsontest.c (mymain): Likewise.
 * tests/seclabeltest.c (main): Likewise.
 * tests/securityselinuxlabeltest.c (mymain): Likewise.
 * tests/securityselinuxtest.c (mymain): Likewise.
 * tests/testutils.h (VIRT_TEST_MAIN_PRELOAD): Likewise.
 * tests/testutils.c (virtTestMain): Likewise.
 (virtTestCaptureProgramOutput): Use symbolic name.
 ---
  cfg.mk   | 7 +++
  tests/commandhelper.c| 7 +++
  tests/qemumonitorjsontest.c  | 8 +++-
  tests/seclabeltest.c | 8 
  tests/securityselinuxlabeltest.c | 6 +++---
  tests/securityselinuxtest.c  | 6 +++---
  tests/testutils.c| 8 
  tests/testutils.h| 4 ++--
  8 files changed, 29 insertions(+), 25 deletions(-)

ACK

Michal

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


Re: [libvirt] [PATCHv2 7/8] tests: Add tests for virtio-rng device handling

2013-02-25 Thread Peter Krempa

On 02/23/13 01:29, Eric Blake wrote:

On 02/21/2013 07:47 AM, Peter Krempa wrote:

Adds XML parsing and qemu commandline tests for the VirtIO RNG device
support.
---

Notes:
 Version 2:
 - ACKed

  .../qemuxml2argv-virtio-rng-egd.args   |  1 +
  .../qemuxml2argv-virtio-rng-egd.xml| 26 ++
  .../qemuxml2argv-virtio-rng-random.args|  1 +
  .../qemuxml2argv-virtio-rng-random.xml | 23 +++
  tests/qemuxml2argvtest.c   |  5 +
  tests/qemuxml2xmltest.c|  3 +++
  6 files changed, 59 insertions(+)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-egd.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-egd.xml
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml


Is it worth testing that a filename containing an XML-special character
is properly escaped?  Other than that, this one is still good to go.


+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
@@ -0,0 +1,23 @@



+rng model='virtio'
+  backend model='random'/test/phile/backend


That is, should this use something like /test/lt;phile as the XML
encoded file name?



Uh, I'm not following you on this one. You mean that if the user 
specifies some characters that are invalid from the perspective of XML 
as a source path?


Anyways, I fixed the issues you pointed out in 1-6 and provided 
explanation for the other stuff. I'm pushing patches 1-7 (the test suite 
can be improved at any time) now and will follow up later with a 
improved version of 8 as well as with a patch that will allow multiple 
RNG devices. That should be better to review as slicing apart the 
existing patches.


Peter

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


[libvirt] [PATCH v2 resend 1/4] add domain NIC model enum macro

2013-02-25 Thread Guannan Ren
---
 src/conf/domain_conf.c | 31 +++
 src/conf/domain_conf.h | 23 +++
 2 files changed, 54 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0c75838..90a6359 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -700,6 +700,37 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode,
   static,
   auto);
 
+/* For NIC model macro, a comment marks the start of a model
+ * group which ends with the model just before next comment
+ * or extends to the end of list.
+ */
+VIR_ENUM_IMPL(virDomainNICModel,
+  VIR_DOMAIN_NIC_MODEL_LAST,
+  default,
+  spapr-vlan, /* qemu */
+
+  virtio, /* qemu and vbox */
+
+  ne2k_isa,   /* qemu and Xen */
+  ne2k_pci,
+  pcnet,
+  rtl8139,
+
+  e1000,  /* qemu, Xen and VMX */
+
+  netfront,   /* Xen(hvm) and libxl */
+
+  vlance, /* VMX */
+  vmxnet,
+  vmxnet2,
+  vmxnet3,
+
+  Am79C970A,  /* vbox */
+  Am79C973,
+  82540EM,
+  82545EM,
+  82543GC);
+
 #define VIR_DOMAIN_XML_WRITE_FLAGS  VIR_DOMAIN_XML_SECURE
 #define VIR_DOMAIN_XML_READ_FLAGS   VIR_DOMAIN_XML_INACTIVE
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4ffa4aa..0a06e11 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -782,6 +782,28 @@ struct _virDomainFSDef {
 unsigned long long space_soft_limit; /* in bytes */
 };
 
+enum virDomainNICModel {
+VIR_DOMAIN_NIC_MODEL_DEFAULT = 0,
+VIR_DOMAIN_NIC_MODEL_SPAPR_VLAN,
+VIR_DOMAIN_NIC_MODEL_VIRTIO,
+VIR_DOMAIN_NIC_MODEL_NE2K_ISA,
+VIR_DOMAIN_NIC_MODEL_NE2K_PCI,
+VIR_DOMAIN_NIC_MODEL_PCNET,
+VIR_DOMAIN_NIC_MODEL_RTL8139,
+VIR_DOMAIN_NIC_MODEL_E1000,
+VIR_DOMAIN_NIC_MODEL_NETFRONT,
+VIR_DOMAIN_NIC_MODEL_VLANCE,
+VIR_DOMAIN_NIC_MODEL_VMXNET,
+VIR_DOMAIN_NIC_MODEL_VMXNET2,
+VIR_DOMAIN_NIC_MODEL_VMXNET3,
+VIR_DOMAIN_NIC_MODEL_AM79C970A,
+VIR_DOMAIN_NIC_MODEL_AM79C973,
+VIR_DOMAIN_NIC_MODEL_82540EM,
+VIR_DOMAIN_NIC_MODEL_82545EM,
+VIR_DOMAIN_NIC_MODEL_82543GC,
+
+VIR_DOMAIN_NIC_MODEL_LAST
+};
 
 /* 5 different types of networking config */
 enum virDomainNetType {
@@ -2324,6 +2346,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceClipboardCopypaste)
 VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode)
 VIR_ENUM_DECL(virDomainNumatuneMemMode)
 VIR_ENUM_DECL(virDomainNumatuneMemPlacementMode)
+VIR_ENUM_DECL(virDomainNICModel)
 VIR_ENUM_DECL(virDomainHyperv)
 /* from libvirt.h */
 VIR_ENUM_DECL(virDomainState)
-- 
1.7.11.2

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


[libvirt] [PATCH v2 resend 0/4]use VIR_ENUM_DECL for domain NIC model and add usb-net

2013-02-25 Thread Guannan Ren

v1 to v2
   *removed i8* NIC models support for QEMU/KVM
   *updated docs
   *rebased

This patchset uses VIR_ENUM_DECL and VIR_ENUM_IMPL macros for NIC models
and changes the related codes to use them.
All of NIC models supported by various hypervisors comprise of the enum,
so the problem is that we need to do further checking in hypervisor-specific
implementation. Fortunately, VMX and Vbox did this checking very well already.
In 4/4, a similar job is performed for QEMU/KVM.

And, adds usb-net support.
The vendorId and productID of is 0525:a4a2. The corresponding item in
usb-ids is as follows:

0525  Netchip Technology, Inc.
a4a2  Linux-USB Ethernet/RNDIS Gadget

In my opinion, using usb-net directly is good enough, so I keep it.
Any other idea is welcome.

Libvirt XML sample:
  devices
interface type='user'
  mac address='52:54:00:32:6a:91'/
  model type='usb-net'/
  alias name='net1'/
  address type='usb' bus='0' port='1'/
/interface
  /devices

qemu commandline:

qemu ${other_vm_args}
-netdev user,id=hostnet1 \
-device usb-net,netdev=hostnet1,id=net1,\
  mac=52:54:00:32:6a:91,bus=usb.0,port=1

Guannan Ren(4)
 [PATCH v2 resend 1/4] add domain NIC model enum macro
 [PATCH v2 resend 2/4] net: use virDomainNICModelType{From|To}String functions
 [PATCH v2 resend 3/4] qemu: add NIC model checking for qemu hypervisor
 [PATCH v2 resend 4/4] qemu: add usb-net support

 docs/formatdomain.html.in| 29 +++--
 src/conf/domain_conf.c   | 78 
++
 src/conf/domain_conf.h   | 32 +++-
 src/libvirt_private.syms |  2 ++
 src/libxl/libxl_conf.c   |  5 +++--
 src/parallels/parallels_driver.c |  2 +-
 src/qemu/qemu_command.c  | 41 +++--
 src/qemu/qemu_driver.c   |  2 +-
 src/qemu/qemu_hotplug.c  | 10 ++
 src/qemu/qemu_process.c  | 14 +++---
 src/vbox/vbox_tmpl.c | 57 
+++--
 src/vmx/vmx.c| 31 ---
 src/xenxs/xen_sxpr.c | 21 +++--
 src/xenxs/xen_xm.c   | 21 +++--
 14 files changed, 208 insertions(+), 137 deletions(-)

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


[libvirt] [PATCH v2 resend 3/4] qemu: add NIC model checking for qemu hypervisor

2013-02-25 Thread Guannan Ren
As the enum virDomainNICModel is a big collection of NIC models
for all of hypervisors, for qemu/kvm, only some of them are
supported, so the patch tries to add a checking for NIC model that
is qemu specific.

The way of doing this is the same as VMX and Vbox do which have
codes to validate the hypervisor-specific NIC model in their
implementation rather than domain XML parsing code.
---
 src/conf/domain_conf.h  | 6 ++
 src/qemu/qemu_command.c | 7 +++
 2 files changed, 13 insertions(+)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6425290..687032b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -791,6 +791,12 @@ enum virDomainNICModel {
 VIR_DOMAIN_NIC_MODEL_PCNET,
 VIR_DOMAIN_NIC_MODEL_RTL8139,
 VIR_DOMAIN_NIC_MODEL_E1000,
+
+/* Add new NIC model for qemu above this.
+ */
+VIR_DOMAIN_NIC_MODEL_FOR_QEMU_END =
+VIR_DOMAIN_NIC_MODEL_E1000,
+
 VIR_DOMAIN_NIC_MODEL_NETFRONT,
 VIR_DOMAIN_NIC_MODEL_VLANCE,
 VIR_DOMAIN_NIC_MODEL_VMXNET,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5cb013c..3a98566 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6032,6 +6032,13 @@ qemuBuildCommandLine(virConnectPtr conn,
 else
 vlan = i;
 
+if (net-model  0 || net-model  
VIR_DOMAIN_NIC_MODEL_FOR_QEMU_END) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(NIC model '%s' is unsupported for QEMU),
+   virDomainNICModelTypeToString(net-model));
+goto error;
+}
+
 /* If appropriate, grab a physical device from the configured
  * network's pool of devices, or resolve bridge device name
  * to the one defined in the network definition.
-- 
1.7.11.2

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


[libvirt] [PATCH v2 resend 2/4] net: use virDomainNICModelType{From|To}String functions

2013-02-25 Thread Guannan Ren
---
 src/conf/domain_conf.c   | 32 --
 src/conf/domain_conf.h   |  2 +-
 src/libvirt_private.syms |  2 ++
 src/libxl/libxl_conf.c   |  5 ++--
 src/parallels/parallels_driver.c |  2 +-
 src/qemu/qemu_command.c  | 26 +-
 src/qemu/qemu_driver.c   |  2 +-
 src/qemu/qemu_hotplug.c  | 10 ---
 src/qemu/qemu_process.c  | 14 +-
 src/vbox/vbox_tmpl.c | 57 +++-
 src/vmx/vmx.c| 31 +++---
 src/xenxs/xen_sxpr.c | 21 ---
 src/xenxs/xen_xm.c   | 21 ---
 13 files changed, 110 insertions(+), 115 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 90a6359..96203e2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1150,8 +1150,6 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
 if (!def)
 return;
 
-VIR_FREE(def-model);
-
 switch (def-type) {
 case VIR_DOMAIN_NET_TYPE_ETHERNET:
 VIR_FREE(def-data.ethernet.dev);
@@ -5220,9 +5218,6 @@ error:
 return ret;
 }
 
-#define NET_MODEL_CHARS \
-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_-
-
 /* Parse the XML definition for a network interface
  * @param node XML nodeset to parse for net definition
  * @return 0 on success, -1 on failure
@@ -5592,23 +5587,17 @@ virDomainNetDefParseXML(virCapsPtr caps,
 ifname = NULL;
 }
 
-/* NIC model (see -net nic,model=?).  We only check that it looks
- * reasonable, not that it is a supported NIC type.  FWIW kvm
- * supports these types as of April 2008:
- * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio
- * QEMU PPC64 supports spapr-vlan
- */
 if (model != NULL) {
-if (strspn(model, NET_MODEL_CHARS)  strlen(model)) {
-virReportError(VIR_ERR_INVALID_ARG, %s,
-   _(Model name contains invalid characters));
+int m;
+if ((m = virDomainNICModelTypeFromString(model))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Unknown NIC model has been specified));
 goto error;
 }
-def-model = model;
-model = NULL;
+def-model = m;
 }
 
-if (def-model  STREQ(def-model, virtio)) {
+if (def-model == VIR_DOMAIN_NIC_MODEL_VIRTIO) {
 if (backend != NULL) {
 int name;
 if ((name = virDomainNetBackendTypeFromString(backend))  0 ||
@@ -11254,10 +11243,11 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr 
src,
 return false;
 }
 
-if (STRNEQ_NULLABLE(src-model, dst-model)) {
+if (src-model != dst-model) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Target network card model %s does not match source 
%s),
-   NULLSTR(dst-model), NULLSTR(src-model));
+   virDomainNICModelTypeToString(dst-model),
+   virDomainNICModelTypeToString(src-model));
 return false;
 }
 
@@ -13175,8 +13165,8 @@ virDomainNetDefFormat(virBufferPtr buf,
 }
 if (def-model) {
 virBufferEscapeString(buf, model type='%s'/\n,
-  def-model);
-if (STREQ(def-model, virtio) 
+  virDomainNICModelTypeToString(def-model));
+if ((def-model == VIR_DOMAIN_NIC_MODEL_VIRTIO) 
 (def-driver.virtio.name || def-driver.virtio.txmode)) {
 virBufferAddLit(buf, driver);
 if (def-driver.virtio.name) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0a06e11..6425290 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -879,7 +879,7 @@ struct _virDomainActualNetDef {
 struct _virDomainNetDef {
 enum virDomainNetType type;
 virMacAddr mac;
-char *model;
+int model;
 union {
 struct {
 enum virDomainNetBackendType name; /* which driver backend to use 
*/
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f399871..499add9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -250,6 +250,8 @@ virDomainNetGetActualVlan;
 virDomainNetInsert;
 virDomainNetRemove;
 virDomainNetTypeToString;
+virDomainNICModelTypeFromString;
+virDomainNICModelTypeToString;
 virDomainNostateReasonTypeFromString;
 virDomainNostateReasonTypeToString;
 virDomainNumatuneMemModeTypeFromString;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 4ce5dec..c963834 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -587,8 +587,9 @@ libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic 
*x_nic)
 
 virMacAddrGetRaw(l_nic-mac, x_nic-mac);
 
-if (l_nic-model  !STREQ(l_nic-model, netfront)) {
-if ((x_nic-model = strdup(l_nic-model)) == NULL) {
+if (l_nic-model  

[libvirt] [PATCH v2 resend 4/4] qemu: add usb-net support

2013-02-25 Thread Guannan Ren
For USB device, the address is optional, so is usb-net.
Currently, we couldn't assign default usb address to usb
devices, so we definitly ignore it during the assignment of PCI
address.

Libvirt XML sample:
  devices
interface type='user'
  mac address='52:54:00:32:6a:91'/
  model type='usb-net'/
  alias name='net1'/
  address type='usb' bus='0' port='1'/
/interface
  /devices

qemu commandline:

qemu ${other_vm_args}
-netdev user,id=hostnet1 \
-device usb-net,netdev=hostnet1,id=net1,\
  mac=52:54:00:32:6a:91,bus=usb.0,port=1
---
 docs/formatdomain.html.in | 29 +++--
 src/conf/domain_conf.c| 15 ---
 src/conf/domain_conf.h|  1 +
 src/qemu/qemu_command.c   |  8 +++-
 4 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a9003d7..eb7b5dc 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2960,24 +2960,17 @@
 
 p
   For hypervisors which support this, you can set the model of
-  emulated network interface card.
-/p
-
-p
-  The values for codetype/code aren't defined specifically by
-  libvirt, but by what the underlying hypervisor supports (if
-  any).  For QEMU and KVM you can get a list of supported models
-  with these commands:
-/p
-
-pre
-qemu -net nic,model=? /dev/null
-qemu-kvm -net nic,model=? /dev/null
-/pre
-
-p
-  Typical values for QEMU and KVM include:
-  ne2k_isa i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio
+  emulated network interface card. For QEMU and KVM, the supported NIC 
models
+  are virtio, ne2k_isa, ne2k_pci, pcnet, rtl8139, e1000, usb-net
+  span class=since(since 1.0.3)/span, spapr-vlan(only PPC64).
+  For Xen, there are ne2k_isa, ne2k_pci, pcnet, rtl8139, e1000, netfront. 
For VMWare,
+  vlance, vmxnet, vmxnet2, vmxnet3 are supported. And for Vbox, Am79C970A, 
Am79C973,
+  82540EM, 82545EM, 82543GC are its supported models.
+  (All of these models are PCI devices, except usb-net which emulates a 
Netchip
+   Linux-USB Ethernet/RNDIS usb ethernet device). For models of PCI type, 
a sub-element
+  codelt;addressgt;/code with codetype='pci'/code can be used to 
tie
+  the device to a particular PCI slot. It is codetype='usb'/code for 
usb-net
+  to tie to USB controller. a href=#elementsAddressdocumented 
above/a.
 /p
 
 h5a name=elementsDriverBackendOptionsSetting NIC driver-specific 
options/a/h5
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 96203e2..78a9a77 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -708,6 +708,7 @@ VIR_ENUM_IMPL(virDomainNICModel,
   VIR_DOMAIN_NIC_MODEL_LAST,
   default,
   spapr-vlan, /* qemu */
+  usb-net,
 
   virtio, /* qemu and vbox */
 
@@ -5427,15 +5428,16 @@ virDomainNetDefParseXML(virCapsPtr caps,
 goto error;
 }
 
-/* XXX what about ISA/USB based NIC models - once we support
+/* XXX what about ISA based NIC models - once we support
  * them we should make sure address type is correct */
 if (def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE 
 def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO 
 def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW 
 def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 
-def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI 
+def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(Network interfaces must use 'pci' address type));
+   _(Network interfaces have incorrect address type));
 goto error;
 }
 
@@ -5642,6 +5644,13 @@ virDomainNetDefParseXML(virCapsPtr caps,
 }
 def-driver.virtio.event_idx = idx;
 }
+} else if (def-model == VIR_DOMAIN_NIC_MODEL_USB_NET) {
+if (def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE 
+def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(Interface of usb-net model requires address of 
usb type));
+goto error;
+}
 }
 
 def-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 687032b..8ef044e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -785,6 +785,7 @@ struct _virDomainFSDef {
 enum virDomainNICModel {
 VIR_DOMAIN_NIC_MODEL_DEFAULT = 0,
 VIR_DOMAIN_NIC_MODEL_SPAPR_VLAN,
+VIR_DOMAIN_NIC_MODEL_USB_NET,
 VIR_DOMAIN_NIC_MODEL_VIRTIO,
 VIR_DOMAIN_NIC_MODEL_NE2K_ISA,
 VIR_DOMAIN_NIC_MODEL_NE2K_PCI,
diff --git a/src/qemu/qemu_command.c 

[libvirt] Entering freeze for libvirt-1.0.3

2013-02-25 Thread Daniel Veillard
  I have just tagged git and pushed the tarball. The rpms for F17
are following too at the usual place:
ftp://libvirt.org/libvirt/

  I gave a try to the set of rpms and this seems to work fine for
relatively simple tests, but of course more testing and checking on
other architectures is needed !
  If everything goes fine, I will make an rc2 in a couple of days
and then we can decide if a final release on Friday is fine or if
we need to postpone to next week,

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


[libvirt] Versioning/Compatibility in the XML schemas

2013-02-25 Thread Pieter Hollants
Out of curiosity, seeing that we doing modifications to the XML schema 
all the time, there does not seem to be some sort of version identifier 
in the XML schemas.


If I take the recently merged patch for DHCP options, for instance, 
suppose virt-manager would add support for it in its GUI. How would such 
a network XML definition be interpreted by older libvirt versions? Is it 
supposed to ignore elements it doesn't know or do we have an implicit 
requirement of always use the latest versions together?


--
Pieter Hollants pie...@hollants.com

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


Re: [libvirt] Versioning/Compatibility in the XML schemas

2013-02-25 Thread Daniel Veillard
On Mon, Feb 25, 2013 at 01:13:37PM +0100, Pieter Hollants wrote:
 Out of curiosity, seeing that we doing modifications to the XML
 schema all the time, there does not seem to be some sort of version
 identifier in the XML schemas.
 
 If I take the recently merged patch for DHCP options, for instance,
 suppose virt-manager would add support for it in its GUI. How would
 such a network XML definition be interpreted by older libvirt
 versions? Is it supposed to ignore elements it doesn't know or do we
 have an implicit requirement of always use the latest versions
 together?

  The base principle is that we are supposed to only do additions
i.e. an XML which was RNG valid for a version A should still be valid
for the RHN shipped with version B , B = A. And an older version of
the code usually still work with a newer XML instance.

  There had been some small breakage IIRC but usually thise rule works.
In general the way it works is that we don't validate XML input, but we
convert it to an internal data structure by picking the information we
know about. So the parser from A may extract only the subset it knows
about. The danger is that it will accept XML definition without fully
understanding the semantic of all elements, instead of filing to load
it. It is usually acceptable, the problem is for example when the
discarded elements includes security or features really needed for
proper work.
  One thing which will fail commonly is that if we add an extra value
for an attribute, usually we use macro mapping the text value in the XML
to an internal enum, and if the match can't be done, you get an error.
So we tend to fail on unknow attribute values, but accept unknown
element most of the time.

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


Re: [libvirt] [PATCHv2 0/5] Add API to allow TCP connection tunelling

2013-02-25 Thread Guido Günther
Hi Peter,
On Mon, Dec 10, 2012 at 09:29:39AM +0100, Peter Krempa wrote:
 This series adds ability for the qemu driver to tunnel connections to TCP
 ports from the host. This is useful for enabling remote VNC/SPICE sessions
 without the need to configure SSH tunnels or portforwards and without the
 need to open the ports for public.

It'd be nice to have this in. Are you still on it?
Cheers,
 -- Guido

 
 There's also an advantage for tools such as virt-viewer that have to guess 
 the remote
 connection parameters and you have to hope that nothing is in your way. With
 spice/VNC clients that have support for read/write callbacks, this would 
 allow also
 direct connection without an intermediate socket.
 
 The API and tunelling works but there's no (stable and good working) client 
 for
 this API. I hacked up a dirty netcat-like terminal into virsh for testing
 purposes (see patch 5/5) but that isn't what I'd like to see.
 
 The client should be able to open a listening socket and when a client 
 connects
 to it, it opens a stream and connects it to the remote host.
 
 For the client there are two options:
 1) do all the stuff in virsh: + one tool to rule them all
   - i'd like to daemonize it and I don't know if 
 that's okay in virsh
 
 2) add a new tool virtunnel: + less virsh pollution
- separate tool ...
 
 As nobody responded, I'd like to re-ask for someones opinion on this.
 
 
 (note: this is my personal effort, I'm annoyed of opening ssh tunnels to 
 remote displays on my server
  and I don't want to open the ports to public. )
 
 After this it would be great to add support for this to virt-viewer. I will 
 have a look at that later.
 
 
 Diff to v1:
 - fixed error reporting in 2/5
 - documented limitation to localhost in 3/5
 - fixed possible segfault in 4/5
 ---
 
 Peter Krempa (5):
   api: Add API to allow TCP tunneling through streams to the host
   fdstream: Add support for TCP connections of streams
   qemu: Add configuration options to enable TCP tunelling
   qemu: Implement virNodeTunnelTcp for the qemu driver
   NOT_TO_BE_APPLIED_UPSTREAM: quick and dirty virsh client to test the 
stuff
 
  include/libvirt/libvirt.h.in | 11 +++
  src/driver.h |  8 +
  src/fdstream.c   | 73 
 
  src/fdstream.h   |  5 +++
  src/libvirt.c| 67 
  src/libvirt_private.syms |  1 +
  src/libvirt_public.syms  |  1 +
  src/qemu/qemu.conf   | 16 ++
  src/qemu/qemu_conf.c | 26 
  src/qemu/qemu_conf.h | 13 
  src/qemu/qemu_driver.c   | 48 +
  src/remote/remote_driver.c   |  1 +
  src/remote/remote_protocol.x |  9 +-
  src/remote_protocol-structs  |  6 
  src/rpc/gendispatch.pl   |  1 +
  tools/console.c  | 66 +--
  tools/console.h  |  9 ++
  tools/virsh-domain.c | 17 +--
  tools/virsh-host.c   | 60 
  19 files changed, 397 insertions(+), 41 deletions(-)
 
 -- 
 1.8.0
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list
 

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


Re: [libvirt] [PATCH 1/2] Trivial fix: in dhcp-host the name is optional

2013-02-25 Thread Gene Czarcinski

On 02/19/2013 04:39 PM, Eric Blake wrote:

On 02/15/2013 12:02 PM, Gene Czarcinski wrote:

Although in IPv4 one must pick either mac or name, either
can be omitted.  Similarly, for IPv6, the name
can be optionally omitted.
Signed-off-by: Gene Czarcinski g...@czarc.net
---
  docs/schemas/network.rng | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 09d7c73..a479453 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -281,7 +281,9 @@
  optional
attribute name=macref 
name=uniMacAddr//attribute
  /optional
-attribute name=nametext//attribute
+optional
+  attribute name=nametext//attribute
+/optional

Hmm.  This says that I can omit both name and mac, and still validate.
Better would be this RelaxNG construct, which says that I must supply at
least one, but can also supply both; the difference is that failure to
supply either will cause a (desired) validation failure.

choice
   group
 attribute name=macref name=uniMacAddr//attribute
 optional
   attribute name=nametext//attribute
 /optional
   /group
   attribute name=nametext//attribute
/choice

I know Laine already ack'd but since we haven't pushed yet, I'm
wondering if it is worth tightening the grammar.

First of all, I like your suggestion since it is closer to reality. 
However, when I posed the question here, this was my answer:


On 02/18/2013 09:12 AM, Daniel P. Berrange wrote:

The schemas are intended to provide a simple syntax check for XML
documents that an app has created. eg detect typos in element/attribute
names, or use of elements which don't use or incorrect nesting of
elements. Even for syntax checks though, the schemas a not going to
provide 100% coverage - todo so would make the schemas far more complicated
and it is just not worth the effort. The XML parser is the ultimate
place where 100% complete syntax checks are performed.

What they generally do not do, is provide any kind of semantic check
about whether the configuration actually makes sense. For example, the
schema won't tell you that requesting virtio disks with a vmware guest
is semantically invalid. This is left to the virt driver.
So, my interpretation is that the schema should not call false negatives 
but getting all the positives correct is not necessary.


This whole thing with dhcp-host is further complicated by IPv6. That is, 
for IPv4 you need MAC whereas for IPv6 you need (client) ID but not 
MAC.  Supposedly, for IPv4 you could specify client-id but I have not 
experimented with that.


In both IPv4 and IPv6, my experiments with dnsmasq say that specifying a 
name is not sufficient to have an IP address assigned whereas for IPv4 
specifying a MAC and for IPv6 specifying an ID is sufficient.



Gene

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


Re: [libvirt] [PATCHv2 0/5] Add API to allow TCP connection tunelling

2013-02-25 Thread Peter Krempa

On 02/25/13 14:51, Guido Günther wrote:

Hi Peter,
On Mon, Dec 10, 2012 at 09:29:39AM +0100, Peter Krempa wrote:

This series adds ability for the qemu driver to tunnel connections to TCP
ports from the host. This is useful for enabling remote VNC/SPICE sessions
without the need to configure SSH tunnels or portforwards and without the
need to open the ports for public.


It'd be nice to have this in. Are you still on it?
Cheers,
  -- Guido


I'm planing to respin this in some time with a few changes. I will 
probably not allow to tunnel arbitrary ports from the machine libvirt is 
running on but limit it to a few selected ones controllable by flags.

(The general approach did not get accepted very well)

Peter

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


[libvirt] [PATCH] qemu: enable direct migration over IPv6

2013-02-25 Thread Ján Tomko
Use virURIParse in qemuMigrationPrepareDirect to allow parsing
IPv6 addresses, which would cause an 'incorrect :port' error message
before.

To be able to migrate over IPv6, QEMU needs to listen on [::] instead
of 0.0.0.0. This patch adds a call to getaddrinfo and sets the listen
address based on the result.

It also uses the same listen address for the NBD server.

This will break migration if a hostname does not resolve to the same
address family on both sides.

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=846013
---

Diff to V1:
* initialize uri_str
* reuse STRSKIP(tcp:) result instead of doing strlen on it
* print a warning instead of failing when the hostname can't be resolved

Diff to V2:
* freeaddrinfo
* separate the listen address to allow reuse in qemuMigrationStartNBDServer

 src/qemu/qemu_migration.c | 77 ---
 1 file changed, 59 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index cae58fa..ff9b959 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -22,7 +22,10 @@
 
 #include config.h
 
+#include netdb.h
+#include sys/socket.h
 #include sys/time.h
+#include sys/types.h
 #ifdef WITH_GNUTLS
 # include gnutls/gnutls.h
 # include gnutls/x509.h
@@ -1103,12 +1106,12 @@ error:
  */
 static int
 qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
-virDomainObjPtr vm)
+virDomainObjPtr vm,
+const char *listenAddr)
 {
 int ret = -1;
 qemuDomainObjPrivatePtr priv = vm-privateData;
 unsigned short port = 0;
-const char *listenAddr = 0.0.0.0;
 char *diskAlias = NULL;
 size_t i;
 
@@ -1981,6 +1984,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 const char *dom_xml,
 const char *migrateFrom,
 virStreamPtr st,
+const char *listenAddr,
 unsigned long flags)
 {
 virDomainDefPtr def = NULL;
@@ -2168,7 +2172,7 @@ done:
 if (flags  VIR_MIGRATE_TUNNELLED)
 VIR_DEBUG(NBD in tunnelled migration is currently not supported);
 else {
-if (qemuMigrationStartNBDServer(driver, vm)  0) {
+if (qemuMigrationStartNBDServer(driver, vm, listenAddr)  0) {
 /* error already reported */
 goto endjob;
 }
@@ -2271,7 +2275,7 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver,
  */
 ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
   cookieout, cookieoutlen, dname, dom_xml,
-  stdio, st, flags);
+  stdio, st, NULL, flags);
 return ret;
 }
 
@@ -2292,9 +2296,14 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
 static int port = 0;
 int this_port;
 char *hostname = NULL;
+char listenAddr[8];
 char migrateFrom [64];
 const char *p;
+char *uri_str = NULL;
 int ret = -1;
+bool ipv6 = false;
+struct addrinfo *info;
+virURIPtr uri;
 
 VIR_DEBUG(driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, 
   cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, 
@@ -2343,16 +2352,39 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
  * URI when passing it to the qemu monitor, so bad
  * characters in hostname part don't matter.
  */
-if (!STRPREFIX(uri_in, tcp:)) {
+if (!(p = STRSKIP(uri_in, tcp:))) {
 virReportError(VIR_ERR_INVALID_ARG, %s,
_(only tcp URIs are supported for KVM/QEMU
   migrations));
 goto cleanup;
 }
 
-/* Get the port number. */
-p = strrchr(uri_in, ':');
-if (p == strchr(uri_in, ':')) {
+/* Convert uri_in to well-formed URI with // after tcp: */
+if (!(STRPREFIX(uri_in, tcp://))) {
+if (virAsprintf(uri_str, tcp://%s, p)  0) {
+virReportOOMError();
+goto cleanup;
+}
+}
+
+uri = virURIParse(uri_str ? uri_str : uri_in);
+VIR_FREE(uri_str);
+
+if (uri == NULL) {
+virReportError(VIR_ERR_INVALID_ARG, _(unable to parse URI: %s),
+   uri_in);
+goto cleanup;
+}
+
+if (uri-server == NULL) {
+virReportError(VIR_ERR_INVALID_ARG, _(missing host in migration
+   URI: %s), uri_in);
+goto cleanup;
+} else {
+hostname = uri-server;
+}
+
+if (uri-port == 0) {
 /* Generate a port */
 this_port = QEMUD_MIGRATION_FIRST_PORT + port++;
 if (port == QEMUD_MIGRATION_NUM_PORTS)
@@ -2365,25 +2397,34 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,

Re: [libvirt] [PATCHv2 3/8] conf: Add support for RNG device configuration in XML

2013-02-25 Thread Peter Krempa

On 02/25/13 10:42, Peter Krempa wrote:

On 02/23/13 00:33, Eric Blake wrote:

On 02/21/2013 07:47 AM, Peter Krempa wrote:

This patch adds basic configuration support for the RNG device suporting


s/suporting/supporting/


the virtio model with the random and egd backend types as described
in the schema in the previous patch.
---

Notes:
 Version 2:
 - fix a ton of memory leaks (I assumed that virXMLGetProp
returns static strings)
 - Add new device type to even more places
 - Fix error message cp error
 - Fix memleak in RNGDef free func



...

@@ -10601,6 +10734,22 @@ virDomainDefParseXML(virCapsPtr caps,
  }
  }

+/* Parse the RNG device */
+if ((n = virXPathNodeSet(./devices/rng, ctxt, nodes))  0)
+goto error;
+
+if (n  1) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(only a single RNG device is supported));


Is this an inherent limit of qemu?  For that matter, is it an inherent
limit, and no hypervisor can ever support more than one?  In the bare
metal case, can't you plug in multiple rng hardware dongles?


AFAIK qemu is able to support multiple RNG devices but I didn't find
that particularly useful. I will follow up with a patch that will add
support for more than one RNG device, although I don't think it will be
used much.


I hacked up the patch to add support for multiple RNG's. qemu appears to 
support the device just fine.


As of guest support it seems that it triggers a 100% reproducible kernel 
panic on linux guest when attempting to read from a machine with two 
virtio-rng devices. I filed a bug to track this issue:


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

Peter


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


Re: [libvirt] [PATCH 4/4] storage: fix uid_t|gid_t handling on 32 bit Linux

2013-02-25 Thread Michal Privoznik
On 22.02.2013 17:55, Philipp Hahn wrote:
 uid_t and gid_t are opaque types, ranging from s32 to u32 to u64.
 
 Unfortunately libvirt uses the value -1 to mean current process, which
 on Linux32 gets converted to ALLONE := +(2^32-1) = 4294967295.
 
 Different libvirt versions used different formatting in the past, which
 break one or the other parsing function:
 virXPathLong(): (signed long)-1 != (double)ALLONE
 virXPathULong(): (unsigned long)-1 != (double)-1
 
 Roll our own version of virXPathULong(), which also accepts -1, which is
 silently converted to ALLONE.
 
 For output: do the reverse and print -1 instead of ALLONE.
 
 Signed-off-by: Philipp Hahn h...@univention.de
 ---
  src/conf/storage_conf.c |   74 
 ++-
  1 file changed, 60 insertions(+), 14 deletions(-)
 

This one breaks storagevolxml2xmltest:
TEST: storagevolxml2xmltest
 1) Storage Vol XML-2-XML vol-file... OK
 2) Storage Vol XML-2-XML vol-file-backing... OK
 3) Storage Vol XML-2-XML vol-qcow2   ... OK
 4) Storage Vol XML-2-XML vol-partition   ... OK
 5) Storage Vol XML-2-XML vol-logical ... OK
 6) Storage Vol XML-2-XML vol-logical-backing ... OK
 7) Storage Vol XML-2-XML vol-sheepdog... 
Offset 283
Expect [4294967295/owner
  group4294967295]
Actual [-1/owner
  group-1]
  ... FAILED

However, the first 3 patches are correct. Even though it's freeze, they are 
pure bugfixes so I've pushed them.

Michal

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


Re: [libvirt] [Qemu-devel] [PATCH] vl.c: Support multiple CPU ranges on -numa option

2013-02-25 Thread Eduardo Habkost
On Fri, Feb 22, 2013 at 11:04:24AM +0100, Markus Armbruster wrote:
 Markus Armbruster arm...@redhat.com writes:
 
  Anthony Liguori anth...@codemonkey.ws writes:
 
  Markus Armbruster arm...@redhat.com writes:
 
  Eduardo Habkost ehabk...@redhat.com writes:
 
  This allows , to be used a separator between each CPU range.  Note
  that commas inside key=value command-line options have to be escaped
  using ,,, so the command-line will look like:
 
-numa node,cpus=A,,B,,C,,D
 
  This is really, really ugly, and an embarrassment to document.  Which
  you didn't ;)
 
  What about
 
  -numa node,cpus=A,cpus=B,cpus=C,cpus=D
 
  Yes, QemuOpts lets you do that.  Getting all the values isn't as easy as
  it could be (unless you use Laszlo's opt-visitor), but that could be
  improved.
 
  No more of this.
 
   -numa node,cpus=A:B:C:D 
 
  if you want to express a list.
 
  Okay for command line and human monitor, just don't let it bleed into
  QMP.
 
 Footnotes:
 
 1. Using colons for lists works only as long as the list elements don't
 contain colons.  Fine for numbers.  No good for filenames, network
 addresses, ...
 
 2. QemuOpts helped us reduce the number of ad hoc option parsers,
 improving consistency and error messages quite a bit.  Having every user
 of colon lists roll their own ad hoc parser slides back into the hole
 that motivated QemuOpts.  Let's try to avoid that, please.
 
 3. The existing QemuOpts syntax for list-valued options (repeating the
 option) doesn't have either of these problems.

The problem here seems to be that we want to reuse option parsing code,
but the only reusable syntax we have for command-line options today is
awful (at least for representing lists).

So our only options seem to be: 1) accept some ugliness and things like
A,,B,,C or cpus=A,cpus=B,cpus=C; 2) write ad hoc option parsers; 3)
define/choose a new reusable syntax.

We already have at least 2 better ways to represent config data (config
files and QMP+JSON), but why do we insist in using command-line options
with an awful syntax for everything?

-- 
Eduardo

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


[libvirt] [PATCH] qemu: do not set unpriv_sgio if neither supported nor requested

2013-02-25 Thread Paolo Bonzini
Currently we call virSetDeviceUnprivSGIO with val == 0 if a block device
has an sgio attribute.  But for sgio='filtered', we know that a
kernel with no unpriv_sgio support will always behave as the user
wanted.  In this case, there is no need to call the function and
report a (bogus) error.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 src/qemu/qemu_process.c | 35 +--
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b560d2e..4c152b2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3455,36 +3455,35 @@ qemuProcessReconnectAll(virConnectPtr conn, 
virQEMUDriverPtr driver)
 int
 qemuSetUnprivSGIO(virDomainDiskDefPtr disk)
 {
+char *sysfs_path = NULL;
 int val = -1;
+int ret = 0;
 
 /* sgio is only valid for block disk; cdrom
  * and floopy disk can have empty source.
  */
 if (disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK ||
+disk-device != VIR_DOMAIN_DISK_DEVICE_LUN ||
 !disk-src)
 return 0;
 
-if (disk-sgio)
-val = (disk-sgio == VIR_DOMAIN_DISK_SGIO_UNFILTERED);
-
-/* Ignore the setting if unpriv_sgio is not supported by the
- * kernel, otherwise defaults to filter the SG_IO commands,
- * I.E. Set unpriv_sgio to 0.
- */
-if (disk-sgio == VIR_DOMAIN_DISK_SGIO_DEFAULT 
-disk-device == VIR_DOMAIN_DISK_DEVICE_LUN) {
-char *sysfs_path = NULL;
+sysfs_path = virGetUnprivSGIOSysfsPath(disk-src, NULL);
+if (sysfs_path == NULL)
+return -1;
 
-if ((sysfs_path = virGetUnprivSGIOSysfsPath(disk-src, NULL)) 
-virFileExists(sysfs_path))
-val = 0;
-VIR_FREE(sysfs_path);
-}
+/* By default, filter the SG_IO commands, i.e. set unpriv_sgio to 0.  */
+val = (disk-sgio == VIR_DOMAIN_DISK_SGIO_UNFILTERED);
 
-if (val = 0  virSetDeviceUnprivSGIO(disk-src, NULL, val)  0)
-return -1;
+/* Do not do anything if unpriv_sgio is not supported by the kernel and the
+ * whitelist is enabled.  But if requesting unfiltered access, always call
+ * virSetDeviceUnprivSGIO, to report an error for unsupported unpriv_sgio.
+ */
+if ((virFileExists(sysfs_path) || val == 1) 
+virSetDeviceUnprivSGIO(disk-src, NULL, val)  0)
+ret = -1;
 
-return 0;
+VIR_FREE(sysfs_path);
+return ret;
 }
 
 int qemuProcessStart(virConnectPtr conn,
-- 
1.8.1.2

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


Re: [libvirt] [PATCH 3/4] storage: cast -1 for uid_t|gid_t

2013-02-25 Thread Eric Blake
On 02/22/2013 09:41 AM, Philipp Hahn wrote:
 uid_t and gid_t are opaque types, ranging from s32 to u32 to u64.

Or even s16 and u16 in older Unix.

 
 Explicitly cast the magic -1 to the appropriate type.

Necessary in some cases, redundant in others; but overall good to be
consistent.

 
 Signed-off-by: Philipp Hahn h...@univention.de
 ---
  src/conf/storage_conf.c   |8 
  src/storage/storage_backend.c |   16 
  src/util/virutil.c|2 +-
  3 files changed, 13 insertions(+), 13 deletions(-)
 
 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index 7a39998..9134a22 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c
 @@ -674,8 +674,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
  if (node == NULL) {
  /* Set default values if there is not permissions element */
  perms-mode = defaultmode;
 -perms-uid = -1;
 -perms-gid = -1;
 +perms-uid = (uid_t) -1;
 +perms-gid = (gid_t) -1;

Redundant.  C guarantees that -1 will promote correctly via assignment
to any size integer, signed or unsigned.

  perms-label = NULL;
  return 0;
  }
 @@ -700,7 +700,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
  }
  
  if (virXPathNode(./owner, ctxt) == NULL) {
 -perms-uid = -1;
 +perms-uid = (uid_t) -1;

Redundant.

  } else {
  if (virXPathLong(number(./owner), ctxt, v)  0) {
  virReportError(VIR_ERR_XML_ERROR,
 @@ -711,7 +711,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
  }
  
  if (virXPathNode(./group, ctxt) == NULL) {
 -perms-gid = -1;
 +perms-gid = (gid_t) -1;

Redundant.

  } else {
  if (virXPathLong(number(./group), ctxt, v)  0) {
  virReportError(VIR_ERR_XML_ERROR,
 diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
 index 964ce29..ec2fa53 100644
 --- a/src/storage/storage_backend.c
 +++ b/src/storage/storage_backend.c
 @@ -277,9 +277,9 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn 
 ATTRIBUTE_UNUSED,
   vol-target.path);
  goto cleanup;
  }
 -uid = (vol-target.perms.uid != st.st_uid) ? vol-target.perms.uid : -1;
 -gid = (vol-target.perms.gid != st.st_gid) ? vol-target.perms.gid : -1;
 -if (((uid != -1) || (gid != -1))
 +uid = (vol-target.perms.uid != st.st_uid) ? vol-target.perms.uid : 
 (uid_t) -1;

Necessary - ternary operator rules for integer promotion are not
necessarily intuitive; and it is conceivable that mixing u16 with int
can produce wrong results.  Forcing both sides of the expression to be
uid_t is indeed worthwhile.

 +gid = (vol-target.perms.gid != st.st_gid) ? vol-target.perms.gid : 
 (gid_t) -1;

Ditto on being necessary.

 +if (((uid != (uid_t) -1) || (gid != (gid_t) -1))

And comparison also needs the cast - definitely necessary.

   (fchown(fd, uid, gid)  0)) {
  virReportSystemError(errno,
   _(cannot chown '%s' to (%u, %u)),
 @@ -542,9 +542,9 @@ static int 
 virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
  
  if ((pool-def-type == VIR_STORAGE_POOL_NETFS)
   (((getuid() == 0)
 -  (vol-target.perms.uid != -1)
 +  (vol-target.perms.uid != (uid_t) -1)
(vol-target.perms.uid != 0))
 -|| ((vol-target.perms.gid != -1)
 +|| ((vol-target.perms.gid != (gid_t) -1)

Necessary.

   (vol-target.perms.gid != getgid() {
  
  virCommandSetUID(cmd, vol-target.perms.uid);
 @@ -572,9 +572,9 @@ static int 
 virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
  }
  }
  
 -uid = (vol-target.perms.uid != st.st_uid) ? vol-target.perms.uid : -1;
 -gid = (vol-target.perms.gid != st.st_gid) ? vol-target.perms.gid : -1;
 -if (((uid != -1) || (gid != -1))
 +uid = (vol-target.perms.uid != st.st_uid) ? vol-target.perms.uid : 
 (uid_t) -1;
 +gid = (vol-target.perms.gid != st.st_gid) ? vol-target.perms.gid : 
 (gid_t) -1;
 +if (((uid != (uid_t) -1) || (gid != (gid_t) -1))

Necessary.

   (chown(vol-target.path, uid, gid)  0)) {
  virReportSystemError(errno,
   _(cannot chown %s to (%u, %u)),
 diff --git a/src/util/virutil.c b/src/util/virutil.c
 index 563f684..4af2599 100644
 --- a/src/util/virutil.c
 +++ b/src/util/virutil.c
 @@ -1206,7 +1206,7 @@ parenterror:
   _(stat of '%s' failed), path);
  goto childerror;
  }
 -if ((st.st_gid != gid)  (chown(path, -1, gid)  0)) {
 +if ((st.st_gid != gid)  (chown(path, (uid_t) -1, gid)  0)) {

Redundant (integer promotion via prototyped function calls does the
right thing).

Since the patch is already pushed, I'm fine leaving the redundant parts in.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library 

Re: [libvirt] RFC: APIs for managing resource groups

2013-02-25 Thread Daniel P. Berrange
On Mon, Feb 25, 2013 at 09:48:16AM -0700, Eric Blake wrote:
 On 02/25/2013 05:41 AM, Daniel P. Berrange wrote:
  Historically for the QEMU/LXC drivers we've simply put each virtual
  instance in a dedicated cgroup, under the path
  
 
  We need to simplify our layout and also introduce some APIs for the
  grouping of VMs. I won't go into specifics of a new cgroups layout
  here, just focus on the question of defining a set of APIs that are
  generic to any hypervisor, for the purpose of setting up VM resource
  groups.
 
 I'm very much in favor of VM resource groups.  In fact, this RFC has
 come up in the past, if it gives you any ideas of what you replied back
 then:
 https://www.redhat.com/archives/libvir-list/2011-March/msg01546.html
 
  
  I'm calling the resource cgroup a partition, since this is all about
  partitioning workloads.
 
 Yes, that naming is workable, and a bit better than what I tried last time.
 
  
  I anticipate a new top level object and APIs for creating/defining it
  in the usual manner:
 
 snip good API
 
  
  There'd also likely be a new VM XML element
  
 partition name=..partition name.../
  
  which is what the Get/SetPartition methods would be touching.
 
 Earlier, you pointed out that it might make sense to have multiple
 partitions per domain - that is, have one partitioning that controls
 only memory usage, and another partitioning that controls only cpu
 usage, then have a domain that belongs to two orthogonal partitions to
 cap both memory and cpu.  Your proposal today doesn't seem to deal with
 the idea of having multiple partitions per domain.  Also, while you
 proposed having a domain belong to a partition, you didn't cover whether
 it makes sense to have a hierarchy of partitions, where one partition
 can provide further constraints on top of a parent partition.

While cgroups currently allows you to setup different hiearchies
for memory, cpu, blockio, etc controls, these days it is agreed
that this is an anti-feature causing no end of trouble for all
involved. In the future I expect the kernel will enforce that we
use the same hierarchy for all cgroup controllers. In other words,
we only want a single partition for all resources.

I do anticipate that we'll be able to create partition hierarchies,
though it may be discouraged since it has performance implications
for the kernel that are currently somewhat unacceptable.


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 tck] Avoid multicast MAC addrs in tests

2013-02-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Certain MAC addrs are used for multicast and recent libvirt
will refuse to start VMs using those addrs.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 scripts/domain/215-nic-hotplug-many.t | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/domain/215-nic-hotplug-many.t 
b/scripts/domain/215-nic-hotplug-many.t
index 5b935cc..8b4afbd 100644
--- a/scripts/domain/215-nic-hotplug-many.t
+++ b/scripts/domain/215-nic-hotplug-many.t
@@ -47,9 +47,9 @@ diag Creating a new transient domain;
 my $dom;
 ok_domain(sub { $dom = $conn-create_domain($xml) }, created transient domain 
object);
 
-my $mac1 = 01:11:22:33:44:55;
-my $mac2 = 02:11:22:33:44:55;
-my $mac3 = 03:11:22:33:44:55;
+my $mac1 = 02:11:22:33:44:51;
+my $mac2 = 02:11:22:33:44:52;
+my $mac3 = 02:11:22:33:44:53;
 my $model = virtio;
 
 my $netxml1 = EOF;
-- 
1.8.1.2

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


Re: [libvirt] RFC: APIs for managing resource groups

2013-02-25 Thread Eric Blake
On 02/25/2013 05:41 AM, Daniel P. Berrange wrote:
 Historically for the QEMU/LXC drivers we've simply put each virtual
 instance in a dedicated cgroup, under the path
 

 We need to simplify our layout and also introduce some APIs for the
 grouping of VMs. I won't go into specifics of a new cgroups layout
 here, just focus on the question of defining a set of APIs that are
 generic to any hypervisor, for the purpose of setting up VM resource
 groups.

I'm very much in favor of VM resource groups.  In fact, this RFC has
come up in the past, if it gives you any ideas of what you replied back
then:
https://www.redhat.com/archives/libvir-list/2011-March/msg01546.html

 
 I'm calling the resource cgroup a partition, since this is all about
 partitioning workloads.

Yes, that naming is workable, and a bit better than what I tried last time.

 
 I anticipate a new top level object and APIs for creating/defining it
 in the usual manner:

snip good API

 
 There'd also likely be a new VM XML element
 
partition name=..partition name.../
 
 which is what the Get/SetPartition methods would be touching.

Earlier, you pointed out that it might make sense to have multiple
partitions per domain - that is, have one partitioning that controls
only memory usage, and another partitioning that controls only cpu
usage, then have a domain that belongs to two orthogonal partitions to
cap both memory and cpu.  Your proposal today doesn't seem to deal with
the idea of having multiple partitions per domain.  Also, while you
proposed having a domain belong to a partition, you didn't cover whether
it makes sense to have a hierarchy of partitions, where one partition
can provide further constraints on top of a parent partition.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH 2/4] storage: Cast uid_t|gid_t to unsigned int

2013-02-25 Thread Eric Blake
On 02/22/2013 09:43 AM, Philipp Hahn wrote:
 uid_t and gid_t are opaque types, ranging from s32 to u32 to u64.

Technically, I've never seen a platform with uid_t or gid_t being u64 -
that's still theoretical at this time.  However, since pid_t is 64-bit
on mingw64, and since id_t is the union of [pug]id_t and therefore also
a 64-bit type, I agree with this commit.

Note that we have a compile-time assertion that on all platforms where
libvirt is currently compiling, that sizeof(uid_t) = sizeof(int)
(likewise for gid_t).  We'd have a LOT more code to clean up if we ever
encounter a platform with u64 [ug]id_t.

Still, since your commit has already been pushed, we can't touch up the
commit message now.  And ACK to your change.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] qemu: Refactor qemuDomainSetMemoryParameters

2013-02-25 Thread Peter Krempa

On 02/22/13 13:56, John Ferlan wrote:

On 02/18/2013 10:18 AM, Peter Krempa wrote:

The new TypedParam helper APIs allow to simplify this function
significantly.

This patch integrates the fix in 75e5bec97b3045e4f926248d5c742f8a50d0f9
by correctly ordering the setting functions instead of reordering the
parameters.
---
  src/qemu/qemu_driver.c | 149 ++---
  1 file changed, 54 insertions(+), 95 deletions(-)


...


  /* It will fail if hard limit greater than swap hard limit anyway */
-if (swap_hard_limit 
-hard_limit 
-(hard_limit-value.ul  swap_hard_limit-value.ul)) {
+if (set_swap_hard_limit  set_memory_hard_limit 
+(memory_hard_limit  swap_hard_limit)) {
  virReportError(VIR_ERR_INVALID_ARG, %s,
 _(hard limit must be lower than swap hard limit));


Consider the messages below, should this be memory hard_limit tunable
value must be lower than swap_hard_limit


I went with the modified spelling of the message ...




...

ACK - seems reasonable to me - your choice on the error message




... and pushed. Thanks for the review.

Peter

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


[libvirt] [PATCH] Fix crash changing CDROM media

2013-02-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

This change tried to fix a crash with changing CDROM media but
failed to actually do so

  commit d0172d2b1b5d865aaa042070d7c2d00effb2ff8c
  Author: Osier Yang jy...@redhat.com
  Date:   Tue Feb 19 20:27:45 2013 +0800

qemu: Remove the shared disk entry if the operation is ejecting or updating

It was still accessing disk-src, when the entire 'disk' object
has been free'd already. Even if it weren't free'd, accessing
the 'src' value of virDomainDiskDef is not allowed without
first validating disk-type is file or block. Just remove the
broken code entirely.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/qemu/qemu_driver.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1e96915..8dae8f9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5778,13 +5778,14 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
 dev-data.disk = tmp;
 
 ret = qemuDomainChangeEjectableMedia(driver, vm, disk, orig_disk, 
false);
+/* 'disk' must not be accessed now - it has been free'd.
+ * 'orig_disk' now points to the new disk, while 'dev_copy'
+ * now points to the old disk */
 
 /* Need to remove the shared disk entry for the original disk src
  * if the operation is either ejecting or updating.
  */
-if (ret == 0 
-orig_disk-src 
-STRNEQ_NULLABLE(orig_disk-src, disk-src))
+if (ret == 0)
 ignore_value(qemuRemoveSharedDisk(driver, dev_copy-data.disk,
   vm-def-name));
 break;
-- 
1.8.1.2

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


[libvirt] [PATCH 04/13] qemu: support named nbd exports

2013-02-25 Thread Paolo Bonzini
These are supported by nbd-server and by the NBD server that QEMU
embeds for live image access.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 docs/formatdomain.html.in  |  6 ++--
 src/qemu/qemu_command.c| 17 +++
 tests/qemuargv2xmltest.c   |  1 +
 ...qemuxml2argv-disk-drive-network-nbd-export.args |  5 
 .../qemuxml2argv-disk-drive-network-nbd-export.xml | 33 ++
 tests/qemuxml2argvtest.c   |  2 ++
 tests/qemuxml2xmltest.c|  1 +
 7 files changed, 62 insertions(+), 3 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a9003d7..90cfc03 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1441,9 +1441,9 @@
 are nbd, rbd, sheepdog or gluster.  If the
 codeprotocol/code attribute is rbd, sheepdog or gluster, an
 additional attribute codename/code is mandatory to specify which
-volume/image will be used.  When the disk codetype/code is
-network, the codesource/code may have zero or
-more codehost/code sub-elements used to specify the hosts
+volume/image will be used; for nbd it is optional.  When the disk
+codetype/code is network, the codesource/code may have zero
+or more codehost/code sub-elements used to specify the hosts
 to connect.
 span class=sinceSince 0.0.3; codetype='dir'/code since
 0.7.5; codetype='network'/code since 0.8.7/spanbr/
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index beb7cfe..4762f0a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2132,6 +2132,7 @@ qemuParseNBDString(virDomainDiskDefPtr disk)
 {
 virDomainDiskHostDefPtr h = NULL;
 char *host, *port;
+char *src;
 
 if (VIR_ALLOC(h)  0)
 goto no_memory;
@@ -2149,11 +2150,24 @@ qemuParseNBDString(virDomainDiskDefPtr disk)
 if (!h-name)
 goto no_memory;
 
+src = strchr(port, ':');
+if (src)
+*src++ = '\0';
+
 h-port = strdup(port);
 if (!h-port)
 goto no_memory;
 
+if (src  STRPREFIX(src, exportname=)) {
+src = strdup(strchr(src, '=') + 1);
+if (!src)
+goto no_memory;
+} else {
+src = NULL;
+}
+
 VIR_FREE(disk-src);
+disk-src = src;
 disk-nhosts = 1;
 disk-hosts = h;
 return 0;
@@ -2254,6 +2268,9 @@ qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr 
opt)
 break;
 }
 
+if (disk-src)
+virBufferEscape(opt, ',', ,, :exportname=%s, disk-src);
+
 return 0;
 }
 
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 3c23010..1731022 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -182,6 +182,7 @@ mymain(void)
 DO_TEST(disk-drive-cache-directsync);
 DO_TEST(disk-drive-cache-unsafe);
 DO_TEST(disk-drive-network-nbd);
+DO_TEST(disk-drive-network-nbd-export);
 DO_TEST(disk-drive-network-gluster);
 DO_TEST(disk-drive-network-rbd);
 DO_TEST(disk-drive-network-rbd-ipv6);
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args
new file mode 100644
index 000..bf411ff
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args
@@ -0,0 +1,5 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
+pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
+-no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 
-drive \
+file=nbd:example.org:6000:exportname=bar,if=virtio,format=raw -net none 
-serial none \
+-parallel none
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml
new file mode 100644
index 000..f2b5ca4
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml
@@ -0,0 +1,33 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219136/memory
+  currentMemory unit='KiB'219136/currentMemory
+  vcpu placement='static'1/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+  source dev='/dev/HostVG/QEMUGuest1'/
+  target dev='hda' bus='ide'/
+  address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+disk type='network' 

[libvirt] [PATCH 02/13] qemu: do not support non-network disks without -drive

2013-02-25 Thread Paolo Bonzini
QEMU added -drive in 2007, and NBD in 2008.  Both appeared first in
release 0.10.0.  Thus the code to support network disks without -drive
is dead, and in fact it incorrectly escapes commas.  Drop it.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 src/qemu/qemu_command.c | 53 ++---
 1 file changed, 2 insertions(+), 51 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5dccaae..a3c5a4e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5901,57 +5901,8 @@ qemuBuildCommandLine(virConnectPtr conn,
 goto no_memory;
 }
 } else if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
-switch (disk-protocol) {
-case VIR_DOMAIN_DISK_PROTOCOL_NBD:
-if (disk-nhosts != 1) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(NBD accepts only one host));
-goto error;
-}
-if (virAsprintf(file, nbd:%s:%s,, disk-hosts-name,
-disk-hosts-port)  0) {
-goto no_memory;
-}
-break;
-case VIR_DOMAIN_DISK_PROTOCOL_RBD:
-{
-virBuffer opt = VIR_BUFFER_INITIALIZER;
-if (qemuBuildRBDString(conn, disk, opt)  0)
-goto error;
-if (virBufferError(opt)) {
-virReportOOMError();
-goto error;
-}
-file = virBufferContentAndReset(opt);
-}
-break;
-case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER:
-{
-virBuffer opt = VIR_BUFFER_INITIALIZER;
-if (qemuBuildGlusterString(disk, opt)  0)
-goto error;
-if (virBufferError(opt)) {
-virReportOOMError();
-goto error;
-}
-file = virBufferContentAndReset(opt);
-}
-break;
-case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
-if (disk-nhosts == 0) {
-if (virAsprintf(file, sheepdog:%s,, disk-src)  0) 
{
-goto no_memory;
-}
-} else {
-/* only one host is supported now */
-if (virAsprintf(file, sheepdog:%s:%s:%s,,
-disk-hosts-name, disk-hosts-port,
-disk-src)  0) {
-goto no_memory;
-}
-}
-break;
-}
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(network disks are only supported with 
-drive));
 } else {
 if (!(file = strdup(disk-src))) {
 goto no_memory;
-- 
1.8.1.2


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


[libvirt] [PATCH 08/13] qemu: add support for libiscsi

2013-02-25 Thread Paolo Bonzini
libiscsi provides a userspace iSCSI initiator.

The main advantage over the kernel initiator is that it is very
easy to provide different initiator names for VMs on the same host.
Thus libiscsi supports usage of persistent reservations in the VM,
which otherwise would only be possible with NPIV.

libiscsi uses iscsi as the scheme, not iscsi+tcp.  We can change
this in the tests (while remaining backwards-compatible manner, because
QEMU uses TCP as the default transport for both Gluster and NBD).

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 src/qemu/qemu_command.c| 49 +-
 tests/qemuargv2xmltest.c   |  1 +
 .../qemuxml2argv-disk-drive-network-gluster.args   |  2 +-
 .../qemuxml2argv-disk-drive-network-iscsi.args |  1 +
 ...ml2argv-disk-drive-network-nbd-ipv6-export.args |  2 +-
 .../qemuxml2argv-disk-drive-network-nbd-ipv6.args  |  2 +-
 tests/qemuxml2argvtest.c   |  2 +
 7 files changed, 54 insertions(+), 5 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 733d3bf..07700cc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2145,6 +2145,23 @@ qemuParseGlusterString(virDomainDiskDefPtr def)
 }
 
 static int
+qemuParseISCSIString(virDomainDiskDefPtr def)
+{
+virURIPtr uri = NULL;
+
+if (!(uri = virURIParse(def-src)))
+return -1;
+
+if (uri-path  strchr(uri-path + 1, '/')) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(invalid address for iSCSI target), disk-src);
+return -1;
+}
+
+return qemuParseDriveURIString(def, uri, iscsi);
+}
+
+static int
 qemuParseNBDString(virDomainDiskDefPtr disk)
 {
 virDomainDiskHostDefPtr h = NULL;
@@ -2238,8 +2255,14 @@ qemuBuildDriveURIString(virDomainDiskDefPtr disk, 
virBufferPtr opt,
 virBufferAddLit(opt, file=);
 transp = 
virDomainDiskProtocolTransportTypeToString(disk-hosts-transport);
 
-if (virAsprintf(tmpscheme, %s+%s, scheme, transp)  0)
-goto no_memory;
+if (disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) {
+tmpscheme = strdup(scheme);
+if (tmpscheme == NULL)
+goto no_memory;
+} else {
+if (virAsprintf(tmpscheme, %s+%s, scheme, transp)  0)
+goto no_memory;
+}
 
 if (disk-src  virAsprintf(volimg, /%s, disk-src)  0)
 goto no_memory;
@@ -2283,6 +2306,12 @@ qemuBuildGlusterString(virDomainDiskDefPtr disk, 
virBufferPtr opt)
 }
 
 static int
+qemuBuildISCSIString(virDomainDiskDefPtr disk, virBufferPtr opt)
+{
+return qemuBuildDriveURIString(disk, opt, iscsi);
+}
+
+static int
 qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt)
 {
 const char *transp;
@@ -2471,6 +2500,11 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 goto error;
 virBufferAddChar(opt, ',');
 break;
+case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
+if (qemuBuildISCSIString(disk, opt)  0)
+goto error;
+virBufferAddChar(opt, ',');
+break;
 
 case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
 if (disk-nhosts == 0) {
@@ -7503,6 +7537,12 @@ qemuParseCommandLineDisk(virCapsPtr qemuCaps,
 
 if (qemuParseGlusterString(def)  0)
 goto error;
+} else if (STRPREFIX(def-src, iscsi:)) {
+def-type = VIR_DOMAIN_DISK_TYPE_NETWORK;
+def-protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI;
+
+if (qemuParseISCSIString(def)  0)
+goto error;
 } else if (STRPREFIX(def-src, sheepdog:)) {
 char *p = def-src;
 char *port, *vdi;
@@ -8793,6 +8833,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
 goto error;
 
 break;
+case VIR_DOMAIN_DISK_PROTOCOL_ISCSI:
+if (qemuParseISCSIString(disk)  0)
+goto error;
+
+break;
 }
 }
 
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 9fbba94..314c7b3 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -186,6 +186,7 @@ mymain(void)
 DO_TEST(disk-drive-network-nbd-ipv6);
 DO_TEST(disk-drive-network-nbd-ipv6-export);
 DO_TEST(disk-drive-network-nbd-unix);
+DO_TEST(disk-drive-network-iscsi);
 DO_TEST(disk-drive-network-gluster);
 DO_TEST(disk-drive-network-rbd);
 DO_TEST(disk-drive-network-rbd-ipv6);
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args
index 6dbb009..9d70586 100644
--- 

[libvirt] [PATCH 05/13] qemu: support NBD with Unix sockets

2013-02-25 Thread Paolo Bonzini
This reuses the XML format that was introduced for Gluster.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 docs/formatdomain.html.in  |  8 ++--
 src/qemu/qemu_command.c| 49 +++---
 tests/qemuargv2xmltest.c   |  1 +
 .../qemuxml2argv-disk-drive-network-nbd-unix.args  |  5 +++
 .../qemuxml2argv-disk-drive-network-nbd-unix.xml   | 33 +++
 tests/qemuxml2argvtest.c   |  2 +
 tests/qemuxml2xmltest.c|  1 +
 7 files changed, 80 insertions(+), 19 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 90cfc03..094b509 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1713,9 +1713,11 @@
 td only one /td
   /tr
 /table
-In case of gluster, valid values for transport attribute are tcp, rdma
-or unix. If nothing is specified, tcp is assumed. If transport is unix,
-socket attribute specifies path to unix socket.
+gluster supports tcp, rdma, unix as valid values for the
+transport attribute.  nbd supports tcp and unix.  Others only
+support tcp.  If nothing is specified, tcp is assumed. If the
+transport is unix, the socket attribute specifies the path to an
+AF_UNIX socket.
   /dd
   dtcodeaddress/code/dt
   ddIf present, the codeaddress/code element ties the disk
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4762f0a..89cd065 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2138,25 +2138,34 @@ qemuParseNBDString(virDomainDiskDefPtr disk)
 goto no_memory;
 
 host = disk-src + strlen(nbd:);
-port = strchr(host, ':');
-if (!port) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(cannot parse nbd filename '%s'), disk-src);
-goto error;
-}
+if (STRPREFIX(host, unix:/)) {
+src = strchr(host + strlen(unix:), ':');
+if (src)
+*src++ = '\0';
 
-*port++ = '\0';
-h-name = strdup(host);
-if (!h-name)
-goto no_memory;
+h-transport = VIR_DOMAIN_DISK_PROTO_TRANS_UNIX;
+h-socket = strdup(host + strlen(unix:));
+} else {
+port = strchr(host, ':');
+if (!port) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(cannot parse nbd filename '%s'), disk-src);
+goto error;
+}
 
-src = strchr(port, ':');
-if (src)
-*src++ = '\0';
+*port++ = '\0';
+h-name = strdup(host);
+if (!h-name)
+goto no_memory;
 
-h-port = strdup(port);
-if (!h-port)
-goto no_memory;
+src = strchr(port, ':');
+if (src)
+*src++ = '\0';
+
+h-port = strdup(port);
+if (!h-port)
+goto no_memory;
+}
 
 if (src  STRPREFIX(src, exportname=)) {
 src = strdup(strchr(src, '=') + 1);
@@ -2261,6 +2270,14 @@ qemuBuildNBDString(virDomainDiskDefPtr disk, 
virBufferPtr opt)
 virBufferEscape(opt, ',', ,, :%s,
 disk-hosts-port ? disk-hosts-port : 10809);
 break;
+case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX:
+if (!disk-hosts-socket) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(socket attribute required for unix transport));
+return -1;
+}
+virBufferEscape(opt, ',', ,, unix:%s, disk-hosts-socket);
+break;
 default:
 transp = 
virDomainDiskProtocolTransportTypeToString(disk-hosts-transport);
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 1731022..12174b3 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -183,6 +183,7 @@ mymain(void)
 DO_TEST(disk-drive-cache-unsafe);
 DO_TEST(disk-drive-network-nbd);
 DO_TEST(disk-drive-network-nbd-export);
+DO_TEST(disk-drive-network-nbd-unix);
 DO_TEST(disk-drive-network-gluster);
 DO_TEST(disk-drive-network-rbd);
 DO_TEST(disk-drive-network-rbd-ipv6);
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args
new file mode 100644
index 000..afea187
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args
@@ -0,0 +1,5 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
+pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
+-no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 
-drive \

[libvirt] [PATCH 12/13] domain: parse XML for iscsi authorization credentials

2013-02-25 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 docs/formatdomain.html.in  | 12 -
 docs/schemas/domaincommon.rng  |  1 +
 src/conf/domain_conf.c | 31 --
 .../qemuxml2argv-disk-drive-network-iscsi-auth.xml | 31 ++
 tests/qemuxml2xmltest.c|  1 +
 5 files changed, 62 insertions(+), 14 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c590427..0906fe9 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1760,12 +1760,12 @@
 holds the actual password or other credentials (the domain XML
 intentionally does not expose the password, only the reference
 to the object that does manage the password).  For now, the
-only known secret codetype/code is ceph, for Ceph RBD
-network sources, and requires either an
-attribute codeuuid/code with the UUID of the Ceph secret
-object, or an attribute codeusage/code with the name
-associated with the Ceph secret
-object.  span class=sincelibvirt 0.9.7/span
+known secret codetype/codes are ceph, for Ceph RBD
+network sources, and iscsi, for CHAP authentication of iSCSI
+targets.  Both require either a codeuuid/code attribute
+with the UUID of the secret object, or a codeusage/code
+attribute matching the key that was specified in the
+secret object.  span class=sincelibvirt 0.9.7/span
   /dd
   dtcodegeometry/code/dt
   ddThe optional codegeometry/code element provides the
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b8c4503..6f85e84 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3592,6 +3592,7 @@
   attribute name='type'
 choice
   valueceph/value
+  valueiscsi/value
 /choice
   /attribute
   choice
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 71da694..e4c3e67 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3885,6 +3885,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 char *wwn = NULL;
 char *vendor = NULL;
 char *product = NULL;
+int expected_secret_usage = -1;
+int auth_secret_usage = -1;
 
 if (VIR_ALLOC(def)  0) {
 virReportOOMError();
@@ -3922,7 +3924,6 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 if (cur-type == XML_ELEMENT_NODE) {
 if (!source  !hosts 
 xmlStrEqual(cur-name, BAD_CAST source)) {
-
 sourceNode = cur;
 
 switch (def-type) {
@@ -3958,6 +3959,9 @@ virDomainDiskDefParseXML(virCapsPtr caps,
_(invalid logical unit number));
 goto error;
 }
+expected_secret_usage = VIR_SECRET_USAGE_TYPE_ISCSI;
+} else if (def-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) {
+expected_secret_usage = VIR_SECRET_USAGE_TYPE_CEPH;
 }
 if (!(source = virXMLPropString(cur, name)) 
 def-protocol != VIR_DOMAIN_DISK_PROTOCOL_NBD) {
@@ -4144,8 +4148,9 @@ virDomainDiskDefParseXML(virCapsPtr caps,
_(missing type for secret));
 goto error;
 }
-if (virSecretUsageTypeTypeFromString(usageType) !=
-VIR_SECRET_USAGE_TYPE_CEPH) {
+auth_secret_usage =
+virSecretUsageTypeTypeFromString(usageType);
+if (auth_secret_usage  0) {
 virReportError(VIR_ERR_XML_ERROR,
_(invalid secret type %s),
usageType);
@@ -4295,6 +4300,13 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 cur = cur-next;
 }
 
+if (auth_secret_usage != -1  auth_secret_usage != expected_secret_usage) 
{
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(invalid secret type '%s'),
+   virSecretUsageTypeTypeToString(auth_secret_usage));
+goto error;
+}
+
 device = virXMLPropString(node, device);
 if (device) {
 if ((def-device = virDomainDiskDeviceTypeFromString(device))  0) {
@@ -12500,15 +12512,18 @@ virDomainDiskDefFormat(virBufferPtr buf,
 if (def-auth.username) {
 virBufferEscapeString(buf,   auth username='%s'\n,
   def-auth.username);
+if (def-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) {
+virBufferAsprintf(buf, secret type='iscsi');
+} else 

[libvirt] [PATCH 07/13] domain: add support for iscsi network disks

2013-02-25 Thread Paolo Bonzini
This plumbs in the XML description of iSCSI shares.  The next patches
will add support for the libiscsi userspace initiator.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 docs/formatdomain.html.in  |  7 +-
 docs/schemas/domaincommon.rng  |  1 +
 src/conf/domain_conf.c |  3 ++-
 src/conf/domain_conf.h |  1 +
 .../qemuxml2argv-disk-drive-network-iscsi.xml  | 27 ++
 tests/qemuxml2xmltest.c|  1 +
 6 files changed, 38 insertions(+), 2 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 094b509..fd6d5ae 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1438,7 +1438,7 @@
 to the directory to use as the disk. If the disk codetype/code
 is network, then the codeprotocol/code attribute specifies
 the protocol to access to the requested image; possible values
-are nbd, rbd, sheepdog or gluster.  If the
+are nbd, iscsi, rbd, sheepdog or gluster.  If the
 codeprotocol/code attribute is rbd, sheepdog or gluster, an
 additional attribute codename/code is mandatory to specify which
 volume/image will be used; for nbd it is optional.  When the disk
@@ -1698,6 +1698,11 @@
 td only one /td
   /tr
   tr
+td iscsi /td
+td an iSCSI server /td
+td only one /td
+  /tr
+  tr
 td rbd /td
 td monitor servers of RBD /td
 td one or more /td
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 63be4aa..980410f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1084,6 +1084,7 @@
 valuerbd/value
 valuesheepdog/value
 valuegluster/value
+valueiscsi/value
   /choice
 /attribute
 optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0c75838..b42c79c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -235,7 +235,8 @@ VIR_ENUM_IMPL(virDomainDiskProtocol, 
VIR_DOMAIN_DISK_PROTOCOL_LAST,
   nbd,
   rbd,
   sheepdog,
-  gluster)
+  gluster,
+  iscsi)
 
 VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST,
   tcp,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4ffa4aa..7cd0264 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -494,6 +494,7 @@ enum virDomainDiskProtocol {
 VIR_DOMAIN_DISK_PROTOCOL_RBD,
 VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG,
 VIR_DOMAIN_DISK_PROTOCOL_GLUSTER,
+VIR_DOMAIN_DISK_PROTOCOL_ISCSI,
 
 VIR_DOMAIN_DISK_PROTOCOL_LAST
 };
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml
new file mode 100644
index 000..dd85032
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml
@@ -0,0 +1,27 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219136/memory
+  currentMemory unit='KiB'219136/currentMemory
+  vcpu placement='static'1/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='network' device='disk'
+  driver name='qemu' type='raw'/
+  source protocol='iscsi' name='iqn.1992-01.com.example'
+host name='example.org' port='6000'/
+  /source
+  target dev='vda' bus='virtio'/
+/disk
+controller type='usb' index='0'/
+memballoon model='virtio'/
+  /devices
+/domain
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 7a01954..e0d3b20 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -171,6 +171,7 @@ mymain(void)
 DO_TEST(disk-drive-network-nbd-ipv6);
 DO_TEST(disk-drive-network-nbd-ipv6-export);
 DO_TEST(disk-drive-network-nbd-unix);
+DO_TEST(disk-drive-network-iscsi);
 DO_TEST(disk-scsi-device);
 DO_TEST(disk-scsi-vscsi);
 DO_TEST(disk-scsi-virtio-scsi);
-- 
1.8.1.2


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


[libvirt] [PATCH 10/13] domain: make port optional for network disks

2013-02-25 Thread Paolo Bonzini
Only sheepdog actually required it in the code, and we can use 7000 as the
default---the same value that QEMU uses for the simple sheepdog:VOLUME
syntax.  With this change, the schema can be fixed to allow no port.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 docs/formatdomain.html.in | 6 ++
 docs/schemas/domaincommon.rng | 8 +---
 src/conf/domain_conf.c| 5 -
 src/qemu/qemu_command.c   | 3 ++-
 4 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 5b4eabe..c590427 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1692,31 +1692,37 @@
 th Protocol /th
 th Meaning /th
 th Number of hosts /th
+th Default port /th
   /tr
   tr
 td nbd /td
 td a server running nbd-server /td
 td only one /td
+td 10809 /td
   /tr
   tr
 td iscsi /td
 td an iSCSI server /td
 td only one /td
+td 3260 /td
   /tr
   tr
 td rbd /td
 td monitor servers of RBD /td
 td one or more /td
+td 6789 /td
   /tr
   tr
 td sheepdog /td
 td one of the sheepdog servers (default is localhost:7000) /td
 td zero or one /td
+td 7000 /td
   /tr
   tr
 td gluster /td
 td a server running glusterd daemon /td
 td only one /td
+td 24007 /td
   /tr
 /table
 gluster supports tcp, rdma, unix as valid values for the
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 82062a3..b8c4503 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1119,9 +1119,11 @@
 ref name=ipAddr/
   /choice
 /attribute
-attribute name=port
-  ref name=unsignedInt/
-/attribute
+optional
+  attribute name=port
+ref name=unsignedInt/
+  /attribute
+/optional
   /group
   group
 attribute name=transport
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b801239..71da694 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4013,11 +4013,6 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 goto error;
 }
 hosts[nhosts - 1].port = 
virXMLPropString(child, port);
-if (!hosts[nhosts - 1].port) {
-virReportError(VIR_ERR_XML_ERROR,
-   %s, _(missing port for 
host));
-goto error;
-}
 }
 }
 child = child-next;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 59773ec..5d52be5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2526,7 +2526,8 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 } else {
 /* only one host is supported now */
 virBufferAsprintf(opt, file=sheepdog:%s:%s:,
-  disk-hosts-name, disk-hosts-port);
+  disk-hosts-name,
+  disk-hosts-port ? disk-hosts-port : 
7000);
 virBufferEscape(opt, ',', ,, %s,, disk-src);
 }
 break;
-- 
1.8.1.2


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


[libvirt] [PATCH 13/13] qemu: pass iscsi authorization credentials

2013-02-25 Thread Paolo Bonzini
A better way to do this would be to use a configuration file like

   [iscsi target-name]
   user = name
   password = pwd

and pass it via -readconfig.  This would remove the username and password
from the ps output.  For now, however, keep this solution.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 src/qemu/qemu_command.c| 80 ++
 ...qemuxml2argv-disk-drive-network-iscsi-auth.args |  1 +
 tests/qemuxml2argvtest.c   |  2 +
 3 files changed, 70 insertions(+), 13 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5d52be5..30ddbd3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1888,8 +1888,8 @@ qemuBuildRBDString(virConnectPtr conn,
 VIR_FREE(base64);
 } else {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(rbd username '%s' specified but secret not 
found),
-   disk-auth.username);
+   _(%s username '%s' specified but secret not 
found),
+   rbd, disk-auth.username);
 goto error;
 }
 } else {
@@ -2057,6 +2057,7 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, 
virURIPtr uri,
 char *transp = NULL;
 char *sock = NULL;
 char *volimg = NULL;
+char *secret = NULL;
 
 if (VIR_ALLOC(def-hosts)  0)
 goto no_memory;
@@ -2117,6 +2118,16 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, 
virURIPtr uri,
 def-src = NULL;
 }
 
+if (uri-user) {
+secret = strchr(uri-user, ':');
+if (secret)
+*secret = '\0';
+
+def-auth.username = strdup(uri-user);
+if (!def-auth.username)
+goto no_memory;
+}
+
 def-nhosts = 1;
 ret = 0;
 
@@ -2237,14 +2248,20 @@ error:
 }
 
 static int
-qemuBuildDriveURIString(virDomainDiskDefPtr disk, virBufferPtr opt,
-const char *scheme)
+qemuBuildDriveURIString(virConnectPtr conn,
+virDomainDiskDefPtr disk, virBufferPtr opt,
+const char *scheme, virSecretUsageType secretType)
 {
 int ret = -1;
 int port = 0;
+virSecretPtr sec = NULL;
+char *secret = NULL;
+size_t secret_size;
+
 char *tmpscheme = NULL;
 char *volimg = NULL;
 char *sock = NULL;
+char *user = NULL;
 char *builturi = NULL;
 const char *transp = NULL;
 virURI uri = {
@@ -2280,8 +2297,42 @@ qemuBuildDriveURIString(virDomainDiskDefPtr disk, 
virBufferPtr opt,
 virAsprintf(sock, socket=%s, disk-hosts-socket)  0)
 goto no_memory;
 
+if (disk-auth.username  secretType != VIR_SECRET_USAGE_TYPE_NONE) {
+/* look up secret */
+switch (disk-auth.secretType) {
+case VIR_DOMAIN_DISK_SECRET_TYPE_UUID:
+sec = virSecretLookupByUUID(conn,
+disk-auth.secret.uuid);
+break;
+case VIR_DOMAIN_DISK_SECRET_TYPE_USAGE:
+sec = virSecretLookupByUsage(conn, secretType,
+ disk-auth.secret.usage);
+break;
+}
+
+if (sec) {
+secret = (char *)conn-secretDriver-getValue(sec, secret_size, 0,
+  
VIR_SECRET_GET_VALUE_INTERNAL_CALL);
+if (secret == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(could not get the value of the secret for 
username %s),
+   disk-auth.username);
+ret = -1;
+goto cleanup;
+}
+if (virAsprintf(user, %s:%s, disk-auth.username, secret)  0)
+goto no_memory;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(%s username '%s' specified but secret not 
found),
+   scheme, disk-auth.username);
+ret = -1;
+goto cleanup;
+}
+}
 uri.scheme = tmpscheme; /* gluster+transport */
 uri.server = disk-hosts-name;
+uri.user = user;
 uri.port = port;
 uri.path = volimg;
 uri.query = sock;
@@ -2305,16 +2356,18 @@ no_memory:
 }
 
 static int
-qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
+qemuBuildGlusterString(virConnectPtr conn, virDomainDiskDefPtr disk, 
virBufferPtr opt)
 {
-return qemuBuildDriveURIString(disk, opt, gluster);
+return qemuBuildDriveURIString(conn, disk, opt, gluster,
+   VIR_SECRET_USAGE_TYPE_NONE);
 }
 
 static int
-qemuBuildISCSIString(virDomainDiskDefPtr disk, virBufferPtr opt)
+qemuBuildISCSIString(virConnectPtr conn, virDomainDiskDefPtr disk, 
virBufferPtr opt)
 {
 int ret;
-ret = qemuBuildDriveURIString(disk, opt, 

Re: [libvirt] [PATCH] Fix crash changing CDROM media

2013-02-25 Thread Eric Blake
On 02/25/2013 10:25 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 This change tried to fix a crash with changing CDROM media but
 failed to actually do so
 
   commit d0172d2b1b5d865aaa042070d7c2d00effb2ff8c
   Author: Osier Yang jy...@redhat.com
   Date:   Tue Feb 19 20:27:45 2013 +0800
 
 qemu: Remove the shared disk entry if the operation is ejecting or 
 updating
 
 It was still accessing disk-src, when the entire 'disk' object
 has been free'd already. Even if it weren't free'd, accessing
 the 'src' value of virDomainDiskDef is not allowed without
 first validating disk-type is file or block. Just remove the
 broken code entirely.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/qemu/qemu_driver.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

ACK.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

[libvirt] [PATCH 00/13] Network disk improvements (NBD libiscsi)

2013-02-25 Thread Paolo Bonzini
This series improves support for NBD disks (patches 1-6), and adds
support for the libiscsi userspace initiator (patches 7-13).

Please review!

Paolo

Paolo Bonzini (13):
  qemu: fix use-after-free when parsing NBD disk
  qemu: do not support non-network disks without -drive
  qemu: rewrite NBD command-line builder and parser
  qemu: support named nbd exports
  qemu: support NBD with Unix sockets
  qemu: support URI syntax for NBD
  domain: add support for iscsi network disks
  qemu: add support for libiscsi
  qemu: support LUN numbers for iSCSI disks
  domain: make port optional for network disks
  secret: add iscsi to possible usage types
  domain: parse XML for iscsi authorization credentials
  qemu: pass iscsi authorization credentials

 docs/formatdomain.html.in  |  42 +-
 docs/formatsecret.html.in  |  12 +
 docs/schemas/domaincommon.rng  |  37 +-
 docs/schemas/secret.rng|  10 +
 include/libvirt/libvirt.h.in   |   1 +
 src/conf/domain_conf.c |  51 ++-
 src/conf/domain_conf.h |   3 +
 src/conf/secret_conf.c |  22 +-
 src/conf/secret_conf.h |   1 +
 src/qemu/qemu_command.c| 432 ++---
 src/secret/secret_driver.c |   8 +
 tests/qemuargv2xmltest.c   |   5 +
 .../qemuxml2argv-disk-drive-network-gluster.args   |   2 +-
 ...qemuxml2argv-disk-drive-network-iscsi-auth.args |   1 +
 .../qemuxml2argv-disk-drive-network-iscsi-auth.xml |  31 ++
 .../qemuxml2argv-disk-drive-network-iscsi.args |   1 +
 .../qemuxml2argv-disk-drive-network-iscsi.xml  |  34 ++
 ...qemuxml2argv-disk-drive-network-nbd-export.args |   5 +
 .../qemuxml2argv-disk-drive-network-nbd-export.xml |  33 ++
 ...ml2argv-disk-drive-network-nbd-ipv6-export.args |   5 +
 ...xml2argv-disk-drive-network-nbd-ipv6-export.xml |  33 ++
 .../qemuxml2argv-disk-drive-network-nbd-ipv6.args  |   5 +
 .../qemuxml2argv-disk-drive-network-nbd-ipv6.xml   |  33 ++
 .../qemuxml2argv-disk-drive-network-nbd-unix.args  |   5 +
 .../qemuxml2argv-disk-drive-network-nbd-unix.xml   |  33 ++
 tests/qemuxml2argvtest.c   |  12 +
 tests/qemuxml2xmltest.c|   7 +
 27 files changed, 687 insertions(+), 177 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml

-- 
1.8.1.2

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


[libvirt] [PATCH 09/13] qemu: support LUN numbers for iSCSI disks

2013-02-25 Thread Paolo Bonzini
Each iSCSI target can provide multiple logical units.  Support this
with an additional attribute in the source element.

libiscsi places the LUN in the path component of the URI.  iSCSI
names cannot contain slashes, so this is unambiguous

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 docs/formatdomain.html.in  |  9 ---
 docs/schemas/domaincommon.rng  | 29 +++---
 src/conf/domain_conf.c | 12 +
 src/conf/domain_conf.h |  2 ++
 src/qemu/qemu_command.c| 23 +
 .../qemuxml2argv-disk-drive-network-iscsi.args |  2 +-
 .../qemuxml2argv-disk-drive-network-iscsi.xml  |  7 ++
 7 files changed, 65 insertions(+), 19 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fd6d5ae..5b4eabe 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1441,10 +1441,11 @@
 are nbd, iscsi, rbd, sheepdog or gluster.  If the
 codeprotocol/code attribute is rbd, sheepdog or gluster, an
 additional attribute codename/code is mandatory to specify which
-volume/image will be used; for nbd it is optional.  When the disk
-codetype/code is network, the codesource/code may have zero
-or more codehost/code sub-elements used to specify the hosts
-to connect.
+volume/image will be used; for nbd it is optional.  iscsi supports
+an additional numeric attribute codeunit/code for the logical unit
+number (default 0).  When the disk codetype/code is network,
+the codesource/code may have zero or more codehost/code
+sub-elements used to specify the hosts to connect.
 span class=sinceSince 0.0.3; codetype='dir'/code since
 0.7.5; codetype='network'/code since 0.8.7/spanbr/
 For a file disk type which represents a cdrom or floppy
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 980410f..82062a3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1078,15 +1078,26 @@
   interleave
 optional
   element name=source
-attribute name=protocol
-  choice
-valuenbd/value
-valuerbd/value
-valuesheepdog/value
-valuegluster/value
-valueiscsi/value
-  /choice
-/attribute
+choice
+  attribute name=protocol
+choice
+  valuenbd/value
+  valuerbd/value
+  valuesheepdog/value
+  valuegluster/value
+/choice
+  /attribute
+  group
+attribute name=protocol
+  valueiscsi/value
+/attribute
+optional
+  attribute name=unit
+data type=integer/
+  /attribute
+/optional
+  /group
+/choice
 optional
   attribute name=name/
 /optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b42c79c..b801239 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3950,6 +3950,15 @@ virDomainDiskDefParseXML(virCapsPtr caps,
protocol);
 goto error;
 }
+if (def-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) {
+if (virXMLPropString(cur, unit) 
+virXPathUInt(string(./source/@unit),
+ ctxt, def-unit)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(invalid logical unit number));
+goto error;
+}
+}
 if (!(source = virXMLPropString(cur, name)) 
 def-protocol != VIR_DOMAIN_DISK_PROTOCOL_NBD) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
@@ -12555,6 +12564,9 @@ virDomainDiskDefFormat(virBufferPtr buf,
 if (def-src) {
 virBufferEscapeString(buf,  name='%s', def-src);
 }
+if (def-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI  def-unit) {
+virBufferAsprintf(buf,  unit='%u', def-unit);
+}
 if (def-nhosts == 0) {
 virBufferAddLit(buf, /\n);
 } else {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 7cd0264..ac8a446 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -606,6 +606,8 @@ struct 

[libvirt] [PATCH 01/13] qemu: fix use-after-free when parsing NBD disk

2013-02-25 Thread Paolo Bonzini
disk-src is still used for disks-hosts-name, do not free it.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 src/qemu/qemu_command.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index dee493f..5dccaae 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8707,7 +8707,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
 disk-hosts-port = strdup(port);
 if (!disk-hosts-port)
 goto no_memory;
-VIR_FREE(disk-src);
 disk-src = NULL;
 break;
 case VIR_DOMAIN_DISK_PROTOCOL_RBD:
-- 
1.8.1.2


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


[libvirt] [PATCH 06/13] qemu: support URI syntax for NBD

2013-02-25 Thread Paolo Bonzini
QEMU 1.3 and newer support an alternative URI-based syntax to specify
the location of an NBD server.  Libvirt can keep on using the old
syntax in general, but only the URI syntax supports IPv6 addresses.

The URI syntax also supports relative paths to Unix sockets.  These
should never be used but aren't explicitly blocked either by the parser,
so support it just in case.

The URI syntax is intentionally compatible with Gluster's, and the
code can be reused.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 src/qemu/qemu_command.c| 97 +++---
 tests/qemuargv2xmltest.c   |  2 +
 ...ml2argv-disk-drive-network-nbd-ipv6-export.args |  5 ++
 ...xml2argv-disk-drive-network-nbd-ipv6-export.xml | 33 
 .../qemuxml2argv-disk-drive-network-nbd-ipv6.args  |  5 ++
 .../qemuxml2argv-disk-drive-network-nbd-ipv6.xml   | 33 
 tests/qemuxml2argvtest.c   |  4 +
 tests/qemuxml2xmltest.c|  2 +
 8 files changed, 153 insertions(+), 28 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 89cd065..733d3bf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2050,35 +2050,36 @@ no_memory:
 }
 
 static int
-qemuParseGlusterString(virDomainDiskDefPtr def)
+qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri,
+const char *scheme)
 {
 int ret = -1;
 char *transp = NULL;
 char *sock = NULL;
 char *volimg = NULL;
-virURIPtr uri = NULL;
-
-if (!(uri = virURIParse(def-src))) {
-return -1;
-}
 
 if (VIR_ALLOC(def-hosts)  0)
 goto no_memory;
 
-if (STREQ(uri-scheme, gluster)) {
+transp = strchr(uri-scheme, '+');
+if (transp)
+*transp++ = 0;
+
+if (!STREQ(uri-scheme, scheme)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Invalid transport/scheme '%s'), uri-scheme);
+goto error;
+}
+
+if (!transp) {
 def-hosts-transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
-} else if (STRPREFIX(uri-scheme, gluster+)) {
-transp = strchr(uri-scheme, '+') + 1;
+} else {
 def-hosts-transport = 
virDomainDiskProtocolTransportTypeFromString(transp);
 if (def-hosts-transport  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(Invalid gluster transport type '%s'), transp);
+   _(Invalid %s transport type '%s'), scheme, 
transp);
 goto error;
 }
-} else {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(Invalid transport/scheme '%s'), uri-scheme);
-goto error;
 }
 def-nhosts = 0; /* set to 1 once everything succeeds */
 
@@ -2105,11 +2106,16 @@ qemuParseGlusterString(virDomainDiskDefPtr def)
 }
 }
 }
-volimg = uri-path + 1; /* skip the prefix slash */
-VIR_FREE(def-src);
-def-src = strdup(volimg);
-if (!def-src)
-goto no_memory;
+if (uri-path) {
+volimg = uri-path + 1; /* skip the prefix slash */
+VIR_FREE(def-src);
+def-src = strdup(volimg);
+if (!def-src)
+goto no_memory;
+} else {
+VIR_FREE(def-src);
+def-src = NULL;
+}
 
 def-nhosts = 1;
 ret = 0;
@@ -2128,12 +2134,31 @@ error:
 }
 
 static int
+qemuParseGlusterString(virDomainDiskDefPtr def)
+{
+virURIPtr uri = NULL;
+
+if (!(uri = virURIParse(def-src)))
+return -1;
+
+return qemuParseDriveURIString(def, uri, gluster);
+}
+
+static int
 qemuParseNBDString(virDomainDiskDefPtr disk)
 {
 virDomainDiskHostDefPtr h = NULL;
 char *host, *port;
 char *src;
 
+virURIPtr uri = NULL;
+
+if (strstr(disk-src, ://)) {
+uri = virURIParse(disk-src);
+if (uri)
+return qemuParseDriveURIString(disk, uri, nbd);
+}
+
 if (VIR_ALLOC(h)  0)
 goto no_memory;
 
@@ -2190,7 +2215,8 @@ error:
 }
 
 static int
-qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
+qemuBuildDriveURIString(virDomainDiskDefPtr disk, virBufferPtr opt,
+const char *scheme)
 {
 int ret = -1;
 int port = 0;
@@ -2204,18 +2230,18 @@ qemuBuildGlusterString(virDomainDiskDefPtr disk, 
virBufferPtr opt)
 };
 
 if (disk-nhosts != 1) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(gluster accepts only one host));
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(%s accepts only one host), scheme);
 

[libvirt] [PATCH] qemu: Don't fail to shutdown domains with unresponsive agent

2013-02-25 Thread Michal Privoznik
Currently, qemuDomainShutdownFlags() chooses the agent method of
shutdown whenever the agent is configured. However, this
assumption is not enough as the guest agent may be unresponsive
at the moment. So unless guest agent method has been explicitly
requested, we should fall back to the ACPI method.
---
 src/qemu/qemu_driver.c | 34 --
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 435c37c..06b14ae 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1702,7 +1702,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, 
unsigned int flags) {
 virDomainObjPtr vm;
 int ret = -1;
 qemuDomainObjPrivatePtr priv;
-bool useAgent = false;
+bool useAgent = false, agentRequested;
 
 virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
   VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1);
@@ -1719,23 +1719,32 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, 
unsigned int flags) {
 goto cleanup;
 
 priv = vm-privateData;
+agentRequested = flags  VIR_DOMAIN_SHUTDOWN_GUEST_AGENT;
 
-if ((flags  VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) ||
+if (agentRequested ||
 (!(flags  VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) 
  priv-agent))
 useAgent = true;
 
 if (useAgent) {
 if (priv-agentError) {
-virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s,
-   _(QEMU guest agent is not 
- available due to an error));
-goto cleanup;
+if (agentRequested) {
+virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s,
+   _(QEMU guest agent is not 
+ available due to an error));
+goto cleanup;
+} else {
+useAgent = false;
+}
 }
 if (!priv-agent) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
-   _(QEMU guest agent is not configured));
-goto cleanup;
+if (agentRequested) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+   _(QEMU guest agent is not configured));
+goto cleanup;
+} else {
+useAgent = false;
+}
 }
 }
 
@@ -1752,7 +1761,12 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, 
unsigned int flags) {
 qemuDomainObjEnterAgent(vm);
 ret = qemuAgentShutdown(priv-agent, QEMU_AGENT_SHUTDOWN_POWERDOWN);
 qemuDomainObjExitAgent(vm);
-} else {
+}
+
+/* If we are not enforced to use just an agent, try ACPI
+ * shutdown as well in case agent did not succeed */
+if (!useAgent ||
+((ret  0)  !agentRequested)) {
 qemuDomainSetFakeReboot(driver, vm, false);
 
 qemuDomainObjEnterMonitor(driver, vm);
-- 
1.8.1.4

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


[libvirt] [PATCH 03/13] qemu: rewrite NBD command-line builder and parser

2013-02-25 Thread Paolo Bonzini
Move the code to an external function, and structure it to prepare
the addition of new features in the next few patches.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 src/qemu/qemu_command.c | 128 
 tests/qemuxml2xmltest.c |   1 +
 2 files changed, 76 insertions(+), 53 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a3c5a4e..beb7cfe 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2128,6 +2128,45 @@ error:
 }
 
 static int
+qemuParseNBDString(virDomainDiskDefPtr disk)
+{
+virDomainDiskHostDefPtr h = NULL;
+char *host, *port;
+
+if (VIR_ALLOC(h)  0)
+goto no_memory;
+
+host = disk-src + strlen(nbd:);
+port = strchr(host, ':');
+if (!port) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(cannot parse nbd filename '%s'), disk-src);
+goto error;
+}
+
+*port++ = '\0';
+h-name = strdup(host);
+if (!h-name)
+goto no_memory;
+
+h-port = strdup(port);
+if (!h-port)
+goto no_memory;
+
+VIR_FREE(disk-src);
+disk-nhosts = 1;
+disk-hosts = h;
+return 0;
+
+no_memory:
+virReportOOMError();
+error:
+virDomainDiskHostDefFree(h);
+VIR_FREE(h);
+return -1;
+}
+
+static int
 qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt)
 {
 int ret = -1;
@@ -2188,6 +2227,36 @@ no_memory:
 goto cleanup;
 }
 
+static int
+qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt)
+{
+const char *transp;
+
+if (disk-nhosts != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(nbd accepts only one host));
+return -1;
+}
+
+virBufferAddLit(opt, file=nbd:);
+
+switch (disk-hosts-transport) {
+case VIR_DOMAIN_DISK_PROTO_TRANS_TCP:
+if (disk-hosts-name)
+virBufferEscape(opt, ',', ,, %s, disk-hosts-name);
+virBufferEscape(opt, ',', ,, :%s,
+disk-hosts-port ? disk-hosts-port : 10809);
+break;
+default:
+transp = 
virDomainDiskProtocolTransportTypeToString(disk-hosts-transport);
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(nbd does not support transport '%s'), transp);
+break;
+}
+
+return 0;
+}
+
 char *
 qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
   virDomainDiskDefPtr disk,
@@ -2314,13 +2383,9 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
 } else if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
 switch (disk-protocol) {
 case VIR_DOMAIN_DISK_PROTOCOL_NBD:
-if (disk-nhosts != 1) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(NBD accepts only one host));
+if (qemuBuildNBDString(disk, opt)  0)
 goto error;
-}
-virBufferAsprintf(opt, file=nbd:%s:%s,,
-  disk-hosts-name, disk-hosts-port);
+virBufferAddChar(opt, ',');
 break;
 case VIR_DOMAIN_DISK_PROTOCOL_RBD:
 virBufferAddLit(opt, file=);
@@ -7337,39 +7402,11 @@ qemuParseCommandLineDisk(virCapsPtr qemuCaps,
 if (STRPREFIX(def-src, /dev/))
 def-type = VIR_DOMAIN_DISK_TYPE_BLOCK;
 else if (STRPREFIX(def-src, nbd:)) {
-char *host, *port;
-
 def-type = VIR_DOMAIN_DISK_TYPE_NETWORK;
 def-protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD;
-host = def-src + strlen(nbd:);
-port = strchr(host, ':');
-if (!port) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(cannot parse nbd filename '%s'),
-   def-src);
-goto error;
-}
-*port++ = '\0';
-if (VIR_ALLOC(def-hosts)  0) {
-virReportOOMError();
-goto error;
-}
-def-nhosts = 1;
-def-hosts-name = strdup(host);
-if (!def-hosts-name) {
-virReportOOMError();
-goto error;
-}
-def-hosts-port = strdup(port);
-if (!def-hosts-port) {
-virReportOOMError();
-goto error;
-}
-def-hosts-transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
-def-hosts-socket = NULL;
 
-VIR_FREE(def-src);
-def-src = NULL;
+if (qemuParseNBDString(def)  0)
+goto error;
 } else if 

[libvirt] [PATCH 11/13] secret: add iscsi to possible usage types

2013-02-25 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 docs/formatsecret.html.in| 12 
 docs/schemas/secret.rng  | 10 ++
 include/libvirt/libvirt.h.in |  1 +
 src/conf/secret_conf.c   | 22 +-
 src/conf/secret_conf.h   |  1 +
 src/secret/secret_driver.c   |  8 
 6 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in
index 01aff2d..c3c4a25 100644
--- a/docs/formatsecret.html.in
+++ b/docs/formatsecret.html.in
@@ -66,6 +66,18 @@
   device/a. span class=sinceSince 0.9.7/span.
 /p
 
+h3Usage type iscsi/h3
+
+p
+  This secret is associated with an iSCSI target for CHAP authentication.
+  The codelt;usage type='iscsi'gt;/code element must contain
+  a single codetarget/code element that specifies a usage name
+  for the secret.  The iSCSI secret can then be used by UUID or by
+  this usage name via the codelt;authgt;/code element of
+  a a href=domain.html#elementsDisksdisk
+  device/a. span class=sinceSince 1.0.4/span.
+/p
+
 h2a name=exampleExample/a/h2
 
 pre
diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng
index e49bd5a..d7b8f83 100644
--- a/docs/schemas/secret.rng
+++ b/docs/schemas/secret.rng
@@ -41,6 +41,7 @@
 choice
   ref name='usagevolume'/
   ref name='usageceph'/
+  ref name='usageiscsi'/
   !-- More choices later --
 /choice
   /element
@@ -67,4 +68,13 @@
 /element
   /define
 
+  define name='usageiscsi'
+attribute name='type'
+  valueiscsi/value
+/attribute
+element name='target'
+  ref name='genericName'/
+/element
+  /define
+
 /grammar
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index ad30cd0..3f98ebe 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3641,6 +3641,7 @@ typedef enum {
 VIR_SECRET_USAGE_TYPE_NONE = 0,
 VIR_SECRET_USAGE_TYPE_VOLUME = 1,
 VIR_SECRET_USAGE_TYPE_CEPH = 2,
+VIR_SECRET_USAGE_TYPE_ISCSI = 3,
 
 #ifdef VIR_ENUM_SENTINELS
 VIR_SECRET_USAGE_TYPE_LAST
diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c
index 891af65..06b9bb2 100644
--- a/src/conf/secret_conf.c
+++ b/src/conf/secret_conf.c
@@ -36,7 +36,7 @@
 #define VIR_FROM_THIS VIR_FROM_SECRET
 
 VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_LAST,
-  none, volume, ceph)
+  none, volume, ceph, iscsi)
 
 void
 virSecretDefFree(virSecretDefPtr def)
@@ -57,6 +57,10 @@ virSecretDefFree(virSecretDefPtr def)
 VIR_FREE(def-usage.ceph);
 break;
 
+case VIR_SECRET_USAGE_TYPE_ISCSI:
+VIR_FREE(def-usage.target);
+break;
+
 default:
 VIR_ERROR(_(unexpected secret usage type %d), def-usage_type);
 break;
@@ -108,6 +112,15 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt,
 }
 break;
 
+case VIR_SECRET_USAGE_TYPE_ISCSI:
+def-usage.target = virXPathString(string(./usage/target), ctxt);
+if (!def-usage.target) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Ceph usage specified, but target is missing));
+return -1;
+}
+break;
+
 default:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(unexpected secret usage type %d),
@@ -262,6 +275,13 @@ virSecretDefFormatUsage(virBufferPtr buf,
 }
 break;
 
+case VIR_SECRET_USAGE_TYPE_ISCSI:
+if (def-usage.target != NULL) {
+virBufferEscapeString(buf, target%s/target\n,
+  def-usage.target);
+}
+break;
+
 default:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(unexpected secret usage type %d),
diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h
index 6079d5b..53517f9 100644
--- a/src/conf/secret_conf.h
+++ b/src/conf/secret_conf.h
@@ -39,6 +39,7 @@ struct _virSecretDef {
 union {
 char *volume;   /* May be NULL */
 char *ceph;
+char *target;
 } usage;
 };
 
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 5be33b9..c577817 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -149,6 +149,11 @@ secretFindByUsage(virSecretDriverStatePtr driver, int 
usageType, const char *usa
 if (STREQ(s-def-usage.ceph, usageID))
 return s;
 break;
+
+case VIR_SECRET_USAGE_TYPE_ISCSI:
+if (STREQ(s-def-usage.target, usageID))
+return s;
+break;
 }
 }
 return NULL;
@@ -614,6 +619,9 @@ secretUsageIDForDef(virSecretDefPtr def)
 case VIR_SECRET_USAGE_TYPE_CEPH:
 return def-usage.ceph;
 
+case VIR_SECRET_USAGE_TYPE_ISCSI:
+return def-usage.target;
+
 default:
 return 

[libvirt] [PATCH 14/13] qemu: support passthrough for iscsi disks

2013-02-25 Thread Paolo Bonzini
This enables usage of commands like persistent reservations.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 src/libvirt_private.syms   |  1 +
 src/qemu/qemu_command.c|  9 ++-
 .../qemuxml2argv-disk-drive-network-iscsi-lun.args |  1 +
 .../qemuxml2argv-disk-drive-network-iscsi-lun.xml  | 28 ++
 tests/qemuxml2argvtest.c   |  4 
 5 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f399871..8acff1a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -156,6 +156,7 @@ virDomainDiskIoTypeToString;
 virDomainDiskPathByName;
 virDomainDiskProtocolTransportTypeFromString;
 virDomainDiskProtocolTransportTypeToString;
+virDomainDiskProtocolTypeToString;
 virDomainDiskRemove;
 virDomainDiskRemoveByName;
 virDomainDiskTypeFromString;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 30ddbd3..22e9d81 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2844,7 +2844,14 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
bus);
 goto error;
 }
-if (disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK) {
+if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
+if (disk-protocol != VIR_DOMAIN_DISK_PROTOCOL_ISCSI) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(disk device='lun' is not supported for 
protocol='%s'),
+   
virDomainDiskProtocolTypeToString(disk-protocol));
+goto error;
+}
+} else if (disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(disk device='lun' is not supported for 
type='%s'),
virDomainDiskTypeToString(disk-type));
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args
new file mode 100644
index 000..baa7760
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args
@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M 
pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor 
unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device 
virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 -usb -drive 
file=iscsi://example.org:3260/iqn.1992-01.com.example,if=none,id=drive-scsi0-0-0-0,format=raw
 -device 
scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml
new file mode 100644
index 000..b6d6aaa
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml
@@ -0,0 +1,28 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219136/memory
+  currentMemory unit='KiB'219136/currentMemory
+  vcpu placement='static'1/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='network' device='lun'
+  driver name='qemu' type='raw'/
+  source protocol='iscsi' name='iqn.1992-01.com.example'
+host name='example.org' port='3260'/
+  /source
+  target dev='sda' bus='scsi'/
+/disk
+controller type='usb' index='0'/
+controller type='scsi' model='virtio-scsi'/
+memballoon model='none'/
+  /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0afecf3..5e0d976 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -497,6 +497,10 @@ mymain(void)
 QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT);
 DO_TEST(disk-drive-network-iscsi-auth,
 QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT);
+DO_TEST(disk-drive-network-iscsi-lun,
+QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE_FORMAT,
+QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI_PCI,
+QEMU_CAPS_VIRTIO_BLK_SG_IO, QEMU_CAPS_SCSI_BLOCK);
 DO_TEST(disk-drive-network-gluster,
 QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT);
 DO_TEST(disk-drive-network-rbd,
-- 
1.8.1.2

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


[libvirt] [PATCHv2] network: add force attribute for dhcp options

2013-02-25 Thread Laine Stump
If a dhcp option has force='yes', the dhcp server will send that
option back to every client regardless of whether or not the client
requests that option (without force='yes', an option will only be
sent to those clients that ask for it in their request packet). For example:

  option number='40' value='libvirt' force='yes'/

This information is relayed to dnsmasq by using the
dhcp-option-force option, rather than dhcp-option.
---
Changes from V1:
* add forgotten addition to RNG
* add forgotten addition to virNetworkIpDefFormat
* add networkxml2xml test (which would have caught the above)

 docs/formatnetwork.html.in | 21 +
 docs/schemas/network.rng   |  8 
 src/conf/network_conf.c| 16 
 src/conf/network_conf.h|  3 ++-
 src/network/bridge_driver.c|  3 ++-
 tests/networkxml2confdata/nat-network.conf |  2 +-
 tests/networkxml2confdata/nat-network.xml  |  2 +-
 tests/networkxml2xmlin/nat-network.xml |  5 +
 tests/networkxml2xmlout/nat-network.xml|  5 +
 9 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 41a83fa..97e3bd4 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -557,6 +557,10 @@
 lt;range start=192.168.122.100 end=192.168.122.254 /gt;
 lt;host mac=00:16:3e:77:e2:ed name=foo.example.com 
ip=192.168.122.10 /gt;
 lt;host mac=00:16:3e:3e:a9:1a name=bar.example.com 
ip=192.168.122.11 /gt;
+lt;option number='252' value='\n'/gt;
+lt;option number='4' value='192.168.122.11'/gt;
+lt;option number='6' value='192.168.145.23'/gt;
+lt;option number='23' value='64' force='yes'/gt;
   lt;/dhcpgt;
 lt;/ipgt;
 lt;ip family=ipv6 address=2001:db8:ca2:2::1 prefix=64 /gt;
@@ -699,6 +703,23 @@
 for all address ranges and statically assigned addresses.span
 class=sinceSince 0.7.1 (codeserver/code since 
0.7.3)./span
   /dd
+  dtcodeoption/code/dt
+  ddThe optional codeoption/code element (which can
+be repeated for multiple DHCP options) specifies
+generic DHCP options (as defined in RFC 2132, or later
+RFCs in some cases) that will be sent by the DHCP
+server to requesting clients (IPv4 only).  There are
+three possible attributes: codenumber/code is
+mandatory and gives the standard option
+number; codevalue/code is optional and gives the
+value of that option to be provided to the client
+codeforce/code is also optional, and defaults to
+no; if codeforce/code is yes, the option will
+be sent to all DHCP clients regardless of whether or
+not the client requests it (usually only options
+specifically requested by the client are sent in the
+DHCP response).span class=sinceSince 1.0.3/span
+  /dd
 /dl
   /dd
 /dl
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index da7d8ad..ed6964a 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -305,6 +305,14 @@
   element name=option
 attribute name=numberref 
name=uint8range//attribute
 attribute name=valuetext//attribute
+optional
+  attribute name=force
+choice
+  valueyes/value
+  valueno/value
+/choice
+  /attribute
+/optional
   /element
 /zeroOrMore
   /element
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 8b57242..4d07bac 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -796,6 +796,7 @@ virNetworkDHCPOptionDefParseXML(const char *networkName,
 virNetworkDHCPOptionDefPtr option)
 {
 char *number = NULL;
+char *force = NULL;
 int ret = -1;
 
 if (!(number = virXMLPropString(node, number))) {
@@ -811,12 +812,25 @@ virNetworkDHCPOptionDefParseXML(const char *networkName,
number);
 goto cleanup;
 }
+
 option-value = virXMLPropString(node, value);
 
+if ((force = virXMLPropString(node, force))) {
+   if (STRCASEEQ(force, yes)) {
+  option-force = true;
+   } else if (STRCASENEQ(force, no)) {
+   virReportError(VIR_ERR_XML_ERROR,
+  _(Invalid option force attribute %s 
+in network '%s'), force, networkName);
+   goto cleanup;
+   }
+}
+
 ret = 0;
 
 cleanup:
 

Re: [libvirt] [PATCH] qemu: Don't fail to shutdown domains with unresponsive agent

2013-02-25 Thread Eric Blake
On 02/25/2013 10:55 AM, Michal Privoznik wrote:
 Currently, qemuDomainShutdownFlags() chooses the agent method of
 shutdown whenever the agent is configured. However, this
 assumption is not enough as the guest agent may be unresponsive
 at the moment. So unless guest agent method has been explicitly
 requested, we should fall back to the ACPI method.
 ---
  src/qemu/qemu_driver.c | 34 --
  1 file changed, 24 insertions(+), 10 deletions(-)

When Daniel added two new shutdown methods for LXC, I had the question
on whether shutdown methods should be mutually exclusive, or a bitmask
of all permissible things to attempt (where passing 0 leaves the
attempts up the hypervisor).  The consensus was that allowing the user
to pass more than one method makes sense (although the hypervisor then
gets to choose method priorities).  I'm not sure your logic matches the
goal of allowing the user to request both agent and acpi at the same time.

 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 435c37c..06b14ae 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -1702,7 +1702,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, 
 unsigned int flags) {
  virDomainObjPtr vm;
  int ret = -1;
  qemuDomainObjPrivatePtr priv;
 -bool useAgent = false;
 +bool useAgent = false, agentRequested;
  
  virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1);
 @@ -1719,23 +1719,32 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, 
 unsigned int flags) {
  goto cleanup;
  
  priv = vm-privateData;
 +agentRequested = flags  VIR_DOMAIN_SHUTDOWN_GUEST_AGENT;
  
 -if ((flags  VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) ||
 +if (agentRequested ||
  (!(flags  VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) 
   priv-agent))
  useAgent = true;

A better logic might be:

/* Explicitly requested agent, or no requests made and agent seems
possible */
if (agentRequested ||
(!flags  priv-agent))

  
  if (useAgent) {
  if (priv-agentError) {
 -virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s,
 -   _(QEMU guest agent is not 
 - available due to an error));
 -goto cleanup;
 +if (agentRequested) {
 +virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s,
 +   _(QEMU guest agent is not 
 + available due to an error));
 +goto cleanup;
 +} else {
 +useAgent = false;
 +}
  }

Here, if there is an agent error, but the user requested both agent and
acpi flags at the same time, then you want to fall back to acpi instead
of erroring out completely.

  if (!priv-agent) {
 -virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
 -   _(QEMU guest agent is not configured));
 -goto cleanup;
 +if (agentRequested) {
 +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
 +   _(QEMU guest agent is not configured));
 +goto cleanup;
 +} else {
 +useAgent = false;
 +}
  }
  }
  
 @@ -1752,7 +1761,12 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, 
 unsigned int flags) {
  qemuDomainObjEnterAgent(vm);
  ret = qemuAgentShutdown(priv-agent, QEMU_AGENT_SHUTDOWN_POWERDOWN);
  qemuDomainObjExitAgent(vm);
 -} else {
 +}
 +
 +/* If we are not enforced to use just an agent, try ACPI
 + * shutdown as well in case agent did not succeed */
 +if (!useAgent ||
 +((ret  0)  !agentRequested)) {

Redundant () around ret0.

Here, the fallback seems like it should be:

/* Get here if agent request failed or was not attempted.  Try acpi
unless it was excluded from an explicit request */
if (ret  0 
(!flags || (flags  VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN))) {


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH 4/4] storage: fix uid_t|gid_t handling on 32 bit Linux

2013-02-25 Thread Eric Blake
On 02/22/2013 09:55 AM, Philipp Hahn wrote:
 uid_t and gid_t are opaque types, ranging from s32 to u32 to u64.
 
 Unfortunately libvirt uses the value -1 to mean current process, which
 on Linux32 gets converted to ALLONE := +(2^32-1) = 4294967295.

I like the standardized name INT_MAX better than the invented name ALLONE.

 
 Different libvirt versions used different formatting in the past, which
 break one or the other parsing function:
 virXPathLong(): (signed long)-1 != (double)ALLONE
 virXPathULong(): (unsigned long)-1 != (double)-1
 
 Roll our own version of virXPathULong(), which also accepts -1, which is
 silently converted to ALLONE.
 
 For output: do the reverse and print -1 instead of ALLONE.
 
 Signed-off-by: Philipp Hahn h...@univention.de
 ---
  src/conf/storage_conf.c |   74 
 ++-
  1 file changed, 60 insertions(+), 14 deletions(-)
 
 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index 9134a22..b267f00 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c
 @@ -665,7 +665,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
  const char *permxpath,
  int defaultmode) {
  char *mode;
 -long v;
 +double d;

Eww - why do we need to use a double?  Just parse to a 32-bit int, then
check for truncation after the fact (as I said elsewhere in the series,
we already do a compile-time verify that uid_t is not larger than int;
so the real problem is if uid_t is smaller than int).

  int ret = -1;
  xmlNodePtr relnode;
  xmlNodePtr node;
 @@ -699,26 +699,57 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
  VIR_FREE(mode);
  }
  
 +/* uid_t and gid_t are *opaque* types: on Linux they're u32, on Windows64
 + * they're u64, on Solaris they were s32 in the past. There may be 
 others.

Not accurate.  mingw64 has s64 pid_t (not u64); and completely lacks
uid_t and gid_t natively:

$ printf '#include sys/types.h\nuid_t\n' | \
x86_64-w64-mingw32-gcc -E -|grep id_t
typedef long long _pid_t;
typedef _pid_t pid_t;
uid_t

When using gnulib (as libvirt does), gnulib provides [ug]id_t as 32-bit
types even on mingw64.

Furthermore, a lot of the problems stem from the fact that s16 or u16
uid_t/gid_t have been used in the past (u16 promotes to s32 without sign
extension, so it does not compare equal to (s32)-1).

I like the idea of outputting -1 rather than the unsigned all-ones
value; which means you need to respin this patch to avoid testsuite
failures where we change the test output.  Also, anywhere that the
parser doesn't accept -1, we need to fix that; accepting -1 as the only
valid negative value and requiring all other values to be non-negative
makes sense.

Looking forward to v2; this patch is appropriate to get into the 1.0.3
release if we get it respun in time.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH tck] Avoid multicast MAC addrs in tests

2013-02-25 Thread Laine Stump
On 02/25/2013 11:52 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com

 Certain MAC addrs are used for multicast and recent libvirt
 will refuse to start VMs using those addrs.

 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  scripts/domain/215-nic-hotplug-many.t | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/scripts/domain/215-nic-hotplug-many.t 
 b/scripts/domain/215-nic-hotplug-many.t
 index 5b935cc..8b4afbd 100644
 --- a/scripts/domain/215-nic-hotplug-many.t
 +++ b/scripts/domain/215-nic-hotplug-many.t
 @@ -47,9 +47,9 @@ diag Creating a new transient domain;
  my $dom;
  ok_domain(sub { $dom = $conn-create_domain($xml) }, created transient 
 domain object);
  
 -my $mac1 = 01:11:22:33:44:55;
 -my $mac2 = 02:11:22:33:44:55;
 -my $mac3 = 03:11:22:33:44:55;
 +my $mac1 = 02:11:22:33:44:51;
 +my $mac2 = 02:11:22:33:44:52;
 +my $mac3 = 02:11:22:33:44:53;
  my $model = virtio;
  
  my $netxml1 = EOF;

ACK

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


Re: [libvirt] [PATCH 0/4] uid_t|gid_t fixes for 32 bit Linux

2013-02-25 Thread Eric Blake
On 02/22/2013 09:58 AM, Philipp Hahn wrote:
 @Guido: for Debian only the 4th patch is relevant, which applies with some
 re-indention.

Unfortunately, the 4th patch is the one that needs rework.

 
 Additional issues:
  - UID and GID 4294967295 happens to be the same as -1, so hopefully nobody
uses it. Should this be documented somewhere?

Unless someone ever invents a system with 64-bit uid_t (to date, no one
has - your claims about u64 to the contrary), treating 4294967295 as
(uid_t)-1 is safe.  POSIX already guaranteed that (uid_t)-1 cannot be
used as a valid user id.

  - Some regression test?

Did you run 'make check' on your patch?  We already have XML output that
is affected by the change in output format, and can write the test so
that we prove that multiple ways of formatting input all get
canonicalized to the same output.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCHv2] network: add force attribute for dhcp options

2013-02-25 Thread Eric Blake
On 02/25/2013 11:30 AM, Laine Stump wrote:
 If a dhcp option has force='yes', the dhcp server will send that
 option back to every client regardless of whether or not the client
 requests that option (without force='yes', an option will only be
 sent to those clients that ask for it in their request packet). For example:
 
   option number='40' value='libvirt' force='yes'/
 
 This information is relayed to dnsmasq by using the
 dhcp-option-force option, rather than dhcp-option.
 ---
 Changes from V1:
 * add forgotten addition to RNG
 * add forgotten addition to virNetworkIpDefFormat
 * add networkxml2xml test (which would have caught the above)

I'm sure there are useful symbolic names for the various options,
instead of just number='40'; but like you said, that can be added later
as needed.

It's borderline on whether to take this patch without support for option
names, since it was submitted prior to freeze but not reviewed until
after.  It is cleaning up the feature of option parsing that was just
barely applied prior to freeze, so that argues that it can be classed as
rounding out an incomplete feature.  Delaying until post-release would
let you get option names in the same release, but then we might want to
revert the option support that just went in.  Meanwhile, since both
this patch, and any future patch for adding option name support, deals
with just XML additions, they can easily be backported by distros
without a soname bump, regardless of whether they make this release or
not; and since it is just XML additions, I don't see how it could cause
regressions to existing domain XML.  I'm probably 60:40 in favor of
including this patch now, but if anyone else speaks clearly for or
against taking it in 1.0.3, I can be swayed.

 +  dtcodeoption/code/dt
 +  ddThe optional codeoption/code element (which can
 +be repeated for multiple DHCP options) specifies
 +generic DHCP options (as defined in RFC 2132, or later
 +RFCs in some cases) that will be sent by the DHCP
 +server to requesting clients (IPv4 only).  There are
 +three possible attributes: codenumber/code is
 +mandatory and gives the standard option
 +number; codevalue/code is optional and gives the
 +value of that option to be provided to the client
 +codeforce/code is also optional, and defaults to
 +no; if codeforce/code is yes, the option will
 +be sent to all DHCP clients regardless of whether or
 +not the client requests it (usually only options
 +specifically requested by the client are sent in the
 +DHCP response).span class=sinceSince 1.0.3/span

We definitely want the docs for option, even if we defer the force=
attribute until post-release.

 @@ -811,12 +812,25 @@ virNetworkDHCPOptionDefParseXML(const char *networkName,
 number);
  goto cleanup;
  }
 +
  option-value = virXMLPropString(node, value);
  
 +if ((force = virXMLPropString(node, force))) {
 +   if (STRCASEEQ(force, yes)) {

I don't know that we have done case-insensitive matching in many other
places; but being liberal in what we accept doesn't hurt my feelings.
On the other hand, the RNG rejects any uppercase variant.

ACK.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [Qemu-devel] [PATCH] vl.c: Support multiple CPU ranges on -numa option

2013-02-25 Thread Eric Blake
On 02/21/2013 01:57 PM, Eduardo Habkost wrote:

 Note that the following format, currently used by libvirt:

   -numa nodes,cpus=A,B,C,D

 will _not_ work yet, as , is the option separator for the command-line
 option parser, and it will require changing the -numa option parsing
 code to handle cpus as a special case.

 No way.
 
 Agreed.  :-)
 
 The bad news is that libvirt uses this format since forever, this format
 never worked, and nobody ever noticed that this was broken.

Well, libvirt just entered freeze for 1.0.3.  I think the best course of
action on libvirt's side is to patch 1.0.3 to flat-out reject any cpumap
that cannot be represented in a syntax understood by qemu 1.4; then a
future libvirt can re-add support for whatever new syntax qemu 1.5 deems
as appropriate.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCHv2 2/8] doc: schema: Add basic documentation for the virtual RNG device support

2013-02-25 Thread Eric Blake
On 02/25/2013 02:08 AM, Peter Krempa wrote:
 On 02/22/13 19:20, Eric Blake wrote:
 On 02/21/2013 07:47 AM, Peter Krempa wrote:
 This patch documents XML elements used for (basic) support of virtual
 RNG devices.


 For the slightly more advanced EGD backend:
devices
  rng model='virtio'
backend model='egd' type='udp'
  !-- this is a definition of a character device --
  source mode='bind' service='1234'/
  source mode='connect' host='1.2.3.4' service='1234'/
  !-- or other valid character device configuration --


 Do we really want two source in a single backend in the example, or
 would it be easier to show multiple rng devices, one for each type of
 backend?
 
 That actually is valid for the character device backends. The UDP
 backend has to use two separate sources for bi-directional
 communication. The definition of that source type is declared as a type
 in our RNG schema an I merely reused that.

Oh, I didn't realize that.  Do we want to ENFORCE that there are two
source elements present then?  Or can we always provide a sane default
when the user provides only 0 or 1 source?  Depending on that answer, it
might be worth additional documentation on why two source elements are
present, as well as mentioning that order is significant in how they are
listed.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [Qemu-devel] [PATCH] vl.c: Support multiple CPU ranges on -numa option

2013-02-25 Thread Andreas Färber
Am 21.02.2013 21:57, schrieb Eduardo Habkost:
 On Thu, Feb 21, 2013 at 09:23:22PM +0100, Markus Armbruster wrote:
 Eduardo Habkost ehabk...@redhat.com writes:

 This allows , to be used a separator between each CPU range.  Note
 that commas inside key=value command-line options have to be escaped
 using ,,, so the command-line will look like:

   -numa node,cpus=A,,B,,C,,D

 This is really, really ugly, and an embarrassment to document.  Which
 you didn't ;)
 
 I was trying to have an intermediate solution using the current -numa
 parser. I have patches in my queue that will change the code to properly
 use QemuOpts later.

Speaking of which, have you considered using QemuOpts for -cpu? Its
custom parsing code will probably not handle , escaping at all. ;)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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


Re: [libvirt] [PATCHv2 7/8] tests: Add tests for virtio-rng device handling

2013-02-25 Thread Eric Blake
On 02/25/2013 03:45 AM, Peter Krempa wrote:
 On 02/23/13 01:29, Eric Blake wrote:
 On 02/21/2013 07:47 AM, Peter Krempa wrote:
 Adds XML parsing and qemu commandline tests for the VirtIO RNG device
 support.
 ---


 Is it worth testing that a filename containing an XML-special character
 is properly escaped?  Other than that, this one is still good to go.

 +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
 @@ -0,0 +1,23 @@

 +rng model='virtio'
 +  backend model='random'/test/phile/backend

 That is, should this use something like /test/lt;phile as the XML
 encoded file name?

 
 Uh, I'm not following you on this one. You mean that if the user
 specifies some characters that are invalid from the perspective of XML
 as a source path?

We should not get in the way of a user doing:

ln -s /dev/urandom '/tmp/myevilrandom'

then passing:

backend model='random'/tmp/mylt;evilgt;random/backend

in their domain XML.  By using virBufferEscape on the output side, we
allow the user to use XML escapes on their input to specify any valid
file name.

 
 Anyways, I fixed the issues you pointed out in 1-6 and provided
 explanation for the other stuff. I'm pushing patches 1-7 (the test suite
 can be improved at any time) now and will follow up later with a
 improved version of 8 as well as with a patch that will allow multiple
 RNG devices. That should be better to review as slicing apart the
 existing patches.

Sure, doing things as followups is fine, since you already collected
ACKs before my late review.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] qemu: enable direct migration over IPv6

2013-02-25 Thread Eric Blake
On 02/25/2013 07:15 AM, Ján Tomko wrote:
 Use virURIParse in qemuMigrationPrepareDirect to allow parsing
 IPv6 addresses, which would cause an 'incorrect :port' error message
 before.
 
 To be able to migrate over IPv6, QEMU needs to listen on [::] instead
 of 0.0.0.0. This patch adds a call to getaddrinfo and sets the listen
 address based on the result.
 
 It also uses the same listen address for the NBD server.
 
 This will break migration if a hostname does not resolve to the same
 address family on both sides.
 
 Bug: https://bugzilla.redhat.com/show_bug.cgi?id=846013
 ---
 
 Diff to V1:
 * initialize uri_str
 * reuse STRSKIP(tcp:) result instead of doing strlen on it
 * print a warning instead of failing when the hostname can't be resolved
 
 Diff to V2:
 * freeaddrinfo
 * separate the listen address to allow reuse in qemuMigrationStartNBDServer
 
  src/qemu/qemu_migration.c | 77 
 ---
  1 file changed, 59 insertions(+), 18 deletions(-)
 
 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index cae58fa..ff9b959 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -22,7 +22,10 @@
  
  #include config.h
  
 +#include netdb.h
 +#include sys/socket.h
  #include sys/time.h
 +#include sys/types.h

You rarely ever need sys/types.h - most types in that header are
guaranteed to be provided by any other standard header that has an
interface using that type; or put another way, POSIX says that inclusion
of other headers may drag in sys/types.h, and we have already been
relying on that reliably happening.  The other two additions are
important, though.

  
 @@ -2292,9 +2296,14 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
  static int port = 0;
  int this_port;
  char *hostname = NULL;
 +char listenAddr[8];
  char migrateFrom [64];

As long as you are touching this, remove the space before [64].  Then
again, 64 was a magic number; a much safer approach would have been

char migrateFrom[sizeof(tcp:0.0.0.0:) + INT_BUFSIZE_BOUND(int)];

(using gnulib's intprops.h), or even getting rid of the stack-alloc
temporary (see below for thoughts about using virBufferPtr instead)

  
 +if (getaddrinfo(hostname, NULL, NULL, info)) {
 +VIR_WARN(unable to get address info for %s, defaulting to IPv4,
 + hostname);

Would it be nice to include the gai_strerror() reason for why
getaddrinfo() failed?

 +} else {
 +ipv6 = info-ai_family == AF_INET6;
 +freeaddrinfo(info);
 +}
 +
  if (*uri_out)
  VIR_DEBUG(Generated uri_out=%s, *uri_out);
  
 -/* QEMU will be started with -incoming tcp:0.0.0.0:port */
 -snprintf(migrateFrom, sizeof(migrateFrom), tcp:0.0.0.0:%d, this_port);
 +/* QEMU will be started with -incoming tcp:0.0.0.0:port
 + * or -incoming tcp:[::]:port for IPv6 */
 +if (ipv6)
 +snprintf(listenAddr, sizeof(listenAddr), [::]);
 +else
 +snprintf(listenAddr, sizeof(listenAddr), 0.0.0.0);

Eww.  Even though this is safe (because you sized listenAddr big
enough), I'd rather see:

const char *listenAddr;

if (ipv6)
listenAddr = [::];
else
listenAddr = 0.0.0.0;

 +
 +snprintf(migrateFrom, sizeof(migrateFrom),
 + tcp:%s:%d, listenAddr, this_port);

And while this snprintf was pre-existing and also happens to be large
enough, I can't help but think it would be nicer to pass a virBufferPtr
instead of a const char * to qemuMigrationPrepareAny, although that
would mean more refactoring done in an independent patch.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCHv2 2/8] doc: schema: Add basic documentation for the virtual RNG device support

2013-02-25 Thread Peter Krempa

On 02/25/13 22:01, Eric Blake wrote:

On 02/25/2013 02:08 AM, Peter Krempa wrote:

On 02/22/13 19:20, Eric Blake wrote:

On 02/21/2013 07:47 AM, Peter Krempa wrote:

This patch documents XML elements used for (basic) support of virtual
RNG devices.


For the slightly more advanced EGD backend:
devices
  rng model='virtio'
backend model='egd' type='udp'
  !-- this is a definition of a character device --
  source mode='bind' service='1234'/
  source mode='connect' host='1.2.3.4' service='1234'/
  !-- or other valid character device configuration --





Do we really want two source in a single backend in the example, or
would it be easier to show multiple rng devices, one for each type of
backend?


That actually is valid for the character device backends. The UDP
backend has to use two separate sources for bi-directional
communication. The definition of that source type is declared as a type
in our RNG schema an I merely reused that.


Oh, I didn't realize that.  Do we want to ENFORCE that there are two
source elements present then?  Or can we always provide a sane default
when the user provides only 0 or 1 source?  Depending on that answer, it
might be worth additional documentation on why two source elements are
present, as well as mentioning that order is significant in how they are
listed.

I reused the code to facilitate the operations done with character 
devices. There's present infrastructure [1] that allows to parse, format 
and create commandlines for qemu. The defaults for the syntax of the XML 
and the expected values should be documented in the section describing 
character devices. (But I did not really check if the description is 
exhaustive)


In hindsight I should have linked that section in the description of the 
egd backend model explicitly rather than by just mentioning it.


Peter


[1]: virDomainChrSourceDefParseXML, virDomainChrSourceDefFormat, 
qemuBuildChrChardevStr


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


[libvirt] [PATCH] conf: Avoid leaking of RNG device definition

2013-02-25 Thread Peter Krempa
---
Yet another place not guarded by the compiler :(

This should qualify as trivial, so I'm pushing it directly. 

 src/conf/domain_conf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3cb780d..b65e52a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1775,6 +1775,8 @@ void virDomainDefFree(virDomainDefPtr def)
 virDomainRedirdevDefFree(def-redirdevs[i]);
 VIR_FREE(def-redirdevs);

+virDomainRNGDefFree(def-rng);
+
 VIR_FREE(def-os.type);
 VIR_FREE(def-os.machine);
 VIR_FREE(def-os.init);
-- 
1.8.1.1

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


Re: [libvirt] [PATCHv2 2/8] doc: schema: Add basic documentation for the virtual RNG device support

2013-02-25 Thread Peter Krempa

On 02/25/13 22:20, Peter Krempa wrote:

On 02/25/13 22:01, Eric Blake wrote:

On 02/25/2013 02:08 AM, Peter Krempa wrote:

On 02/22/13 19:20, Eric Blake wrote:

On 02/21/2013 07:47 AM, Peter Krempa wrote:

This patch documents XML elements used for (basic) support of virtual
RNG devices.


For the slightly more advanced EGD backend:
devices
  rng model='virtio'
backend model='egd' type='udp'
  !-- this is a definition of a character device --
  source mode='bind' service='1234'/
  source mode='connect' host='1.2.3.4' service='1234'/
  !-- or other valid character device configuration --





Do we really want two source in a single backend in the example, or
would it be easier to show multiple rng devices, one for each type of
backend?


That actually is valid for the character device backends. The UDP
backend has to use two separate sources for bi-directional
communication. The definition of that source type is declared as a type
in our RNG schema an I merely reused that.


Oh, I didn't realize that.  Do we want to ENFORCE that there are two
source elements present then?  Or can we always provide a sane default
when the user provides only 0 or 1 source?  Depending on that answer, it
might be worth additional documentation on why two source elements are
present, as well as mentioning that order is significant in how they are
listed.


I reused the code to facilitate the operations done with character
devices. There's present infrastructure [1] that allows to parse, format
and create commandlines for qemu. The defaults for the syntax of the XML
and the expected values should be documented in the section describing
character devices. (But I did not really check if the description is
exhaustive)

In hindsight I should have linked that section in the description of the
egd backend model explicitly rather than by just mentioning it.


I actually did that:

backend type='egd'   (Hm there's a mistake here - s/type/model/, I'll 
fix that)


This backend connects to a source using the EGD protocol. The source is 
specified as a character device. Refer to character device host 
interface for more information.


(where character device host interface links to :
http://libvirt.org/formatdomain.html#elementsCharHostInterface )



Peter


[1]: virDomainChrSourceDefParseXML, virDomainChrSourceDefFormat,
qemuBuildChrChardevStr

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


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


[libvirt] [PATCH 0/4] Virtio-RNG followup

2013-02-25 Thread Peter Krempa
This series adds rate limiting and multiple device support and fixes
docs and adds test for XML entities and multiple RNG devices.

Peter Krempa (4):
  virtio-rng: allow multiple RNG devices
  docs: Fix attribute name for virtio-rng backend
  virtio-rng: Add rate limiting options for virtio-RNG
  tests: Test multiple RNG devices and support for XML entities in paths

 docs/formatdomain.html.in  | 14 +++-
 docs/schemas/domaincommon.rng  | 18 -
 src/conf/domain_conf.c | 78 +-
 src/conf/domain_conf.h |  6 +-
 src/qemu/qemu_command.c| 28 +---
 .../qemuxml2argv-virtio-rng-random.args|  2 +-
 .../qemuxml2argv-virtio-rng-random.xml |  4 ++
 7 files changed, 106 insertions(+), 44 deletions(-)

-- 
1.8.1.1

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


[libvirt] [PATCH 3/4] virtio-rng: Add rate limiting options for virtio-RNG

2013-02-25 Thread Peter Krempa
Qemu's implementation of virtio RNG supports rate limiting of the
entropy used. This patch exposes the option to tune this fucntionality.

This patch is based on qemu commit 904d6f588063fb5ad2b61998acdf1e73fb4

The rate limiting is exported in the XML as:
devices
  ...
  rng model='virtio'
rate period='1234'4321/rate
backend model='random'/
  /rng
  ...
---

Notes:
This series would benefit from the per-driver XML parsing checks to verify
that rate  8bits, otherwise it will be rounded down to 0 bytes. I will
follow up with that change as soon as the per-driver callbacks get in.

Version 3:
- State the time unit in docs
Version 2:
- Qemu uses bytes/period, adapt the value according to that

 docs/formatdomain.html.in  | 10 ++
 docs/schemas/domaincommon.rng  | 18 +-
 src/conf/domain_conf.c | 17 +
 src/conf/domain_conf.h |  2 ++
 src/qemu/qemu_command.c|  9 +
 .../qemuxml2argv-virtio-rng-random.args|  2 +-
 .../qemuxml2argv-virtio-rng-random.xml |  1 +
 7 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 2a60f61..220884c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4294,6 +4294,7 @@ qemu-kvm -net nic,model=? /dev/null
   ...
   lt;devicesgt;
 lt;rng model='virtio'gt;
+  lt;rate period=2000gt;1234lt;/rategt;
   lt;backend model='random'gt;/dev/randomlt;/backendgt;
   lt;!-- OR --gt;
   lt;backend model='egd' type='udp'gt;
@@ -4316,6 +4317,15 @@ qemu-kvm -net nic,model=? /dev/null
   li'virtio' mdash; supported by qemu and virtio-rng kernel 
module/li
 /ul
   /dd
+  dtcoderate/code/dt
+  dd
+p
+  The rate element allows to limit the rate that the entropy can be
+  read from the source. The value is in bits that the device is allowed
+  to read in the selected period. The period is represented in 
miliseconds.
+  The default period is 1000ms or 1 second.
+/p
+  /dd
   dtcodebackend/code/dt
   dd
 p
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 8330a50..da53095 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3522,7 +3522,12 @@
   valuevirtio/value
 /choice
   /attribute
-  ref name=rng-backend/
+  interleave
+ref name=rng-backend/
+optional
+  ref name=rng-rate/
+/optional
+  /interleave
 /element
   /define

@@ -3546,6 +3551,17 @@
 /element
   /define

+  define name=rng-rate
+element name=rate
+  optional
+attribute name=period
+  ref name=positiveInteger/
+/attribute
+  /optional
+  ref name=positiveInteger/
+/element
+  /define
+
   define name=usbmaster
 element name=master
   attribute name=startport
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 40eded6..0e2f1a9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7506,6 +7506,17 @@ virDomainRNGDefParseXML(const xmlNodePtr node,

 ctxt-node = node;

+if (virXPathUInt(string(./rate), ctxt, def-rate)  -1) {
+virReportError(VIR_ERR_XML_ERROR, %s, _(invalid RNG rate value));
+goto error;
+}
+
+if (def-rate  0 
+virXPathUInt(string(./rate/@period), ctxt, def-period)  -1) {
+virReportError(VIR_ERR_XML_ERROR, %s, _(invalid RNG period value));
+goto error;
+}
+
 if ((nbackends = virXPathNodeSet(./backend, ctxt, backends))  0)
 goto error;

@@ -13812,6 +13823,12 @@ virDomainRNGDefFormat(virBufferPtr buf,
 const char *backend = virDomainRNGBackendTypeToString(def-backend);

 virBufferAsprintf(buf, rng model='%s'\n, model);
+if (def-rate) {
+virBufferAddLit(buf,   rate);
+if (def-period)
+virBufferAsprintf(buf,  period='%u', def-period);
+virBufferAsprintf(buf, %u/rate\n, def-rate);
+}
 virBufferAsprintf(buf,   backend model='%s', backend);

 switch ((enum virDomainRNGBackend) def-backend) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5dc3400..92130f0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1736,6 +1736,8 @@ enum virDomainRNGBackend {
 struct _virDomainRNGDef {
 int model;
 int backend;
+unsigned int rate;
+unsigned int period;

 union {
 char *file; /* file name for 'random' source */
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9270258..4899ccf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4256,6 +4256,15 @@ qemuBuildRNGDeviceArgs(virCommandPtr cmd,

 virBufferAsprintf(buf, 

[libvirt] [PATCH 1/4] virtio-rng: allow multiple RNG devices

2013-02-25 Thread Peter Krempa
qemu supports adding multiple RNG devices. This patch allows libvirt to
support this.
---
 src/conf/domain_conf.c  | 61 +
 src/conf/domain_conf.h  |  4 +++-
 src/qemu/qemu_command.c | 19 +++
 3 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b65e52a..40eded6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1775,7 +1775,9 @@ void virDomainDefFree(virDomainDefPtr def)
 virDomainRedirdevDefFree(def-redirdevs[i]);
 VIR_FREE(def-redirdevs);

-virDomainRNGDefFree(def-rng);
+for (i = 0; i  def-nrngs; i++)
+virDomainRNGDefFree(def-rngs[i]);
+VIR_FREE(def-rngs);

 VIR_FREE(def-os.type);
 VIR_FREE(def-os.machine);
@@ -2357,10 +2359,10 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def,
 if (cb(def, device, def-memballoon-info, opaque)  0)
 return -1;
 }
-if (def-rng) {
-device.type = VIR_DOMAIN_DEVICE_RNG;
-device.data.rng = def-rng;
-if (cb(def, device, def-rng-info, opaque)  0)
+device.type = VIR_DOMAIN_DEVICE_RNG;
+for (i = 0; i  def-nrngs; i++) {
+device.data.rng = def-rngs[i];
+if (cb(def, device, def-rngs[i]-info, opaque)  0)
 return -1;
 }
 device.type = VIR_DOMAIN_DEVICE_HUB;
@@ -10733,21 +10735,21 @@ virDomainDefParseXML(virCapsPtr caps,
 }
 }

-/* Parse the RNG device */
+/* Parse the RNG devices */
 if ((n = virXPathNodeSet(./devices/rng, ctxt, nodes))  0)
 goto error;
-
-if (n  1) {
-virReportError(VIR_ERR_XML_ERROR, %s,
-   _(only a single RNG device is supported));
-goto error;
-}
-
-if (n  0) {
-if (!(def-rng = virDomainRNGDefParseXML(nodes[0], ctxt, flags)))
+if (n  VIR_ALLOC_N(def-rngs, n)  0)
+goto no_memory;
+for (i = 0; i  n; i++) {
+virDomainRNGDefPtr rng = virDomainRNGDefParseXML(nodes[i],
+ ctxt,
+ flags);
+if (!rng)
 goto error;
-VIR_FREE(nodes);
+
+def-rngs[def-nrngs++] = rng;
 }
+VIR_FREE(nodes);

 /* analysis of the hub devices */
 if ((n = virXPathNodeSet(./devices/hub, ctxt, nodes))  0) {
@@ -11702,17 +11704,6 @@ static bool
 virDomainRNGDefCheckABIStability(virDomainRNGDefPtr src,
  virDomainRNGDefPtr dst)
 {
-if (!src  !dst)
-return true;
-
-if (!src || !dst) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _(Target domain RNG device count '%d' 
- does not match source count '%d'),
-   src ? 1 : 0, dst ? 1 : 0);
-return false;
-}
-
 if (src-model != dst-model) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Target RNG model '%s' does not match source '%s'),
@@ -12129,8 +12120,16 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
  dst-memballoon))
 return false;

-if (!virDomainRNGDefCheckABIStability(src-rng, dst-rng))
+if (src-nrngs != dst-nrngs) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Target domain RNG device count %zu 
+ does not match source %zu), dst-nrngs, src-nrngs);
 return false;
+}
+
+for (i = 0 ; i  src-nrngs ; i++)
+if (!virDomainRNGDefCheckABIStability(src-rngs[i], dst-rngs[i]))
+return false;

 return true;
 }
@@ -15054,8 +15053,10 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 if (def-memballoon)
 virDomainMemballoonDefFormat(buf, def-memballoon, flags);

-if (def-rng)
-virDomainRNGDefFormat(buf, def-rng, flags);
+for (n = 0; n  def-nrngs; n++) {
+if (virDomainRNGDefFormat(buf, def-rngs[n], flags))
+goto error;
+}

 virBufferAddLit(buf,   /devices\n);

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0828954..5dc3400 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1878,13 +1878,15 @@ struct _virDomainDef {
 size_t nseclabels;
 virSecurityLabelDefPtr *seclabels;

+size_t nrngs;
+virDomainRNGDefPtr *rngs;
+
 /* Only 1 */
 virDomainWatchdogDefPtr watchdog;
 virDomainMemballoonDefPtr memballoon;
 virCPUDefPtr cpu;
 virSysinfoDefPtr sysinfo;
 virDomainRedirFilterDefPtr redirfilter;
-virDomainRNGDefPtr rng;

 void *namespaceData;
 virDomainXMLNamespace ns;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1c9bfc9..9270258 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -787,8 +787,8 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr 
qemuCaps)
 if 

[libvirt] [PATCH 4/4] tests: Test multiple RNG devices and support for XML entities in paths

2013-02-25 Thread Peter Krempa
Users may want to specify XML entities in paths to devices. Ensure they
are parsed and used properly. Also test support for multiple RNG devices
in one guest.
---
 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml  | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args 
b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
index ced11db..a99931a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
@@ -1 +1 @@
-LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M 
pc -m 214 -smp 1 -nographic -nodefaults -monitor 
unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 -object 
rng-random,id=rng0,filename=/test/phile -device 
virtio-rng-pci,rng=rng0,max-bytes=100,period=1234,bus=pci.0,addr=0x4
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M 
pc -m 214 -smp 1 -nographic -nodefaults -monitor 
unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 -object 
rng-random,id=rng0,filename=/test/phile -device 
virtio-rng-pci,rng=rng0,max-bytes=100,period=1234,bus=pci.0,addr=0x4 -object 
'rng-random,id=rng1,filename=/test/strangephile' -device 
virtio-rng-pci,rng=rng1,bus=pci.0,addr=0x5
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
index 26ddd38..9950f5b 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
@@ -20,5 +20,8 @@
   rate period='1234'800/rate
   backend model='random'/test/phile/backend
 /rng
+rng model='virtio'
+  backend model='random'/test/strangelt;gt;phile/backend
+/rng
   /devices
 /domain
-- 
1.8.1.1

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


Re: [libvirt] [PATCH 08/16] conf: Refactor ABI stability checking and break long lines

2013-02-25 Thread Peter Krempa

On 02/21/13 15:27, Peter Krempa wrote:

On 02/20/13 18:44, Laine Stump wrote:

On 02/20/2013 12:06 PM, Peter Krempa wrote:

Get rid of the identical variable in the ABI stability checks in favor
of return statements and break or refactor very long lines where
possible.
---
  src/conf/domain_conf.c | 469
+
  1 file changed, 200 insertions(+), 269 deletions(-)


ACK (although in general I have some misgivings about cosmetic changes
like this, as they obscure the history of the modified lines, making it
more tedious to sift through the past commit logs to understand why
certain bits of code exist / what changed, etc).



Thankfully there's a script for vim (called fugitive) and for sure for
other editors too that allows to browse the blame history.

Thanks for the reviews, I pushed patches 1-6 and 8.

Peter


Ping? Could anyone please have a look at the rest of the series.

Peter

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


[libvirt] [PATCH 2/4] docs: Fix attribute name for virtio-rng backend

2013-02-25 Thread Peter Krempa
---
 docs/formatdomain.html.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 12c9468..2a60f61 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4328,7 +4328,7 @@ qemu-kvm -net nic,model=? /dev/null
   li'egd' mdash; a EGD protocol backend/li
 /ul
   /dd
-  dtcodebackend type='random'/code/dt
+  dtcodebackend model='random'/code/dt
   dd
 p
   This backend type expects a non-blocking character device as input.
@@ -4337,7 +4337,7 @@ qemu-kvm -net nic,model=? /dev/null
   When no file name is specified the hypervisor default is used.
 /p
   /dd
-  dtcodebackend type='egd'/code/dt
+  dtcodebackend model='egd'/code/dt
   dd
 p
   This backend connects to a source using the EGD protocol.
-- 
1.8.1.1

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


[libvirt] [RESEND] ESX: Fix DISPATCH_FREE generation code to free all extended objects

2013-02-25 Thread Ata Bohra
ping ...
  From: ata.hus...@hotmail.com
 To: libvir-list@redhat.com
 CC: ata.hus...@hotmail.com
 Subject: [PATCH] ESX: Fix DISPATCH_FREE generation code to free all extended 
 objects
 Date: Tue, 1 Jan 2013 22:22:28 -0800
 
 Python code generator generate_source section that handles
 code generation to free inherited objects needs to generate
 DISPATCH_FREE calls for all extended_by objects.
 ---
  src/esx/esx_vi_generator.py |   21 ++---
  1 file changed, 18 insertions(+), 3 deletions(-)
 
 diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py
 index af4e7e8..7a7cf76 100755
 --- a/src/esx/esx_vi_generator.py
 +++ b/src/esx/esx_vi_generator.py
 @@ -4,6 +4,7 @@
  # esx_vi_generator.py: generates most of the SOAP type mapping code
  #
  # Copyright (C) 2010-2012 Matthias Bolte matthias.bo...@googlemail.com
 +# Copyright (C) 2013 Ata E Husain Bohra ata.hus...@hotmail.com
  #
  # This library is free software; you can redistribute it and/or
  # modify it under the terms of the GNU Lesser General Public
 @@ -785,9 +786,7 @@ class Object(Type):
  source += ESX_VI__TEMPLATE__DYNAMIC_FREE(%s,\n % self.name
  source += {\n
 
 -for extended_by in self.extended_by:
 -source += ESX_VI__TEMPLATE__DISPATCH__FREE(%s)\n \
 -  % extended_by
 +source += recurse_dispatch(self, 
 ESX_VI__TEMPLATE__DISPATCH__FREE)
 
  source += },\n
  source += {\n
 @@ -1285,6 +1284,22 @@ class ManagedObject(Type):
 
 
 
 +def recurse_dispatch(object, text):
 +
 +source = 
 +
 +if object.extended_by is None:
 +return source
 +
 +for extended_by in object.extended_by:
 +source += %s(%s)\n % (text, extended_by)
 +child = objects_by_name[extended_by]
 +source += recurse_dispatch(child, text)
 +
 +return source
 +
 +
 +
  class Enum(Type):
  FEATURE__ANY_TYPE = (1  1)
  FEATURE__SERIALIZE = (1  2)
 --
 1.7.9.5
 
  --
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RESEND] ESX: Add AnyType_Serialize routine to esx_vi_types.c

2013-02-25 Thread Ata Bohra
ping ..
 From: ata.hus...@hotmail.com
To: matthias.bo...@googlemail.com
Date: Tue, 1 Jan 2013 12:07:12 -0800
CC: libvir-list@redhat.com
Subject: Re: [libvirt] [PATCH] ESX: Add AnyType_Serialize routine to 
esx_vi_types.c




  ---
   src/esx/esx_vi_types.c |   48 
  
   src/esx/esx_vi_types.h |3 ++-
   2 files changed, 50 insertions(+), 1 deletion(-)
 
 Looks good on a quick view, but I wonder what are you trying to that
 requires to serialize an AnyType directly instead of the specific
 types?

Some of the variables contains anytype variables that are needed during 
serialization time such as: extraConfig field of VirtualMachineConfigSpec 
or ArrayUpdateSpec. I found that without providing serialize method even 
compilation does not work. 
 -- 
 Matthias Bolte
 http://photron.blogspot.com

  

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

[libvirt] Hard requirement on gcrypt.h?

2013-02-25 Thread Eugene Marcotte
Hi,

I'm building libvirt for the first time and figuring out what libraries
are required to get the simplest possible thing working.

Is there a hard requirement on gcrypt? I got libvirt past the configure
script but the build failed on gcrypt.h. It looks like the include for
it could be moved inside the #if WITH_GNUTLS section of libirt.c, if I
understand what configure.ac is trying to set up.

Thoughts?

Thanks!
Eugene

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


Re: [libvirt] [PATCH 5/5] tests: uniformly report test failures

2013-02-25 Thread Eric Blake
On 02/25/2013 03:38 AM, Michal Privoznik wrote:
 On 23.02.2013 00:09, Eric Blake wrote:
 testutils.c likes to print summaries after a test completes,
 including if it failed.  But if the test outright exit()s,
 this summary is skipped.  Enforce that we return instead of exit.

 
 ACK

Thanks for the reviews.  Series pushed now, as this improves the
testsuite for 1.0.3.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] Hard requirement on gcrypt.h?

2013-02-25 Thread Eric Blake
On 02/25/2013 05:38 PM, Eugene Marcotte wrote:
 Hi,
 
 I'm building libvirt for the first time and figuring out what libraries
 are required to get the simplest possible thing working.
 
 Is there a hard requirement on gcrypt? I got libvirt past the configure
 script but the build failed on gcrypt.h. It looks like the include for
 it could be moved inside the #if WITH_GNUTLS section of libirt.c, if I
 understand what configure.ac is trying to set up.

Can you show the exact compiler error message you received?  You are
correct that this header is non-standard, and should be properly ifdef'd
before we try to use it.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] wrap some long lines

2013-02-25 Thread li guang
ping ...

在 2013-02-19二的 10:22 +0800,liguang写道:
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  src/qemu/qemu_command.c |   71 ++
  1 files changed, 46 insertions(+), 25 deletions(-)
 
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 9ad3c77..ad62120 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -611,7 +611,8 @@ qemuAssignDeviceNetAlias(virDomainDefPtr def, 
 virDomainNetDefPtr net, int idx)
  }
  if ((thisidx = qemuDomainDeviceAliasIndex(def-nets[i]-info, 
 net))  0) {
  virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 -   _(Unable to determine device index for 
 network device));
 +   _(Unable to determine device index for 
 + network device));
  return -1;
  }
  if (thisidx = idx)
 @@ -638,7 +639,8 @@ qemuAssignDeviceHostdevAlias(virDomainDefPtr def, 
 virDomainHostdevDefPtr hostdev
  int thisidx;
  if ((thisidx = 
 qemuDomainDeviceAliasIndex(def-hostdevs[i]-info, hostdev))  0) {
  virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 -   _(Unable to determine device index for 
 hostdev device));
 +   _(Unable to determine device index for 
 + hostdev device));
  return -1;
  }
  if (thisidx = idx)
 @@ -665,7 +667,8 @@ qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, 
 virDomainRedirdevDefPtr redir
  int thisidx;
  if ((thisidx = 
 qemuDomainDeviceAliasIndex(def-redirdevs[i]-info, redir))  0) {
  virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 -   _(Unable to determine device index for 
 redirected device));
 +   _(Unable to determine device index for
 +  redirected device));
  return -1;
  }
  if (thisidx = idx)
 @@ -1001,8 +1004,8 @@ static int qemuCollectPCIAddress(virDomainDefPtr def 
 ATTRIBUTE_UNUSED,
  if (info-addr.pci.function != 0) {
  virReportError(VIR_ERR_XML_ERROR,
 _(Attempted double use of PCI Address '%s' 
 - (may need \multifunction='on'\ for device on 
 function 0)),
 -   addr);
 + (may need \multifunction='on'\ for 
 + device on function 0)), addr);
  } else {
  virReportError(VIR_ERR_XML_ERROR,
 _(Attempted double use of PCI Address '%s'), 
 addr);
 @@ -1036,7 +1039,8 @@ static int qemuCollectPCIAddress(virDomainDefPtr def 
 ATTRIBUTE_UNUSED,
  goto cleanup;
  }
  
 -VIR_DEBUG(Remembering PCI addr %s (multifunction=off for 
 function 0), addr);
 +VIR_DEBUG(Remembering PCI addr %s (multifunction=off
 +   for function 0), addr);
  if (virHashAddEntry(addrs-used, addr, addr))
  goto cleanup;
  addr = NULL;
 @@ -1468,7 +1472,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
  def-controllers[i]-info.addr.pci.slot != 1 ||
  def-controllers[i]-info.addr.pci.function != 2) {
  virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 -   _(PIIX3 USB controller must have PCI 
 address 0:0:1.2));
 +   _(PIIX3 USB controller must have
 +  PCI address 0:0:1.2));
  goto error;
  }
  reservedUSB = true;
 @@ -1524,7 +1529,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
  primaryVideo-info.addr.pci.slot != 2 ||
  primaryVideo-info.addr.pci.function != 0) {
  virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 -   _(Primary video card must have PCI address 
 0:0:2.0));
 +   _(Primary video card must have 
 + PCI address 0:0:2.0));
  goto error;
  }
  /* If TYPE==PCI, then qemuCollectPCIAddress() function
 @@ -1814,12 +1820,14 @@ qemuBuildRomStr(virBufferPtr buf,
  if (info-rombar || info-romfile) {
  if (info-type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 -   %s, _(rombar and romfile are supported only 
 for PCI devices));
 +   %s, _(rombar and romfile are supported 
 +   only for PCI devices));
  return -1;
  }
  if (!qemuCapsGet(caps, QEMU_CAPS_PCI_ROMBAR)) {
  

Re: [libvirt] Hard requirement on gcrypt.h?

2013-02-25 Thread Eugene Marcotte
On Mon, 2013-02-25 at 17:42 -0700, Eric Blake wrote:
 On 02/25/2013 05:38 PM, Eugene Marcotte wrote:
  Hi,
  
  I'm building libvirt for the first time and figuring out what libraries
  are required to get the simplest possible thing working.
  
  Is there a hard requirement on gcrypt? I got libvirt past the configure
  script but the build failed on gcrypt.h. It looks like the include for
  it could be moved inside the #if WITH_GNUTLS section of libirt.c, if I
  understand what configure.ac is trying to set up.
 
 Can you show the exact compiler error message you received?  You are
 correct that this header is non-standard, and should be properly ifdef'd
 before we try to use it.
 

make[3]: Entering directory `/home/emarcotte/src/libvirt/src'
  CC   libvirt_driver_la-libvirt.lo
libvirt.c:34:20: fatal error: gcrypt.h: No such file or directory
compilation terminated.

Of course, installing gcrypt-devel fixes it, but I would have expected
configure to fail. I can try to add an explicit check for gcrypt.h if
it's required. 

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


Re: [libvirt] [PATCH v2 1/1] Add NVRAM device

2013-02-25 Thread Li Zhang

Daniel/Eric,

Any suggestion about this patch?

On 2013年02月25日 13:49, Li Zhang wrote:

From: Li Zhang zhlci...@linux.vnet.ibm.com

For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device.
Users are allowed to specify spapr-vio devices'address.
But NVRAM is not supported in libvirt. So this patch is to
add NVRAM device to allow users to specify its address.

In QEMU, NVRAM device's address is specified by
  -global spapr-nvram.reg=x.

In libvirt, XML file is defined as the following:

   nvram
 address type='spapr-vio' reg='0x3000'/
   /nvram

Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
  * v2 - v1:
 Add NVRAM parameters parsing in qemuParseCommandLine.

  src/conf/domain_conf.c  |   83 ++-
  src/conf/domain_conf.h  |   10 ++
  src/qemu/qemu_command.c |   48 +++
  src/qemu/qemu_command.h |2 ++
  4 files changed, 142 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10f361c..8c1e8ae 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -175,7 +175,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
redirdev,
smartcard,
chr,
-  memballoon)
+  memballoon,
+  nvram)
  
  VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,

none,
@@ -1442,6 +1443,16 @@ void 
virDomainMemballoonDefFree(virDomainMemballoonDefPtr def)
  VIR_FREE(def);
  }
  
+void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def)

+{
+if (!def)
+return;
+
+virDomainDeviceInfoClear(def-info);
+
+VIR_FREE(def);
+}
+
  void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def)
  {
  if (!def)
@@ -1602,6 +1613,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
  case VIR_DOMAIN_DEVICE_SMARTCARD:
  case VIR_DOMAIN_DEVICE_CHR:
  case VIR_DOMAIN_DEVICE_MEMBALLOON:
+case VIR_DOMAIN_DEVICE_NVRAM:
  case VIR_DOMAIN_DEVICE_LAST:
  break;
  }
@@ -1791,6 +1803,7 @@ void virDomainDefFree(virDomainDefPtr def)
  virDomainWatchdogDefFree(def-watchdog);
  
  virDomainMemballoonDefFree(def-memballoon);

+virDomainNVRAMDefFree(def-nvram);
  
  for (i = 0; i  def-nseclabels; i++)

  virSecurityLabelDefFree(def-seclabels[i]);
@@ -2342,6 +2355,12 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def,
  if (cb(def, device, def-memballoon-info, opaque)  0)
  return -1;
  }
+if (def-nvram) {
+device.type = VIR_DOMAIN_DEVICE_NVRAM;
+device.data.nvram = def-nvram;
+if (cb(def, device, def-nvram-info, opaque)  0)
+return -1;
+}
  device.type = VIR_DOMAIN_DEVICE_HUB;
  for (i = 0; i  def-nhubs ; i++) {
  device.data.hub = def-hubs[i];
@@ -7461,6 +7480,23 @@ error:
  goto cleanup;
  }
  
+static virDomainNVRAMDefPtr

+virDomainNVRAMDefParseXML(const xmlNodePtr node,
+   unsigned int flags)
+{
+   virDomainNVRAMDefPtr def;
+
+if (VIR_ALLOC(def)  0) {
+virReportOOMError();
+return NULL;
+}
+
+if ( virDomainDeviceInfoParseXML(node, NULL, def-info, flags)  0 )
+return NULL;
+
+return def;
+}
+
  static virSysinfoDefPtr
  virSysinfoParseXML(const xmlNodePtr node,
xmlXPathContextPtr ctxt)
@@ -10572,6 +10608,32 @@ virDomainDefParseXML(virCapsPtr caps,
  }
  }
  
+def-nvram = NULL;

+if ((n = virXPathNodeSet(./devices/nvram, ctxt, nodes))  0) {
+goto error;
+}
+
+if (n  1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(only a single nvram device is supported));
+goto error;
+}
+
+if (n  0) {
+virDomainNVRAMDefPtr nvram =
+virDomainNVRAMDefParseXML(nodes[0], flags);
+if (!nvram)
+goto error;
+def-nvram = nvram;
+VIR_FREE(nodes);
+} else {
+virDomainNVRAMDefPtr nvram;
+if (VIR_ALLOC(nvram)  0)
+goto no_memory;
+nvram-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
+def-nvram = nvram;
+}
+
  /* analysis of the hub devices */
  if ((n = virXPathNodeSet(./devices/hub, ctxt, nodes))  0) {
  goto error;
@@ -13547,6 +13609,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
  }
  
  static int

+virDomainNVRAMDefFormat(virBufferPtr buf,
+ virDomainNVRAMDefPtr def,
+ unsigned int flags)
+{
+virBufferAsprintf(buf, nvram\n);
+if (virDomainDeviceInfoIsSet(def-info, flags))
+if (virDomainDeviceInfoFormat(buf, def-info, flags)  0)
+return -1;
+
+ virBufferAddLit(buf, /nvram\n);
+
+ return 0;
+}
+
+static int
  virDomainSysinfoDefFormat(virBufferPtr buf,
virSysinfoDefPtr def)
  {
@@ -14787,6 +14864,9 @@ 

Re: [libvirt] [PATCHv2 1/1] Optimize machine option to set more options with it.

2013-02-25 Thread Li Zhang

Hi Daniel,

This patch is still use USB option to disable the controller which 
created in machine init of QEMU.
Because with USB model none, other USB devices can't be used to attach 
USB controller.
So we disable the controller in machine init and choose the controller 
defined in libvirt by -device .


That's why I send it again with some fixes.
Can you consider about it?

Thanks a lot.
-Li

On 2013年02月22日 17:09, Li Zhang wrote:

From: Li Zhang zhlci...@linux.vnet.ibm.com

Currently, -machine option is used only when dump-guest-core is used.

To use options defined in machine option for new version of QEMU,
it needs to use -machine xxx, and to be compatible with older version
  -M, this patch addes QEMU_CAPS_MACH_OPT capability, and assumes
  -machine is used for QEMU v1.0 onwards.

To avoid the collision for creating USB controllers when using USB
option and -device , it needs to set usb=off in machine option.
QEMU_CAPS_USB_OPT capability is added, and it will be for QEMU
v1.3.0-rc0 onwards which supports USB option.

Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
  src/qemu/qemu_capabilities.c |   10 ++
  src/qemu/qemu_capabilities.h |2 ++
  src/qemu/qemu_command.c  |   36 +---
  tests/qemuxml2argvtest.c |4 ++--
  4 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 51fc9dc..79eb83f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -205,6 +205,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
usb-serial, /* 125 */
usb-net,
add-fd,
+  mach-opt,
+  usb-opt,
  
  );
  
@@ -2416,6 +2418,14 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
  
  virQEMUCapsInitQMPBasic(qemuCaps);
  
+/* Assuming to use machine option v1.0 onwards*/

+if (qemuCaps-version = 100)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_OPT);
+
+/* USB option is supported v1.3.0-rc0 onwards */
+if (qemuCaps-version = 1002090)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_USB_OPT);
+
  if (!(archstr = qemuMonitorGetTargetArch(mon)))
  goto cleanup;
  
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h

index e69d558..06aaa68 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -166,6 +166,8 @@ enum virQEMUCapsFlags {
  QEMU_CAPS_DEVICE_USB_SERIAL  = 125, /* -device usb-serial */
  QEMU_CAPS_DEVICE_USB_NET = 126, /* -device usb-net */
  QEMU_CAPS_ADD_FD = 127, /* -add-fd */
+QEMU_CAPS_MACH_OPT   = 128, /* -machine */
+QEMU_CAPS_USB_OPT= 129, /* -machine ,usb=off*/
  
  QEMU_CAPS_LAST,   /* this must always be the last item */

  };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6c28123..e7dde21 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4614,6 +4614,8 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
 const virDomainDefPtr def,
 virQEMUCapsPtr qemuCaps)
  {
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
  /* This should *never* be NULL, since we always provide
   * a machine in the capabilities data for QEMU. So this
   * check is just here as a safety in case the unexpected
@@ -4621,27 +4623,39 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
  if (!def-os.machine)
  return 0;
  
-if (!def-mem.dump_core) {

+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACH_OPT)) {
  /* if no parameter to the machine type is needed, we still use
   * '-M' to keep the most of the compatibility with older versions.
   */
  virCommandAddArgList(cmd, -M, def-os.machine, NULL);
  } else {
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   %s, _(dump-guest-core is not available 
-with this QEMU binary));
-return -1;
-}
  
  /* However, in case there is a parameter to be added, we need to

   * use the -machine parameter because qemu is not parsing the
   * -M correctly */
+
  virCommandAddArg(cmd, -machine);
-virCommandAddArgFormat(cmd,
-   %s,dump-guest-core=%s,
-   def-os.machine,
-   
virDomainMemDumpTypeToString(def-mem.dump_core));
+virBufferAsprintf(buf, %s, def-os.machine);
+
+/* To avoid the collision of creating USB controllers when calling
+ * machine-init in QEMU, it needs to set usb=off
+ */
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_OPT))
+virBufferAsprintf(buf, ,usb=off);
+
+if (def-mem.dump_core) {
+if (!virQEMUCapsGet(qemuCaps, 

Re: [libvirt] [Dnsmasq-discuss] Chaining instances?

2013-02-25 Thread Gao Yongwei
2013/2/25 Laine Stump la...@laine.org

 On 02/25/2013 02:05 AM, Laine Stump wrote:
  This discussion should really be taking place on libvir-list - I'm
  Cc'ing it there.

 Bah. I'm not sure what happened, but I ended up not actually Cc'ing...


 
 
  On 02/24/2013 04:11 PM, TJ wrote:
  On 24/02/13 19:19, Laine Stump wrote:
  On 02/24/2013 05:09 AM, TJ wrote:
  I wondered if maybe configuring the libvirtd dnsmasq instances to be
 dhcp proxies for the LAN dnsmasq, and use multiple dhcp-range's with tags
 might do it?
 
  My brain is a bit fried right now having had to fix several bugs in
 vmbuilder and libvirt, plus add new functionality to libvirtd to allow its
 dnsmasq instance to read a conf-file and use a separate log-facility,
  What you're talking about doing sounds *very* useful to have supported
  directly in libvirt, but you may want to contact libvirt's upstream
  developers (at libvir-list@redhat.com, or on irc.oftc.net in #virt)
  prior to expending a lot of effort on libvirt code - in general a
  problem may already be solved in a different manner, or somebody else
  may already be working on it. Failing that, there may have already been
  considerable debate on a particular subject, and a path to a solution
  generally agreed on, but with nobody yet working on the code.
  It turns out that what I need(ed) to do was completely *disable*
 libvirt's use of dnsmasq
  and instead use Simon's related dhcp-helper program which acts as a
 full dhcp-relay using, f.e:
 
  /usr/sbin/dhcp-helper -u libvirt-dnsmasq -i virbr1 -b eth0
 
  This forwards all requests to the primary dnsmasq DHCP server on the
 LAN which matches against the
  appropriate dhcp-range:
 
  # physical network leases
  dhcp-range=set:phys,10.254.251.50,10.254.251.199,255.255.255.0,1440m
  # subnet for VMs on server1
  dhcp-range=set:vmlan1,10.254.1.100,10.254.1.199,255.255.255.0,1440m
  # default route (gateway)
  dhcp-option=tag:phys,option:router,10.254.251.1
  dhcp-option=tag:vmlan1,option:router,10.254.1.1
  # DNS server
  dhcp-option=6,10.254.251.1
 
 
  To that end this evening I added two additional options to libvirt:
 
  dnsmasq enabled='true'/
  dhcphelper enabled='false'/
  libvirt's XML should remain implementation-agnostic. We don't want to
  have an option to disable the specific implementation of dns+dhcp called
  dnsmasq; rather we want to be able to enable/disable a network's dhcp
  server and/or dns server. (The reason for this is that we want to leave
  the door open for someone to implement a different backend using a
  different dhcp server and/or dns server and be able to use the same
  configuration.)
 
  We have actually previously discussed disabling dns and/or dhcp (see
  http://www.redhat.com/archives/libvir-list/2012-November/msg00861.html).
  It is already possible to completely disable dhcp - simply don't include
  a dhcp element in the network definition. As for dns, from the very
  beginning dns has been *always* enabled on all networks, so even though
  there is a dns element, simply having not having one is not a valid
  way to say no dns server (as it would cause backward compatibility
  problems with existing installations).
 
  Instead, the conclusion of the above-mentioned thread was that the
  proper way to handle this would be to add an enable element to the
  existing dns element, which would default to yes. To disable
  libvirt-supplied dns services for a subnet, you would just put:
 
  dns enable='no'/
 
  in the network definition. In the case that dns had enable='no' AND
  there was no dhcp element, dnsmasq simply wouldn't be started at all.
  That hasn't been implemented yet, but shouldn't be too complex to do.
 
  As for dhcp-helper, aside from the fact that again you've created an
  option that is specific to a particular implementation of what is
  needed, that command isn't universally available anyway (it's not in any
  package for Fedora/RHEL, for example), and I'm not sure that it's really
  necessary to have it started up by libvirt anyway - can it detect newly
  created/upped interfaces as dnsmasq can? If so, it could be started up
  by regular host system config even before libvirt was started.
 
 
  These are the default values when the options are *not* defined. They
 allow the admin to disable dnsmasq entirely:
 
  dnsmasq enabled='false'/
 
  and to enable dhcp-helper:
 
  dhcphelper enabled='true'/
 
  Using two new functions:
 
  int networkBuildDhcphelperArgv(...)
  int networkBuildDhcpHelperCommandLine(...)
 
  the existing function:
 
  int networkStartDhcpDaemon(...)
 
  is able to launch either or both of dnsmasq and dhcp-helper with the
 correct options.
 
  dhcp-helper's -i option is filled from network-def-bridge and its
 -b option is taken from
  the first forward device declared in a forward dev='?'/ or interface
 dev='?'/. If no forward
  device is declared it throws a VIR_ERR_INVALID_INTERFACE error with
 appropriate explanatory text.
 

 --
 libvir-list 

Re: [libvirt] Entering freeze for libvirt-1.0.3

2013-02-25 Thread Li Zhang

On 2013年02月25日 19:52, Daniel Veillard wrote:

   I have just tagged git and pushed the tarball. The rpms for F17
are following too at the usual place:
 ftp://libvirt.org/libvirt/

   I gave a try to the set of rpms and this seems to work fine for
relatively simple tests, but of course more testing and checking on
other architectures is needed !
   If everything goes fine, I will make an rc2 in a couple of days
and then we can decide if a final release on Friday is fine or if
we need to postpone to next week,


There are too few changes in this release for pp64.
Testing it by building source for ppc64, it works well.

Thanks.
--Li

Daniel



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

Re: [libvirt] Entering freeze for libvirt-1.0.3

2013-02-25 Thread Daniel Veillard
On Tue, Feb 26, 2013 at 11:10:35AM +0800, Li Zhang wrote:
 On 2013年02月25日 19:52, Daniel Veillard wrote:
I have just tagged git and pushed the tarball. The rpms for F17
 are following too at the usual place:
  ftp://libvirt.org/libvirt/
 
I gave a try to the set of rpms and this seems to work fine for
 relatively simple tests, but of course more testing and checking on
 other architectures is needed !
If everything goes fine, I will make an rc2 in a couple of days
 and then we can decide if a final release on Friday is fine or if
 we need to postpone to next week,
 
 There are too few changes in this release for pp64.

  By too few do you mean that we are missing anything important ?

 Testing it by building source for ppc64, it works well.

  Cool, thanks for testing :-)

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] Entering freeze for libvirt-1.0.3

2013-02-25 Thread Li Zhang

On 2013年02月26日 12:51, Daniel Veillard wrote:

On Tue, Feb 26, 2013 at 11:10:35AM +0800, Li Zhang wrote:

On 2013年02月25日 19:52, Daniel Veillard wrote:

   I have just tagged git and pushed the tarball. The rpms for F17
are following too at the usual place:
 ftp://libvirt.org/libvirt/

   I gave a try to the set of rpms and this seems to work fine for
relatively simple tests, but of course more testing and checking on
other architectures is needed !
   If everything goes fine, I will make an rc2 in a couple of days
and then we can decide if a final release on Friday is fine or if
we need to postpone to next week,

There are too few changes in this release for pp64.

   By too few do you mean that we are missing anything important ?

I have sent out 2 patches which are not accepted for this release.

One patch is to fix one bug for ppc64 which is:
[PATCHv2 1/1] Optimize machine option to set more options with it.
Without this patch, USB controllers doesn't work well with VGA enabled 
for ppc64.

And testing with VDSM, it will always report errors when using VGA.




Testing it by building source for ppc64, it works well.

   Cool, thanks for testing :-)

Daniel



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

[libvirt] The problem of CPU index

2013-02-25 Thread Li Zhang

Hi all,

In the function qemuMonitorJSONExtractCPUInfo(),
it assumes that CPU indexes should be contiguous.
As the following:
   if (cpu != i) {
virReportError(VIR_ERR_INTERNAL_ERROR,
   _(unexpected cpu index %d expecting %d),
   i, cpu);
goto cleanup;
}

For x86, in what kind of situation will cause this error?

Actually, on ppc64, the CPU indexes always are not contiguous,
because smt is always disabled because of hardware's limitation for KVM.

For example, smt=4, CPUs=64, cores=16.

If with SMT enabled, 0~3 will run on core 0, 4~7 will run on core 1, 
If SMT is disabled, only one thread is active. It should be 0, 4, 8,

CPU information on my machine is as the following:

Architecture:  ppc64
Byte Order:Big Endian
CPU(s):64
On-line CPU(s) list:   0,4,8,12,16,20,24,28,32,36,40,44,48,52,56,60
Off-line CPU(s) list: 
1-3,5-7,9-11,13-15,17-19,21-23,25-27,29-31,33-35,37-39,41-43,45-47,49-51,53-55,57-59,61-63

Thread(s) per core:1
Core(s) per socket:1
Socket(s): 16
NUMA node(s):  2
Model: 8246-L2C
L1d cache: 32K
L1i cache: 32K
L2 cache:  256K
L3 cache:  4096K
L4 cache:  4096K
NUMA node0 CPU(s): 0,36,40,44,48,52,56,60
NUMA node1 CPU(s): 4,8,12,16,20,24,28,32

Actually, only 16 CPUs are online.
All we get CPUs are 0,4,8,...,60.

If possible, could this assumption be removed?

Thanks
--Li

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


Re: [libvirt] Entering freeze for libvirt-1.0.3

2013-02-25 Thread Daniel Veillard
On Tue, Feb 26, 2013 at 01:03:17PM +0800, Li Zhang wrote:
 On 2013年02月26日 12:51, Daniel Veillard wrote:
 On Tue, Feb 26, 2013 at 11:10:35AM +0800, Li Zhang wrote:
 On 2013年02月25日 19:52, Daniel Veillard wrote:
I have just tagged git and pushed the tarball. The rpms for F17
 are following too at the usual place:
  ftp://libvirt.org/libvirt/
 
I gave a try to the set of rpms and this seems to work fine for
 relatively simple tests, but of course more testing and checking on
 other architectures is needed !
If everything goes fine, I will make an rc2 in a couple of days
 and then we can decide if a final release on Friday is fine or if
 we need to postpone to next week,
 There are too few changes in this release for pp64.
By too few do you mean that we are missing anything important ?
 I have sent out 2 patches which are not accepted for this release.
 
 One patch is to fix one bug for ppc64 which is:
 [PATCHv2 1/1] Optimize machine option to set more options with it.
 Without this patch, USB controllers doesn't work well with VGA
 enabled for ppc64.
 And testing with VDSM, it will always report errors when using VGA.

  Hum, i just read your patch but i don't have enough knowledge of
qemu parameters to know if there is large side effect to your patch.
Also you remove code which is about core and replace it with something
completely different.
  As is and with the amount of details you provided I can't make any
jugement of how bad or good those changes are. One thing is sure,
bug fixes can go in, they are fine during freeze !

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

[libvirt] [PATCH] interface: udev backend coverity NULL deref

2013-02-25 Thread Doug Goldstein
This fixes a potential NULL deref identified by John Ferlan
jfer...@redhat.com if scandir() didn't return an expected value.
---
 src/interface/interface_backend_udev.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/interface/interface_backend_udev.c 
b/src/interface/interface_backend_udev.c
index dca85b3..1034429 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -779,6 +779,13 @@ udevIfaceGetIfaceDefBond(struct udev *udev,
  * so we use the part after the _
  */
 tmp_str = strchr(slave_list[i]-d_name, '_');
+if (!tmp_str || strlen(tmp_str)  2) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Invalid enslaved interface name '%s' seen for 
+ bond '%s', slave_list[i]-d_name, name));
+goto cleanup;
+}
+/* go past the _ */
 tmp_str++;
 
 ifacedef-data.bond.itf[i] =
-- 
1.7.12.4

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


Re: [libvirt] Entering freeze for libvirt-1.0.3

2013-02-25 Thread Li Zhang

On 2013年02月26日 14:04, Daniel Veillard wrote:

On Tue, Feb 26, 2013 at 01:03:17PM +0800, Li Zhang wrote:

On 2013年02月26日 12:51, Daniel Veillard wrote:

On Tue, Feb 26, 2013 at 11:10:35AM +0800, Li Zhang wrote:

On 2013年02月25日 19:52, Daniel Veillard wrote:

   I have just tagged git and pushed the tarball. The rpms for F17
are following too at the usual place:
 ftp://libvirt.org/libvirt/

   I gave a try to the set of rpms and this seems to work fine for
relatively simple tests, but of course more testing and checking on
other architectures is needed !
   If everything goes fine, I will make an rc2 in a couple of days
and then we can decide if a final release on Friday is fine or if
we need to postpone to next week,

There are too few changes in this release for pp64.

   By too few do you mean that we are missing anything important ?

I have sent out 2 patches which are not accepted for this release.

One patch is to fix one bug for ppc64 which is:
[PATCHv2 1/1] Optimize machine option to set more options with it.
Without this patch, USB controllers doesn't work well with VGA
enabled for ppc64.
And testing with VDSM, it will always report errors when using VGA.

   Hum, i just read your patch but i don't have enough knowledge of
qemu parameters to know if there is large side effect to your patch.
Also you remove code which is about core and replace it with something
completely different.

Thanks so much for your reviewing.

In qemu, it provides some options for some kind of devices or hardware 
by -machine option.
I think its purpose is to provide the configurations for machines to 
compare with -device xxx.


There will be  more and more options to be set  by -machine according to 
QEMU's development.
So, in my patch,  -machine is used to be set more options not only 
dump_core with newer QEMU versions



   As is and with the amount of details you provided I can't make any
jugement of how bad or good those changes are. One thing is sure,
bug fixes can go in, they are fine during freeze !

Got it, thanks. :)
I will wait for more comments from mailing list.


Daniel



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

Re: [libvirt] [PATCH] qemu: Don't fail to shutdown domains with unresponsive agent

2013-02-25 Thread Michal Privoznik
On 25.02.2013 19:33, Eric Blake wrote:
 On 02/25/2013 10:55 AM, Michal Privoznik wrote:
 Currently, qemuDomainShutdownFlags() chooses the agent method of
 shutdown whenever the agent is configured. However, this
 assumption is not enough as the guest agent may be unresponsive
 at the moment. So unless guest agent method has been explicitly
 requested, we should fall back to the ACPI method.
 ---
  src/qemu/qemu_driver.c | 34 --
  1 file changed, 24 insertions(+), 10 deletions(-)
 
 When Daniel added two new shutdown methods for LXC, I had the question
 on whether shutdown methods should be mutually exclusive, or a bitmask
 of all permissible things to attempt (where passing 0 leaves the
 attempts up the hypervisor).  The consensus was that allowing the user
 to pass more than one method makes sense (although the hypervisor then
 gets to choose method priorities).  I'm not sure your logic matches the
 goal of allowing the user to request both agent and acpi at the same time.
 

Aah. Okay; I thought they were considered as mutually exclusive. Hence
my code is not correct. Let me re-spin v2.

Michal

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