Re: [libvirt] Two issues regarding TFTP support in virtual networks...

2010-01-13 Thread Paolo Bonzini



You are not giving BOOTP info (you are missing thebootp  tag,
which goes insidedhcp).


Okay, here's the XML that's now being generated by my test script:

network
nametestbr6253/name
forward mode='nat' /
bridge name='testbr6253' stp='on' forwardDelay='0' /
ip address='192.168.133.1' netmask='255.255.255.0'
tftp root='/tmp/tmp.dEf7FPCaKm/tftpboot' /
bootp file='pxelinux.0' /
dhcp
range start='192.168.133.100' end='192.168.133.199' /
/dhcp
/ip
/network

However, I still get the message No filename or root path specified
within the VM and it does not boot from the above directory.


bootp must be within dhcp (think of elements within ip as services 
that run, such as TFTP or DHCP; bootp is just additional data that is 
passed via DHCP).


Paolo

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


Re: [libvirt] [PATCH] Implement CPU topology support for QEMU driver

2010-01-13 Thread Daniel P. Berrange
On Wed, Jan 13, 2010 at 09:47:00AM +0100, Jiri Denemark wrote:
ADD_ARG_LIT(-smp);
   -ADD_ARG_LIT(vcpus);
   +if ((qemuCmdFlags  QEMUD_CMD_FLAG_SMP_TOPOLOGY)  def-cpu) {
   +virBuffer buf = VIR_BUFFER_INITIALIZER;
   +
   +virBufferVSprintf(buf, %lu, def-vcpus);
   +
   +if (def-cpu-sockets  0)
   +virBufferVSprintf(buf, ,sockets=%u, def-cpu-sockets);
   +
   +if (def-cpu-cores  0)
   +virBufferVSprintf(buf, ,cores=%u, def-cpu-cores);
   +
   +if (def-cpu-threads  0)
   +virBufferVSprintf(buf, ,threads=%u, def-cpu-threads);
   +
   +if (virBufferError(buf)) {
   +virBufferFreeAndReset(buf);
   +goto no_memory;
   +}
   +
   +ADD_ARG(virBufferContentAndReset(buf));
   +}
   +else {
   +char vcpus[50];
   +snprintf(vcpus, sizeof(vcpus), %lu, def-vcpus);
   +ADD_ARG_LIT(vcpus);
   +}
  
  
  Can you split this piece of code out into a separate method with a
  signature like
  
static char *qemuBuildCPUStr(virDomainDefPtr def, int qemuCmdFlags);
  
  The main qemuBuildCommandLine method is getting out of control :-)
 
 With pleasure.
 
  Also, in the case where 'def-cpu' is NULL, but QEMUD_CMD_FLAG_SMP_TOPOLOGY
  is available, I think we should explicitly set an arg based on
  
 sockets=def-vcpus
 cores=1
 threads=1
  
  so that we avoid relying on any changable QEMU default values.
 
 Makes sense. I was also considering a check for vcpus == sockets * cores *
 threads, but I'm not totally convinced it is a good idea.

You don't want todo that because it is fine to have a topology that is
not fully populated with CPUs

 
   +do {
   +if (*p == '\0' || *p == ',')
   +goto syntax;
   +
   +if ((next = strchr(p, ',')))
   +next++;
   +
   +if (c_isdigit(*p)) {
   +int n;
   +if (virStrToLong_i(p, end, 10, n)  0 ||
   +!end || (*end != ','  *end != '\0'))
   +goto syntax;
   +dom-vcpus = n;
   +} else {
   +int i;
   +int n = -1;
   +for (i = 0; i  ARRAY_CARDINALITY(options); i++) {
   +if (STRPREFIX(p, options[i])) {
   +p += strlen(options[i]);
   +if (virStrToLong_i(p, end, 10, n)  0 ||
   +!end || (*end != ','  *end != '\0'))
   +goto syntax;
   +topology[i] = n;
   +break;
   +}
   +}
   +
   +if (n  0)
   +goto syntax;
   +}
   +} while ((p = next));
  
  If you strip off the initial  CPU count value, then you can use
  
 qemuParseCommandLineKeywords
  
  to parse the reset of the  name=value  args.
 
 Ah, cool, I didn't know about it. I guess because it's not used very often in
 the code. BTW, would it make sense to extend the function to (optionally)
 support parsing of any comma-separated list such as keyword,name=value,...?
 The keyword parts would result in NULL value in retvalues array and the
 name=value parts would behave normally.

Yes, there are a couple of places where that might be useful.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] [PATCH] util: Remove logging handlers in virExec

2010-01-13 Thread Daniel P. Berrange
On Tue, Jan 12, 2010 at 03:54:39PM -0500, Cole Robinson wrote:
 On 01/12/2010 03:46 PM, Daniel P. Berrange wrote:
  On Tue, Jan 12, 2010 at 03:26:26PM -0500, Cole Robinson wrote:
  This allows debug statements and raised errors in hook functions to
  actually be logged somewhere (stderr). Users can enable debugging in the
  daemon and now see more info in /var/log/libvirt/...
 
  Signed-off-by: Cole Robinson crobi...@redhat.com
  ---
   src/util/util.c |6 ++
   1 files changed, 6 insertions(+), 0 deletions(-)
 
  diff --git a/src/util/util.c b/src/util/util.c
  index 44a4b2f..23d781d 100644
  --- a/src/util/util.c
  +++ b/src/util/util.c
  @@ -334,6 +334,7 @@ __virExec(virConnectPtr conn,
   int pipeerr[2] = {-1,-1};
   int childout = -1;
   int childerr = -1;
  +int logprio;
   sigset_t oldmask, newmask;
   struct sigaction sig_action;
   
  @@ -452,6 +453,11 @@ __virExec(virConnectPtr conn,
  of being seen / logged */
   virSetErrorFunc(NULL, NULL);
   
  +/* Make sure any hook logging is sent to stderr */
  +logprio = virLogGetDefaultPriority();
  +virLogReset();
  +virLogSetDefaultPriority(logprio);
  +
  
  I'm not sure that I understand this - surely the child process is already
  inheriting this setup from the parent libvirtd ?
  
  Daniel
 
 Problem is that virExec closes only the file descriptors it is using,
 which means it will close the file descriptors for any nontrivial
 logging handlers. I could add some internal logging API to get the
 relevant fds and keep them from being closed, but not sure if we
 necessarily want to preserve those for a daemonized process?

Oh of course - can you put a comment to that effect in your patch as a 
reminder.  ACK to the patch 

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] [PATCH] virterror: Don't invoke error callback for ERR_OK

2010-01-13 Thread Daniel P. Berrange
On Tue, Jan 12, 2010 at 03:57:33PM -0500, Cole Robinson wrote:
 On 01/12/2010 03:48 PM, Daniel P. Berrange wrote:
  On Tue, Jan 12, 2010 at 03:26:28PM -0500, Cole Robinson wrote:
  Since virDispatchError is now responsible for invoking the error callback,
  give it the same semantics as ReportError, which will skip VIR_ERR_OK
  (which is encountered when no error was raised).
 
  This fixes invoking the error callback after every non-erroring API call.
 
  Signed-off-by: Cole Robinson crobi...@redhat.com
  ---
   src/util/virterror.c |6 +-
   1 files changed, 5 insertions(+), 1 deletions(-)
 
  diff --git a/src/util/virterror.c b/src/util/virterror.c
  index e2128b9..78974ee 100644
  --- a/src/util/virterror.c
  +++ b/src/util/virterror.c
  @@ -603,8 +603,12 @@ virDispatchError(virConnectPtr conn)
   if (!err)
   return;
   
  -/* Set a generic error message if none is already set */
  +/* We never used to raise ERR_OK, so maintain existing behavior */
   if (err-code == VIR_ERR_OK)
  +return;
  +
  +/* Set a generic error message if none is already set */
  +if (!err-message)
   virErrorGenericFailure(err);
   
   /* Copy the global error to per-connection error if needed */
  
  We should only ever be invoking virDispatchError() in error paths, so
  if err-code == VIR_ERR_OK, this means we do need set a generic message
  because the earlier code indicated an error but forgot to report one.
  So I don't think this is correct.
  
 
 Ah, I think I wanted to check VIR_ERR_NONE here actually.
 virDispatchError is called regardless of whether an error is actually
 raised, so it may receive a zero'd out/empty virLastErrorObject, which
 is what I'm trying to avoid reporting.

I still don't think you are correct in that. If you run

  # grep --after 1 virDispatchError libvirt.c
virDispatchError(NULL);
return (-1);
  --
  virDispatchError(net-conn);
  return -1;
  --
  virDispatchError(NULL);
  return (-1);
  --
  virDispatchError(pool-conn);
  return -1;

Then all cases where virDispatchError() is called should be followed by the
return of an error code, 99% of them are -1 or NULL. There are one or two
where we use '0' for error as a special case. I don't see any places where
virDispatchError() is called in a successful return path. So we should
always be invoking the error callback, and ensuring an error is actually 
set before doing so.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] [PATCH] Implement path lookup for USB by vendor:product

2010-01-13 Thread Daniel P. Berrange
On Tue, Jan 12, 2010 at 03:26:29PM -0500, Cole Robinson wrote:
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index deb8adc..f03ce91 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -2105,14 +2105,11 @@ static int 
 qemuDomainSetHostdevUSBOwnership(virConnectPtr conn,
  struct qemuFileOwner owner = { uid, gid };
  int ret = -1;
  
 -/* XXX what todo for USB devs assigned based on product/vendor ? Doom 
 :-( */
 -if (!def-source.subsys.u.usb.bus ||
 -!def-source.subsys.u.usb.device)
 -return 0;
 -
  usbDevice *dev = usbGetDevice(conn,
def-source.subsys.u.usb.bus,
 -  def-source.subsys.u.usb.device);
 +  def-source.subsys.u.usb.device,
 +  def-source.subsys.u.usb.vendor,
 +  def-source.subsys.u.usb.product);
  
  if (!dev)
  goto cleanup;
 diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
 index 000bc8a..e015c06 100644
 --- a/src/security/security_selinux.c
 +++ b/src/security/security_selinux.c
 @@ -484,7 +484,9 @@ SELinuxSetSecurityHostdevLabel(virConnectPtr conn,
  if (dev-source.subsys.u.usb.bus  dev-source.subsys.u.usb.device) 
 {
  usbDevice *usb = usbGetDevice(conn,
dev-source.subsys.u.usb.bus,
 -  dev-source.subsys.u.usb.device);
 +  dev-source.subsys.u.usb.device,
 +  dev-source.subsys.u.usb.vendor,
 +  dev-source.subsys.u.usb.product);
  
  if (!usb)
  goto done;

You need to remove the surrounding  if/else clause here


 diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
 index 35b29ad..6a52d4c 100644
 --- a/src/security/virt-aa-helper.c
 +++ b/src/security/virt-aa-helper.c
 @@ -839,8 +839,11 @@ get_files(vahControl * ctl)
  if (dev-source.subsys.u.usb.bus 
  dev-source.subsys.u.usb.device) {
  usbDevice *usb = usbGetDevice(NULL,
 -   dev-source.subsys.u.usb.bus,
 -   dev-source.subsys.u.usb.device);
 +  dev-source.subsys.u.usb.bus,
 +  dev-source.subsys.u.usb.device,
 +  dev-source.subsys.u.usb.vendor,
 +  dev-source.subsys.u.usb.product);
 +
  if (usb == NULL)
  continue;
  rc = usbDeviceFileIterate(NULL, usb,

And likewise here.

 diff --git a/src/util/hostusb.c b/src/util/hostusb.c
 index 07e10b1..3e9ac83 100644
 --- a/src/util/hostusb.c
 +++ b/src/util/hostusb.c
 @@ -37,9 +37,10 @@
  #include util.h
  #include virterror_internal.h
  
 +#define USB_SYSFS /sys/bus/usb
  #define USB_DEVFS /dev/bus/usb/
 -#define USB_ID_LEN 10 /*   */
 -#define USB_ADDR_LEN 8 /* XXX:XXX */
 +#define USB_ID_LEN 10 /* 1234 5678 */
 +#define USB_ADDR_LEN 8 /* 123:456 */
  
  struct _usbDevice {
  unsigned  bus;
 @@ -57,11 +58,95 @@ struct _usbDevice {
  virReportErrorHelper(conn, VIR_FROM_NONE, code, __FILE__,  \
   __FUNCTION__, __LINE__, fmt)
  
 +static int usbSysReadFile(virConnectPtr conn,
 +  const char *f_name, const char *d_name,
 +  const char *fmt, unsigned *value)
 +{
 +int ret = -1;
 +char *buf = NULL;
 +char filename[PATH_MAX];
 +
 +snprintf(filename, PATH_MAX, USB_SYSFS /devices/%s/%s, d_name, f_name);

Lets us virAsprintf() here.

 +
 +if (virFileReadAll(filename, 1024, buf)  0)
 +goto error;
 +
 +if (sscanf(buf, fmt, value) != 1) {
 +virReportSystemError(conn, errno,
 + _(Could not parse usb file %s), filename);
 +goto error;
 +}

The callers all either pass in %x or %d to this, so how about just
using virStrToLong  and passing in the 'base' arg instead of format.

 +static int usbFindBusByVendor(virConnectPtr conn,
 +  unsigned vendor, unsigned product,
 +  unsigned *bus, unsigned *devno)
 +{
 +DIR *dir = NULL;
 +int ret = -1, found = 0;
 +struct dirent *de;
 +
 +dir = opendir(USB_SYSFS /devices);
 +if (!dir) {
 +virReportSystemError(conn, errno,
 + _(Could not open directory %s),
 + USB_SYSFS /devices);
 +goto error;
 +}
 +
 +while ((de = readdir(dir))) {
 +unsigned found_prod, found_vend;
 +if (de-d_name[0] == '.' || strchr(de-d_name, ':'))
 +continue;
 +
 +if 

Re: [libvirt] [PATCH] node_device: udev: Enumerate floppy devices

2010-01-13 Thread Daniel P. Berrange
On Tue, Jan 12, 2010 at 03:26:30PM -0500, Cole Robinson wrote:
 There are quite a few differences between how udev exposes legacy
 and USB floppy devs, but this patch takes care of both variants.
 
 Signed-off-by: Cole Robinson crobi...@redhat.com
 ---
  src/node_device/node_device_udev.c |   88 ---
  1 files changed, 70 insertions(+), 18 deletions(-)
 
 diff --git a/src/node_device/node_device_udev.c 
 b/src/node_device/node_device_udev.c
 index 55cfee2..a9e196d 100644
 --- a/src/node_device/node_device_udev.c
 +++ b/src/node_device/node_device_udev.c
 @@ -840,30 +840,19 @@ out:
  }
  
  
 -static int udevProcessCDROM(struct udev_device *device,
 -virNodeDeviceDefPtr def)
 +static int udevProcessRemoveableMedia(struct udev_device *device,
 +  virNodeDeviceDefPtr def,
 +  int has_media)
  {
  union _virNodeDevCapData *data = def-caps-data;
  int tmp_int = 0, ret = 0;
  
 -/* NB: the drive_type string provided by udev is different from
 - * that provided by HAL; now it's cd instead of cdrom We
 - * change it to cdrom to preserve compatibility with earlier
 - * versions of libvirt.  */
 -VIR_FREE(def-caps-data.storage.drive_type);
 -def-caps-data.storage.drive_type = strdup(cdrom);
 -if (def-caps-data.storage.drive_type == NULL) {
 -virReportOOMError(NULL);
 -goto out;
 -}
 -
  if ((udevGetIntSysfsAttr(device, removable, tmp_int, 0) == 
 PROPERTY_FOUND) 
  (tmp_int == 1)) {
  def-caps-data.storage.flags |= VIR_NODE_DEV_CAP_STORAGE_REMOVABLE;
  }
  
 -if ((udevGetIntProperty(device, ID_CDROM_MEDIA, tmp_int, 0)
 - == PROPERTY_FOUND)  (tmp_int == 1)) {
 +if (has_media) {
  
  def-caps-data.storage.flags |=
  VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE;
 @@ -898,6 +887,53 @@ out:
  return ret;
  }
  
 +static int udevProcessCDROM(struct udev_device *device,
 +virNodeDeviceDefPtr def)
 +{
 +int ret = -1;
 +int tmp_int = 0;
 +int has_media = 0;
 +
 +/* NB: the drive_type string provided by udev is different from
 + * that provided by HAL; now it's cd instead of cdrom We
 + * change it to cdrom to preserve compatibility with earlier
 + * versions of libvirt.  */
 +VIR_FREE(def-caps-data.storage.drive_type);
 +def-caps-data.storage.drive_type = strdup(cdrom);
 +if (def-caps-data.storage.drive_type == NULL) {
 +virReportOOMError(NULL);
 +goto out;
 +}
 +
 +if ((udevGetIntProperty(device, ID_CDROM_MEDIA,
 +tmp_int, 0) == PROPERTY_FOUND))
 +has_media = tmp_int;
 +
 +ret = udevProcessRemoveableMedia(device, def, has_media);
 +out:
 +return ret;
 +}
 +
 +static int udevProcessFloppy(struct udev_device *device,
 + virNodeDeviceDefPtr def)
 +{
 +int tmp_int = 0;
 +int has_media = 0;
 +char *tmp_str = NULL;
 +
 +if ((udevGetIntProperty(device, DKD_MEDIA_AVAILABLE,
 +tmp_int, 0) == PROPERTY_FOUND))
 +/* USB floppy */
 +has_media = tmp_int;
 +else if (udevGetStringProperty(device, ID_FS_LABEL,
 +   tmp_str) == PROPERTY_FOUND) {
 +/* Legacy floppy */
 +has_media = 1;
 +VIR_FREE(tmp_str);
 +}
 +
 +return udevProcessRemoveableMedia(device, def, has_media);
 +}
  
  /* This function exists to deal with the case in which a driver does
   * not provide a device type in the usual place, but udev told us it's
 @@ -996,9 +1032,23 @@ static int udevProcessStorage(struct udev_device 
 *device,
  if (udevGetStringProperty(device,
ID_TYPE,
data-storage.drive_type) != PROPERTY_FOUND) {
 -/* If udev doesn't have it, perhaps we can guess it. */
 -if (udevKludgeStorageType(def) != 0) {
 -goto out;
 +int tmp_int = 0;
 +
 +/* All floppy drives have the ID_DRIVE_FLOPPY prop. This is
 + * needed since legacy floppies don't have a drive_type */
 +if ((udevGetIntProperty(device, ID_DRIVE_FLOPPY,
 +tmp_int, 0) == PROPERTY_FOUND) 
 +(tmp_int == 1)) {
 +
 +data-storage.drive_type = strdup(floppy);
 +if (!data-storage.drive_type)
 +goto out;
 +} else {
 +
 +/* If udev doesn't have it, perhaps we can guess it. */
 +if (udevKludgeStorageType(def) != 0) {
 +goto out;
 +}
  }
  }
  
 @@ -1006,6 +1056,8 @@ static int udevProcessStorage(struct udev_device 
 *device,
  ret = udevProcessCDROM(device, def);
  } else if (STREQ(def-caps-data.storage.drive_type, disk)) {
  ret = udevProcessDisk(device, def);
 +} else if 

Re: [libvirt] [PATCH 2/3] New utility functions virFileCreate and virDirCreate

2010-01-13 Thread Daniel P. Berrange
On Wed, Jan 13, 2010 at 01:25:06AM -0500, Laine Stump wrote:
 These functions create a new file or directory with the given
 uid/gid. If the flag VIR_FILE_CREATE_AS_UID is given, they do this by
 forking a new process, calling setuid/setgid in the new process, and
 then creating the file. This is better than simply calling open then
 fchown, because in the latter case, a root-squashing nfs server would
 create the new file as user nobody, then refuse to allow fchown.
 
 If VIR_FILE_CREATE_AS_UID is not specified, the simpler tactic of
 creating the file/dir, then chowning is is used. This gives better
 results in cases where the parent directory isn't on a root-squashing
 NFS server, but doesn't give permission for the specified uid/gid to
 create files. (Note that if the fork/setuid method fails to create the
 file due to access privileges, the parent process will make a second
 attempt using this simpler method.)
 
 Return from both of these functions is 0 on success, or the value of
 errno if there was a failure.
 ---
  src/util/util.c |  247 
 +++
  src/util/util.h |9 ++
  2 files changed, 256 insertions(+), 0 deletions(-)
 
 diff --git a/src/util/util.c b/src/util/util.c
 index 1d493de..1cb29f4 100644
 --- a/src/util/util.c
 +++ b/src/util/util.c
 @@ -1126,6 +1126,253 @@ int virFileExists(const char *path)
  return(0);
  }
  
 +
 +static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, 
 gid_t gid) {
 +int fd = -1;
 +int ret = 0;
 +
 +if ((fd = open(path, O_RDWR | O_CREAT | O_EXCL, mode))  0) {
 +ret = errno;
 +virReportSystemError(NULL, errno, _(failed to create file '%s'),
 + path);
 +goto error;
 +}
 +if ((getuid() == 0)  ((uid != 0) || (gid != 0))) {
 +if (fchown(fd, uid, gid)  0) {
 +ret = errno;
 +virReportSystemError(NULL, errno, _(cannot chown '%s' to (%u, 
 %u)),
 + path, uid, gid);
 +close(fd);
 +goto error;
 +}
 +}
 +if (close(fd)  0) {
 +ret = errno;
 +virReportSystemError(NULL, errno, _(failed to close new file '%s'),
 + path);
 +goto error;
 +}
 +fd = -1;
 +error:
 +return ret;
 +}
 +
 +static int virDirCreateSimple(const char *path, mode_t mode, uid_t uid, 
 gid_t gid) {
 +int ret = 0;
 +
 +if (mkdir(path, mode)  0) {
 +ret = errno;
 +virReportSystemError(NULL, errno, _(failed to create directory 
 '%s'),
 + path);
 +goto error;
 +}
 +
 +if ((getuid() == 0)  ((uid != 0) || (gid != getgid( {
 +if (chown(path, uid, gid)  0) {
 +ret = errno;
 +virReportSystemError(NULL, errno, _(cannot chown '%s' to (%u, 
 %u)),
 + path, uid, gid);
 +goto error;
 +}
 +}
 +error:
 +return ret;
 +}
 +
 +#ifndef WIN32
 +int virFileCreate(const char *path, mode_t mode,
 +  uid_t uid, gid_t gid, unsigned int flags) {
 +pid_t pid;
 +int waitret, status, ret = 0;
 +int fd = -1;
 +
 +if ((!(flags  VIR_FILE_CREATE_AS_UID))
 +|| (getuid() != 0)
 +|| ((uid == 0)  (gid == 0))) {
 +return virFileCreateSimple(path, mode, uid, gid);
 +}
 +
 +/* parent is running as root, but caller requested that the
 + * file be created as some other user and/or group). The
 + * following dance avoids problems caused by root-squashing
 + * NFS servers. */
 +
 +if ((pid = fork())  0) {
 +ret = errno;
 +virReportSystemError(NULL, errno,
 + _(cannot fork o create file '%s'), path);
 +return ret;
 +}
 +
 +if (pid) { /* parent */
 +/* wait for child to complete, and retrieve its exit code */
 +while ((waitret = waitpid(pid, status, 0) == -1)
 +(errno == EINTR));
 +if (waitret == -1) {
 +ret = errno;
 +virReportSystemError(NULL, errno,
 + _(failed to wait for child creating '%s'),
 + path);
 +return ret;
 +}
 +ret = WEXITSTATUS(status);
 +if (!WIFEXITED(status) || (ret == EACCES)) {
 +/* fall back to the simpler method, which works better in
 + * some cases */
 +/* Should we log a warning here? */
 +return virFileCreateSimple(path, mode, uid, gid);
 +}
 +if ((ret == 0)  (gid != 0)) {
 +/* check if group was set properly by creating after
 + * setgid. If not, try doing it with chown */
 +struct stat st;
 +if (stat(path, st) == -1) {
 +ret = errno;
 +virReportSystemError(NULL, errno,
 + _(stat of 

Re: [libvirt] [BUG?] no domain with matching uuid error, when vm restarts

2010-01-13 Thread Daniel P. Berrange
On Tue, Jan 12, 2010 at 04:06:32PM -0800, su disheng wrote:
 Hi,
 In the following libvirt API calling sequence, I always get an error no
 domain with matching uuid
 Connect _conn = new Connect(qemu:///system, false);
  _conn.domainDefineXML(kvm_guest_xml);
  Domain dm = _conn.domainLookupByName(kvm_guest_name);
  dm.create();
 
  /* stop, undefine, and re-start the vm*/
  dm.shutdown();
  dm.undefine();
  dm.domainDefineXML(kvm_guest_xml);
  /A/
  Domain dm = _conn.domainLookupByName(kvm_guest_name);
  dm.create()  /Error**/
 
if I close the connection, and re-connect qemu at the end of
 /A/, then everything is OK.

I think the first 'dm'  object you create is not being freed / garbage
collected. This means the underlying libvirt virDomainPtr object is
not released. And so when you then _conn.domainLookupByName() after
'A'  you are still getting an handle to the old object and
thus the stale UUID.


From the log, seems that uuid is not updated immediately, for this
 connection, the uuid is in the stale state?
I am using libvirt 0.6.3, if it's a bug, does it fixed in the latest
 code? or Have I need to close the connection for each vm shutdown/re-define?

You should not need to close the connection, provided you ensure all the
'Domain' objects are freed/garbage collected

Alternatively, if you wish to re-use the same name,then you could try
auto-generating the  UUID yourself, so you use the same UUID when
defining the domain for the second time


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] [PATCH 1/3] Add virRunWithHook.

2010-01-13 Thread Daniel P. Berrange
On Wed, Jan 13, 2010 at 01:25:05AM -0500, Laine Stump wrote:
 Similar to virExecWithHook, but waits for child to exit. Useful for
 doing things like setuid after the fork but before the exec.
 ---
  src/util/util.c |   25 ++---
  src/util/util.h |3 +++
  2 files changed, 21 insertions(+), 7 deletions(-)
 
 diff --git a/src/util/util.c b/src/util/util.c
 index 44a4b2f..1d493de 100644
 --- a/src/util/util.c
 +++ b/src/util/util.c
 @@ -792,9 +792,11 @@ error:
   * only if the command could not be run.
   */
  int
 -virRun(virConnectPtr conn,
 -   const char *const*argv,
 -   int *status) {
 +virRunWithHook(virConnectPtr conn,
 +   const char *const*argv,
 +   virExecHook hook,
 +   void *data,
 +   int *status) {
  pid_t childpid;
  int exitstatus, execret, waitret;
  int ret = -1;
 @@ -811,7 +813,7 @@ virRun(virConnectPtr conn,
  
  if ((execret = __virExec(conn, argv, NULL, NULL,
   childpid, -1, outfd, errfd,
 - VIR_EXEC_NONE, NULL, NULL, NULL))  0) {
 + VIR_EXEC_NONE, hook, data, NULL))  0) {
  ret = execret;
  goto error;
  }
 @@ -867,9 +869,11 @@ virRun(virConnectPtr conn,
  #else /* __MINGW32__ */
  
  int
 -virRun(virConnectPtr conn,
 -   const char *const *argv ATTRIBUTE_UNUSED,
 -   int *status)
 +virRunWithHook(virConnectPtr conn,
 +   const char *const *argv ATTRIBUTE_UNUSED,
 +   virExecHook hook ATTRIBUTE_UNUSED,
 +   void *data ATTRIBUTE_UNUSED,
 +   int *status)
  {
  if (status)
  *status = ENOTSUP;
 @@ -895,6 +899,13 @@ virExec(virConnectPtr conn,
  
  #endif /* __MINGW32__ */
  
 +int
 +virRun(virConnectPtr conn,
 +   const char *const*argv,
 +   int *status) {
 +return virRunWithHook(conn, argv, NULL, NULL, status);
 +}
 +
  /* Like gnulib's fread_file, but read no more than the specified maximum
 number of bytes.  If the length of the input is = max_len, and
 upon error while reading that data, it works just like fread_file.  */
 diff --git a/src/util/util.h b/src/util/util.h
 index d556daa..5e70038 100644
 --- a/src/util/util.h
 +++ b/src/util/util.h
 @@ -81,6 +81,9 @@ int virExec(virConnectPtr conn,
  int *errfd,
  int flags) ATTRIBUTE_RETURN_CHECK;
  int virRun(virConnectPtr conn, const char *const*argv, int *status) 
 ATTRIBUTE_RETURN_CHECK;
 +int virRunWithHook(virConnectPtr conn, const char *const*argv,
 +   virExecHook hook, void *data,
 +   int *status) ATTRIBUTE_RETURN_CHECK;
  
  int virFileReadLimFD(int fd, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;
  
 -- 

ACK


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] [PATCH] virterror: Don't invoke error callback for ERR_OK

2010-01-13 Thread Cole Robinson
On 01/13/2010 04:23 AM, Daniel P. Berrange wrote:
 On Tue, Jan 12, 2010 at 03:57:33PM -0500, Cole Robinson wrote:
 On 01/12/2010 03:48 PM, Daniel P. Berrange wrote:
 On Tue, Jan 12, 2010 at 03:26:28PM -0500, Cole Robinson wrote:
 Since virDispatchError is now responsible for invoking the error callback,
 give it the same semantics as ReportError, which will skip VIR_ERR_OK
 (which is encountered when no error was raised).

 This fixes invoking the error callback after every non-erroring API call.

 Signed-off-by: Cole Robinson crobi...@redhat.com
 ---
  src/util/virterror.c |6 +-
  1 files changed, 5 insertions(+), 1 deletions(-)

 diff --git a/src/util/virterror.c b/src/util/virterror.c
 index e2128b9..78974ee 100644
 --- a/src/util/virterror.c
 +++ b/src/util/virterror.c
 @@ -603,8 +603,12 @@ virDispatchError(virConnectPtr conn)
  if (!err)
  return;
  
 -/* Set a generic error message if none is already set */
 +/* We never used to raise ERR_OK, so maintain existing behavior */
  if (err-code == VIR_ERR_OK)
 +return;
 +
 +/* Set a generic error message if none is already set */
 +if (!err-message)
  virErrorGenericFailure(err);
  
  /* Copy the global error to per-connection error if needed */

 We should only ever be invoking virDispatchError() in error paths, so
 if err-code == VIR_ERR_OK, this means we do need set a generic message
 because the earlier code indicated an error but forgot to report one.
 So I don't think this is correct.


 Ah, I think I wanted to check VIR_ERR_NONE here actually.
 virDispatchError is called regardless of whether an error is actually
 raised, so it may receive a zero'd out/empty virLastErrorObject, which
 is what I'm trying to avoid reporting.
 
 I still don't think you are correct in that. If you run
 
   # grep --after 1 virDispatchError libvirt.c
 virDispatchError(NULL);
 return (-1);
   --
   virDispatchError(net-conn);
   return -1;
   --
   virDispatchError(NULL);
   return (-1);
   --
   virDispatchError(pool-conn);
   return -1;
 
 Then all cases where virDispatchError() is called should be followed by the
 return of an error code, 99% of them are -1 or NULL. There are one or two
 where we use '0' for error as a special case. I don't see any places where
 virDispatchError() is called in a successful return path. So we should
 always be invoking the error callback, and ensuring an error is actually 
 set before doing so.
 

Whoops, sorry for the confusion. I'll investigate more.

Thanks,
Cole

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


Re: [libvirt] [PATCH 00/34] Add device addressing and disk controller support

2010-01-13 Thread Wolfgang Mauerer
Hi,

On Fri, Jan 8, 2010 at 6:22 PM, Daniel P. Berrange berra...@redhat.com
wrote:
 This series is a merge of two previous series I posted

  http://www.redhat.com/archives/libvir-list/2009-December/msg00232.html
  http://www.redhat.com/archives/libvir-list/2009-December/msg00392.html

 It accomplishes quite a lot of things, having major impact on the QEMU
 driver, hopefully all in a postive way :-)

do you happen to have a git repository from which the patches can
be pulled? This would ease testing quite a bit since it's become
a rather voluminous series ;-)

Thanks, Wolfgang

 In particular it does

  * Add standard XML syntax for addressing of PCI devices, and disk drives
  * Add support for disk controllers as a managed device in XML
  * Add support for disk controller hotplug/unplug
  * Convert everything over to use QEMU's -device flag where available
  * Add PCI addressing when using -device
  * Introduce a way to give every device a unique 'alias' name in
   the XML format

 I can't promise it works perfectly, but i've done quite alot of positive
 testing with it now  believe all the back comptability stuff is working
 right.

 --
 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 00/34] Add device addressing and disk controller support

2010-01-13 Thread Daniel P. Berrange
On Wed, Jan 13, 2010 at 02:16:45PM +0100, Wolfgang Mauerer wrote:
 Hi,
 
 On Fri, Jan 8, 2010 at 6:22 PM, Daniel P. Berrange berra...@redhat.com
 wrote:
  This series is a merge of two previous series I posted
 
   http://www.redhat.com/archives/libvir-list/2009-December/msg00232.html
   http://www.redhat.com/archives/libvir-list/2009-December/msg00392.html
 
  It accomplishes quite a lot of things, having major impact on the QEMU
  driver, hopefully all in a postive way :-)
 
 do you happen to have a git repository from which the patches can
 be pulled? This would ease testing quite a bit since it's become
 a rather voluminous series ;-)

Yep, its on the 'device-info' branch on gitorious

http://gitorious.org/~berrange/libvirt/staging/commits/device-info

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


[libvirt] Problem with libvirt-0.7.5 windows port

2010-01-13 Thread anuj rampal
Hi,

I'm trying to cross compile the libvirt-0.7.5 for windows.

This is the error I'm getting:

# make
make  all-recursive
make[1]: Entering directory `/root/libvirt-0.7.5'
Making all in gnulib/lib
make[2]: Entering directory `/root/libvirt-0.7.5/gnulib/lib'

[...]

Making all in tools
make[2]: Entering directory `/root/libvirt-0.7.5/tools'
make  all-am
make[3]: Entering directory `/root/libvirt-0.7.5/tools'
  CC virsh-console.o
  CC virsh-virsh.o
  CCLD   virsh.exe
../src/.libs/libvirt.a: could not read symbols: Archive has no index; run
ranlib to add one
collect2: ld returned 1 exit status
make[3]: *** [virsh.exe] Error 1
make[3]: Leaving directory `/root/libvirt-0.7.5/tools'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/root/libvirt-0.7.5/tools'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/root/libvirt-0.7.5'
make: *** [all] Error 2


Does any1 has any idea.

Thanks in advance.

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

Re: [libvirt] [PATCH 2/3] New utility functions virFileCreate and virDirCreate

2010-01-13 Thread Serge E. Hallyn
Quoting Laine Stump (la...@laine.org):
 These functions create a new file or directory with the given
 uid/gid. If the flag VIR_FILE_CREATE_AS_UID is given, they do this by
 forking a new process, calling setuid/setgid in the new process, and
 then creating the file. This is better than simply calling open then
 fchown, because in the latter case, a root-squashing nfs server would
 create the new file as user nobody, then refuse to allow fchown.
 
 If VIR_FILE_CREATE_AS_UID is not specified, the simpler tactic of
 creating the file/dir, then chowning is is used. This gives better
 results in cases where the parent directory isn't on a root-squashing
 NFS server, but doesn't give permission for the specified uid/gid to
 create files. (Note that if the fork/setuid method fails to create the
 file due to access privileges, the parent process will make a second
 attempt using this simpler method.)
 
 Return from both of these functions is 0 on success, or the value of
 errno if there was a failure.
 ---
  src/util/util.c |  247 
 +++
  src/util/util.h |9 ++
  2 files changed, 256 insertions(+), 0 deletions(-)
 
 diff --git a/src/util/util.c b/src/util/util.c
 index 1d493de..1cb29f4 100644
 --- a/src/util/util.c
 +++ b/src/util/util.c
 @@ -1126,6 +1126,253 @@ int virFileExists(const char *path)
  return(0);
  }
 
 +
 +static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, 
 gid_t gid) {
 +int fd = -1;
 +int ret = 0;
 +
 +if ((fd = open(path, O_RDWR | O_CREAT | O_EXCL, mode))  0) {
 +ret = errno;
 +virReportSystemError(NULL, errno, _(failed to create file '%s'),
 + path);
 +goto error;
 +}
 +if ((getuid() == 0)  ((uid != 0) || (gid != 0))) {

How about checking for CAP_CHOWN instead of getuid()==0?  Otherwise
if I try installing this certain ways, virFileCreateSimple() will refuse
to try the chown even though it could succeed.

-serge

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


Re: [libvirt] [PATCH 2/3] New utility functions virFileCreate and virDirCreate

2010-01-13 Thread Serge E. Hallyn
Quoting Serge E. Hallyn (se...@us.ibm.com):
 Quoting Laine Stump (la...@laine.org):
  These functions create a new file or directory with the given
  uid/gid. If the flag VIR_FILE_CREATE_AS_UID is given, they do this by
  forking a new process, calling setuid/setgid in the new process, and
  then creating the file. This is better than simply calling open then
  fchown, because in the latter case, a root-squashing nfs server would
  create the new file as user nobody, then refuse to allow fchown.
  
  If VIR_FILE_CREATE_AS_UID is not specified, the simpler tactic of
  creating the file/dir, then chowning is is used. This gives better
  results in cases where the parent directory isn't on a root-squashing
  NFS server, but doesn't give permission for the specified uid/gid to
  create files. (Note that if the fork/setuid method fails to create the
  file due to access privileges, the parent process will make a second
  attempt using this simpler method.)
  
  Return from both of these functions is 0 on success, or the value of
  errno if there was a failure.
  ---
   src/util/util.c |  247 
  +++
   src/util/util.h |9 ++
   2 files changed, 256 insertions(+), 0 deletions(-)
  
  diff --git a/src/util/util.c b/src/util/util.c
  index 1d493de..1cb29f4 100644
  --- a/src/util/util.c
  +++ b/src/util/util.c
  @@ -1126,6 +1126,253 @@ int virFileExists(const char *path)
   return(0);
   }
  
  +
  +static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, 
  gid_t gid) {
  +int fd = -1;
  +int ret = 0;
  +
  +if ((fd = open(path, O_RDWR | O_CREAT | O_EXCL, mode))  0) {
  +ret = errno;
  +virReportSystemError(NULL, errno, _(failed to create file '%s'),
  + path);
  +goto error;
  +}
  +if ((getuid() == 0)  ((uid != 0) || (gid != 0))) {
 
 How about checking for CAP_CHOWN instead of getuid()==0?  Otherwise
 if I try installing this certain ways, virFileCreateSimple() will refuse
 to try the chown even though it could succeed.

I guess for certain netfs's the uid might matter more, so checking for
either condition - or in fact just trying the fchown, then doing a stat,
then making sure st.st_{ug}id are correct after the fact - seems useful.

-serge

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


Re: [libvirt] [PATCH 07/34] Convert monitor over to use virDomainDeviceAddress

2010-01-13 Thread Paolo Bonzini

On 01/08/2010 06:23 PM, Daniel P. Berrange wrote:

@@ -1206,9 +1206,7 @@ int qemuMonitorJSONAddUSBDeviceMatch(qemuMonitorPtr mon,
  /* XXX qemu also returns a 'function' number now */


Wrong comment now.

Paolo

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


Re: [libvirt] VMware vCenter support

2010-01-13 Thread Matthias Bolte
2010/1/8 Fulop, Balazs balazs.fu...@morganstanley.com:
 Dear List,



 I find the ESX driver in libvirt quite useful to perform powercycle
 operations on a VM. However, I need to connect to an ESX server even when
 the VM is managed on the VMware 4.0 platform.



 Reading this mail:

 http://www.mail-archive.com/libvir-list@redhat.com/msg18259.html

 makes me think that support to connect directly to a vCenter using the
 scheme vpx:// is not far from becoming a reality. Could you please provide
 an estimated timeline for this and notes on possible further enhancements in
 the ESX driver code?



 Thank you very much in advance.



 Regards,

    Balazs Fulop


I have looked into this some time ago because there was a request for
direct vCenter support on the mailing list (the on you referenced).

As I said back then, some tweaks to the ESX driver are necessary to
enable direct vCenter support. The driver currently assumes that there
is exactly one datacenter and each compute resource has exactly one
host attached. Within a vCenter this assumptions aren't true anymore.
A vCenter can contain multiple datacenters, a datacenter can contain
clusters (cluster compute resource) and a cluster can contain multiple
hosts. The ESX driver needs to be made aware of this.

Also the connection URI format needs to be extended in order to allow
to specify a explicit datacenter/cluster/host when connecting to a
vCenter. Explicit host reference for example is necessary to perform a
migration.

The extended URI format may look like this:

  vpx://vcenter/datacenter1/cluster5/host7

If clusters aren't used it may look like this

  vpx://vcenter/datacenter1/-/host7

The path part would be optional. For example

  virsh -c vpx://vcenter list

would list all virtual machines of all datacenters of this vCenter and

  virsh -c vpx://vcenter/datacenter2 list

would only list all virtual machines of datacenter2.

You see, I already thought about direct vCenter support and how to
implement it. Just yesterday there was a request for this by one of
visualization projects here at my university.

I don't think this will be part of libvirt 0.7.6, that's scheduled for
the end of this month, so feature freeze for 0.7.6 will probably begin
by the end of next week. But I'm going to work on this in the next
days/weeks, so probably it'll be part of libvirt 0.7.7. Stay tuned :)

Regarding other ESX driver enhancements: The next bigger steps will be
storage, network and interface drivers for ESX. But those will require
some more research on how to implement the driver functions based on
what ESX can provide.

If you have any suggestions or priorities, tell me.

Matthias

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

Re: [libvirt] [PATCH 21/34] Convert character devices over to use -device

2010-01-13 Thread Paolo Bonzini

On 01/08/2010 06:23 PM, Daniel P. Berrange wrote:

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5dcd50f..eded887 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1592,6 +1592,9 @@ static char *qemuDiskDriveName(const virDomainDiskDefPtr 
disk)
  case VIR_DOMAIN_DISK_BUS_VIRTIO:
  ret = virAsprintf(devname, virtio%d, devid);
  break;
+case VIR_DOMAIN_DISK_BUS_XEN:
+ret = virAsprintf(devname, xenblk%d, devid);
+break;
  default:
  qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_SUPPORT,
   _(Unsupported disk name mapping for bus '%s'),


Belongs in previous patch.

Paolo

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


Re: [libvirt] Two issues regarding TFTP support in virtual networks...

2010-01-13 Thread Darryl L. Pierce
On Wed, Jan 13, 2010 at 10:17:01AM +0100, Paolo Bonzini wrote:
 You are not giving BOOTP info (you are missing thebootp  tag,
 which goes insidedhcp).
 
 Okay, here's the XML that's now being generated by my test script:
 
 network
 nametestbr6253/name
 forward mode='nat' /
 bridge name='testbr6253' stp='on' forwardDelay='0' /
 ip address='192.168.133.1' netmask='255.255.255.0'
 tftp root='/tmp/tmp.dEf7FPCaKm/tftpboot' /
 bootp file='pxelinux.0' /
 dhcp
 range start='192.168.133.100' end='192.168.133.199' /
 /dhcp
 /ip
 /network
 
 However, I still get the message No filename or root path specified
 within the VM and it does not boot from the above directory.
 
 bootp must be within dhcp (think of elements within ip as
 services that run, such as TFTP or DHCP; bootp is just additional
 data that is passed via DHCP).

Ah, okay. That wasn't clear to me in the wiki page on the network XML.
I'll give that a shot then.

(tries)

Okay, still the same error. Here's the new XML being generated:

network
nametestbr31018/name
forward mode='nat' /
bridge name='testbr31018' stp='on' forwardDelay='0' /
ip address='192.168.163.1' netmask='255.255.255.0'
tftp root='/tmp/tmp.6LUTF06Gkm/tftpboot' /
dhcp
bootp file='pxelinux.0' /
range start='192.168.163.100' end='192.168.163.199' /
/dhcp
/ip
/network

Are there any relative pathing values that should be used by the tftp
and bootp tags? Should the tftp-root attribute point to the work
directory and then bootp-file refer to tftpboot/pxelinux.0?

-- 
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/



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

[libvirt] [PATCH] Enhance qemuParseCommandLineKeywords

2010-01-13 Thread Jiri Denemark
Current version expects name=value,... list and when an incorrect string
such as a,b,c=d would be parsed as a,b,c keyword with d value
without reporting any error, which is probably not the expected
behavior.

This patch adds an extra argument called allowEmptyValue, which if
non-zero will permit keywords with no value; a,b=c,,d= will be parsed
as follows:
keyword value
a NULL
b c
  NULL
d 

In case allowEmptyValue is zero, the string is required to contain
name=value pairs only; retvalues is guaranteed to contain non-NULL
pointers. Now, a,b,c=d will result in an error.

Signed-off-by: Jiri Denemark jdene...@redhat.com
---
 src/qemu/qemu_conf.c |   51 ++---
 1 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index d3da776..9f35217 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -3066,47 +3066,58 @@ static const char *qemuFindEnv(const char **progenv,
 /*
  * Takes a string containing a set of key=value,key=value,key...
  * parameters and splits them up, returning two arrays with
- * the individual keys and values
+ * the individual keys and values. If allowEmptyValue is nonzero,
+ * the =value part is optional and if a key with no value is found,
+ * NULL is be placed into corresponding place in retvalues.
  */
 static int
 qemuParseCommandLineKeywords(virConnectPtr conn,
  const char *str,
  char ***retkeywords,
- char ***retvalues)
+ char ***retvalues,
+ int allowEmptyValue)
 {
 int keywordCount = 0;
 int keywordAlloc = 0;
 char **keywords = NULL;
 char **values = NULL;
 const char *start = str;
+const char *end;
 int i;
 
 *retkeywords = NULL;
 *retvalues = NULL;
+end = start + strlen(str);
 
 while (start) {
 const char *separator;
 const char *endmark;
 char *keyword;
-char *value;
+char *value = NULL;
 
-if (!(separator = strchr(start, '='))) {
-qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _(malformed keyword arguments in '%s'), str);
-goto error;
+if (!(endmark = strchr(start, ',')))
+endmark = end;
+if (!(separator = strchr(start, '=')))
+separator = end;
+
+if (separator = endmark) {
+if (!allowEmptyValue) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(malformed keyword arguments in '%s'), 
str);
+goto error;
+}
+separator = endmark;
 }
+
 if (!(keyword = strndup(start, separator - start)))
 goto no_memory;
 
-separator++;
-endmark = strchr(separator, ',');
-
-value = endmark ?
-strndup(separator, endmark - separator) :
-strdup(separator);
-if (!value) {
-VIR_FREE(keyword);
-goto no_memory;
+if (separator  endmark) {
+separator++;
+if (!(value = strndup(separator, endmark - separator))) {
+VIR_FREE(keyword);
+goto no_memory;
+}
 }
 
 if (keywordAlloc == keywordCount) {
@@ -3123,7 +3134,7 @@ qemuParseCommandLineKeywords(virConnectPtr conn,
 values[keywordCount] = value;
 keywordCount++;
 
-start = endmark ? endmark + 1 : NULL;
+start = endmark  end ? endmark + 1 : NULL;
 }
 
 *retkeywords = keywords;
@@ -3163,7 +3174,7 @@ qemuParseCommandLineDisk(virConnectPtr conn,
 
 if ((nkeywords = qemuParseCommandLineKeywords(conn, val,
   keywords,
-  values))  0)
+  values, 0))  0)
 return NULL;
 
 if (VIR_ALLOC(def)  0) {
@@ -3347,7 +3358,7 @@ qemuParseCommandLineNet(virConnectPtr conn,
 if ((nkeywords = qemuParseCommandLineKeywords(conn,
   tmp+1,
   keywords,
-  values))  0)
+  values, 0))  0)
 return NULL;
 } else {
 nkeywords = 0;
@@ -3420,7 +3431,7 @@ qemuParseCommandLineNet(virConnectPtr conn,
 if ((nkeywords = qemuParseCommandLineKeywords(conn,
   nic + strlen(nic,),
   keywords,
-  values))  0) {
+  values, 0))  0) {
 

Re: [libvirt] Two issues regarding TFTP support in virtual networks...

2010-01-13 Thread Paolo Bonzini



network
nametestbr31018/name
forward mode='nat' /
bridge name='testbr31018' stp='on' forwardDelay='0' /
ip address='192.168.163.1' netmask='255.255.255.0'
tftp root='/tmp/tmp.6LUTF06Gkm/tftpboot' /
dhcp
bootp file='pxelinux.0' /
range start='192.168.163.100' end='192.168.163.199' /
/dhcp
/ip
/network

Are there any relative pathing values that should be used by the tftp
and bootp tags? Should the tftp-root attribute point to the work
directory and then bootp-file refer to tftpboot/pxelinux.0?


No, the above seems fine (I should have pointed you to 
tests/networkxml2xmlin/netboot-network.xml before).  Can you post the 
generated dnsmasq command line?


Also, have you tried using wireshark to see what's going on?

Paolo

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


Re: [libvirt] [PATCH] node_device: udev: Enumerate floppy devices

2010-01-13 Thread Cole Robinson
On 01/12/2010 03:26 PM, Cole Robinson wrote:
 There are quite a few differences between how udev exposes legacy
 and USB floppy devs, but this patch takes care of both variants.
 

Pushed now.

- Cole

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


Re: [libvirt] [PATCH] util: Remove logging handlers in virExec

2010-01-13 Thread Cole Robinson
On 01/13/2010 04:20 AM, Daniel P. Berrange wrote:
 On Tue, Jan 12, 2010 at 03:54:39PM -0500, Cole Robinson wrote:
 On 01/12/2010 03:46 PM, Daniel P. Berrange wrote:
 On Tue, Jan 12, 2010 at 03:26:26PM -0500, Cole Robinson wrote:
 This allows debug statements and raised errors in hook functions to
 actually be logged somewhere (stderr). Users can enable debugging in the
 daemon and now see more info in /var/log/libvirt/...

 Signed-off-by: Cole Robinson crobi...@redhat.com
 ---
  src/util/util.c |6 ++
  1 files changed, 6 insertions(+), 0 deletions(-)

 diff --git a/src/util/util.c b/src/util/util.c
 index 44a4b2f..23d781d 100644
 --- a/src/util/util.c
 +++ b/src/util/util.c
 @@ -334,6 +334,7 @@ __virExec(virConnectPtr conn,
  int pipeerr[2] = {-1,-1};
  int childout = -1;
  int childerr = -1;
 +int logprio;
  sigset_t oldmask, newmask;
  struct sigaction sig_action;
  
 @@ -452,6 +453,11 @@ __virExec(virConnectPtr conn,
 of being seen / logged */
  virSetErrorFunc(NULL, NULL);
  
 +/* Make sure any hook logging is sent to stderr */
 +logprio = virLogGetDefaultPriority();
 +virLogReset();
 +virLogSetDefaultPriority(logprio);
 +

 I'm not sure that I understand this - surely the child process is already
 inheriting this setup from the parent libvirtd ?

 Daniel

 Problem is that virExec closes only the file descriptors it is using,
 which means it will close the file descriptors for any nontrivial
 logging handlers. I could add some internal logging API to get the
 relevant fds and keep them from being closed, but not sure if we
 necessarily want to preserve those for a daemonized process?
 
 Oh of course - can you put a comment to that effect in your patch as a 
 reminder.  ACK to the patch 
 
 Daniel

Thanks, pushed with an updated comment.

- Cole

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


Re: [libvirt] [PATCH] node_device: udev: Fix memory leak

2010-01-13 Thread Cole Robinson
On 01/12/2010 03:26 PM, Cole Robinson wrote:
 We are setting the same property two different ways without
 free'ing in between. Just drop the second assignment.
 
 Signed-off-by: Cole Robinson crobi...@redhat.com
 ---
  src/node_device/node_device_udev.c |6 +-
  1 files changed, 1 insertions(+), 5 deletions(-)
 
 diff --git a/src/node_device/node_device_udev.c 
 b/src/node_device/node_device_udev.c
 index a9e196d..ff6bd46 100644
 --- a/src/node_device/node_device_udev.c
 +++ b/src/node_device/node_device_udev.c
 @@ -997,11 +997,7 @@ static int udevProcessStorage(struct udev_device *device,
  goto out;
  }
  data-storage.block = strdup(devnode);
 -if (udevGetStringProperty(device,
 -  DEVNAME,
 -  data-storage.block) == PROPERTY_ERROR) {
 -goto out;
 -}
 +
  if (udevGetStringProperty(device,
ID_BUS,
data-storage.bus) == PROPERTY_ERROR) {

Pushed now.

- Cole

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


Re: [libvirt] [PATCH] node_device: udev: Use base 16 for product/vendor

2010-01-13 Thread Cole Robinson
On 01/12/2010 03:26 PM, Cole Robinson wrote:
 udev doesn't prefix USB product/vendor info with '0x', so the
 strtol conversions were wrong for the product field (vendor already
 set the correct base). Make the change for PCI product/vendor as
 well to be safe.
 
 This fixes USB device assignment via virt-manager.
 

Pushed now.

- Cole

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


Re: [libvirt] [PATCH 2/3] New utility functions virFileCreate and virDirCreate

2010-01-13 Thread Laine Stump

On 01/13/2010 09:32 AM, Serge E. Hallyn wrote:

Quoting Serge E. Hallyn (se...@us.ibm.com):
   

Quoting Laine Stump (la...@laine.org):
 

These functions create a new file or directory with the given
uid/gid. If the flag VIR_FILE_CREATE_AS_UID is given, they do this by
forking a new process, calling setuid/setgid in the new process, and
then creating the file. This is better than simply calling open then
fchown, because in the latter case, a root-squashing nfs server would
create the new file as user nobody, then refuse to allow fchown.

If VIR_FILE_CREATE_AS_UID is not specified, the simpler tactic of
creating the file/dir, then chowning is is used. This gives better
results in cases where the parent directory isn't on a root-squashing
NFS server, but doesn't give permission for the specified uid/gid to
create files. (Note that if the fork/setuid method fails to create the
file due to access privileges, the parent process will make a second
attempt using this simpler method.)

Return from both of these functions is 0 on success, or the value of
errno if there was a failure.
---
  src/util/util.c |  247 +++
  src/util/util.h |9 ++
  2 files changed, 256 insertions(+), 0 deletions(-)

diff --git a/src/util/util.c b/src/util/util.c
index 1d493de..1cb29f4 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1126,6 +1126,253 @@ int virFileExists(const char *path)
  return(0);
  }

+
+static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t 
gid) {
+int fd = -1;
+int ret = 0;
+
+if ((fd = open(path, O_RDWR | O_CREAT | O_EXCL, mode))  0) {
+ret = errno;
+virReportSystemError(NULL, errno, _(failed to create file '%s'),
+ path);
+goto error;
+}
+if ((getuid() == 0)  ((uid != 0) || (gid != 0))) {
   

How about checking for CAP_CHOWN instead of getuid()==0?  Otherwise
if I try installing this certain ways, virFileCreateSimple() will refuse
to try the chown even though it could succeed.
 

I guess for certain netfs's the uid might matter more, so checking for
either condition - or in fact just trying the fchown, then doing a stat,
then making sure st.st_{ug}id are correct after the fact - seems useful.
   


That was actually just a duplication of existing functionality from the 
code that will now call virFileCreate() (right down to the lack of any 
error reporting if you're not running as root and the caller requests 
the file to be created with a different uid).
The same check/chown is done in a couple other places in this patch 
series, so your comment applies to those as well.


If I were to remove the check of current uid, should a failure of chown 
then be reported as an error, or ignored (making it closer to current 
behavior for non-root users)? And does this need to be added to the 
current patch, or can it be considered a follow-on? (I guess the 
question behind that is do people commonly use libvirt in a manner that 
would be affected by this, or is this theorizing about something someone 
*might* do?)


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


Re: [libvirt] Two issues regarding TFTP support in virtual networks...

2010-01-13 Thread Darryl L. Pierce
On Wed, Jan 13, 2010 at 05:03:59PM +0100, Paolo Bonzini wrote:
 
 network
 nametestbr31018/name
 forward mode='nat' /
 bridge name='testbr31018' stp='on' forwardDelay='0' /
 ip address='192.168.163.1' netmask='255.255.255.0'
 tftp root='/tmp/tmp.6LUTF06Gkm/tftpboot' /
 dhcp
 bootp file='pxelinux.0' /
 range start='192.168.163.100' end='192.168.163.199' /
 /dhcp
 /ip
 /network
 
 Are there any relative pathing values that should be used by the tftp
 and bootp tags? Should the tftp-root attribute point to the work
 directory and then bootp-file refer to tftpboot/pxelinux.0?
 
 No, the above seems fine (I should have pointed you to
 tests/networkxml2xmlin/netboot-network.xml before).  Can you post
 the generated dnsmasq command line?

Here is what's output by libvirtd:

11:36:57.608: debug : virRun:809 : /usr/sbin/dnsmasq --strict-order 
--bind-interfaces --pid-file=/var/run/libvirt/network/testbr6762.pid 
--conf-file=  --listen-address 192.168.132.1 --except-interface lo --dhcp-range 
192.168.132.100,192.168.132.199 --dhcp-lease-max=100 --enable-tftp --tftp-root 
/tmp/tmp.HRG19a7Udi --dhcp-boot tftpboot/pxelinux.0

Now, this command line matches, WRT the pxe booting elements, our
pre-0.7 libvirtd test script that worked just fine. And I do see in the
vm getting an address via dhcp. But i'm still getting the error
No filename or root path specified.

 Also, have you tried using wireshark to see what's going on?

No, I haven't.

-- 
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/



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

Re: [libvirt] [RFC] Proposal for introduction of network traffic filtering capabilities for filtering of network traffic from and to VMs

2010-01-13 Thread Daniel Veillard
On Mon, Jan 11, 2010 at 12:55:27PM -0500, Stefan Berger wrote:
 Hello!
 
 The following is a proposal to introduce network traffic filtering 
 capabilities for the network traffic originating from and destined to 
 virtual machines. Libvirt's capabilities will be extended to allow users 
 to describe what traffic filtering rules are to be applied on a virtual 
 machine (using XML) and libvirt then creates the appropriate ebtables and 
 iptables rules and applies those on the host when the virtual machine 
 starts up, resumes after suspension or resumes on a new host after 
 migration. libvirt tears down the traffic filtering rules when the virtual 
 machine shuts down, suspends, or a VM is migrated to another host. It will 
 also be possible to modify the filtering rules while a virtual machine is 
 running. In this architecture we apply the firewall rules on the outside 
 of the virtual machines on the Linux host and make use of the fact that 
 virtual machines can be configured by libvirt to have their network 
 traffic pass through the host and the host exposes (tap) 'backend' 
 interfaces on which the firewall rules can be applied. The application of 
 the firewall rules is optional and those who do not want to introduce a 
 raw network throughput performance hit on their system due to the 
 evaluation of every packet passing through the filtering chains, do not 
 have to use these capabilities. An initial implementation would be done 
 for libvirt's Qemu support.

 It's relatively clear that this would not work with devices exposed
natively to the domain, say though PCI passthough. I'm wondering what
other case of limitiations could be found. Also this may not map well
for other kind of hypervisors like VMWare, right ?

 As stated above, the application of firewall rules on virtual machines 
 will require some form of XML description that lets the user choose what 
 type of filtering is to be performed. In effect the user needs to be able 
 to tell libvirt which ebtables and iptables rules to generate so that the 
 appropriate filtering can be done. We realize that  ebtables and iptables 
 have lots of capabilities but think that we need to restrict which 
 capabilities can actually be 'reached' through this XML.
 
 The following proposal is based on an XML as defined by the DMTF (
 http://www.dmtf.org/standards/cim/cim_schema_v2230/CIM_Network.pdf, slide 
 10) with extensions for processing of ARP packets.It gives control over 
 the evaluation of Ethernet (MAC) frames, ARP packet data and IP header 
 information. A (draft) XML skeleton in this case could look as follows:

  Humpf, extending astandard XML schemas is really bad practice. Even if
you think that the extension may be accepted later, it may have
different semantic for the same construct and that's a disaster.
I would at least put the new elements in a separate namespace to
avoid something insane if DMTF extends its specification.

 FilterEntry 
 ActionDROP|ACCEPT/Action   !-- from FilterEntry -- 
 
 TrafficTypeincoming [to VM]|outgoing [from VM]/TrafficType 
 
 Hdr8021Filter 
 HdrSrcMACAddr8021   /HdrSrcMACAddr8021 
 HdrSrcMACMask8021   /HdrSrcMACMask8021 
 HdrDestMACAddr8021  /HdrDestMACAddr8021 
 HdrDestMACMask8021  /HdrDestMACMask8021 
 HdrProtocolID8021 numeric and/or string? 
 /HdrProtocolID8021 
 HdrPriorityValue8021/HdrPriorityValue8021 
 HdrVLANID8021   /HdrVLANID8021 
 /Hdr8021Filter 
 
 ARPFilter 
 HWType   /HWType 
 ProtocolType /ProtocolType 
 OPCode   /OPCode 
 SrcMACAddr   /SrcMACAddr 
 SrcIPAddr/SrcIPAddr 
 DestMACAddr  /DestMACAddr 
 DestIPAddr   /DestIPAddr 
 /ARPFilter 
 
 IPHeadersFilter 
 HdrIPVersion /HdrIPVersion 
 HdrSrcAddress/HdrSrcAddress 
 HdrSrcMask   /HdrSrcMask 
 HdrDestAddress   /HdrDestAddress 
 HdrDestMask  /HdrDestMask 
 HdrProtocolID  numeric and/or string?  /HdrProtocolID 
 HdrSrcPortStart  /HdrSrcPortStart 
 HdrSrcPortEnd/HdrSrcPortEnd 
 HdrDestPortStart /HdrDestPortStart 
 HdrDestPortEnd   /HdrDestPortEnd 
 HdrDSCP  /HdrDSCP 
 /IPHeadersFilter 
 
 /FilterEntry 

  hum the types for each values should be made explicit if possible,
But in general I find that extremely bulky for a single entry, while
I would expect any domain to have a dozen filters or so just for
simple stuff. I wonder if empty default should just be dropped, so
that basically if you're filtering on a MAC address you don't carry
a bunch of unused fields.

 Since the ebtables and iptables command cannot accept all possible 
 parameters at 

Re: [libvirt] [PATCH 0/4] Update interface xml to match netcf 0.1.5

2010-01-13 Thread Laine Stump
It's been 12 days with no responses to these patches, so I figured I'd 
better give them some visibility so they're not forgotten.


https://www.redhat.com/archives/libvir-list/2010-January/msg3.html

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


Re: [libvirt] [RFC] Proposal for introduction of network traffic filtering capabilities for filtering of network traffic from and to VMs

2010-01-13 Thread Daniel P. Berrange
On Wed, Jan 13, 2010 at 06:03:22PM +0100, Daniel Veillard wrote:
 On Mon, Jan 11, 2010 at 12:55:27PM -0500, Stefan Berger wrote:
  As stated above, the application of firewall rules on virtual machines 
  will require some form of XML description that lets the user choose what 
  type of filtering is to be performed. In effect the user needs to be able 
  to tell libvirt which ebtables and iptables rules to generate so that the 
  appropriate filtering can be done. We realize that  ebtables and iptables 
  have lots of capabilities but think that we need to restrict which 
  capabilities can actually be 'reached' through this XML.
  
  The following proposal is based on an XML as defined by the DMTF (
  http://www.dmtf.org/standards/cim/cim_schema_v2230/CIM_Network.pdf, slide 
  10) with extensions for processing of ARP packets.It gives control over 
  the evaluation of Ethernet (MAC) frames, ARP packet data and IP header 
  information. A (draft) XML skeleton in this case could look as follows:
 
   Humpf, extending astandard XML schemas is really bad practice. Even if
 you think that the extension may be accepted later, it may have
 different semantic for the same construct and that's a disaster.
 I would at least put the new elements in a separate namespace to
 avoid something insane if DMTF extends its specification.
 
  FilterEntry 
  ActionDROP|ACCEPT/Action   !-- from FilterEntry -- 
  
  TrafficTypeincoming [to VM]|outgoing [from VM]/TrafficType 
  
  Hdr8021Filter 
  HdrSrcMACAddr8021   /HdrSrcMACAddr8021 
  HdrSrcMACMask8021   /HdrSrcMACMask8021 
  HdrDestMACAddr8021  /HdrDestMACAddr8021 
  HdrDestMACMask8021  /HdrDestMACMask8021 
  HdrProtocolID8021 numeric and/or string? 
  /HdrProtocolID8021 
  HdrPriorityValue8021/HdrPriorityValue8021 
  HdrVLANID8021   /HdrVLANID8021 
  /Hdr8021Filter 
  
  ARPFilter 
  HWType   /HWType 
  ProtocolType /ProtocolType 
  OPCode   /OPCode 
  SrcMACAddr   /SrcMACAddr 
  SrcIPAddr/SrcIPAddr 
  DestMACAddr  /DestMACAddr 
  DestIPAddr   /DestIPAddr 
  /ARPFilter 
  
  IPHeadersFilter 
  HdrIPVersion /HdrIPVersion 
  HdrSrcAddress/HdrSrcAddress 
  HdrSrcMask   /HdrSrcMask 
  HdrDestAddress   /HdrDestAddress 
  HdrDestMask  /HdrDestMask 
  HdrProtocolID  numeric and/or string?  /HdrProtocolID 
  HdrSrcPortStart  /HdrSrcPortStart 
  HdrSrcPortEnd/HdrSrcPortEnd 
  HdrDestPortStart /HdrDestPortStart 
  HdrDestPortEnd   /HdrDestPortEnd 
  HdrDSCP  /HdrDSCP 
  /IPHeadersFilter 
  
  /FilterEntry 
 
   hum the types for each values should be made explicit if possible,
 But in general I find that extremely bulky for a single entry, while
 I would expect any domain to have a dozen filters or so just for
 simple stuff. I wonder if empty default should just be dropped, so
 that basically if you're filtering on a MAC address you don't carry
 a bunch of unused fields.
 
  Since the ebtables and iptables command cannot accept all possible 
  parameters at the same time, only a certain subset of the above parameters 
  may be set for any given filter command. Higher level application writers 
  will likely use a library that lets them choose which features they would 
  want to have enforced, such as no-broadcast or no-multicast, enforcement 
  of MAC spoofing prevention or ARP poisoning prevention, which then 
  generates this lower level XML rules in the appropriate order of the 
  rules.
 
I wonder if a tailored set of simpler XML matching what is actually
 possible and getting away from the DMTF records wouldn't be better,
 since the original spec is extended on the feature side and the
 applicability is restricted on the implementation side, I'm not sure I
 see the point of trying to be compatible.

In addition the DMTF XML format illustrated above is seriously ugly
and not following the style of other libvirt XML formats. The use
of UPPERCASE alone is reason enough to create a native libvirt format
for this.  We should of course ensure that whatever we do can be
reliably mapped into the DMTF format via a simple XSL transform.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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


Re: [libvirt] [PATCH] virterror: Don't invoke error callback for ERR_OK

2010-01-13 Thread Cole Robinson
On 01/13/2010 07:45 AM, Cole Robinson wrote:
 On 01/13/2010 04:23 AM, Daniel P. Berrange wrote:
 On Tue, Jan 12, 2010 at 03:57:33PM -0500, Cole Robinson wrote:
 On 01/12/2010 03:48 PM, Daniel P. Berrange wrote:
 On Tue, Jan 12, 2010 at 03:26:28PM -0500, Cole Robinson wrote:
 Since virDispatchError is now responsible for invoking the error callback,
 give it the same semantics as ReportError, which will skip VIR_ERR_OK
 (which is encountered when no error was raised).

 This fixes invoking the error callback after every non-erroring API call.

 Signed-off-by: Cole Robinson crobi...@redhat.com
 ---
  src/util/virterror.c |6 +-
  1 files changed, 5 insertions(+), 1 deletions(-)

 diff --git a/src/util/virterror.c b/src/util/virterror.c
 index e2128b9..78974ee 100644
 --- a/src/util/virterror.c
 +++ b/src/util/virterror.c
 @@ -603,8 +603,12 @@ virDispatchError(virConnectPtr conn)
  if (!err)
  return;
  
 -/* Set a generic error message if none is already set */
 +/* We never used to raise ERR_OK, so maintain existing behavior */
  if (err-code == VIR_ERR_OK)
 +return;
 +
 +/* Set a generic error message if none is already set */
 +if (!err-message)
  virErrorGenericFailure(err);
  
  /* Copy the global error to per-connection error if needed */

 We should only ever be invoking virDispatchError() in error paths, so
 if err-code == VIR_ERR_OK, this means we do need set a generic message
 because the earlier code indicated an error but forgot to report one.
 So I don't think this is correct.


 Ah, I think I wanted to check VIR_ERR_NONE here actually.
 virDispatchError is called regardless of whether an error is actually
 raised, so it may receive a zero'd out/empty virLastErrorObject, which
 is what I'm trying to avoid reporting.

 I still don't think you are correct in that. If you run

   # grep --after 1 virDispatchError libvirt.c
 virDispatchError(NULL);
 return (-1);
   --
   virDispatchError(net-conn);
   return -1;
   --
   virDispatchError(NULL);
   return (-1);
   --
   virDispatchError(pool-conn);
   return -1;

 Then all cases where virDispatchError() is called should be followed by the
 return of an error code, 99% of them are -1 or NULL. There are one or two
 where we use '0' for error as a special case. I don't see any places where
 virDispatchError() is called in a successful return path. So we should
 always be invoking the error callback, and ensuring an error is actually 
 set before doing so.

 
 Whoops, sorry for the confusion. I'll investigate more.

A couple issues were conspiring to raise the 'Unknown error'. In the daemon we
were blindly trying to unregister domain events via an API call, which would
rightly error if not events had ever been registered. However, the
implementation wasn't actually setting an error message. Patches coming shortly.

Thanks,
Cole

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


[libvirt] [PATCH] daemon: Don't blindly unregister domain events

2010-01-13 Thread Cole Robinson
The daemon will attempt to unregister domain events on client disconnect,
even if no events were ever registered. This raises an unneeded error.

Track in the qemu_client structure if events have been registered, and
check this when performing cleanup.

Signed-off-by: Cole Robinson crobi...@redhat.com
---
 daemon/libvirtd.c |2 +-
 daemon/libvirtd.h |1 +
 daemon/remote.c   |4 
 3 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index c53ef0a..61a9728 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1447,7 +1447,7 @@ void qemudDispatchClientFailure(struct qemud_client 
*client) {
 }
 
 /* Deregister event delivery callback */
-if(client-conn) {
+if (client-conn  client-domain_events_registered) {
 DEBUG0(Deregistering to relay remote events);
 virConnectDomainEventDeregister(client-conn, remoteRelayDomainEvent);
 }
diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h
index e3624c6..2f647f3 100644
--- a/daemon/libvirtd.h
+++ b/daemon/libvirtd.h
@@ -177,6 +177,7 @@ struct qemud_client {
 int watch;
 int readonly:1;
 int closing:1;
+int domain_events_registered:1;
 
 struct sockaddr_storage addr;
 socklen_t addrlen;
diff --git a/daemon/remote.c b/daemon/remote.c
index 41b9974..e39d6d1 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -4775,6 +4775,8 @@ remoteDispatchDomainEventsRegister (struct qemud_server 
*server ATTRIBUTE_UNUSED
 
 if (ret)
 ret-cb_registered = 1;
+
+client-domain_events_registered = 1;
 return 0;
 }
 
@@ -4796,6 +4798,8 @@ remoteDispatchDomainEventsDeregister (struct qemud_server 
*server ATTRIBUTE_UNUS
 
 if (ret)
 ret-cb_registered = 0;
+
+client-domain_events_registered = 0;
 return 0;
 }
 
-- 
1.6.5.2

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


[libvirt] [PATCH] daemon: Fix various error reporting issues

2010-01-13 Thread Cole Robinson
Many node device calls weren't properly relaying error messages, and
domain event registeration was not checking for error.

Signed-off-by: Cole Robinson crobi...@redhat.com
---
 daemon/remote.c |   38 +-
 1 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 0b30131..41b9974 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -231,7 +231,7 @@ remoteDispatchGetType (struct qemud_server *server 
ATTRIBUTE_UNUSED,
  */
 ret-type = strdup (type);
 if (!ret-type) {
-remoteDispatchFormatError (rerr, %s, _(out of memory in strdup));
+remoteDispatchOOMError(rerr);
 return -1;
 }
 
@@ -4489,7 +4489,7 @@ remoteDispatchNodeDeviceDumpXml (struct qemud_server 
*server ATTRIBUTE_UNUSED,
 
 dev = virNodeDeviceLookupByName(conn, args-name);
 if (dev == NULL) {
-remoteDispatchFormatError(rerr, %s, _(node_device not found));
+remoteDispatchConnError(rerr, conn);
 return -1;
 }
 
@@ -4520,7 +4520,7 @@ remoteDispatchNodeDeviceGetParent (struct qemud_server 
*server ATTRIBUTE_UNUSED,
 
 dev = virNodeDeviceLookupByName(conn, args-name);
 if (dev == NULL) {
-remoteDispatchFormatError(rerr, %s, _(node_device not found));
+remoteDispatchConnError(rerr, conn);
 return -1;
 }
 
@@ -4562,7 +4562,7 @@ remoteDispatchNodeDeviceNumOfCaps (struct qemud_server 
*server ATTRIBUTE_UNUSED,
 
 dev = virNodeDeviceLookupByName(conn, args-name);
 if (dev == NULL) {
-remoteDispatchFormatError(rerr, %s, _(node_device not found));
+remoteDispatchConnError(rerr, conn);
 return -1;
 }
 
@@ -4591,7 +4591,7 @@ remoteDispatchNodeDeviceListCaps (struct qemud_server 
*server ATTRIBUTE_UNUSED,
 
 dev = virNodeDeviceLookupByName(conn, args-name);
 if (dev == NULL) {
-remoteDispatchFormatError(rerr, %s, _(node_device not found));
+remoteDispatchConnError(rerr, conn);
 return -1;
 }
 
@@ -4634,7 +4634,7 @@ remoteDispatchNodeDeviceDettach (struct qemud_server 
*server ATTRIBUTE_UNUSED,
 
 dev = virNodeDeviceLookupByName(conn, args-name);
 if (dev == NULL) {
-remoteDispatchFormatError(rerr, %s, _(node_device not found));
+remoteDispatchConnError(rerr, conn);
 return -1;
 }
 
@@ -4661,7 +4661,7 @@ remoteDispatchNodeDeviceReAttach (struct qemud_server 
*server ATTRIBUTE_UNUSED,
 
 dev = virNodeDeviceLookupByName(conn, args-name);
 if (dev == NULL) {
-remoteDispatchFormatError(rerr, %s, _(node_device not found));
+remoteDispatchConnError(rerr, conn);
 return -1;
 }
 
@@ -4688,7 +4688,7 @@ remoteDispatchNodeDeviceReset (struct qemud_server 
*server ATTRIBUTE_UNUSED,
 
 dev = virNodeDeviceLookupByName(conn, args-name);
 if (dev == NULL) {
-remoteDispatchFormatError(rerr, %s, _(node_device not found));
+remoteDispatchConnError(rerr, conn);
 return -1;
 }
 
@@ -4738,7 +4738,7 @@ remoteDispatchNodeDeviceDestroy(struct qemud_server 
*server ATTRIBUTE_UNUSED,
 
 dev = virNodeDeviceLookupByName(conn, args-name);
 if (dev == NULL) {
-remoteDispatchFormatError(rerr, %s, _(node_device not found));
+remoteDispatchConnError(rerr, conn);
 return -1;
 }
 
@@ -4766,11 +4766,14 @@ remoteDispatchDomainEventsRegister (struct qemud_server 
*server ATTRIBUTE_UNUSED
 {
 CHECK_CONN(client);
 
-/* Register event delivery callback */
-REMOTE_DEBUG(%s,Registering to relay remote events);
-virConnectDomainEventRegister(conn, remoteRelayDomainEvent, client, NULL);
+if (virConnectDomainEventRegister(conn,
+  remoteRelayDomainEvent,
+  client, NULL)  0) {
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
 
-if(ret)
+if (ret)
 ret-cb_registered = 1;
 return 0;
 }
@@ -4786,11 +4789,12 @@ remoteDispatchDomainEventsDeregister (struct 
qemud_server *server ATTRIBUTE_UNUS
 {
 CHECK_CONN(client);
 
-/* Deregister event delivery callback */
-REMOTE_DEBUG(%s,Deregistering to relay remote events);
-virConnectDomainEventDeregister(conn, remoteRelayDomainEvent);
+if (virConnectDomainEventDeregister(conn, remoteRelayDomainEvent)  0) {
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
 
-if(ret)
+if (ret)
 ret-cb_registered = 0;
 return 0;
 }
-- 
1.6.5.2

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


[libvirt] [PATCH] events: Report errors on failure

2010-01-13 Thread Cole Robinson

Signed-off-by: Cole Robinson crobi...@redhat.com
---
 po/POTFILES.in  |1 +
 src/conf/domain_event.c |   36 +---
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 1ab0859..c9c78b6 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -4,6 +4,7 @@ daemon/remote.c
 daemon/stream.c
 src/conf/cpu_conf.c
 src/conf/domain_conf.c
+src/conf/domain_event.c
 src/conf/interface_conf.c
 src/conf/network_conf.c
 src/conf/node_device_conf.c
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 0fa2822..72f499c 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -26,6 +26,13 @@
 #include logging.h
 #include datatypes.h
 #include memory.h
+#include virterror_internal.h
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+#define eventReportError(conn, code, fmt...)\
+virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__,   \
+ __FUNCTION__, __LINE__, fmt)
 
 
 /**
@@ -87,6 +94,9 @@ virDomainEventCallbackListRemove(virConnectPtr conn,
 return 0;
 }
 }
+
+eventReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(could not find event callback for removal));
 return -1;
 }
 
@@ -140,6 +150,9 @@ int virDomainEventCallbackListMarkDelete(virConnectPtr conn,
 return 0;
 }
 }
+
+eventReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(could not find event callback for deletion));
 return -1;
 }
 
@@ -199,13 +212,14 @@ virDomainEventCallbackListAdd(virConnectPtr conn,
 for (n=0; n  cbList-count; n++) {
 if(cbList-callbacks[n]-cb == callback 
conn == cbList-callbacks[n]-conn) {
-DEBUG0(WARNING: Callback already tracked);
+eventReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(event callback already tracked));
 return -1;
 }
 }
 /* Allocate new event */
 if (VIR_ALLOC(event)  0) {
-DEBUG0(Error allocating event);
+virReportOOMError(conn);
 return -1;
 }
 event-conn = conn;
@@ -216,7 +230,7 @@ virDomainEventCallbackListAdd(virConnectPtr conn,
 /* Make space on list */
 n = cbList-count;
 if (VIR_REALLOC_N(cbList-callbacks, n + 1)  0) {
-DEBUG0(Error reallocating list);
+virReportOOMError(conn);
 VIR_FREE(event);
 return -1;
 }
@@ -242,8 +256,10 @@ virDomainEventQueuePtr virDomainEventQueueNew(void)
 {
 virDomainEventQueuePtr ret;
 
-if (VIR_ALLOC(ret)  0)
+if (VIR_ALLOC(ret)  0) {
+virReportOOMError(NULL);
 return NULL;
+}
 
 return ret;
 }
@@ -254,12 +270,15 @@ virDomainEventPtr virDomainEventNew(int id, const char 
*name,
 {
 virDomainEventPtr event;
 
-if (VIR_ALLOC(event)  0)
+if (VIR_ALLOC(event)  0) {
+virReportOOMError(NULL);
 return NULL;
+}
 
 event-type = type;
 event-detail = detail;
 if (!(event-name = strdup(name))) {
+virReportOOMError(NULL);
 VIR_FREE(event);
 return NULL;
 }
@@ -318,8 +337,11 @@ virDomainEventQueuePop(virDomainEventQueuePtr evtQueue)
 {
 virDomainEventPtr ret;
 
-if(!evtQueue || evtQueue-count == 0 )
+if (!evtQueue || evtQueue-count == 0 ) {
+eventReportError(NULL, VIR_ERR_INTERNAL_ERROR, %s,
+ _(event queue is empty, nothing to pop));
 return NULL;
+}
 
 ret = evtQueue-events[0];
 
@@ -357,7 +379,7 @@ virDomainEventQueuePush(virDomainEventQueuePtr evtQueue,
 /* Make space on queue */
 if (VIR_REALLOC_N(evtQueue-events,
   evtQueue-count + 1)  0) {
-DEBUG0(Error reallocating queue);
+virReportOOMError(NULL);
 return -1;
 }
 
-- 
1.6.5.2

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


[libvirt] [PATCH v2] Implement path lookup for USB by vendor:product

2010-01-13 Thread Cole Robinson
Based off how QEMU does it, look through /sys/bus/usb/devices/* for
matching vendor:product info, and if found, use info from the surrounding
files to build the device's /dev/bus/usb path.

This fixes USB device assignment by vendor:product when running qemu
as non-root (well, it should, but for some reason I couldn't reproduce
the failure people are seeing in [1], but it appears to work properly)

[1] https://bugzilla.redhat.com/show_bug.cgi?id=542450

v2:
Drop 'bus.addr only' checks in security drivers
Use various util helpers

Signed-off-by: Cole Robinson crobi...@redhat.com
---
 po/POTFILES.in  |1 +
 src/qemu/qemu_driver.c  |9 +--
 src/security/security_selinux.c |   25 -
 src/security/virt-aa-helper.c   |   32 +--
 src/util/hostusb.c  |  110 +-
 src/util/hostusb.h  |4 +-
 6 files changed, 141 insertions(+), 40 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index c9c78b6..3b82a74 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -55,6 +55,7 @@ src/uml/uml_conf.c
 src/uml/uml_driver.c
 src/util/bridge.c
 src/util/conf.c
+src/util/hostusb.c
 src/util/json.c
 src/util/logging.c
 src/util/pci.c
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index deb8adc..f03ce91 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2105,14 +2105,11 @@ static int 
qemuDomainSetHostdevUSBOwnership(virConnectPtr conn,
 struct qemuFileOwner owner = { uid, gid };
 int ret = -1;
 
-/* XXX what todo for USB devs assigned based on product/vendor ? Doom :-( 
*/
-if (!def-source.subsys.u.usb.bus ||
-!def-source.subsys.u.usb.device)
-return 0;
-
 usbDevice *dev = usbGetDevice(conn,
   def-source.subsys.u.usb.bus,
-  def-source.subsys.u.usb.device);
+  def-source.subsys.u.usb.device,
+  def-source.subsys.u.usb.vendor,
+  def-source.subsys.u.usb.product);
 
 if (!dev)
 goto cleanup;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 000bc8a..cb585ed 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -481,20 +481,17 @@ SELinuxSetSecurityHostdevLabel(virConnectPtr conn,
 
 switch (dev-source.subsys.type) {
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
-if (dev-source.subsys.u.usb.bus  dev-source.subsys.u.usb.device) {
-usbDevice *usb = usbGetDevice(conn,
-  dev-source.subsys.u.usb.bus,
-  dev-source.subsys.u.usb.device);
+usbDevice *usb = usbGetDevice(conn,
+  dev-source.subsys.u.usb.bus,
+  dev-source.subsys.u.usb.device,
+  dev-source.subsys.u.usb.vendor,
+  dev-source.subsys.u.usb.product);
 
-if (!usb)
-goto done;
+if (!usb)
+goto done;
 
-ret = usbDeviceFileIterate(conn, usb, SELinuxSetSecurityUSBLabel, 
vm);
-usbFreeDevice(conn, usb);
-} else {
-/* XXX deal with product/vendor better */
-ret = 0;
-}
+ret = usbDeviceFileIterate(conn, usb, SELinuxSetSecurityUSBLabel, vm);
+usbFreeDevice(conn, usb);
 break;
 }
 
@@ -556,7 +553,9 @@ SELinuxRestoreSecurityHostdevLabel(virConnectPtr conn,
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
 usbDevice *usb = usbGetDevice(conn,
   dev-source.subsys.u.usb.bus,
-  dev-source.subsys.u.usb.device);
+  dev-source.subsys.u.usb.device,
+  dev-source.subsys.u.usb.vendor,
+  dev-source.subsys.u.usb.product);
 
 if (!usb)
 goto done;
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 35b29ad..3c8b49a 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -836,24 +836,22 @@ get_files(vahControl * ctl)
 virDomainHostdevDefPtr dev = ctl-def-hostdevs[i];
 switch (dev-source.subsys.type) {
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
-if (dev-source.subsys.u.usb.bus 
-dev-source.subsys.u.usb.device) {
-usbDevice *usb = usbGetDevice(NULL,
-   dev-source.subsys.u.usb.bus,
-   dev-source.subsys.u.usb.device);
-if (usb == NULL)
-continue;
-rc = usbDeviceFileIterate(NULL, usb,
-  

[libvirt] [PATCH] qemu: Check for ia64 kvm

2010-01-13 Thread Cole Robinson
From: Dustin Xiong x_k_...@hotmail.com

Ported to current code. Untested, but builds fine.

Signed-off-by: Cole Robinson crobi...@redhat.com
---
 src/qemu/qemu_conf.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index d3da776..0d970d6 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -373,12 +373,19 @@ static const struct qemu_feature_flags const 
arch_info_x86_64_flags [] = {
 { apic, 1, 0 },
 };
 
+static const struct qemu_feature_flags const arch_info_ia64_flags [] = {
+{ acpi, 1, 1 },
+{ apic, 1, 0 },
+};
+
 /* The archicture tables for supported QEMU archs */
 static const struct qemu_arch_info const arch_info_hvm[] = {
 {  i686,   32, NULL, /usr/bin/qemu,
/usr/bin/qemu-system-x86_64, arch_info_i686_flags, 4 },
 {  x86_64, 64, NULL, /usr/bin/qemu-system-x86_64,
NULL, arch_info_x86_64_flags, 2 },
+{  itanium, 64, NULL, /usr/bin/qemu-system-ia64,
+   NULL, arch_info_ia64_flags, 2},
 {  arm,32, NULL, /usr/bin/qemu-system-arm,NULL, NULL, 0 },
 {  mips,   32, NULL, /usr/bin/qemu-system-mips,   NULL, NULL, 0 },
 {  mipsel, 32, NULL, /usr/bin/qemu-system-mipsel, NULL, NULL, 0 },
-- 
1.6.5.2

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


[libvirt] [PATCH] util: Make sure virExec hook failures are raised

2010-01-13 Thread Cole Robinson
With the introduction virDispatchError, hook function errors are
never sent through the error callback, so users will never see
these messages.

Fix this by calling virDispatchError after hook failure.

Signed-off-by: Cole Robinson crobi...@redhat.com
---
 src/util/util.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/util/util.c b/src/util/util.c
index ba6b0db..45ca657 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -557,8 +557,11 @@ __virExec(virConnectPtr conn,
 }
 
 if (hook)
-if ((hook)(data) != 0)
+if ((hook)(data) != 0) {
+VIR_DEBUG0(Hook function failed.);
+virDispatchError(NULL);
 _exit(1);
+}
 
 /* The steps above may need todo something privileged, so
  * we delay clearing capabilities until the last minute */
-- 
1.6.5.2

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


Re: [libvirt] [RFC] Proposal for introduction of network traffic filtering capabilities for filtering of network traffic from and to VMs

2010-01-13 Thread Stefan Berger
Daniel Veillard veill...@redhat.com wrote on 01/13/2010 12:03:22 PM:

 On Mon, Jan 11, 2010 at 12:55:27PM -0500, Stefan Berger wrote:
  Hello!
  
[...]
  raw network throughput performance hit on their system due to the 
  evaluation of every packet passing through the filtering chains, do 
not 
  have to use these capabilities. An initial implementation would be 
done 
  for libvirt's Qemu support.
 
  It's relatively clear that this would not work with devices exposed
 natively to the domain, say though PCI passthough. I'm wondering what

Future SR-IOV devices may be programmable and provide at least a subset 
of the filtering that can be done with this XML. In that case we would 
probably need hardware specific drivers in libvirt that can issue proper
ioctl()s with the proper parameters to activate the corresponding 
filtering.

 other case of limitiations could be found. Also this may not map well
 for other kind of hypervisors like VMWare, right ?

I don't know much about the API that VMWare is exposing. Maybe only a 
certain subset of what would be possible with this XML could be applied 
to their API, if such functionality exist. Otherwise, if libvirt
can run ebtables and iptables commands on their management VM and
all traffic passes through VM=specific network interface (tap) in that VM,
it *should* work as well.

 
  As stated above, the application of firewall rules on virtual machines 

  will require some form of XML description that lets the user choose 
what 
  type of filtering is to be performed. In effect the user needs to be 
able 
  to tell libvirt which ebtables and iptables rules to generate so that 
the 
  appropriate filtering can be done. We realize that  ebtables and 
iptables 
  have lots of capabilities but think that we need to restrict which 
  capabilities can actually be 'reached' through this XML.
  
  The following proposal is based on an XML as defined by the DMTF (
  http://www.dmtf.org/standards/cim/cim_schema_v2230/CIM_Network.pdf, 
slide 
  10) with extensions for processing of ARP packets.It gives control 
over 
  the evaluation of Ethernet (MAC) frames, ARP packet data and IP header 

  information. A (draft) XML skeleton in this case could look as 
follows:
 
   Humpf, extending astandard XML schemas is really bad practice. Even if
 you think that the extension may be accepted later, it may have
 different semantic for the same construct and that's a disaster.
 I would at least put the new elements in a separate namespace to
 avoid something insane if DMTF extends its specification.

Yes, we could do that. As said, this was meant as a draft.

 
  FilterEntry 
  ActionDROP|ACCEPT/Action   !-- from FilterEntry -- 
  
  TrafficTypeincoming [to VM]|outgoing [from VM]/TrafficType 

  
  Hdr8021Filter 
  HdrSrcMACAddr8021   /HdrSrcMACAddr8021 
  HdrSrcMACMask8021   /HdrSrcMACMask8021 
  HdrDestMACAddr8021  /HdrDestMACAddr8021 
  HdrDestMACMask8021  /HdrDestMACMask8021 
  HdrProtocolID8021 numeric and/or string? 
  /HdrProtocolID8021 
  HdrPriorityValue8021/HdrPriorityValue8021 
  HdrVLANID8021   /HdrVLANID8021 
  /Hdr8021Filter 
  
  ARPFilter 
  HWType   /HWType 
  ProtocolType /ProtocolType 
  OPCode   /OPCode 
  SrcMACAddr   /SrcMACAddr 
  SrcIPAddr/SrcIPAddr 
  DestMACAddr  /DestMACAddr 
  DestIPAddr   /DestIPAddr 
  /ARPFilter 
  
  IPHeadersFilter 
  HdrIPVersion /HdrIPVersion 
  HdrSrcAddress/HdrSrcAddress 
  HdrSrcMask   /HdrSrcMask 
  HdrDestAddress   /HdrDestAddress 
  HdrDestMask  /HdrDestMask 
  HdrProtocolID  numeric and/or string? 
/HdrProtocolID 
  HdrSrcPortStart  /HdrSrcPortStart 
  HdrSrcPortEnd/HdrSrcPortEnd 
  HdrDestPortStart /HdrDestPortStart 
  HdrDestPortEnd   /HdrDestPortEnd 
  HdrDSCP  /HdrDSCP 
  /IPHeadersFilter 
  
  /FilterEntry 
 
   hum the types for each values should be made explicit if possible,
 But in general I find that extremely bulky for a single entry, while
 I would expect any domain to have a dozen filters or so just for
 simple stuff. I wonder if empty default should just be dropped, so
 that basically if you're filtering on a MAC address you don't carry
 a bunch of unused fields.

Of course empty XML elements would not be shown. The above XML is meant 
to show all the network packet 'elements' that can be tested and for which 

also ebtables and iptables support exists. Also only a certain combination
of the above elements would be set for a specific corresponding ebtables 
command.

An example for a (template) XML for testing 

[libvirt] Cannot use console with 0.7.5, error: internal error no assigned pty for device serial0

2010-01-13 Thread Marc Haber
Hi,

I have one test host running Debian unstable (kernel 2.6.32.3), and I
would like to virtualize on it using KVM and virsh. Debian unstable
has libvirt 0.7.5.

On this host, I cannot start any KVM domain using console with these
XML parts:

serial type='pty'
  target port='0'/
/serial
console type='pty'
  target port='0'/
/console

When I try virsh start $domain, I get the error message:

error: internal error no assigned pty for device serial0

When I remove the serial and console stanza, the machine starts up.
This is the configuration documented everywhere. At first, I suspected
a change in the configuration syntax and filed Debian bug #565145, But
on the #virt IRC channel, people suggested that there was a change in
libvirt parsing KVM's output, which may be a genuine bug breaking the
console.

Is this already a known issue? Can I do anything to help debugging?

Greetings
Marc

-- 
-
Marc Haber | I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things.Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190

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


Re: [libvirt] Cannot use console with 0.7.5, error: internal error no assigned pty for device serial0

2010-01-13 Thread Marc Haber
I apologize for thread hijacking, I missed editing out the
IN-Reply-To-Header. Sorry.

Greetings
Marc

-- 
-
Marc Haber | I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things.Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190

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


Re: [libvirt] [BUG?] no domain with matching uuid error, when vm restarts

2010-01-13 Thread su disheng
On Wed, Jan 13, 2010 at 2:39 AM, Daniel P. Berrange berra...@redhat.comwrote:

 On Tue, Jan 12, 2010 at 04:06:32PM -0800, su disheng wrote:
  Hi,
  In the following libvirt API calling sequence, I always get an error
 no
  domain with matching uuid
  Connect _conn = new Connect(qemu:///system, false);
   _conn.domainDefineXML(kvm_guest_xml);
   Domain dm = _conn.domainLookupByName(kvm_guest_name);
   dm.create();
 
   /* stop, undefine, and re-start the vm*/
   dm.shutdown();
   dm.undefine();
   dm.domainDefineXML(kvm_guest_xml);
   /A/
   Domain dm = _conn.domainLookupByName(kvm_guest_name);
   dm.create()  /Error**/
 
 if I close the connection, and re-connect qemu at the end of
  /A/, then everything is OK.

 I think the first 'dm'  object you create is not being freed / garbage
 collected. This means the underlying libvirt virDomainPtr object is
 not released. And so when you then _conn.domainLookupByName() after
 'A'  you are still getting an handle to the old object and
 thus the stale UUID.


Hmm, got it. If free all the returned virDomainPtr, then the issue is gone.
One frustrated thing is that the API virDomainDefineXML, which returns a
virDomainPtr, I need to explicitly free it, even I don't reference it after
that function call... Got me crazy to debug it. Maybe it's better to mention
it in the API reference?

Another thing is /* TODO search by UUID first as they are better
differenciators */ in virGetDomain, if libvirt does what it said here, my
issue will not be an issue again, right?



 From the log, seems that uuid is not updated immediately, for this
  connection, the uuid is in the stale state?
 I am using libvirt 0.6.3, if it's a bug, does it fixed in the
 latest
  code? or Have I need to close the connection for each vm
 shutdown/re-define?

 You should not need to close the connection, provided you ensure all the
 'Domain' objects are freed/garbage collected

 Alternatively, if you wish to re-use the same name,then you could try
 auto-generating the  UUID yourself, so you use the same UUID when
 defining the domain for the second time


 Daniel
 --
 |: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/:|
 |: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org:|
 |: http://autobuild.org   -o- 
 http://search.cpan.org/~danberr/http://search.cpan.org/%7Edanberr/:|
 |: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505
 :|

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

Re: [libvirt] [RFC] Proposal for introduction of network traffic filtering capabilities for filtering of network traffic from and to VMs

2010-01-13 Thread Dimitrios Pendarakis

Note that this is the case for the standard VMware ESX vSwitch. However,
the Cisco Nexus 1000v,
which is an optional virtual switch for ESX, does appear to provide (guest)
VM firewall capabilities.
See for example the section on isolation and protection and ACLs in the
link below:

http://www.cisco.com/en/US/prod/collateral/switches/ps9441/ps9902/dmz_virtualization_vsphere4_nexus1000V.pdf

In terms of interfaces, the 1000v claims a CLI (likely similar to physical
Cisco switches), support for SNMP
and an XML API.

Thanks,
Dimitrios


   
  From:   Matthias Bolte matthias.bo...@googlemail.com   
   
  To: Stefan Berger/Watson/i...@ibmus   
   
  Cc: Gerhard Stenzel gerhard.sten...@de.ibm.com, 
libvir-list@redhat.com, Vivek
  Kashyap/Beaverton/i...@ibmus  
   
  Date:   01/13/2010 05:43 PM  
   
  Subject:Re: [libvirt] [RFC] Proposal for introduction of network traffic  
filtering
  capabilities for filtering of network traffic fromand to 
VMs
   
  Sent by:libvir-list-boun...@redhat.com   
   





2010/1/13 Stefan Berger stef...@us.ibm.com:

 Daniel Veillard veill...@redhat.com wrote on 01/13/2010 12:03:22 PM:

 On Mon, Jan 11, 2010 at 12:55:27PM -0500, Stefan Berger wrote:
  Hello!
 
 [...]
[...]
 other case of limitiations could be found. Also this may not map well
 for other kind of hypervisors like VMWare, right ?

 I don't know much about the API that VMWare is exposing. Maybe only a
 certain subset of what would be possible with this XML could be applied
 to their API, if such functionality exist. Otherwise, if libvirt
 can run ebtables and iptables commands on their management VM and
 all traffic passes through VM=specific network interface (tap) in that
VM,
 it *should* work as well.

VMware ESX hosts allow to configure the host level firewall via the
remote VI API. But AFAIK there is no virtual machine level firewall.

You're not supposed to do something like that in the service console,
doing anything in the service console is not supported in general.
Also there is no libvirtd in the service console because of that and
because it is not necessary. The ESX driver does everything using the
remote VI API.

Matthias

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

inline: graycol.gifinline: ecblank.gif--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Check for ia64 kvm

2010-01-13 Thread Dustin Xiong


 

 From: crobi...@redhat.com
 To: libvirt-l...@redhat.com
 CC: x_k_...@hotmail.com; crobi...@redhat.com
 Subject: [PATCH] qemu: Check for ia64 kvm
 Date: Wed, 13 Jan 2010 15:50:06 -0500
 
 From: Dustin Xiong x_k_...@hotmail.com
 
 Ported to current code. Untested, but builds fine.
 
 Signed-off-by: Cole Robinson crobi...@redhat.com
 ---
 src/qemu/qemu_conf.c | 7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)
 
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index d3da776..0d970d6 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -373,12 +373,19 @@ static const struct qemu_feature_flags const 
 arch_info_x86_64_flags [] = {
 { apic, 1, 0 },
 };
 
 +static const struct qemu_feature_flags const arch_info_ia64_flags [] = {
 + { acpi, 1, 1 },
 + { apic, 1, 0 },
 +};
 +
 /* The archicture tables for supported QEMU archs */
 static const struct qemu_arch_info const arch_info_hvm[] = {
 { i686, 32, NULL, /usr/bin/qemu,
 /usr/bin/qemu-system-x86_64, arch_info_i686_flags, 4 },
 { x86_64, 64, NULL, /usr/bin/qemu-system-x86_64,
 NULL, arch_info_x86_64_flags, 2 },
 + { itanium, 64, NULL, /usr/bin/qemu-system-ia64,
 + NULL, arch_info_ia64_flags, 2},
 { arm, 32, NULL, /usr/bin/qemu-system-arm, NULL, NULL, 0 },
 { mips, 32, NULL, /usr/bin/qemu-system-mips, NULL, NULL, 0 },
 { mipsel, 32, NULL, /usr/bin/qemu-system-mipsel, NULL, NULL, 0 },
 -- 
 1.6.5.2
 


There another problem when you make the libvirt src on the arch of itanium, you 
need modify the function clone().

   On IA-64, a different system call is used:

   int __clone2(int (*fn)(void *),  void *child_stack_base,
size_t stack_size, int flags, void *arg, ...
/* pid_t *pid, struct user_desc *tls, pid_t *ctid */ );

   The  __clone2() system call operates in the same way as clone(), except
   that child_stack_base points to the lowest address of the child's stack
   area,  and  stack_size  specifies  the  size of the stack pointed to by
   child_stack_base.


The function clone() is used in /src/lxc_container.c.

 

-Dustin.
  
_
Windows Live: Friends get your Flickr, Yelp, and Digg updates when they e-mail 
you.
http://www.microsoft.com/middleeast/windows/windowslive/see-it-in-action/social-network-basics.aspx?ocid=PID23461::T:WLMTAGL:ON:WL:en-xm:SI_SB_3:092010--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Fix linkage of virt-aa-helper to libgnu.a

2010-01-13 Thread Matthias Bolte
---
 src/Makefile.am |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index dbf708b..324030b 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -876,9 +876,9 @@ virt_aa_helper_LDFLAGS = $(WARN_CFLAGS)
 virt_aa_helper_LDADD = \
$(WARN_CFLAGS)  \
$(LIBXML_LIBS)  \
-   @top_srcdir@/gnulib/lib/libgnu.la   \
@top_srcdir@/src/libvirt_conf.la\
-   @top_srcdir@/src/libvirt_util.la
+   @top_srcdir@/src/libvirt_util.la\
+   @top_srcdir@/gnulib/lib/libgnu.la
 virt_aa_helper_CFLAGS =\
-...@top_srcdir@/src/conf   \
-...@top_srcdir@/src/security
-- 
1.6.3.3

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


[libvirt] [PATCH] Fix event-test segfault for mixed remote/local driver usage

2010-01-13 Thread Matthias Bolte
If examples/domain-events/events-c/event-test.c uses a local driver
for the hyperviror and a remote driver for storage then the segfault
occurs in remoteDomainEventDispatchFunc because conn-privateData is
used.

The problem is as follows. The doRemoteOpen function registers
remoteDomainEventFired as event handle and remoteDomainEventQueueFlush
as event timeout. The doRemoteOpen function is used in remoteOpen and
remoteOpenSecondaryDriver. So both handlers are registered for all
types of remote drivers, but remoteDomainEventQueueFlush always uses
conn-privateData. That's wrong when remoteDomainEventQueueFlush is
called for any other driver than the hypervisor driver, it should use
conn-storagePrivateData for the storage driver for example.

In the common case this doesn't result in a segfault because in the
common case all drivers for a connection are either all remote or all
local and conn-privateData and conn-storagePrivateData are the same.
But in the local Xen case ('event-test xen:///' executed in Domain-0,
with a libvirtd running in Domain-0) the hypervisor driver is local
and the storage driver is remote. This results in a mismatch between
conn-privateData and conn-storagePrivateData. Now the call to
remoteDomainEventQueueFlush in the context of the storage driver
results in a segfault, because it assumes that conn-privateData points
to remote driver private data, but it points to Xen driver private
data.

To fix this, the pointer to the private driver data gets passed as
opaque parameter instead of the connection pointer. Because some
functions called by the event handler functions need the connection
pointer for other purposes than error reporting the connection
pointer is passed as member of the private remote driver data struct.

Now remoteDomainEventFired and remoteDomainEventQueueFlush have the
correct private data pointer for the respective calling context.
---
 src/remote/remote_driver.c |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index d6f5fce..582b85c 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -152,6 +152,7 @@ struct private_stream_data {
 
 struct private_data {
 virMutex lock;
+virConnectPtr conn;
 
 int sock;   /* Socket. */
 int watch;  /* File handle watch */
@@ -893,7 +894,7 @@ doRemoteOpen (virConnectPtr conn,
 if ((priv-watch = virEventAddHandle(priv-sock,
  VIR_EVENT_HANDLE_READABLE,
  remoteDomainEventFired,
- conn, NULL))  0) {
+ priv, NULL))  0) {
 DEBUG0(virEventAddHandle failed: No addHandleImpl defined.
 continuing without events.);
 } else {
@@ -901,7 +902,7 @@ doRemoteOpen (virConnectPtr conn,
 DEBUG0(Adding Timeout for remote event queue flushing);
 if ( (priv-eventFlushTimer = virEventAddTimeout(-1,
  
remoteDomainEventQueueFlush,
- conn, NULL))  0) {
+ priv, NULL))  0) {
 DEBUG0(virEventAddTimeout failed: No addTimeoutImpl defined. 
 continuing without events.);
 virEventRemoveHandle(priv-watch);
@@ -980,6 +981,7 @@ remoteAllocPrivateData(virConnectPtr conn)
 return NULL;
 }
 remoteDriverLock(priv);
+priv-conn = conn;
 priv-localUses = 1;
 priv-watch = -1;
 priv-sock = -1;
@@ -8659,8 +8661,7 @@ remoteDomainEventFired(int watch,
int event,
void *opaque)
 {
-virConnectPtrconn = opaque;
-struct private_data *priv = conn-privateData;
+struct private_data *priv = opaque;
 
 remoteDriverLock(priv);
 
@@ -8684,7 +8685,7 @@ remoteDomainEventFired(int watch,
 goto done;
 }
 
-if (remoteIOHandleInput(conn, priv, 0)  0)
+if (remoteIOHandleInput(priv-conn, priv, 0)  0)
 DEBUG0(Something went wrong during async message processing);
 
 done:
@@ -8708,8 +8709,7 @@ static void remoteDomainEventDispatchFunc(virConnectPtr 
conn,
 void
 remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, void *opaque)
 {
-virConnectPtr conn = opaque;
-struct private_data *priv = conn-privateData;
+struct private_data *priv = opaque;
 virDomainEventQueue tempQueue;
 
 remoteDriverLock(priv);
@@ -8731,7 +8731,7 @@ remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, 
void *opaque)
 
 if ( priv-callbackList-count == 0 ) {
 /* Tell the server when we are the last callback deregistering */
-if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER,
+if (call (priv-conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER,
  

[libvirt] Set MAC address for host virtual interface?

2010-01-13 Thread Neil Aggarwal
Hello:

Is there a way to set the MAC address for the virtual
interface on the host?

I have this in my guests's XML file:
   interface type='bridge'
 mac address='54:52:00:7d:f8:fc'/
 source bridge='br0'/
 target dev='tkl-joomla'/
   /interface

the mac address line controls the guest interface
but the host interface seems to get a random MAC
address.

This is causing a problem for me since I use Cacti
to monitor the bandwidth usage of the guest machines.
Every time I stop and start a guest, Cacti creates a
new interface and I have to reset the graphs to use
the new interfaces.

Thanks,
Neil

--
Neil Aggarwal, (281)846-8957, http://UnmeteredVPS.net
Host Joomla!, Wordpress, phpBB, or vBulletin for $25/mo
Unmetered bandwidth = no overage charges, 7 day free trial

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


[libvirt] Man page patch for BZ 548485

2010-01-13 Thread David Jorm
See https://bugzilla.redhat.com/show_bug.cgi?id=548485

git diff for the patch:

---SNIP---

--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -342,7 +342,9 @@ severed upon restore, as TCP timeouts may have expired.
 
 =item Bschedinfo optional I--weight Bnumber optional I--cap Bnumber 
Idomain-id
 
-Allows to show (and set) the domain scheduler parameters.
+Allows you to show (and set) the domain scheduler parameters.
+
+BNote: The only parameter currently available for all schedulers is 
cpu_shares. It has a valid value range of 0-262144.
 
 BNote: The weight and cap parameters are defined only for the
 XEN_CREDIT scheduler and are now IDEPRECATED.

---SNIP---

I hope that's right, I found that BZ a bit confusing to comprehend. Learning 
curve.

David

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