Re: [libvirt] [PATCH v2 1/8] hellolibvirt: Update hellolibvirt example

2013-02-24 Thread Laine Stump
On 02/22/2013 07:46 AM, John Ferlan wrote:
 On 02/20/2013 08:05 PM, Dave Allan wrote:
 On Wed, Feb 20, 2013 at 12:38:38PM -0500, John Ferlan wrote:
 Update the code to be more in line with how code looks elsewhere in
 libvirt.  Allow listing of domains, networks, storage pools, and
 network interfaces.
 I like the changes to make the style more in line with the rest of the
 codebase, however, I really think that this example code should be
 about a minimal use of the API to list a few domains and that's all,
 so I'm not in favor of extending it to list other kinds of objects.  I
 think people can find the details of how to do that kind of thing in
 the API docs.

 Dave

 Update the function prototypes in libvirt.c to include a message about
 the client needing to free() returned name fields.  Fix the all domains
 example flags values.
 ---
  examples/hellolibvirt/hellolibvirt.c | 131 
 ++-
  src/libvirt.c|  21 +++---
  2 files changed, 91 insertions(+), 61 deletions(-)
 Any other (strong) opinions one way or another?  Should I remove the
 hellolibvirt.c for now?

 Adding network, volume groups, and interfaces was (mostly) trivial once
 I got past the set up the structure to handle multiple types. It
 certainly shows how similar the API's are for various objects and how
 trivial it is to gather generic information for each.

That's a neat idea, but I think putting the actual functions being
called into a table and calling them indirectly obscures what's trying
to be exhibited.

Aside from that, the virInterface API *still* isn't available on all
platforms, so I don't think that's a good thing to have in a basic
example program - we would get too many noobs asking why the example
program is broken.

I think it would be better to just provide simple examples for domain,
with a comment somewhere that says storage pool, network, and interface
APIs work in a similar fashion or something like that. (And maybe you
could have this more complicated example called hellolibvirt2.c or
something)


 Any thoughts on the libvirt.c changes which are primarily documentation
 of the need to free the 'names' returned.  That was a result of a review
 comment which noted that the comment in hellolibvirt.c if true should be
 rectified. I forgot to put the v2-v1 differences marker when generating
 the patch to call this one out specifically.

Those changes seem fine to me, although maybe they should be in a
separate patch, since they aren't really part of updating hellolibvirt.

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


Re: [libvirt] [PATCH 5/5] storage: qemu-img: change INFO to DEBUG

2013-02-24 Thread Ján Tomko
On 02/22/13 14:28, John Ferlan wrote:
 On 02/18/2013 09:27 AM, Ján Tomko wrote:
 For really old qemu-img binaries which do not support specifying
 the format of the backing file, display a DEBUG message instead of
 INFO that this can't be done.
 ---
  src/storage/storage_backend.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 ACK
 
 John

Thanks. I've pushed the series.

Jan

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


[libvirt] [PATCH] Hook log the exit status of the hook not 256

2013-02-24 Thread Guido Günther
Adjust the docs accordingly. See http://bugs.debian.org/701570.
---
 docs/hooks.html.in |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/hooks.html.in b/docs/hooks.html.in
index 5f9963d..b75fbeb 100644
--- a/docs/hooks.html.in
+++ b/docs/hooks.html.in
@@ -239,13 +239,14 @@
 pIf a hook script returns with an exit code of 0, the libvirt daemon
regards this as successful and performs no logging of it./p
 pHowever, if a hook script returns with a non zero exit code, the libvirt
-   daemon regards this as a failure, logs it with return code 256, and
+   daemon regards this as a failure, logs it's return code, and
additionally logs anything on stderr the hook script returns./p
 pFor example, a hook script might use this code to indicate failure,
and send a text string to stderr:/p
 preecho Could not find required XYZZY gt;amp;2
 exit 1/pre
 pThe resulting entry in the libvirt log will appear as:/p
-pre20:02:40.297: error : virHookCall:416 : Hook script execution failed: 
Hook script /etc/libvirt/hooks/qemu qemu failed with error code 256:Could not 
find required XYZZY/pre
+pre20:02:40.297: error : virHookCall:285 : Hook script execution failed: 
internal error Child process (LC_ALL=C 
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
+   HOME=/root USER=root LOGNAME=root 
/etc/libvirt/hooks/qemu qemu prepare begin -) unexpected exit status 1: Could 
not find required XYZZY/pre
   /body
 /html
-- 
1.7.10.4

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


Re: [libvirt] [PATCH v4 0/4] add pci-bridge support

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

在 2013-02-19二的 10:25 +0800,liguang写道:
 Now, it's impossible to arrange devices into multi-pci-bus,
 for example:
sound model='ac97'
   address type='pci' domain='0x' bus='0x00' slot='0x04' 
 function='0x0'/
/sound
video
   model type='cirrus' vram='9216' heads='1'/
   address type='pci' domain='0x' bus='0x1' slot='0x02' 
 function='0x0'/
/video
 libvirt will complain about bus != 0,
 fortunately, qemu supports pci-to-pci bridge,
 if we want to use multi-pci-bus, we can define
 2 pci bridge controllers, then attach 1 to the other
 as a subordinate pci-bus, so, 2 pci-buses appear.
 for example:
controller type='pci-bridge' index='0'/
controller type='pci-bridge' index='1'
   address type='pci' domain='0x' bus='0x00' slot='0x05' 
 function='0x0'/
/controller
sound model='ac97'
   address type='pci' domain='0x' bus='0x01' slot='0x02' 
 function='0x0'/
/sound
video
   model type='cirrus' vram='9216' heads='1'/
   address type='pci' domain='0x' bus='0x00' slot='0x02' 
 function='0x0'/
/video
 
 
  src/conf/domain_conf.c|   98 -
  src/conf/domain_conf.h|1 +
  docs/schemas/domaincommon.rng |1 +
  src/qemu/qemu_capabilities.c  |4 +
  src/qemu/qemu_capabilities.h  |1 +
  src/qemu/qemu_command.c   |   43 -
  6 files changed, 126 insertions(+), 22 deletions(-)



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

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

2013-02-24 Thread Li Zhang

Hello,

Does anyone have any comments?

On 2013年02月22日 17:09, 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
---
  src/conf/domain_conf.c  |   83 ++-
  src/conf/domain_conf.h  |   10 ++
  src/qemu/qemu_command.c |   31 ++
  src/qemu/qemu_command.h |2 ++
  4 files changed, 125 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 @@ virDomainDefFormatInternal(virDomainDefPtr def,
  if (def-memballoon)
  

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

2013-02-24 Thread Li Zhang

Ping ...

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, QEMU_CAPS_DUMP_GUEST_CORE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(dump-guest-core is not available 
+ with this QEMU binary));
+return -1;
+}
+
+virBufferAsprintf(buf, ,dump-guest-core=%s,
+  

[libvirt] [PATCH] interface: Fix udev backend bridge device display

2013-02-24 Thread Doug Goldstein
The bridge device was showing the vnet devices created for the domains
as connected to the bridge. libvirt should only show host devices when
trying to get the interface definition rather than the domain devices as
well.
---
Honestly this method sucks. But it makes the code path work and doesn't
result in brokenness. I was really thinking of sscanf() but I don't really
care to store the values. Suggestions?
---
 src/interface/interface_backend_udev.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/interface/interface_backend_udev.c 
b/src/interface/interface_backend_udev.c
index bd83545..dca85b3 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -24,7 +24,9 @@
 #include libudev.h
 
 #include virerror.h
+#include c-ctype.h
 #include datatypes.h
+#include domain_conf.h
 #include interface_driver.h
 #include interface_conf.h
 #include viralloc.h
@@ -527,6 +529,16 @@ udevIfaceBridgeScanDirFilter(const struct dirent *entry)
 if (STREQ(entry-d_name, .) || STREQ(entry-d_name, ..))
 return 0;
 
+/* Omit the domain interfaces from the list of bridge attached
+ * devices. All we can do is check for the device name matching
+ * vnet%d. Improvements to this check are welcome.
+ */
+if (strlen(entry-d_name) = 5) {
+if (STRPREFIX(entry-d_name, VIR_NET_GENERATED_PREFIX) 
+c_isdigit(entry-d_name[4]))
+return 0;
+}
+
 return 1;
 }
 
-- 
1.7.12.4

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


[libvirt] [PATCH] virsh: add --start option to the define command

2013-02-24 Thread Doug Goldstein
Adds the --start option to the define command to simplify the often used
case of virsh define dom.xml; start dom or virsh define dom.xml 
virsh start dom.

This is just a rebased version of:
https://www.redhat.com/archives/libvir-list/2013-January/msg00490.html

There's a competing patchset available at:
https://www.redhat.com/archives/libvir-list/2013-February/msg01298.html
---
 tools/virsh-domain.c | 23 +--
 tools/virsh.pod  |  5 +++--
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 96dd4fa..6d62197 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6602,6 +6602,10 @@ static const vshCmdOptDef opts_define[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_(file containing an XML domain description)
 },
+{.name = start,
+ .type = VSH_OT_BOOL,
+ .help = N(start the domain after definition)
+},
 {.name = NULL}
 };
 
@@ -6612,6 +6616,7 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd)
 const char *from = NULL;
 bool ret = true;
 char *buffer;
+bool start = false;
 
 if (vshCommandOptStringReq(ctl, cmd, file, from)  0)
 return false;
@@ -6619,17 +6624,31 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd)
 if (virFileReadAll(from, VSH_MAX_XML_FILE, buffer)  0)
 return false;
 
+start = vshCommandOptBool(cmd, start);
+
 dom = virDomainDefineXML(ctl-conn, buffer);
 VIR_FREE(buffer);
 
 if (dom != NULL) {
 vshPrint(ctl, _(Domain %s defined from %s\n),
  virDomainGetName(dom), from);
-virDomainFree(dom);
 } else {
 vshError(ctl, _(Failed to define domain from %s), from);
-ret = false;
+return false;
 }
+
+/* Start the domain if the user requested it and it was defined */
+if (start) {
+if (virDomainCreate(dom)  0) {
+vshError(ctl, _(Failed to start domain %s, which was 
+successfully defined.), virDomainGetName(dom));
+ret = false;
+} else {
+vshPrint(ctl, _(Domain %s started\n), virDomainGetName(dom));
+}
+}
+
+virDomainFree(dom);
 return ret;
 }
 
diff --git a/tools/virsh.pod b/tools/virsh.pod
index a5d8fe6..660331c 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -555,11 +555,12 @@ BExample
  vi domain.xml (or make changes with your other text editor)
  virsh create domain.xml
 
-=item Bdefine IFILE
+=item Bdefine IFILE [I--start]
 
 Define a domain from an XML file. The domain definition is registered
 but not started.  If domain is already running, the changes will take
-effect on the next boot.
+effect on the next boot. If I--start is requested, start the domain
+after defining it.
 
 =item Bdesc Idomain [[I--live] [I--config] |
   [I--current]] [I--title] [I--edit] [I--new-desc
-- 
1.7.12.4

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


Re: [libvirt] git send-email should not allow 'y' for in-reply-to

2013-02-24 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Fri, Jan 11, 2013 at 10:43:39AM -0800, Hilco Wijbenga wrote:
 ...
 More seriously, I agree that re-wording the question is a reasonable
 thing to do. I do not use send-email, either, so I don't have a strong
 opinion on it. The suggestions you made:

 How about What Message-ID to use as In-Reply-To for the first email?
 or Provide the Message-ID to use as In-Reply-To for the first
 email:.

 seem fine to me. Maybe somebody who has been confused by it can offer
 more. At any rate, patches welcome.

Has anything come out of this discussion?  Is the current phrasing
fine as-is?

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


[libvirt] [PATCH] docs: add forward mode='hostdev' example

2013-02-24 Thread Laine Stump
Also rearrange examples so that all managed networks (those with
bridges created by libvirt) are together.
---
 docs/formatnetwork.html.in | 52 +-
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 4b4c47b..46b7270 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -332,7 +332,7 @@
 /p
 pre
 ...
-  lt;forward mode='passthrough'gt;
+  lt;forward mode='passthrough' managed='yes'gt;
 lt;pf dev='eth0'/gt;
   lt;/forwardgt;
 ...
@@ -863,6 +863,27 @@
 lt;/ipgt;
   lt;/networkgt;/pre
 
+h3a name=examplesNoGatewayNetwork config with no gateway 
addresses/a/h3
+
+p
+A valid network definition can contain no IPv4 or IPv6 addresses.
+Such a definition can be used for a very private or very
+isolated network since it will not be possible to communicate
+with the virtualization host via this network.  However, this
+virtual network interface can be used for communication between
+virtual guest systems.  This works for IPv4
+and span class=since(Since 1.0.1)/span IPv6.  However,
+the codeipv6/code attribute must be set to yes for
+functional guest-to-guest IPv6 communication.
+/p
+
+pre
+  lt;network ipv6='yes'gt;
+lt;namegt;nogwlt;/namegt;
+lt;bridge name=virbr2 stp=on delay=0/gt;
+  lt;/networkgt;
+/pre
+
 h3a name=examplesBridgeUsing an existing host bridge/a/h3
 
 p
@@ -916,25 +937,26 @@
 lt;/forwardgt;
   lt;/networkgt;/pre
 
-h3a name=examplesNoGatewayNetwork config with no gateway 
addresses/a/h3
+h3a name=examplesPCIPassthroughAssigning SR-IOV Virtual Functions 
via PCI Passthrough/a/h3
 
 p
-A valid network definition can contain no IPv4 or IPv6 addresses.  Such a 
definition
-can be used for a very private or very isolated network since it will 
not be
-possible to communicate with the virtualization host via this network.  
However,
-this virtual network interface can be used for communication between 
virtual guest
-systems.  This works for IPv4 and span class=since(Since 1.0.1)/span 
IPv6.
-However, the new ipv6='yes' must be added for guest-to-guest IPv6
-communication.
+  span class=sinceSince 0.9.10, requires a host with working
+  IOMMU/span This example shows how to use a libvirt network to
+  assign individual SR-IOV Virtual Functions (VF) to guests using
+  PCI passthrough. Only the Physical Function (PF) of the SR-IOV
+  network adapter needs to be listed in the network definition; a
+  list of all VFs will be automatically derived from that, and VFs
+  will be allocated to guest domains as requested.
 /p
 
 pre
-  lt;network ipv6='yes'gt;
-lt;namegt;nogwlt;/namegt;
-lt;uuidgt;7a3b7497-1ec7-8aef-6d5c-38dff9109e93lt;/uuidgt;
-lt;bridge name=virbr2 stp=on delay=0 /gt;
-lt;mac address='00:16:3E:5D:C7:9E'/gt;
-  lt;/networkgt;/pre
+  lt;networkgt;
+lt;namegt;passthroughlt;/namegt;
+lt;forward mode='hostdev' managed='yes'gt;
+  lt;pf dev='eth3'/gt;
+lt;/forwardgt;
+  lt;/networkgt;
+/pre
 
   /body
 /html
-- 
1.7.11.7

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


[libvirt] [PATCH 1/2] conf: use VIR_EXPAND instead of VIR_REALLOC in virNetworkDHCPDefParseXML

2013-02-24 Thread Laine Stump
VIR_REALLOC_N doesn't initialize the newly allocated memory to 0,
which can result in unpleasant surprises for those who have become
accustomed to the 0-initializing behavior of most libvirt memory
allocation functions, and add a new field to a struct thinking that it
will default to 0.
---

(in particular, this patch assures that the upcoming force attribute
in virNetworkDHCPOptionDef will be initialized to false.)

 src/conf/network_conf.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 34fd05a..b3e2858 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -820,29 +820,29 @@ virNetworkDHCPDefParseXML(const char *networkName,
 if (cur-type == XML_ELEMENT_NODE 
 xmlStrEqual(cur-name, BAD_CAST range)) {
 
-if (VIR_REALLOC_N(def-ranges, def-nranges + 1)  0) {
+if (VIR_EXPAND_N(def-ranges, def-nranges, 1)  0) {
 virReportOOMError();
 return -1;
 }
 if (virSocketAddrRangeParseXML(networkName, cur,
-   def-ranges[def-nranges])  
0) {
+   def-ranges[def-nranges - 1])  
0) {
+def-nranges--;
 return -1;
 }
-def-nranges++;
 
 } else if (cur-type == XML_ELEMENT_NODE 
 xmlStrEqual(cur-name, BAD_CAST host)) {
 
-if (VIR_REALLOC_N(def-hosts, def-nhosts + 1)  0) {
+if (VIR_EXPAND_N(def-hosts, def-nhosts, 1)  0) {
 virReportOOMError();
 return -1;
 }
 if (virNetworkDHCPHostDefParseXML(networkName, def, cur,
-  def-hosts[def-nhosts],
+  def-hosts[def-nhosts - 1],
   false)  0) {
+def-nhosts--;
 return -1;
 }
-def-nhosts++;
 
 } else if (VIR_SOCKET_ADDR_IS_FAMILY(def-address, AF_INET) 
cur-type == XML_ELEMENT_NODE 
@@ -870,15 +870,15 @@ virNetworkDHCPDefParseXML(const char *networkName,
 VIR_FREE(server);
 } else if (cur-type == XML_ELEMENT_NODE 
 xmlStrEqual(cur-name, BAD_CAST option)) {
-if (VIR_REALLOC_N(def-options, def-noptions + 1)  0) {
+if (VIR_EXPAND_N(def-options, def-noptions, 1)  0) {
 virReportOOMError();
 return -1;
 }
 if (virNetworkDHCPOptionDefParseXML(networkName, cur,
-def-options[def-noptions])) 
{
+def-options[def-noptions - 
1])) {
+def-noptions--;
 return -1;
 }
-def-noptions++;
 }
 
 cur = cur-next;
-- 
1.7.11.7

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


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

2013-02-24 Thread Laine Stump
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(-)

-- 
1.7.11.7

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


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

2013-02-24 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.
---
 docs/formatnetwork.html.in | 21 +
 src/conf/network_conf.c| 14 ++
 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, 41 insertions(+), 4 deletions(-)

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index f7c483d..4b4c47b 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/src/conf/network_conf.c b/src/conf/network_conf.c
index b3e2858..8434dc4 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -782,6 +782,7 @@ virNetworkDHCPOptionDefParseXML(const char *networkName,
 virNetworkDHCPOptionDefPtr option)
 {
 char *number = NULL;
+char *force = NULL;
 int ret = -1;
 
 if (!(number = virXMLPropString(node, number))) {
@@ -797,12 +798,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:
 VIR_FREE(number);
+VIR_FREE(force);
 return ret;
 }
 
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index e7a4f95..fd7a03c 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -1,7 +1,7 @@
 /*
  * network_conf.h: network XML handling
  *
- * Copyright (C) 2006-2008, 2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -75,6 +75,7 @@ typedef virNetworkDHCPOptionDef *virNetworkDHCPOptionDefPtr;
 struct _virNetworkDHCPOptionDef {
 unsigned int number;
 char *value;
+bool force;
 };
 
 typedef struct _virNetworkDNSTxtDef virNetworkDNSTxtDef;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0c3f778..e66a55d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -928,7 +928,8 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
 }
 
 for (r = 0 ; r  ipdef-noptions ; r++) {
-

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

2013-02-24 Thread Li Zhang
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 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 if (def-memballoon)
 virDomainMemballoonDefFormat(buf, def-memballoon, flags);
 
+if 

Re: [libvirt] [PATCH] interface: Fix udev backend bridge device display

2013-02-24 Thread Laine Stump
On 02/24/2013 09:36 PM, Doug Goldstein wrote:
 The bridge device was showing the vnet devices created for the domains
 as connected to the bridge. libvirt should only show host devices when
 trying to get the interface definition rather than the domain devices as
 well.
 ---
 Honestly this method sucks. But it makes the code path work and doesn't
 result in brokenness. I was really thinking of sscanf() but I don't really
 care to store the values. Suggestions?

This is of course still inexact, since 1) it is possible/acceptable for
someone to name their tap devices something else, and 2) other pieces of
software may also be creating transient devices that could be connected
to a bridge. In the end, this is just one of the symptoms of using
current live status to make an approximation of what is really
supposed to be persistent configuration. I've always been a bit
lukewarm on the idea (as you know :-).

I don't really have any better idea for this (other than directly
reading the configuration off of disk, in which case you should be
writing a netcf backend rather than a libvirt interface driver backend),
and this apparently makes the situation better than it was before, so
ACK to this patch.

 ---
  src/interface/interface_backend_udev.c | 12 
  1 file changed, 12 insertions(+)

 diff --git a/src/interface/interface_backend_udev.c 
 b/src/interface/interface_backend_udev.c
 index bd83545..dca85b3 100644
 --- a/src/interface/interface_backend_udev.c
 +++ b/src/interface/interface_backend_udev.c
 @@ -24,7 +24,9 @@
  #include libudev.h
  
  #include virerror.h
 +#include c-ctype.h
  #include datatypes.h
 +#include domain_conf.h
  #include interface_driver.h
  #include interface_conf.h
  #include viralloc.h
 @@ -527,6 +529,16 @@ udevIfaceBridgeScanDirFilter(const struct dirent *entry)
  if (STREQ(entry-d_name, .) || STREQ(entry-d_name, ..))
  return 0;
  
 +/* Omit the domain interfaces from the list of bridge attached
 + * devices. All we can do is check for the device name matching
 + * vnet%d. Improvements to this check are welcome.
 + */
 +if (strlen(entry-d_name) = 5) {
 +if (STRPREFIX(entry-d_name, VIR_NET_GENERATED_PREFIX) 
 +c_isdigit(entry-d_name[4]))
 +return 0;
 +}
 +
  return 1;
  }
  

--
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-24 Thread Laine Stump
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.


Looks good to me. I'm pushing it with that change.

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


Re: [libvirt] [PATCH 2/2] use client id for IPv6 DHCP host definition

2013-02-24 Thread Laine Stump
On 02/19/2013 04:44 PM, Eric Blake wrote:
 On 02/18/2013 07:38 PM, Laine Stump wrote:
 On 02/15/2013 02:02 PM, Gene Czarcinski wrote:
 Originally, only a host name was used to associate a
 DHCPv6 request with a specific IPv6 address.  Further testing
 demonstrates that this is an unreliable method and, instead,
 a client-id or DUID needs to be used.  According to DHCPv6
 standards, this id can be a duid-LLT, duid-LL, or duid-UUID
 even though dnsmasq will accept almost any text string.

 Other than the suggestion to use strcspn() instead of a while loop, this
 all looks fine to me. ACK with that change made. If you (or someone
 else) wants to ACK the short interdiff I've attached that makes that
 change, I'll push it.

 From 0b2f2f0794e931af901bc31e4fe6eadc799bee33 Mon Sep 17 00:00:00 2001
 From: Laine Stump la...@laine.org
 Date: Mon, 18 Feb 2013 21:33:49 -0500
 Subject: [PATCH] Changes to squash into use client id for IPv6 DHCP host
  definition

 ---
  src/conf/network_conf.c | 15 +--
  src/util/virdnsmasq.c   |  3 +--
  2 files changed, 6 insertions(+), 12 deletions(-)

 diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
 index 8657284..12bf4d7 100644
 --- a/src/conf/network_conf.c
 +++ b/src/conf/network_conf.c
 @@ -710,16 +710,11 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
  
  id = virXMLPropString(node, id);
  if (id) {
 -char *cp = id;
 -
 -while (*cp) {
 -if ((*cp != ':')  (!c_isxdigit(*cp))) {
 -virReportError(VIR_ERR_XML_ERROR,
 -   _(Cannot use id '%s' in network '%s'),
 -   id, networkName);
 -goto cleanup;
 -}
 -cp++;
 +char *cp = id + strcspn(id, 0123456789abcdefABCDEF:);
 +if (*cp) {
 +virReportError(VIR_ERR_XML_ERROR,
 +   _(Invalid character '%c' in id '%s' of network 
 '%s'),
 +   *cp, id, networkName);
 That's not quite right.  You want to use strspn(), not strcspn().

Argh! I would have caught that if I'd paid attention to make check.

In the meantime, I noticed that there weren't any networkxml2xml tests
for dhcp6 host records, so I added one before pushing.


 +++ b/src/util/virdnsmasq.c
 @@ -328,8 +328,7 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
  id, ipstr)  0)
  goto alloc_error;
  }
 -}
 -else if (name  mac) {
 +} else if (name  mac) {
 This hunk is good.

 ACK to your interdiff once you use the correct function.


I've now pushed both the patches in this series.

Thanks Gene! (and thanks for Eric for catching my mistake!)

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