[libvirt] ARM libvirt compiling error
Hello, I'm trying to set up a development environment on an Arndale (ARM Samsung Exynos 5250) board to work on sVirt. I'm using Debian 7.0, I've downloaded the source code from GIT and than: ./autogen.sh --prefix=$HOME/usr make but in the middle of make execution, the program fails with this error: conf/domain_conf.c: In function 'virDomainHostdevDefParseXML': conf/domain_conf.c:3915:36: error: 'next_unit' may be used uninitialized in this function [-Werror=uninitialized] conf/domain_conf.c:3886:9: note: 'next_unit' was declared here conf/domain_conf.c: At top level: cc1: error: unrecognized command line option "-Wno-unused-command-line-argument" [-Werror] I've solved this problem simply initializing the "next_unit" variable (file src/conf/domain_conf.c, line 3886). This is the diff between the original file and the modified one: 3886c3886 < int next_unit; --- > int next_unit = -1; Another way to solve is obviously with --disable-werror, but I guess this is not the best way. My gcc version is 4.6.3 (Debian 4.6.3-14), kernel version is 3.8.0-rc4. Maybe it's only a compiler problem, but can anyone confirm this? Is it worth to submit a new bug report/patch the source? Regards, Michele -- *Michele Paolino ** *Virtual Open Systems* **Open Source KVM Virtualization Developments Multicore Systems Virtualization Porting Services *Web*:* *www.virtualopensystems.com* -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] need custom /dev entries in LXC
On Wed, May 22, 2013 at 06:22:12PM -0400, Michael R. Hines wrote: > Hi, > > We run nvidia devices inside libvirt-managed LXC containers. > > It used to be that simply doing: > > $ echo 'c 195:* rwm' > /sys/fs/cgroup/devices/libvirt/lxc > > Then, after booting the container, we would do: > > $ mknod -m 666 /dev/nvidia0 c 195 0 > > would be good enough to run our CUDA applications. > > But, according to: > > $ cat src/lxc/lxc_container.c > > The CAP_MKNOD capability is being dropped and only a specific > set of devices is being created before booting the container. > > Is there any reason why this is not per-device configurable? With recent libvirt you can pass through arbitrary block and character devices explicitly, using the following XML: http://libvirt.org/formatdomain.html#elementsHostDevCaps As such there is never any need to change cgroups or use mknod as you describe. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] nodedev_hal: Enumerate scsi generic device
The xml outputed by HAL backend for scsi generic device: pci_8086_2922_scsi_host_scsi_device_lun0_scsi_generic /sys/devices/pci:00/:00:1f.2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0 pci_8086_2922_scsi_host_scsi_device_lun0 /dev/sg0 --- src/node_device/node_device_hal.c | 9 + 1 file changed, 9 insertions(+) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 99b5044..794f923 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -303,6 +303,14 @@ gather_storage_cap(LibHalContext *ctx, const char *udi, return 0; } +static int +gather_scsi_generic_cap(LibHalContext *ctx, const char *udi, +union _virNodeDevCapData *d) +{ +(void)get_str_prop(ctx, udi, "scsi_generic.device", &d->sg.path); +return 0; +} + static int gather_system_cap(LibHalContext *ctx, const char *udi, @@ -350,6 +358,7 @@ static caps_tbl_entry caps_tbl[] = { { "scsi_host", VIR_NODE_DEV_CAP_SCSI_HOST, gather_scsi_host_cap }, { "scsi", VIR_NODE_DEV_CAP_SCSI, gather_scsi_cap }, { "storage",VIR_NODE_DEV_CAP_STORAGE, gather_storage_cap }, +{ "scsi_generic", VIR_NODE_DEV_CAP_SCSI_GENERIC, gather_scsi_generic_cap }, }; -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] nodedev: Support SCSI_GENERIC cap flag for listAllNodeDevices
--- include/libvirt/libvirt.h.in | 1 + src/conf/node_device_conf.c | 4 +++- src/conf/node_device_conf.h | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1804c93..e7e003c 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3258,6 +3258,7 @@ typedef enum { VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE = 1 << 8, /* Storage device */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST = 1 << 9, /* FC Host Bus Adapter */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS= 1 << 10, /* Capable of vport */ +VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC = 1 << 11, /* Capable of scsi_generic */ } virConnectListAllNodeDeviceFlags; int virConnectListAllNodeDevices (virConnectPtr conn, diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index a0b6338..b764cb8 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1492,7 +1492,9 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj, (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST) && virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_FC_HOST)) || (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS) && - virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_VPORTS + virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_VPORTS)) || + (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC) && + virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_SCSI_GENERIC return false; } diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index e326e82..17240be 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -274,7 +274,8 @@ void virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj); VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST | \ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS) + VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS| \ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC) int virNodeDeviceList(virConnectPtr conn, virNodeDeviceObjList devobjs, -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] nodedev: Expose sysfs path of device
The name format is constructed by libvirt, it's not that clear to get what the device's sysfs path should be. This exposes the device's sysfs path by a new tag . Since the sysfspath is filled during enumerating the devices by either udev or HAL. It's an output-only tag. --- src/conf/node_device_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4eeb3b3..cc6f297 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -236,6 +236,7 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) virBufferAddLit(&buf, "\n"); virBufferEscapeString(&buf, " %s\n", def->name); +virBufferEscapeString(&buf, " %s\n", def->sysfs_path); if (def->parent) { virBufferEscapeString(&buf, " %s\n", def->parent); } -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] nodedev_udev: Refactor udevGetDeviceType
Checking if the "devtype" is NULL along with each "if" statements is bad. It wastes the performance, and also not good for reading. And also when the "devtype" is NULL, the logic is also not clear. This reorgnizes the logic of with "if...else" and a bunch of "else if". Other changes: * Change the function style. * Remove the useless debug statement. * Get rid of the goto * New helper udevDeviceHasProperty to simplify the logic for checking if a property is existing for the device. * Add comment to clarify "PCI devices don't set the DEVTYPE property" * s/sysfs path/sysfs name/, as udev_device_get_sysname returns the name instead of the full path. E.g. "sg0" * Refactor the comment for setting VIR_NODE_DEV_CAP_NET cap type a bit. --- src/node_device/node_device_udev.c | 119 - 1 file changed, 52 insertions(+), 67 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 620cd58..3c91298 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -106,7 +106,6 @@ static int udevStrToLong_i(char const *s, return ret; } - /* This function allocates memory from the heap for the property * value. That memory must be later freed by some other code. */ static int udevGetDeviceProperty(struct udev_device *udev_device, @@ -1100,78 +1099,64 @@ out: return ret; } - -static int udevGetDeviceType(struct udev_device *device, - enum virNodeDevCapType *type) +static bool +udevDeviceHasProperty(struct udev_device *dev, + const char *key) { -const char *devtype = NULL; -char *tmp_string = NULL; -unsigned int tmp = 0; -int ret = 0; +const char *value = NULL; +bool ret = false; -devtype = udev_device_get_devtype(device); -VIR_DEBUG("Found device type '%s' for device '%s'", - NULLSTR(devtype), udev_device_get_sysname(device)); - -if (devtype != NULL && STREQ(devtype, "usb_device")) { -*type = VIR_NODE_DEV_CAP_USB_DEV; -goto out; -} - -if (devtype != NULL && STREQ(devtype, "usb_interface")) { -*type = VIR_NODE_DEV_CAP_USB_INTERFACE; -goto out; -} - -if (devtype != NULL && STREQ(devtype, "scsi_host")) { -*type = VIR_NODE_DEV_CAP_SCSI_HOST; -goto out; -} - -if (devtype != NULL && STREQ(devtype, "scsi_target")) { -*type = VIR_NODE_DEV_CAP_SCSI_TARGET; -goto out; -} - -if (devtype != NULL && STREQ(devtype, "scsi_device")) { -*type = VIR_NODE_DEV_CAP_SCSI; -goto out; -} +if ((value = udev_device_get_property_value(dev, key))) +ret = true; -if (devtype != NULL && STREQ(devtype, "disk")) { -*type = VIR_NODE_DEV_CAP_STORAGE; -goto out; -} - -if (devtype != NULL && STREQ(devtype, "wlan")) { -*type = VIR_NODE_DEV_CAP_NET; -goto out; -} - -if (udevGetUintProperty(device, "PCI_CLASS", &tmp, 16) == PROPERTY_FOUND) { -*type = VIR_NODE_DEV_CAP_PCI_DEV; -goto out; -} +return ret; +} -/* It does not appear that wired network interfaces set the - * DEVTYPE property. USB devices also have an INTERFACE property, - * but they do set DEVTYPE, so if devtype is NULL and the - * INTERFACE property exists, we have a network device. */ -if (devtype == NULL && -udevGetStringProperty(device, - "INTERFACE", - &tmp_string) == PROPERTY_FOUND) { -VIR_FREE(tmp_string); -*type = VIR_NODE_DEV_CAP_NET; -goto out; -} +static int +udevGetDeviceType(struct udev_device *device, + enum virNodeDevCapType *type) +{ +const char *devtype = NULL; +int ret = -1; -VIR_DEBUG("Could not determine device type for device " - "with sysfs path '%s'", - udev_device_get_sysname(device)); -ret = -1; +devtype = udev_device_get_devtype(device); +*type = 0; + +if (devtype) { +if (STREQ(devtype, "usb_device")) +*type = VIR_NODE_DEV_CAP_USB_DEV; +else if (STREQ(devtype, "usb_interface")) +*type = VIR_NODE_DEV_CAP_USB_INTERFACE; +else if (STREQ(devtype, "scsi_host")) +*type = VIR_NODE_DEV_CAP_SCSI_HOST; +else if (STREQ(devtype, "scsi_target")) +*type = VIR_NODE_DEV_CAP_SCSI_TARGET; +else if (STREQ(devtype, "scsi_device")) +*type = VIR_NODE_DEV_CAP_SCSI; +else if (STREQ(devtype, "disk")) +*type = VIR_NODE_DEV_CAP_STORAGE; +else if (STREQ(devtype, "wlan")) +*type = VIR_NODE_DEV_CAP_NET; +} else { +/* PCI devices don't set the DEVTYPE property. */ +if (udevDeviceHasProperty(device, "PCI_CLASS")) +*type = VIR_NODE_DEV_CAP_PCI_DEV; + +/* Wired network interfaces
[libvirt] [PATCH 0/6] Enumerate scsi generic device
The scsi generic device is listed as a child of the parent of the mapped disk device. E.g. +- pci__00_1f_2 | | | +- scsi_host0 | | | | | +- scsi_target0_0_0 | | | | | +- scsi_0_0_0_0 | | | | | +- block_sda_TOSHIBA_MK5061GSY_9243PG8CT | | +- scsi_generic_sg0 This is consistent with the sysfs/udev. The XML produced by udev backend: scsi_generic_sg0 /sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0 scsi_0_0_0_0 /dev/sg0 The XML produced by HAL backend: pci_8086_2922_scsi_host_scsi_device_lun0_scsi_generic /sys/devices/pci:00/:00:1f.2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0 pci_8086_2922_scsi_host_scsi_device_lun0 /dev/sg0 Osier Yang (6): nodedev: Expose sysfs path of device nodedev_udev: Refactor udevGetDeviceType nodedev_udev: Enumerate scsi generic device nodedev_hal: Enumerate scsi generic device nodedev: Support SCSI_GENERIC cap flag for listAllNodeDevices virsh: Support SCSI_GENERIC cap flag for nodedev-list include/libvirt/libvirt.h.in | 1 + src/conf/node_device_conf.c| 11 ++- src/conf/node_device_conf.h| 8 ++- src/node_device/node_device_hal.c | 9 +++ src/node_device/node_device_udev.c | 137 - tools/virsh-nodedev.c | 3 + tools/virsh.pod| 6 +- 7 files changed, 105 insertions(+), 70 deletions(-) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] nodedev_udev: Enumerate scsi generic device
Since scsi generic device doesn't have DEVTYPE property set, the only way to we have to get it's a scsi generic device is from the "SUBSYSTEM" property. The XML of the scsi generic device will be like: scsi_generic_sg0 /sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/scsi_generic/sg0 scsi_0_0_0_0 /dev/sg0 --- src/conf/node_device_conf.c| 6 +- src/conf/node_device_conf.h| 5 + src/node_device/node_device_udev.c | 24 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index cc6f297..a0b6338 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -49,7 +49,8 @@ VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST, "scsi", "storage", "fc_host", - "vports") + "vports", + "scsi_generic") VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", @@ -472,6 +473,9 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) virBufferAddLit(&buf, "\n"); break; +case VIR_NODE_DEV_CAP_SCSI_GENERIC: +virBufferEscapeString(&buf, "%s\n", + data->sg.path); case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_LAST: diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 1c5855c..e326e82 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -48,6 +48,8 @@ enum virNodeDevCapType { VIR_NODE_DEV_CAP_STORAGE, /* Storage device */ VIR_NODE_DEV_CAP_FC_HOST, /* FC Host Bus Adapter */ VIR_NODE_DEV_CAP_VPORTS, /* HBA which is capable of vports */ +VIR_NODE_DEV_CAP_SCSI_GENERIC, /* SCSI generic device */ + VIR_NODE_DEV_CAP_LAST }; @@ -165,6 +167,9 @@ struct _virNodeDevCapsDef { char *media_label; unsigned int flags;/* virNodeDevStorageCapFlags bits */ } storage; +struct { +char *path; +} sg; /* SCSI generic device */ } data; virNodeDevCapsDefPtr next; /* next capability */ }; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 3c91298..27a48cf 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1099,6 +1099,21 @@ out: return ret; } +static int +udevProcessScsiGeneric(struct udev_device *dev, + virNodeDeviceDefPtr def) +{ +if (udevGetStringProperty(dev, + "DEVNAME", + &def->caps->data.sg.path) != PROPERTY_FOUND) +return -1; + +if (udevGenerateDeviceName(dev, def, NULL) != 0) +return -1; + +return 0; +} + static bool udevDeviceHasProperty(struct udev_device *dev, const char *key) @@ -1117,6 +1132,7 @@ udevGetDeviceType(struct udev_device *device, enum virNodeDevCapType *type) { const char *devtype = NULL; +char *subsystem = NULL; int ret = -1; devtype = udev_device_get_devtype(device); @@ -1148,6 +1164,11 @@ udevGetDeviceType(struct udev_device *device, * property exists, we have a network device. */ if (udevDeviceHasProperty(device, "INTERFACE")) *type = VIR_NODE_DEV_CAP_NET; + +if (udevGetStringProperty(device, "SUBSYSTEM", &subsystem) == +PROPERTY_FOUND && +STREQ(subsystem, "scsi_generic")) +*type = VIR_NODE_DEV_CAP_SCSI_GENERIC; } if (!*type) @@ -1194,6 +1215,9 @@ static int udevGetDeviceDetails(struct udev_device *device, case VIR_NODE_DEV_CAP_STORAGE: ret = udevProcessStorage(device, def); break; +case VIR_NODE_DEV_CAP_SCSI_GENERIC: +ret = udevProcessScsiGeneric(device, def); +break; default: VIR_ERROR(_("Unknown device type %d"), def->caps->type); ret = -1; -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] virsh: Support SCSI_GENERIC cap flag for nodedev-list
Document for nodedev-list is also updated. --- tools/virsh-nodedev.c | 3 +++ tools/virsh.pod | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 3657c5f..da93b8e 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -453,6 +453,9 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) case VIR_NODE_DEV_CAP_VPORTS: flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS; break; +case VIR_NODE_DEV_CAP_SCSI_GENERIC: +flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC; +break; default: break; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 7c8ce18..f9e0287 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2086,9 +2086,9 @@ List all of the devices available on the node that are known by libvirt. I is used to filter the list by capability types, the types must be separated by comma, e.g. --cap pci,scsi, valid capability types include 'system', 'pci', 'usb_device', 'usb', 'net', 'scsi_host', 'scsi_target', -'scsi', 'storage', 'fc_host', 'vports'. If I<--tree> is used, the output -is formatted in a tree representing parents of each node. I and -I<--tree> are mutually exclusive. +'scsi', 'storage', 'fc_host', 'vports', 'scsi_generic'. If I<--tree> is +used, the output is formatted in a tree representing parents of each node. +I and I<--tree> are mutually exclusive. =item B I -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESEND 0/5] VirtualBox version 4.2 support for libvirt vbox driver
On Wed, May 22, 2013 at 10:00:12AM +0200, Manuel VIVES wrote: > Hello, > > I'm re-sending this patch for reviewing. > If necessary I'm willing to make > some changes to those patches. > > I'm currently working on a better management for snapshots with virtualbox, > and my work is based on Virtualbox 4.2 so that's why I'm re sending this > patch. > > Regards, > Manuel VIVES > > Ryan Woodsmall said originally: > > "This patch set adds VirtualBox 4.2 initial support for the libvirt vbox > driver. > I've tested enough to check capabilities, create a VM, destroy it, etc. Five > patches total: > > - Patch 1 is the C API header file from Oracle, cleaned up for libvirt. > - Patch 2 is the version specific source file for version dependency. > - Patch 3 is the src/Makefile.am change to pick up the two new files. > - Patch 4 is the vbox driver support for the new VirtualBox API/version. > - Patch 5 is the vbox_tmpl.c template support for the new version. > > A few things have changed in the VirtualBox API - some small (capitalizations > of things in function names like Ip to IP and Dhcp to DHCP) and some much > larger > (FindMedium is superceded by OpenMedium). The biggest change for the sake of > this > patch set is the signature of CreateMachine is quite a bit different. Using > the > Oracle source as a guide, to spin up a VM with a given UUID, it looks like a > text > flag has to be passed in a new argument to CreateMachine. This flag is built > in the > VirtualBox 4.2 specific ifdefs and is kind of ugly but works. Additionally, > there > is now (unused) VM groups support in CreateMachine and the previous > 'osTypeId' arg > is currently set to nsnull as in the Oracle code. > > The FindMedium to OpenMedium changes were more straightforward and are pretty > clear. > The rest of the vbox template changes are basically spelling/capitalization > changes > from the looks of things. > > This probably isn't perfect, but it works on git and patched against 0.10.2 > for a > few quick tests. Not currently on the list, so ping me directly if you need > any > other info on these, or if anything could use additional work. Thanks! " > > ryan woodsmall (5): > vbox C API header for VirtualBox v4.2 > vbox version-specific C file for VirtualBox v4.2 > Makefile.am additions for VirtualBox v4.2 > vbox driver support for VirtualBox v4.2 > vbox template support for VirtualBox v4.2 > > src/Makefile.am |3 +- > src/vbox/vbox_CAPI_v4_2.h | 8855 > + > src/vbox/vbox_V4_2.c | 13 + > src/vbox/vbox_driver.c|8 + > src/vbox/vbox_tmpl.c | 90 +- > 5 files changed, 8958 insertions(+), 11 deletions(-) > create mode 100644 src/vbox/vbox_CAPI_v4_2.h > create mode 100644 src/vbox/vbox_V4_2.c ACK to all pastches, they look like the sane minimum required to make vbox 4.2 work. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix warning about using an uninitialized next_unit value
Using an uninitialized value and a bool saying if the value is valid may confuse compilators. --- src/conf/domain_conf.c | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 46d49a2..6dc8cf3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3883,26 +3883,28 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, virDomainDefPtr def, virDomainHostdevDefPtr hostdev) { -int next_unit; +int next_unit = 0; unsigned nscsi_controllers = 0; -bool found = false; int i; +int ret; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) return -1; -for (i = 0; i < def->ncontrollers && !found; i++) { +for (i = 0; i < def->ncontrollers; i++) { if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) continue; nscsi_controllers++; -next_unit = virDomainControllerSCSINextUnit(def, - xmlopt->config.hasWideScsiBus ? - SCSI_WIDE_BUS_MAX_CONT_UNIT : - SCSI_NARROW_BUS_MAX_CONT_UNIT, -def->controllers[i]->idx); -if (next_unit >= 0) -found = true; +ret = virDomainControllerSCSINextUnit(def, + xmlopt->config.hasWideScsiBus ? + SCSI_WIDE_BUS_MAX_CONT_UNIT : + SCSI_NARROW_BUS_MAX_CONT_UNIT, + def->controllers[i]->idx); +if (ret >= 0) { +next_unit = ret; +break; +} } hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; @@ -3912,7 +3914,7 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, nscsi_controllers; hostdev->info->addr.drive.bus = 0; hostdev->info->addr.drive.target = 0; -hostdev->info->addr.drive.unit = found ? next_unit : 0; +hostdev->info->addr.drive.unit = next_unit; return 0; } -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh iface-bridge: Ignore delay if stp is turned off
Delay only makes sense with STP enabled. --- tools/virsh-interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 1c2e40b..2bdb215 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -894,7 +894,7 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd) goto cleanup; } -if ((delay || stp) && +if (stp && ((virAsprintf(&delay_str, "%d", delay) < 0) || !xmlSetProp(br_node, BAD_CAST "delay", BAD_CAST delay_str))) { vshError(ctl, _("Failed to set bridge delay %d in xml document"), delay); -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Release of libvirt-1.0.6
According to plans, the release is available, I tagged it in git and the tarball and rpms should be available shortly on the FTP: ftp://libvirt.org/libvirt/ This is a fairly large release considering the number of commits which went in, this is due in part to a lot of refactoring of the code base. This still seem an homogenous release with a large set of bug fixes, a few new user visible features, and a very large amount of incremental improvements. There have been significant updates at the localization and documentation level too ! Features: - Move VirtualBox driver into libvirtd (Daniel P. Berrange) - Support for static routes on a virtual bridge (Gene Czarcinski) - Various improvement for hostdev SCSI support (Osier Yang and Han Cheng) - Switch to VIR_STRDUP and VIR_STRNDUP (Michal Privoznik) - Various cleanups and improvement in Xen and LXC drivers (Daniel P. Berrange) Documentation: - Document that runtime changes may be lost after S4 suspend (Jiri Denemark) - domain: /dev/urandom isn't a valid rng patch (Cole Robinson) - formatdomain: fix links in the table of contents (Ján Tomko) - add another user (Eric Blake) - datatypes: fix virGetStoragePool's comment (Ján Tomko) - Expand documentation for LXC driver (Daniel P. Berrange) - Fix/update syntax in Sysinfo/SMBIOS description (John Ferlan) - Update formatdomain for lifecycle events (John Ferlan) - Fix the wrong links in secret documentation (Osier Yang) - Add the missed usage type 'iscsi' (Osier Yang) - Add docs about cgroups layout and usage (Daniel P. Berrange) - Point users to Virt-Viewer MSI installers for Windows builds (Daniel P. Berrange) - Fix namespace bugs in API docs, todo page & hv support page (Daniel P. Berrange) - Fix a few more docs XSL bugs related to the TOC (Daniel P. Berrange) - Fix docs generator regression in previous commit (Daniel P. Berrange) - Fix multiple formatting problems in HTML docs (Daniel P. Berrange) - fix 'since' for socket path generation (Ján Tomko) Portability: - vbox: define DYNLIB_NAME for kFreeBSD (Guido Günther) - build: skip qemu in tests when !WITH_QEMU (Eric Blake) - build: use correct rpc.h for virtlockd (Eric Blake) - build: work around cygwin header bug (Eric Blake) - build: cast [ug]id_t when printing (Eric Blake) - build: port qemu to cygwin (Eric Blake) - build: use correct rpc.h for lockd (Eric Blake) - build: work around broken sasl header (Eric Blake) - build: fix build without libvirtd (Eric Blake) - build: fix build with newer gnutls (Eric Blake) - build: fix build with older gcc (Eric Blake) - qemu: Fix build without gnutls (Jiri Denemark) - spec: Build vbox packages only for x86 architectures (Viktor Mihajlovski) - Add missing c-ctype.h to virfile.c (Daniel P. Berrange) - test: fix VPATH fchosttest failure (Viktor Mihajlovski) - libxl: fix build with Xen4.3 (Jim Fehlig) - build: Fix check-driverimpls in VPATH (Jiri Denemark) - util: Fix build without devmapper (Jiri Denemark) - FreeBSD: disable buggy -fstack-protector-all (Roman Bogorodskiy) - build: avoid gcrypt deprecation warnings (Roman Bogorodskiy) - build: avoid shadowed variable in fdstreamtest (Eric Blake) - fix virNetDevSetMAC and virNetDevExists on BSD (Roman Bogorodskiy) - Disable some URI tests on older libxml2 (Daniel P. Berrange) - Fix build of python bindings on Python 2.4 (Daniel P. Berrange) - build: fix build with old polkit0 (Jim Fehlig) - Fixup rpcgen code on kFreeBSD too (Guido Günther) - build: avoid non-portable cast of pthread_t (Eric Blake) - build: Fix build when WITH_HAL is defined (Jim Fehlig) - build: fix mingw build of vbox (Eric Blake) - build: fix mingw build of virprocess.c (Eric Blake) - build: fix FreeBSD build (Eric Blake) Bug Fixes: - conf: Generate address for scsi host device automatically (Osier Yang) - qemu: prevent termination of guests w/hostdev on driver reconnect (Laine Stump) - qemu: escape literal IPv6 address in NBD migration (Ján Tomko) - Check for existence of interface prior to setting terminate flag (John Ferlan) - Resolve memory leak found by valgrind (John Ferlan) - qemu: snapshot: Don't kill access to disk if snapshot creation fails (Peter Krempa) - virsh: migrate: Don't disallow --p2p and --migrateuri (Cole Robinson) - qemu: Don't report error on successful media eject (Cole Robinson) - qemu: save domain state to XML after reboot (Sergey Fionov) - esx: Fix dynamic VI object type detection (Matthias Bolte) - storage_conf: Don't leak "uuid" in virStoragePoolDefParseAuthCephx (Osier Yang) - storage_conf: Fix the wrong error message (Osier Yang) - Fix blkdeviotune for shutoff domain (Martin Kletzander) - virsh: Fix regression of vol-resize (Osier Yang) - xen: Resolve Coverity FORWARD_NULL issue (John Ferlan) - qemu: fix NBD migration to hosts with IPv6 enabled (Ján Tomko) - conf: fix use after free in virChrdevOpen (Ján Tomko) - virNetMessageSaveError: Fix copy and paste error (Michal Privoznik) - virNWFilterHashTablePut: Free the correct variable (Michal Privoznik) - umlConne
Re: [libvirt] [PATCH 1/2] Make virNetDevSetupControl() public.
On Sat, May 25, 2013 at 07:07:49AM -0600, Eric Blake wrote: > On 05/15/2013 11:47 PM, Roman Bogorodskiy wrote: > > This method is useful not only in virnetdev.c. > > --- > > src/libvirt_private.syms | 1 + > > src/util/virnetdev.c | 15 +-- > > src/util/virnetdev.h | 12 > > src/util/virnetdevmacvlan.c | 2 +- > > src/util/virnetdevvportprofile.c | 2 +- > > 5 files changed, 28 insertions(+), 4 deletions(-) > > > > > +#else > > +int > > +virNetDevSetupControl(const char *ifname ATTRIBUTE_UNUSED, > > + void *ifr ATTRIBUTE_UNUSED) > > +{ > > +virReportSystemError(ENOSYS, > > + _("%s is not supported on this platform"), > > + __func__); > > virReportSystemError is already a macro that reports the function name, > so your message would include the name twice. But offhand, I can't > think of any better wording to use. How about "Network device configuration is not supported on this platform" Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix warning about using an uninitialized next_unit value
On 03/06/13 18:22, Jiri Denemark wrote: Using an uninitialized value and a bool saying if the value is valid may confuse compilators. --- src/conf/domain_conf.c | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 46d49a2..6dc8cf3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3883,26 +3883,28 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, virDomainDefPtr def, virDomainHostdevDefPtr hostdev) { -int next_unit; +int next_unit = 0; unsigned nscsi_controllers = 0; -bool found = false; int i; +int ret; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) return -1; -for (i = 0; i < def->ncontrollers && !found; i++) { +for (i = 0; i < def->ncontrollers; i++) { if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) continue; nscsi_controllers++; -next_unit = virDomainControllerSCSINextUnit(def, - xmlopt->config.hasWideScsiBus ? - SCSI_WIDE_BUS_MAX_CONT_UNIT : - SCSI_NARROW_BUS_MAX_CONT_UNIT, -def->controllers[i]->idx); -if (next_unit >= 0) -found = true; +ret = virDomainControllerSCSINextUnit(def, + xmlopt->config.hasWideScsiBus ? + SCSI_WIDE_BUS_MAX_CONT_UNIT : + SCSI_NARROW_BUS_MAX_CONT_UNIT, + def->controllers[i]->idx); +if (ret >= 0) { +next_unit = ret; +break; +} } hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; @@ -3912,7 +3914,7 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, nscsi_controllers; This statement still uses the bool variable "found": hostdev->info->addr.drive.controller = found ? def->controllers[i - 1]->idx : nscsi_controllers; And the controller index "i - 1" above should be changed to "i" instead. Since the second expression of the for loop was changed. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] RPC: Support up to 4096 cpus on the host and 256 in the guest
On Tue, May 28, 2013 at 02:35:50PM +0200, Peter Krempa wrote: > The RPC limits for cpu maps didn't allow to use libvirt on ultra big > boxes. This patch increases size of the limits to support a maximum of > 4096 cpus on the host with the built-in maximum of 256 cpus per guest. > The full cpu map of such a system takes 128 kilobytes and the map for > vcpu pinning is 512 bytes long. > --- > src/remote/remote_protocol.x | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x > index 1ebbce7..a94e0db 100644 > --- a/src/remote/remote_protocol.x > +++ b/src/remote/remote_protocol.x > @@ -82,13 +82,13 @@ const REMOTE_DOMAIN_ID_LIST_MAX = 16384; > const REMOTE_DOMAIN_NAME_LIST_MAX = 16384; > > /* Upper limit on cpumap (bytes) passed to virDomainPinVcpu. */ > -const REMOTE_CPUMAP_MAX = 256; > +const REMOTE_CPUMAP_MAX = 512; > > /* Upper limit on number of info fields returned by virDomainGetVcpus. */ > -const REMOTE_VCPUINFO_MAX = 2048; > +const REMOTE_VCPUINFO_MAX = 4096; > > /* Upper limit on cpumaps (bytes) passed to virDomainGetVcpus. */ > -const REMOTE_CPUMAPS_MAX = 16384; > +const REMOTE_CPUMAPS_MAX = 131072; > > /* Upper limit on migrate cookie. */ > const REMOTE_MIGRATE_COOKIE_MAX = 16384; 4096 sounds large, but I can't help wondering if we should pre-empt the inevitable and go even bigger. In terms of RPC message size, we can afford to allow 16384 host CPUS and 4096 guest CPUS ? Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Ongoing work on lock contention in qemu driver?
On Fri, May 24, 2013 at 11:37:04AM -0400, Peter Feiner wrote: > On Wed, May 22, 2013 at 7:31 PM, Peter Feiner wrote: > > Since some security driver operations are costly, I think it's > > worthwhile to reduce the scope of the security manager lock or > > increase the granularity by introducing more locks. After a cursory > > look, the security manager lock seems to have a much broader scope > > than necessary. The system / library calls underlying the security > > drivers are all thread safe (e.g., defining apparmor security profiles > > or chowning disk files), so a global lock isn't strictly necessary. > > Moreover, since most virSecurity calls are made whilst a virDomainObj > > lock is held and the security calls are generally domain specific, > > *most* of the security calls are probably thread safe in the absence > > of the global security manager lock. Obviously some work will have to > > be done to see where the security lock actually matters and some > > finer-grained locks will have to be introduced to handle these > > situations. > > To verify that this is worthwhile, I disabled the apparmor driver > entirely. My 20 VM creation test ran about 10s faster (down from 35s > to 25s). > > After giving this approach a little more thought, I think an > incremental series of patches is a good way to go. The responsibility > of locking could be pushed down into the security drivers. At first, > all of the drivers would lock where their managers' locked. Then each > driver could be updated to do more fine-grained locking. I'm going to > work on a patch to push the locking down into the drivers, then I'm > going to work on a patch for better locking in the apparmor driver. Yep, that sounds like a sane approach to me. Previously the security drivers had no locking at all, since they were relying on the global lock at the QEMU driver level. When I introduced the lock into the security manager module, I was pessimistic and used coarse locking. As you say, we can clearly relax this somewhat, if we have the locking in the individual security drivers. > > I also think it's worthwhile to eliminate locking from the the > > virDomainObjList lookups and traversals. Since virDomainObjLists are > > accessed in a bunch of places, I think it's a good defensive idea to > > decouple the performance of these accesses from virDomainObj locks, > > which are held during potentially long-running operations like domain > > creation. An easy way to divorce virDomainObjListSearchName from the > > virDomainObj lock would be to keep a copy of the domain names in the > > virDomainObjList and protect that list with the virDomainObjList lock. > > After removing the security driver contention, this was still a > substantial bottleneck: virConnectDefineXML could still take a few > seconds. I removed the contention by keeping a copy of the domain > definition's name in the domain object. Since the name is immutable > and the domain object is protected by the list lock, the list > traversal can read the name without taking any additional locks. This > patch reduced virConnectDefineXML to tens of milliseconds. Yep, I had a patch to add a security hash table to the domain object list, hashing based on name, but I lost the code when a disk died. I didn't find it made any difference, but agree we should just do it anyway, since it'll almost certainly be a problem in some scenarios. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] use virBitmapFree instead of VIR_FREE for cpumask
Found by 'git grep FREE.*cpumask' after looking at 31f1f6b. --- src/conf/domain_conf.c | 2 +- src/libxl/libxl_driver.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 46d49a2..b335b58 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13542,7 +13542,7 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) for (n = 0; n < def->cputune.nvcpupin; n++) { if (vcpupin_list[n]->vcpuid == vcpu) { -VIR_FREE(vcpupin_list[n]->cpumask); +virBitmapFree(vcpupin_list[n]->cpumask); VIR_FREE(vcpupin_list[n]); memmove(&vcpupin_list[n], &vcpupin_list[n+1], diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 7245f97..bed583b 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -651,7 +651,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, /* Remove any cputune settings */ if (vm->def->cputune.nvcpupin) { for (i = 0; i < vm->def->cputune.nvcpupin; ++i) { -VIR_FREE(vm->def->cputune.vcpupin[i]->cpumask); +virBitmapFree(vm->def->cputune.vcpupin[i]->cpumask); VIR_FREE(vm->def->cputune.vcpupin[i]); } VIR_FREE(vm->def->cputune.vcpupin); -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] RPC: Support up to 4096 cpus on the host and 256 in the guest
On 06/03/13 12:32, Daniel P. Berrange wrote: On Tue, May 28, 2013 at 02:35:50PM +0200, Peter Krempa wrote: The RPC limits for cpu maps didn't allow to use libvirt on ultra big boxes. This patch increases size of the limits to support a maximum of 4096 cpus on the host with the built-in maximum of 256 cpus per guest. The full cpu map of such a system takes 128 kilobytes and the map for vcpu pinning is 512 bytes long. --- src/remote/remote_protocol.x | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 1ebbce7..a94e0db 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -82,13 +82,13 @@ const REMOTE_DOMAIN_ID_LIST_MAX = 16384; const REMOTE_DOMAIN_NAME_LIST_MAX = 16384; /* Upper limit on cpumap (bytes) passed to virDomainPinVcpu. */ -const REMOTE_CPUMAP_MAX = 256; +const REMOTE_CPUMAP_MAX = 512; /* Upper limit on number of info fields returned by virDomainGetVcpus. */ -const REMOTE_VCPUINFO_MAX = 2048; +const REMOTE_VCPUINFO_MAX = 4096; /* Upper limit on cpumaps (bytes) passed to virDomainGetVcpus. */ -const REMOTE_CPUMAPS_MAX = 16384; +const REMOTE_CPUMAPS_MAX = 131072; /* Upper limit on migrate cookie. */ const REMOTE_MIGRATE_COOKIE_MAX = 16384; 4096 sounds large, but I can't help wondering if we should pre-empt the inevitable and go even bigger. In terms of RPC message size, we can afford to allow 16384 host CPUS and 4096 guest CPUS ? We can do that. The cpu map for a 16384 host with 4096 guest cpus is 8 MiB large. The RPC message limit is almost 16MiB so we should be fine even with those (now seemingly insane) numbers. I'll send a v2 in a moment. Daniel Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix warning about using an uninitialized next_unit value
On Mon, Jun 03, 2013 at 18:32:15 +0800, Osier Yang wrote: > On 03/06/13 18:22, Jiri Denemark wrote: > > Using an uninitialized value and a bool saying if the value is valid may > > confuse compilators. ... > This statement still uses the bool variable "found": > > hostdev->info->addr.drive.controller = found ? > def->controllers[i - 1]->idx : > nscsi_controllers; > Heh, I should have looked at the compilation result before sending this patch :-) V2 is on the way. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] use virBitmapFree instead of VIR_FREE for cpumask
On 06/03/2013 07:11 PM, Ján Tomko wrote: Found by 'git grep FREE.*cpumask' after looking at 31f1f6b. --- src/conf/domain_conf.c | 2 +- src/libxl/libxl_driver.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 46d49a2..b335b58 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13542,7 +13542,7 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) for (n = 0; n < def->cputune.nvcpupin; n++) { if (vcpupin_list[n]->vcpuid == vcpu) { -VIR_FREE(vcpupin_list[n]->cpumask); +virBitmapFree(vcpupin_list[n]->cpumask); VIR_FREE(vcpupin_list[n]); memmove(&vcpupin_list[n], &vcpupin_list[n+1], diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 7245f97..bed583b 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -651,7 +651,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, /* Remove any cputune settings */ if (vm->def->cputune.nvcpupin) { for (i = 0; i < vm->def->cputune.nvcpupin; ++i) { -VIR_FREE(vm->def->cputune.vcpupin[i]->cpumask); +virBitmapFree(vm->def->cputune.vcpupin[i]->cpumask); VIR_FREE(vm->def->cputune.vcpupin[i]); } VIR_FREE(vm->def->cputune.vcpupin); ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Fix warning about using an uninitialized next_unit value
Using an uninitialized value and a bool saying if the value is valid may confuse compilators. --- src/conf/domain_conf.c | 34 +- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 46d49a2..21750ac 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3883,36 +3883,36 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, virDomainDefPtr def, virDomainHostdevDefPtr hostdev) { -int next_unit; -unsigned nscsi_controllers = 0; -bool found = false; +int next_unit = 0; +unsigned controller = 0; int i; +int ret; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) return -1; -for (i = 0; i < def->ncontrollers && !found; i++) { +for (i = 0; i < def->ncontrollers; i++) { if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) continue; -nscsi_controllers++; -next_unit = virDomainControllerSCSINextUnit(def, - xmlopt->config.hasWideScsiBus ? - SCSI_WIDE_BUS_MAX_CONT_UNIT : - SCSI_NARROW_BUS_MAX_CONT_UNIT, -def->controllers[i]->idx); -if (next_unit >= 0) -found = true; +controller++; +ret = virDomainControllerSCSINextUnit(def, + xmlopt->config.hasWideScsiBus ? + SCSI_WIDE_BUS_MAX_CONT_UNIT : + SCSI_NARROW_BUS_MAX_CONT_UNIT, + def->controllers[i]->idx); +if (ret >= 0) { +next_unit = ret; +controller = def->controllers[i]->idx; +break; +} } hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; - -hostdev->info->addr.drive.controller = found ? - def->controllers[i - 1]->idx : - nscsi_controllers; +hostdev->info->addr.drive.controller = controller; hostdev->info->addr.drive.bus = 0; hostdev->info->addr.drive.target = 0; -hostdev->info->addr.drive.unit = found ? next_unit : 0; +hostdev->info->addr.drive.unit = next_unit; return 0; } -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] RPC: Support up to 16384 cpus on the host and 4096 in the guest
The RPC limits for cpu maps didn't allow to use libvirt on ultra big boxes. This patch increases size of the limits to support a maximum of 16384 cpus on the host with a maximum of 4096 cpus per guest. The full cpu map of such a system takes 8 megabytes and the map for vcpu pinning is 2 kilobytes long. --- src/remote/remote_protocol.x | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 1ebbce7..9723377 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -82,13 +82,13 @@ const REMOTE_DOMAIN_ID_LIST_MAX = 16384; const REMOTE_DOMAIN_NAME_LIST_MAX = 16384; /* Upper limit on cpumap (bytes) passed to virDomainPinVcpu. */ -const REMOTE_CPUMAP_MAX = 256; +const REMOTE_CPUMAP_MAX = 2048; /* Upper limit on number of info fields returned by virDomainGetVcpus. */ -const REMOTE_VCPUINFO_MAX = 2048; +const REMOTE_VCPUINFO_MAX = 16384; /* Upper limit on cpumaps (bytes) passed to virDomainGetVcpus. */ -const REMOTE_CPUMAPS_MAX = 16384; +const REMOTE_CPUMAPS_MAX = 8388608; /* Upper limit on migrate cookie. */ const REMOTE_MIGRATE_COOKIE_MAX = 16384; -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] usb: don't spoil decadic addresses
On 06/02/2013 02:41 AM, Eric Blake wrote: > On 06/01/2013 01:52 AM, Martin Kletzander wrote: >> For USB devices, dev->name gets formated as %.3o:%.3o even though the >> numbers are decadic. > > s/decadic/decimal/ here and in the subject line. > >> >> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=967712 >> >> Signed-off-by: Martin Kletzander >> --- >> > > ACK. > Thanks, fixed both occurrences and pushed. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/11] vCPU hotplug mega-series
On 05/27/13 19:59, Peter Krempa wrote: This patch series consists of multiple parts: -patch 1,2: Two trivial cleanups -patch 3: Improve and refactor vCPU data parsing -patch 4: Add agent monitor helpers for cpu-hotplug stuff -patch 5,6,7: Implement universal guest vCPU mapping function -patch 8: Implement new qemu monitor command for cpu hotplug -patch 9,10,11: Implement agent based cpu state manipulation API/command Tip: The new "virsh vcpumap guest --active" command may be used that the agent actually offlined the vCPU in the guest. Peter Krempa (11): virsh-domain-monitor: Remove ATTRIBUTE_UNUSED from a argument qemu: Use bool instead of int in qemuMonitorSetCPU APIs qemu: Extract more information about vCPUs and threads qemu_agent: Introduce helpers for agent based CPU hot(un)plug lib: Add API to map virtual cpus of a guest virsh-domain-monitor: Implement command to map guest vCPUs qemu: Implement virDomainGetVCPUMap for the qemu driver qemu: Implement new QMP command for cpu hotplug lib: Add API to modify vCPU state from the guest using the guest agent virsh-domain: Implement command for virDomainSetGuestVcpu qemu: Implement virDomainSetGuetVcpu in qemu driver Ping. Now that 1.0.6 is out of the door this series could get some reviews ;). Thanks. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] use virBitmapFree instead of VIR_FREE for cpumask
On 06/03/2013 02:00 PM, Guannan Ren wrote: > On 06/03/2013 07:11 PM, Ján Tomko wrote: >> Found by 'git grep FREE.*cpumask' after looking at 31f1f6b. >> --- >> src/conf/domain_conf.c | 2 +- >> src/libxl/libxl_driver.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 46d49a2..b335b58 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -13542,7 +13542,7 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) >> for (n = 0; n < def->cputune.nvcpupin; n++) { >> if (vcpupin_list[n]->vcpuid == vcpu) { >> -VIR_FREE(vcpupin_list[n]->cpumask); >> +virBitmapFree(vcpupin_list[n]->cpumask); >> VIR_FREE(vcpupin_list[n]); >> memmove(&vcpupin_list[n], >> &vcpupin_list[n+1], >> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c >> index 7245f97..bed583b 100644 >> --- a/src/libxl/libxl_driver.c >> +++ b/src/libxl/libxl_driver.c >> @@ -651,7 +651,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, >> /* Remove any cputune settings */ >> if (vm->def->cputune.nvcpupin) { >> for (i = 0; i < vm->def->cputune.nvcpupin; ++i) { >> -VIR_FREE(vm->def->cputune.vcpupin[i]->cpumask); >> +virBitmapFree(vm->def->cputune.vcpupin[i]->cpumask); >> VIR_FREE(vm->def->cputune.vcpupin[i]); >> } >> VIR_FREE(vm->def->cputune.vcpupin); > >ACK > Thanks, pushed now. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virsh-domain: Add --live, --config, --current logic to cmdAttachInterface
On 05/28/13 17:33, Peter Krempa wrote: On 05/28/13 14:01, Guannan Ren wrote: On 05/28/2013 06:52 PM, Peter Krempa wrote: Use the approach established in commit 69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too. --- ACK with the rest. I've pushed this series, now that the release is done. Thanks for the review. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Fix warning about using an uninitialized next_unit value
On 06/03/2013 01:56 PM, Jiri Denemark wrote: > Using an uninitialized value and a bool saying if the value is valid may > confuse compilators. > --- > src/conf/domain_conf.c | 34 +- > 1 file changed, 17 insertions(+), 17 deletions(-) > ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] spec: Explicitly require gcrypt-devel
Our configure.ac says: Not all versions of gnutls include -lgcrypt, and so we add it explicitly for the calls to gcry_control/check_version Thus we cannot rely on gnutls-devel to bring grcypt-devel as a dependency. --- libvirt.spec.in | 1 + 1 file changed, 1 insertion(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 37d656d..6aea8f4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -418,6 +418,7 @@ BuildRequires: readline-devel BuildRequires: ncurses-devel BuildRequires: gettext BuildRequires: libtasn1-devel +BuildRequires: gcrypt-devel BuildRequires: gnutls-devel BuildRequires: libattr-devel %if %{with_libvirtd} -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh iface-bridge: Ignore delay if stp is turned off
On 06/03/2013 04:23 AM, Jiri Denemark wrote: > Delay only makes sense with STP enabled. > --- > tools/virsh-interface.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) ACK. > > diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c > index 1c2e40b..2bdb215 100644 > --- a/tools/virsh-interface.c > +++ b/tools/virsh-interface.c > @@ -894,7 +894,7 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd) > goto cleanup; > } > > -if ((delay || stp) && > +if (stp && > ((virAsprintf(&delay_str, "%d", delay) < 0) || > !xmlSetProp(br_node, BAD_CAST "delay", BAD_CAST delay_str))) { > vshError(ctl, _("Failed to set bridge delay %d in xml document"), > delay); > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Explicitly require gcrypt-devel
On Mon, Jun 03, 2013 at 14:59:10 +0200, Jiri Denemark wrote: > Our configure.ac says: > > Not all versions of gnutls include -lgcrypt, and so we add > it explicitly for the calls to gcry_control/check_version > > Thus we cannot rely on gnutls-devel to bring grcypt-devel as a > dependency. > --- > libvirt.spec.in | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libvirt.spec.in b/libvirt.spec.in > index 37d656d..6aea8f4 100644 > --- a/libvirt.spec.in > +++ b/libvirt.spec.in > @@ -418,6 +418,7 @@ BuildRequires: readline-devel > BuildRequires: ncurses-devel > BuildRequires: gettext > BuildRequires: libtasn1-devel > +BuildRequires: gcrypt-devel Hmm, this should have been libcrypt-devel Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Explicitly require gcrypt-devel
On 06/03/13 15:06, Jiri Denemark wrote: On Mon, Jun 03, 2013 at 14:59:10 +0200, Jiri Denemark wrote: Our configure.ac says: Not all versions of gnutls include -lgcrypt, and so we add it explicitly for the calls to gcry_control/check_version Thus we cannot rely on gnutls-devel to bring grcypt-devel as a dependency. --- libvirt.spec.in | 1 + 1 file changed, 1 insertion(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 37d656d..6aea8f4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -418,6 +418,7 @@ BuildRequires: readline-devel BuildRequires: ncurses-devel BuildRequires: gettext BuildRequires: libtasn1-devel +BuildRequires: gcrypt-devel Hmm, this should have been libcrypt-devel ACK to a version fixed this way. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] use virBitmapFree instead of VIR_FREE for cpumask
On 06/03/2013 05:11 AM, Ján Tomko wrote: > Found by 'git grep FREE.*cpumask' after looking at 31f1f6b. > --- > src/conf/domain_conf.c | 2 +- > src/libxl/libxl_driver.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh iface-bridge: Ignore delay if stp is turned off
On Mon, Jun 03, 2013 at 06:49:06 -0600, Eric Blake wrote: > On 06/03/2013 04:23 AM, Jiri Denemark wrote: > > Delay only makes sense with STP enabled. > > --- > > tools/virsh-interface.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > ACK. Pushed, thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Fix warning about using an uninitialized next_unit value
On Mon, Jun 03, 2013 at 14:34:22 +0200, Jano Tomko wrote: > On 06/03/2013 01:56 PM, Jiri Denemark wrote: > > Using an uninitialized value and a bool saying if the value is valid may > > confuse compilators. > > --- > > src/conf/domain_conf.c | 34 +- > > 1 file changed, 17 insertions(+), 17 deletions(-) > > > > ACK Pushed, thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Explicitly require gcrypt-devel
On Mon, Jun 03, 2013 at 15:09:59 +0200, Peter Krempa wrote: > On 06/03/13 15:06, Jiri Denemark wrote: > > On Mon, Jun 03, 2013 at 14:59:10 +0200, Jiri Denemark wrote: > >> Our configure.ac says: > >> > >> Not all versions of gnutls include -lgcrypt, and so we add > >> it explicitly for the calls to gcry_control/check_version > >> > >> Thus we cannot rely on gnutls-devel to bring grcypt-devel as a > >> dependency. > >> --- > >> libvirt.spec.in | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/libvirt.spec.in b/libvirt.spec.in > >> index 37d656d..6aea8f4 100644 > >> --- a/libvirt.spec.in > >> +++ b/libvirt.spec.in > >> @@ -418,6 +418,7 @@ BuildRequires: readline-devel > >> BuildRequires: ncurses-devel > >> BuildRequires: gettext > >> BuildRequires: libtasn1-devel > >> +BuildRequires: gcrypt-devel > > > > Hmm, this should have been libcrypt-devel > > ACK to a version fixed this way. Pushed, thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] conf: Generate address for scsi host device automatically
On 05/31/2013 12:09 PM, Osier Yang wrote: + +static int +virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, + virDomainDefPtr def, + virDomainHostdevDefPtr hostdev) +{ +int next_unit; +unsigned nscsi_controllers = 0; +bool found = false; +int i; + +if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) +return -1; + +for (i = 0; i < def->ncontrollers && !found; i++) { +if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) +continue; + +nscsi_controllers++; +next_unit = virDomainControllerSCSINextUnit(def, + xmlopt->config.hasWideScsiBus ? + SCSI_WIDE_BUS_MAX_CONT_UNIT : + SCSI_NARROW_BUS_MAX_CONT_UNIT, +def->controllers[i]->idx); +if (next_unit >= 0) +found = true; +} + +hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; + +hostdev->info->addr.drive.controller = found ? + def->controllers[i - 1]->idx : + nscsi_controllers; +hostdev->info->addr.drive.bus = 0; +hostdev->info->addr.drive.target = 0; +hostdev->info->addr.drive.unit = found ? next_unit : 0; well, 1.0.6 is out of the door, but with this I hit the following problem on Ubuntu 12.04 (gcc 4.6.3): ../../src/conf/domain_conf.c: In function 'virDomainHostdevDefParseXML': ../../src/conf/domain_conf.c:3915:36: error: 'next_unit' may be used uninitialized in this function [-Werror=uninitialized] ../../src/conf/domain_conf.c:3886:9: note: 'next_unit' was declared here It seems that the older compiler is not smart enough to grasp the tie between 'found' and 'next_unit'... + +return 0; +} -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] conf: Generate address for scsi host device automatically
On 03/06/13 22:15, Viktor Mihajlovski wrote: On 05/31/2013 12:09 PM, Osier Yang wrote: + +static int +virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, + virDomainDefPtr def, + virDomainHostdevDefPtr hostdev) +{ +int next_unit; +unsigned nscsi_controllers = 0; +bool found = false; +int i; + +if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) +return -1; + +for (i = 0; i < def->ncontrollers && !found; i++) { +if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) +continue; + +nscsi_controllers++; +next_unit = virDomainControllerSCSINextUnit(def, + xmlopt->config.hasWideScsiBus ? + SCSI_WIDE_BUS_MAX_CONT_UNIT : + SCSI_NARROW_BUS_MAX_CONT_UNIT, + def->controllers[i]->idx); +if (next_unit >= 0) +found = true; +} + +hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; + +hostdev->info->addr.drive.controller = found ? + def->controllers[i - 1]->idx : + nscsi_controllers; +hostdev->info->addr.drive.bus = 0; +hostdev->info->addr.drive.target = 0; +hostdev->info->addr.drive.unit = found ? next_unit : 0; well, 1.0.6 is out of the door, but with this I hit the following problem on Ubuntu 12.04 (gcc 4.6.3): ../../src/conf/domain_conf.c: In function 'virDomainHostdevDefParseXML': ../../src/conf/domain_conf.c:3915:36: error: 'next_unit' may be used uninitialized in this function [-Werror=uninitialized] ../../src/conf/domain_conf.c:3886:9: note: 'next_unit' was declared here It seems that the older compiler is not smart enough to grasp the tie between 'found' and 'next_unit'... fixed by Jirka with 4db39e3fee6 + +return 0; +} -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] virsh-domain: Report errors and don't deref NULL in qemu-agent-command
Check the returned value for NULL and take the cleanup path appropriately if the API fails. --- tools/virsh-domain.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 46f07ed..9ea5ffc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7739,7 +7739,10 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd) vshError(ctl, "%s", _("timeout, async and block options are exclusive")); goto cleanup; } + result = virDomainQemuAgentCommand(dom, guest_agent_cmd, timeout, flags); +if (!result) +goto cleanup; if (vshCommandOptBool(cmd, "pretty")) { char *tmp; -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Fix error reporting of virsh qemu-agent-command and underlying API's.
The code was horribly broken in multiple ways. This series fixes dispatching of errors from the API, null dereference in virsh and shadowed error messages. Peter Krempa (3): virsh-domain: Report errors and don't deref NULL in qemu-agent-command qemu: Properly report guest agent errors on command passthrough libvirt-qemu: Dispatch errors from virDomainQemuAgentCommand() src/libvirt-qemu.c | 9 +++-- src/qemu/qemu_agent.c | 31 +++ src/qemu/qemu_driver.c | 10 +++--- tools/virsh-domain.c | 3 +++ 4 files changed, 32 insertions(+), 21 deletions(-) -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] libvirt-qemu: Dispatch errors from virDomainQemuAgentCommand()
The original implementation didn't follow the established pattern and did not dispatch errors in case of failure. --- src/libvirt-qemu.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c index 747488d..e884e32 100644 --- a/src/libvirt-qemu.c +++ b/src/libvirt-qemu.c @@ -211,6 +211,7 @@ virDomainQemuAgentCommand(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; +char *ret; VIR_DEBUG("domain=%p, cmd=%s, timeout=%d, flags=%x", domain, cmd, timeout, flags); @@ -228,13 +229,17 @@ virDomainQemuAgentCommand(virDomainPtr domain, conn = domain->conn; if (conn->driver->domainQemuAgentCommand) { -return conn->driver->domainQemuAgentCommand(domain, cmd, -timeout, flags); +ret = conn->driver->domainQemuAgentCommand(domain, cmd, + timeout, flags); +if (!ret) +goto error; +return ret; } virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); /* Copy to connection error object for back compatibility */ +error: virDispatchError(conn); return NULL; } -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: Properly report guest agent errors on command passthrough
The code for arbitrary guest agent passthrough was horribly broken since introduciton. Fix it to correctly report errors. --- src/qemu/qemu_agent.c | 31 +++ src/qemu/qemu_driver.c | 10 +++--- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index c7a9681..00fe13f 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1408,25 +1408,32 @@ qemuAgentArbitraryCommand(qemuAgentPtr mon, int timeout) { int ret = -1; -virJSONValuePtr cmd; +virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; *result = NULL; -if (timeout < VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN) -return ret; +if (timeout < VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN) { +virReportError(VIR_ERR_INVALID_ARG, + _("guest agent timeout '%d' is " + "less than the minimum '%d'"), + timeout, VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN); +goto cleanup; +} -cmd = virJSONValueFromString(cmd_str); -if (!cmd) -return ret; +if (!(cmd = virJSONValueFromString(cmd_str))) +goto cleanup; + +if ((ret = qemuAgentCommand(mon, cmd, &reply, timeout)) < 0) +goto cleanup; -ret = qemuAgentCommand(mon, cmd, &reply, timeout); +if ((ret = qemuAgentCheckError(cmd, reply)) < 0) +goto cleanup; -if (ret == 0) { -ret = qemuAgentCheckError(cmd, reply); -if (!(*result = virJSONValueToString(reply, false))) -ret = -1; -} +if (!(*result = virJSONValueToString(reply, false))) +ret = -1; + +cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5ca0fd4..9d3f632 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14881,16 +14881,12 @@ qemuDomainQemuAgentCommand(virDomainPtr domain, qemuDomainObjEnterAgent(vm); ret = qemuAgentArbitraryCommand(priv->agent, cmd, &result, timeout); qemuDomainObjExitAgent(vm); -if (ret < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to execute agent command")); -goto endjob; -} +if (ret < 0) +VIR_FREE(result); endjob: -if (qemuDomainObjEndJob(driver, vm) == 0) { +if (qemuDomainObjEndJob(driver, vm) == 0) vm = NULL; -} cleanup: if (vm) -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] conf: Generate address for scsi host device automatically
On 06/03/2013 04:29 PM, Osier Yang wrote: fixed by Jirka with 4db39e3fee6 the post somehow went past me, sorry... :$ -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] RPC: Support up to 16384 cpus on the host and 4096 in the guest
On Mon, Jun 03, 2013 at 02:02:00PM +0200, Peter Krempa wrote: > The RPC limits for cpu maps didn't allow to use libvirt on ultra big > boxes. This patch increases size of the limits to support a maximum of > 16384 cpus on the host with a maximum of 4096 cpus per guest. > The full cpu map of such a system takes 8 megabytes and the map for > vcpu pinning is 2 kilobytes long. > --- > src/remote/remote_protocol.x | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x > index 1ebbce7..9723377 100644 > --- a/src/remote/remote_protocol.x > +++ b/src/remote/remote_protocol.x > @@ -82,13 +82,13 @@ const REMOTE_DOMAIN_ID_LIST_MAX = 16384; > const REMOTE_DOMAIN_NAME_LIST_MAX = 16384; > > /* Upper limit on cpumap (bytes) passed to virDomainPinVcpu. */ > -const REMOTE_CPUMAP_MAX = 256; > +const REMOTE_CPUMAP_MAX = 2048; > > /* Upper limit on number of info fields returned by virDomainGetVcpus. */ > -const REMOTE_VCPUINFO_MAX = 2048; > +const REMOTE_VCPUINFO_MAX = 16384; > > /* Upper limit on cpumaps (bytes) passed to virDomainGetVcpus. */ > -const REMOTE_CPUMAPS_MAX = 16384; > +const REMOTE_CPUMAPS_MAX = 8388608; > > /* Upper limit on migrate cookie. */ > const REMOTE_MIGRATE_COOKIE_MAX = 16384; ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Make logical pools independent on target path
When using logical pools, we had to trust the target->pth provided. This parameter, however, can be completely ommited and we can get the correct path using 'lvdisplay' command. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=952973 Signed-off-by: Martin Kletzander --- Notes: I'm not sure we can drop the target.path from the XML (see tests), so another variant is to keep it there the same way it was defined by user/mgmt app. If that's desired, mentioning it with an ack is enough for me to know I should drop the conf/storage_conf.c hunk as well as the hunks patching tests. configure.ac | 4 ++ src/conf/storage_conf.c| 19 +++--- src/storage/storage_backend_logical.c | 79 +++--- tests/storagepoolxml2xmlin/pool-logical-create.xml | 1 - tests/storagepoolxml2xmlin/pool-logical.xml| 1 - .../storagepoolxml2xmlout/pool-logical-create.xml | 1 - tests/storagepoolxml2xmlout/pool-logical.xml | 1 - 7 files changed, 67 insertions(+), 39 deletions(-) diff --git a/configure.ac b/configure.ac index 5d1bc6b..753139d 100644 --- a/configure.ac +++ b/configure.ac @@ -1603,6 +1603,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then AC_PATH_PROG([PVS], [pvs], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([VGS], [vgs], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([LVS], [lvs], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([LVDISPLAY], [lvdisplay], [], [$PATH:/sbin:/usr/sbin]) if test "$with_storage_lvm" = "yes" ; then if test -z "$PVCREATE" ; then AC_MSG_ERROR([We need pvcreate for LVM storage driver]) ; fi @@ -1617,6 +1618,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then if test -z "$PVS" ; then AC_MSG_ERROR([We need pvs for LVM storage driver]) ; fi if test -z "$VGS" ; then AC_MSG_ERROR([We need vgs for LVM storage driver]) ; fi if test -z "$LVS" ; then AC_MSG_ERROR([We need lvs for LVM storage driver]) ; fi +if test -z "$LVDISPLAY" ; then AC_MSG_ERROR([We need lvdisplay for LVM storage driver]) ; fi else if test -z "$PVCREATE" ; then with_storage_lvm=no ; fi if test -z "$VGCREATE" ; then with_storage_lvm=no ; fi @@ -1630,6 +1632,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then if test -z "$PVS" ; then with_storage_lvm=no ; fi if test -z "$VGS" ; then with_storage_lvm=no ; fi if test -z "$LVS" ; then with_storage_lvm=no ; fi +if test -z "$LVDISPLAY" ; then with_storage_lvm=no ; fi if test "$with_storage_lvm" = "check" ; then with_storage_lvm=yes ; fi fi @@ -1648,6 +1651,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then AC_DEFINE_UNQUOTED([PVS],["$PVS"],[Location of pvs program]) AC_DEFINE_UNQUOTED([VGS],["$VGS"],[Location of vgs program]) AC_DEFINE_UNQUOTED([LVS],["$LVS"],[Location of lvs program]) +AC_DEFINE_UNQUOTED([LVDISPLAY],["$LVDISPLAY"],[Location of lvdisplay program]) fi fi AM_CONDITIONAL([WITH_STORAGE_LVM], [test "$with_storage_lvm" = "yes"]) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index cc3d3d9..948e190 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1,7 +1,7 @@ /* * storage_conf.c: config handling for storage driver * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -947,15 +947,16 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) /* When we are working with a virtual disk we can skip the target * path and permissions */ if (!(options->flags & VIR_STORAGE_POOL_SOURCE_NETWORK)) { -if (!(target_path = virXPathString("string(./target/path)", ctxt))) { -virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing storage pool target path")); -goto error; +if (ret->type != VIR_STORAGE_POOL_LOGICAL) { +if (!(target_path = virXPathString("string(./target/path)", ctxt))) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing storage pool target path")); +goto error; +} +ret->target.path = virFileSanitizePath(target_path); +if (!ret->target.path) +goto error; } -ret->target.path = virFileSanitizePath(target_path); -if (!ret->target.path) -goto error; - if (virStorageDefParsePerms(ctxt, &ret->target.perms, "./target/permissions", DEFAULT_POOL_PERM_MODE) < 0) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 944aa0e..66a4e42 100644 --- a/src/storage/storage_backend_lo
Re: [libvirt] [PATCH 1/3] virsh-domain: Report errors and don't deref NULL in qemu-agent-command
On 06/03/2013 04:34 PM, Peter Krempa wrote: > Check the returned value for NULL and take the cleanup path > appropriately if the API fails. > --- > tools/virsh-domain.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index 46f07ed..9ea5ffc 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -7739,7 +7739,10 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd) > vshError(ctl, "%s", _("timeout, async and block options are > exclusive")); > goto cleanup; > } > + > result = virDomainQemuAgentCommand(dom, guest_agent_cmd, timeout, flags); > +if (!result) > +goto cleanup; > > if (vshCommandOptBool(cmd, "pretty")) { > char *tmp; > ACK, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] RPC: Support up to 16384 cpus on the host and 4096 in the guest
On 06/03/13 17:03, Daniel P. Berrange wrote: On Mon, Jun 03, 2013 at 02:02:00PM +0200, Peter Krempa wrote: The RPC limits for cpu maps didn't allow to use libvirt on ultra big boxes. This patch increases size of the limits to support a maximum of 16384 cpus on the host with a maximum of 4096 cpus per guest. The full cpu map of such a system takes 8 megabytes and the map for vcpu pinning is 2 kilobytes long. --- src/remote/remote_protocol.x | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) ACK Pushed. Thanks. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: Properly report guest agent errors on command passthrough
On 06/03/2013 04:35 PM, Peter Krempa wrote: > The code for arbitrary guest agent passthrough was horribly broken since > introduciton. Fix it to correctly report errors. s/introduciton/introduction/ ACK, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] libvirt-qemu: Dispatch errors from virDomainQemuAgentCommand()
On 06/03/2013 04:35 PM, Peter Krempa wrote: > The original implementation didn't follow the established pattern and > did not dispatch errors in case of failure. > --- > src/libvirt-qemu.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c > index 747488d..e884e32 100644 > --- a/src/libvirt-qemu.c > +++ b/src/libvirt-qemu.c > @@ -211,6 +211,7 @@ virDomainQemuAgentCommand(virDomainPtr domain, >unsigned int flags) > { > virConnectPtr conn; > +char *ret; > > VIR_DEBUG("domain=%p, cmd=%s, timeout=%d, flags=%x", >domain, cmd, timeout, flags); > @@ -228,13 +229,17 @@ virDomainQemuAgentCommand(virDomainPtr domain, > conn = domain->conn; > > if (conn->driver->domainQemuAgentCommand) { > -return conn->driver->domainQemuAgentCommand(domain, cmd, > -timeout, flags); > +ret = conn->driver->domainQemuAgentCommand(domain, cmd, > + timeout, flags); > +if (!ret) > +goto error; > +return ret; > } > > virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); > > /* Copy to connection error object for back compatibility */ > +error: > virDispatchError(conn); > return NULL; > } > One more fix would fit here, so ACK with this squashed in: diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c index e884e32..9dd76dd 100644 --- a/src/libvirt-qemu.c +++ b/src/libvirt-qemu.c @@ -221,13 +221,14 @@ virDomainQemuAgentCommand(virDomainPtr domain, virDispatchError(NULL); return NULL; } + +conn = domain->conn; + if (domain->conn->flags & VIR_CONNECT_RO) { virLibDomainError(NULL, VIR_ERR_OPERATION_DENIED, __FUNCTION__); -return NULL; +goto error; } -conn = domain->conn; - if (conn->driver->domainQemuAgentCommand) { ret = conn->driver->domainQemuAgentCommand(domain, cmd, timeout, flags); -- Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] libvirt-qemu: Dispatch errors from virDomainQemuAgentCommand()
On 06/03/13 17:22, Martin Kletzander wrote: On 06/03/2013 04:35 PM, Peter Krempa wrote: The original implementation didn't follow the established pattern and did not dispatch errors in case of failure. --- src/libvirt-qemu.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) One more fix would fit here, so ACK with this squashed in: I squashed the fix you've proposed and pushed the whole series. Thanks for the review. Martin Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Refresh pool after creating volume
On 01/06/13 16:05, Martin Kletzander wrote: On 05/29/2013 06:16 PM, Osier Yang wrote: On 30/05/13 00:04, Martin Kletzander wrote: On 05/29/2013 05:55 PM, Osier Yang wrote: On 29/05/13 20:51, Martin Kletzander wrote: On 05/29/2013 11:53 AM, Osier Yang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=965442 One has to refresh the pool to get the correct pool info, this patch refreshes the pool after creating a volume in code instead. Pool refreshing failure is fine to ignore with a warning. --- src/storage/storage_driver.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index a2b0c21..2a55095 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1443,6 +1443,9 @@ storageVolCreateXML(virStoragePoolPtr obj, } +if (backend->refreshPool && backend->refreshPool(obj->conn, pool) < 0) +VIR_WARN("Failed to refresh pool after creating volume"); + VIR_INFO("Creating volume '%s' in storage pool '%s'", volobj->name, pool->def->name); ret = volobj; @@ -1606,6 +1609,9 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, goto cleanup; } +if (backend->refreshPool && backend->refreshPool(obj->conn, pool) < 0) +VIR_WARN("Failed to refresh pool after creating volume"); + VIR_INFO("Creating volume '%s' in storage pool '%s'", volobj->name, pool->def->name); ret = volobj; I don't want to say NACK just like that, but I think the bug indicates there's a problem in the storage driver. It should automatically reflect the changes made to that pool. That's the thing I mentioned long time ago. Using things like inotify to update the pool's info (though inotify doesn't work for all pool types, such as iscsi). I didn't mean responding to user-created volumes. The user has to do pool-refresh after that, that's their business as they do something behind libvirt's back. Right in principle. But when the driver does something (with the pool), it should update its info accordingly without the need to refresh it. Right too. This applies to deleteVol/resizeVol too. But as I said, though calliing refreshPool in these APIs is not the best method, but it's the generic thing what current storage driver does. Assuming that we won't reply on 3rd project/tools, we have to introduce something like event to let the pool refresh itself. It's just not much better than calling refreshPool in the APIs, as it should call refreshPool anyway finally. What's the structure that gets updated only with refresh and not after the vol is created? Can you explain more about this? I guess you mean the vol is created outside of libvirt? such as a iscsi target create a new LUN? Does it do with all the drivers? Not sure what do you mean for "drivers". But I guess you mean "storage backends" here? if so, yes. All the current storage backends support "refreshPool"/ Yes, backends. I meant what drivers has this issue. Long story short; I think this bug fixes the symptom, not the problem. As said above, you have a right opinion. The pool should be notified asynchronously, but it's thing I don't have enough time to do currently. Not quite what I meant, corrected myself above. Martin I think we are still talking aout two different things, let me explain on code. This is how I would solve the issue (just for creating the volume, this would have to be similarly adapted to resize, wipe and delete, but that shouldn't be more than few lines): Okay, I know your meaning now. commit 416247880399f88ec382a16c7cebc5460d6b67ad Author: Martin Kletzander Date: Fri May 31 14:25:48 2013 +0200 test diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 1a85afc..a119fc4 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1,7 +1,7 @@ /* * storage_backend_fs.c: storage backend for FS and directory handling * - * Copyright (C) 2007-2012 Red Hat, Inc. + * Copyright (C) 2007-2013 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -797,6 +797,27 @@ error: } +static int +virStorageBackendStatVFS(virStoragePoolDefPtr def) +{ +struct statvfs sb; + +if (statvfs(def->target.path, &sb) < 0) { +virReportSystemError(errno, + _("cannot statvfs path '%s'"), + def->target.path); +return -1; +} +def->capacity = ((unsigned long long)sb.f_frsize * + (unsigned long long)sb.f_blocks); +def->available = ((unsigned long long)sb.f_bfree * +(unsigned long long)sb.f_frsize); +def->allocation = def->capacity - def->available; + +return 0; +} + Though this is only for fs backend, I could see your thought (only refresh the poo
[libvirt] nwfilter: grab driver lock earlier during init (bz96649)
This patch is in relation to Bug 966449: https://bugzilla.redhat.com/show_bug.cgi?id=966449 Below is a possible patch addressing the coredump. Thread 1 must be calling nwfilterDriverRemoveDBusMatches(). It does so with nwfilterDriverLock held. In the patch below I am now moving the nwfilterDriverLock(driverState) further up so that the initialization, which seems to either take a long time or is entirely stuck, occurs with the lock held and the shutdown cannot occur at the same time. To avoid having to make the nwfilterDriverLock lockable multiple times / recursive I changed the virNWFilterDriverIsWatchingFirewallD to take as an argument whether it has to grab that lock. There's only a single caller at the moment that calls this function during initialization. We could remove this lock entirely and maybe append to the name of the function NoLock (?). --- src/nwfilter/nwfilter_driver.c| 18 +- src/nwfilter/nwfilter_driver.h|2 +- src/nwfilter/nwfilter_ebiptables_driver.c |7 ++- 3 files changed, 20 insertions(+), 7 deletions(-) Index: libvirt/src/nwfilter/nwfilter_driver.c === --- libvirt.orig/src/nwfilter/nwfilter_driver.c +++ libvirt/src/nwfilter/nwfilter_driver.c @@ -191,6 +191,8 @@ nwfilterStateInitialize(bool privileged, if (!privileged) return 0; +nwfilterDriverLock(driverState); + if (virNWFilterIPAddrMapInit() < 0) goto err_free_driverstate; if (virNWFilterLearnInit() < 0) @@ -203,8 +205,6 @@ nwfilterStateInitialize(bool privileged, if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB) < 0) goto err_techdrivers_shutdown; -nwfilterDriverLock(driverState); - /* * startup the DBus late so we don't get a reload signal while * initializing @@ -309,21 +309,29 @@ nwfilterStateReload(void) { /** * virNWFilterIsWatchingFirewallD: * + * @needDriverLock: Provide 'true' if this function needs to grab + * the nwfilter driver lock, 'false' otherwise, + * which may be the case during initialization + * * Checks if the nwfilter has the DBus watches for FirewallD installed. * * Returns true if it is watching firewalld, false otherwise */ bool -virNWFilterDriverIsWatchingFirewallD(void) +virNWFilterDriverIsWatchingFirewallD(bool needDriverLock) { bool ret; if (!driverState) return false; -nwfilterDriverLock(driverState); +if (needDriverLock) +nwfilterDriverLock(driverState); + ret = driverState->watchingFirewallD; -nwfilterDriverUnlock(driverState); + +if (needDriverLock) +nwfilterDriverUnlock(driverState); return ret; } Index: libvirt/src/nwfilter/nwfilter_driver.h === --- libvirt.orig/src/nwfilter/nwfilter_driver.h +++ libvirt/src/nwfilter/nwfilter_driver.h @@ -33,6 +33,6 @@ int nwfilterRegister(void); -bool virNWFilterDriverIsWatchingFirewallD(void); +bool virNWFilterDriverIsWatchingFirewallD(bool needDriverLock); #endif /* __VIR_NWFILTER_DRIVER_H__ */ Index: libvirt/src/nwfilter/nwfilter_ebiptables_driver.c === --- libvirt.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt/src/nwfilter/nwfilter_ebiptables_driver.c @@ -4191,7 +4191,12 @@ ebiptablesDriverInitWithFirewallD(void) int status; int ret = -1; -if (!virNWFilterDriverIsWatchingFirewallD()) +/* + * check whether we are watching firewalld + * Since we call this function during initialization we won't need + * to have it get the lock, so we pass 'false'. + */ +if (!virNWFilterDriverIsWatchingFirewallD(false)) return -1; firewall_cmd_path = virFindFileInPath("firewall-cmd"); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make logical pools independent on target path
On 03/06/13 23:05, Martin Kletzander wrote: When using logical pools, we had to trust the target->pth provided. s/pth/path/, This parameter, however, can be completely ommited and we can get the correct path using 'lvdisplay' command. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=952973 Signed-off-by: Martin Kletzander --- Notes: I'm not sure we can drop the target.path from the XML (see tests), so another variant is to keep it there the same way it was defined by user/mgmt app. How about one file a bug "The created volume is not under specified pool's target path"? Since with this patch, the specified pool's target path is silently ignored. If you have a good answer/reason/fix for above question, I see no problem of ignore the pool->target.path, but the path you got from lvdisplay (with vol name is truncated) should be reflected to the pool's XML. If that's desired, mentioning it with an ack is enough for me to know I should drop the conf/storage_conf.c hunk as well as the hunks patching tests. configure.ac | 4 ++ src/conf/storage_conf.c| 19 +++--- src/storage/storage_backend_logical.c | 79 +++--- tests/storagepoolxml2xmlin/pool-logical-create.xml | 1 - tests/storagepoolxml2xmlin/pool-logical.xml| 1 - .../storagepoolxml2xmlout/pool-logical-create.xml | 1 - tests/storagepoolxml2xmlout/pool-logical.xml | 1 - 7 files changed, 67 insertions(+), 39 deletions(-) diff --git a/configure.ac b/configure.ac index 5d1bc6b..753139d 100644 --- a/configure.ac +++ b/configure.ac @@ -1603,6 +1603,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then AC_PATH_PROG([PVS], [pvs], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([VGS], [vgs], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([LVS], [lvs], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([LVDISPLAY], [lvdisplay], [], [$PATH:/sbin:/usr/sbin]) if test "$with_storage_lvm" = "yes" ; then if test -z "$PVCREATE" ; then AC_MSG_ERROR([We need pvcreate for LVM storage driver]) ; fi @@ -1617,6 +1618,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then if test -z "$PVS" ; then AC_MSG_ERROR([We need pvs for LVM storage driver]) ; fi if test -z "$VGS" ; then AC_MSG_ERROR([We need vgs for LVM storage driver]) ; fi if test -z "$LVS" ; then AC_MSG_ERROR([We need lvs for LVM storage driver]) ; fi +if test -z "$LVDISPLAY" ; then AC_MSG_ERROR([We need lvdisplay for LVM storage driver]) ; fi else if test -z "$PVCREATE" ; then with_storage_lvm=no ; fi if test -z "$VGCREATE" ; then with_storage_lvm=no ; fi @@ -1630,6 +1632,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then if test -z "$PVS" ; then with_storage_lvm=no ; fi if test -z "$VGS" ; then with_storage_lvm=no ; fi if test -z "$LVS" ; then with_storage_lvm=no ; fi +if test -z "$LVDISPLAY" ; then with_storage_lvm=no ; fi if test "$with_storage_lvm" = "check" ; then with_storage_lvm=yes ; fi fi @@ -1648,6 +1651,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then AC_DEFINE_UNQUOTED([PVS],["$PVS"],[Location of pvs program]) AC_DEFINE_UNQUOTED([VGS],["$VGS"],[Location of vgs program]) AC_DEFINE_UNQUOTED([LVS],["$LVS"],[Location of lvs program]) +AC_DEFINE_UNQUOTED([LVDISPLAY],["$LVDISPLAY"],[Location of lvdisplay program]) fi fi AM_CONDITIONAL([WITH_STORAGE_LVM], [test "$with_storage_lvm" = "yes"]) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index cc3d3d9..948e190 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1,7 +1,7 @@ /* * storage_conf.c: config handling for storage driver * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -947,15 +947,16 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) /* When we are working with a virtual disk we can skip the target * path and permissions */ if (!(options->flags & VIR_STORAGE_POOL_SOURCE_NETWORK)) { -if (!(target_path = virXPathString("string(./target/path)", ctxt))) { -virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing storage pool target path")); -goto error; +if (ret->type != VIR_STORAGE_POOL_LOGICAL) { +if (!(target_path = virXPathString("string(./target/path)", ctxt))) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing storage pool target path")); +goto error; +} +ret->target.path = virFileSanitizePath(target_path); +if (!ret->target.path) +
Re: [libvirt] nwfilter: grab driver lock earlier during init (bz96649)
On 06/03/2013 11:57 AM, Stefan Berger wrote: > This patch is in relation to Bug 966449: > > https://bugzilla.redhat.com/show_bug.cgi?id=966449 > > Below is a possible patch addressing the coredump. > > Thread 1 must be calling nwfilterDriverRemoveDBusMatches(). It does so > with nwfilterDriverLock held. In the patch below I am now moving the > nwfilterDriverLock(driverState) further up so that the initialization, > which seems to either take a long time or is entirely stuck, occurs with > the lock held and the shutdown cannot occur at the same time. As Dan pointed out in the BZ comments, this is just one example of an class of problems with libvirt's virStateInitialize and virStateCleanup, and virStateReload - these three functions need to be serialized, otherwise this patch will only be narrowing the window of opportunity for a problem, but not completely eliminating it. Still, it *is* proper for the nwfilter lock to be acquired earlier as you're doing in this patch. I don't know that it's necessary to have either the "needDriverLock arg virNWFilterDriverIsWatchingFirewallD or to add "NoLock" to the name. If this is the only caller, I would be just as happy removing the locking from it completely and leaving the name as is (it's highly likely that it will never be called from anywhere else, or that if it is, the new place it's called from will also already have the lock. So, ACK to the movement of the lock acquisition, and ACK to either adding the needDriverLock arg or just removing the locking from virNWFilterDriverIsWatchingFirewallD completely - the choice is yours. > > To avoid having to make the nwfilterDriverLock lockable multiple times / > recursive I changed the virNWFilterDriverIsWatchingFirewallD to take as > an argument whether it has to grab that lock. There's only a single > caller at the moment that calls this function during initialization. We > could remove this lock entirely and maybe append to the name of the > function NoLock (?). > > --- > src/nwfilter/nwfilter_driver.c| 18 +- > src/nwfilter/nwfilter_driver.h|2 +- > src/nwfilter/nwfilter_ebiptables_driver.c |7 ++- > 3 files changed, 20 insertions(+), 7 deletions(-) > > Index: libvirt/src/nwfilter/nwfilter_driver.c > === > --- libvirt.orig/src/nwfilter/nwfilter_driver.c > +++ libvirt/src/nwfilter/nwfilter_driver.c > @@ -191,6 +191,8 @@ nwfilterStateInitialize(bool privileged, > if (!privileged) > return 0; > > +nwfilterDriverLock(driverState); > + > if (virNWFilterIPAddrMapInit() < 0) > goto err_free_driverstate; > if (virNWFilterLearnInit() < 0) > @@ -203,8 +205,6 @@ nwfilterStateInitialize(bool privileged, > if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB) < 0) > goto err_techdrivers_shutdown; > > -nwfilterDriverLock(driverState); > - > /* > * startup the DBus late so we don't get a reload signal while > * initializing > @@ -309,21 +309,29 @@ nwfilterStateReload(void) { > /** > * virNWFilterIsWatchingFirewallD: > * > + * @needDriverLock: Provide 'true' if this function needs to grab > + * the nwfilter driver lock, 'false' otherwise, > + * which may be the case during initialization > + * > * Checks if the nwfilter has the DBus watches for FirewallD installed. > * > * Returns true if it is watching firewalld, false otherwise > */ > bool > -virNWFilterDriverIsWatchingFirewallD(void) > +virNWFilterDriverIsWatchingFirewallD(bool needDriverLock) > { > bool ret; > > if (!driverState) > return false; > > -nwfilterDriverLock(driverState); > +if (needDriverLock) > +nwfilterDriverLock(driverState); > + > ret = driverState->watchingFirewallD; > -nwfilterDriverUnlock(driverState); > + > +if (needDriverLock) > +nwfilterDriverUnlock(driverState); > > return ret; > } > Index: libvirt/src/nwfilter/nwfilter_driver.h > === > --- libvirt.orig/src/nwfilter/nwfilter_driver.h > +++ libvirt/src/nwfilter/nwfilter_driver.h > @@ -33,6 +33,6 @@ > > int nwfilterRegister(void); > > -bool virNWFilterDriverIsWatchingFirewallD(void); > +bool virNWFilterDriverIsWatchingFirewallD(bool needDriverLock); > > #endif /* __VIR_NWFILTER_DRIVER_H__ */ > Index: libvirt/src/nwfilter/nwfilter_ebiptables_driver.c > === > --- libvirt.orig/src/nwfilter/nwfilter_ebiptables_driver.c > +++ libvirt/src/nwfilter/nwfilter_ebiptables_driver.c > @@ -4191,7 +4191,12 @@ ebiptablesDriverInitWithFirewallD(void) > int status; > int ret = -1; > > -if (!virNWFilterDriverIsWatchingFirewallD()) > +/* > + * check whether we are watching firewalld > + * Since we call this function during initialization we won't n
Re: [libvirt] need custom /dev entries in LXC
On 06/03/2013 06:03 AM, Daniel P. Berrange wrote: On Wed, May 22, 2013 at 06:22:12PM -0400, Michael R. Hines wrote: Hi, We run nvidia devices inside libvirt-managed LXC containers. It used to be that simply doing: $ echo 'c 195:* rwm' > /sys/fs/cgroup/devices/libvirt/lxc Then, after booting the container, we would do: $ mknod -m 666 /dev/nvidia0 c 195 0 would be good enough to run our CUDA applications. But, according to: $ cat src/lxc/lxc_container.c The CAP_MKNOD capability is being dropped and only a specific set of devices is being created before booting the container. Is there any reason why this is not per-device configurable? With recent libvirt you can pass through arbitrary block and character devices explicitly, using the following XML: http://libvirt.org/formatdomain.html#elementsHostDevCaps As such there is never any need to change cgroups or use mknod as you describe. Daniel Thanks for the response, Daniel. That's good news. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] nwfilter: grab driver lock earlier during init (bz96649)
On 06/03/2013 12:29 PM, Laine Stump wrote: On 06/03/2013 11:57 AM, Stefan Berger wrote: This patch is in relation to Bug 966449: https://bugzilla.redhat.com/show_bug.cgi?id=966449 Below is a possible patch addressing the coredump. Thread 1 must be calling nwfilterDriverRemoveDBusMatches(). It does so with nwfilterDriverLock held. In the patch below I am now moving the nwfilterDriverLock(driverState) further up so that the initialization, which seems to either take a long time or is entirely stuck, occurs with the lock held and the shutdown cannot occur at the same time. As Dan pointed out in the BZ comments, this is just one example of an class of problems with libvirt's virStateInitialize and virStateCleanup, and virStateReload - these three functions need to be serialized, otherwise this patch will only be narrowing the window of opportunity for a problem, but not completely eliminating it. Still, it *is* proper for the nwfilter lock to be acquired earlier as you're doing in this patch. I don't know that it's necessary to have either the "needDriverLock arg virNWFilterDriverIsWatchingFirewallD or to add "NoLock" to the name. If this is the only caller, I would be just as happy removing the locking from it completely and leaving the name as is (it's highly likely that it will never be called from anywhere else, or that if it is, the new place it's called from will also already have the lock. So, ACK to the movement of the lock acquisition, and ACK to either adding the needDriverLock arg or just removing the locking from virNWFilterDriverIsWatchingFirewallD completely - the choice is yours. I'll remove the lock entirely. I'll post v2 then. Stefan To avoid having to make the nwfilterDriverLock lockable multiple times / recursive I changed the virNWFilterDriverIsWatchingFirewallD to take as an argument whether it has to grab that lock. There's only a single caller at the moment that calls this function during initialization. We could remove this lock entirely and maybe append to the name of the function NoLock (?). --- src/nwfilter/nwfilter_driver.c| 18 +- src/nwfilter/nwfilter_driver.h|2 +- src/nwfilter/nwfilter_ebiptables_driver.c |7 ++- 3 files changed, 20 insertions(+), 7 deletions(-) Index: libvirt/src/nwfilter/nwfilter_driver.c === --- libvirt.orig/src/nwfilter/nwfilter_driver.c +++ libvirt/src/nwfilter/nwfilter_driver.c @@ -191,6 +191,8 @@ nwfilterStateInitialize(bool privileged, if (!privileged) return 0; +nwfilterDriverLock(driverState); + if (virNWFilterIPAddrMapInit() < 0) goto err_free_driverstate; if (virNWFilterLearnInit() < 0) @@ -203,8 +205,6 @@ nwfilterStateInitialize(bool privileged, if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB) < 0) goto err_techdrivers_shutdown; -nwfilterDriverLock(driverState); - /* * startup the DBus late so we don't get a reload signal while * initializing @@ -309,21 +309,29 @@ nwfilterStateReload(void) { /** * virNWFilterIsWatchingFirewallD: * + * @needDriverLock: Provide 'true' if this function needs to grab + * the nwfilter driver lock, 'false' otherwise, + * which may be the case during initialization + * * Checks if the nwfilter has the DBus watches for FirewallD installed. * * Returns true if it is watching firewalld, false otherwise */ bool -virNWFilterDriverIsWatchingFirewallD(void) +virNWFilterDriverIsWatchingFirewallD(bool needDriverLock) { bool ret; if (!driverState) return false; -nwfilterDriverLock(driverState); +if (needDriverLock) +nwfilterDriverLock(driverState); + ret = driverState->watchingFirewallD; -nwfilterDriverUnlock(driverState); + +if (needDriverLock) +nwfilterDriverUnlock(driverState); return ret; } Index: libvirt/src/nwfilter/nwfilter_driver.h === --- libvirt.orig/src/nwfilter/nwfilter_driver.h +++ libvirt/src/nwfilter/nwfilter_driver.h @@ -33,6 +33,6 @@ int nwfilterRegister(void); -bool virNWFilterDriverIsWatchingFirewallD(void); +bool virNWFilterDriverIsWatchingFirewallD(bool needDriverLock); #endif /* __VIR_NWFILTER_DRIVER_H__ */ Index: libvirt/src/nwfilter/nwfilter_ebiptables_driver.c === --- libvirt.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt/src/nwfilter/nwfilter_ebiptables_driver.c @@ -4191,7 +4191,12 @@ ebiptablesDriverInitWithFirewallD(void) int status; int ret = -1; -if (!virNWFilterDriverIsWatchingFirewallD()) +/* + * check whether we are watching firewalld + * Since we call this function during initialization we won't need + * to have it get the lock, so we
[libvirt] [PATCH v2] nwfilter: grab driver lock earlier during init (bz96649)
This patch is in _relation_ to Bug 966449: https://bugzilla.redhat.com/show_bug.cgi?id=966449 Below is a possible patch addressing the coredump. Thread 1 must be calling nwfilterDriverRemoveDBusMatches(). It does so with nwfilterDriverLock held. In the patch below I am now moving the nwfilterDriverLock(driverState) further up so that the initialization, which seems to either take a long time or is entirely stuck, occurs with the lock held and the shutdown cannot occur at the same time. Remove the lock in virNWFilterDriverIsWatchingFirewallD to avoid trying to lock the same lock again. --- src/nwfilter/nwfilter_driver.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) Index: libvirt/src/nwfilter/nwfilter_driver.c === --- libvirt.orig/src/nwfilter/nwfilter_driver.c +++ libvirt/src/nwfilter/nwfilter_driver.c @@ -191,6 +191,8 @@ nwfilterStateInitialize(bool privileged, if (!privileged) return 0; +nwfilterDriverLock(driverState); + if (virNWFilterIPAddrMapInit() < 0) goto err_free_driverstate; if (virNWFilterLearnInit() < 0) @@ -203,8 +205,6 @@ nwfilterStateInitialize(bool privileged, if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB) < 0) goto err_techdrivers_shutdown; -nwfilterDriverLock(driverState); - /* * startup the DBus late so we don't get a reload signal while * initializing @@ -316,16 +316,10 @@ nwfilterStateReload(void) { bool virNWFilterDriverIsWatchingFirewallD(void) { -bool ret; - if (!driverState) return false; -nwfilterDriverLock(driverState); -ret = driverState->watchingFirewallD; -nwfilterDriverUnlock(driverState); - -return ret; +return driverState->watchingFirewallD; } /** -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: Allow attach-disk to specify disk wwn
Commit 6e73850b01ee support to set wwn for disks, but it was not exposed to attach-disk. --- tools/virsh-domain.c | 14 +- tools/virsh.pod | 8 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 9ea5ffc..767e288 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -307,6 +307,10 @@ static const vshCmdOptDef opts_attach_disk[] = { .type = VSH_OT_STRING, .help = N_("serial of disk device") }, +{.name = "wwn", + .type = VSH_OT_STRING, + .help = N_("wwn of disk device") +}, {.name = "shareable", .type = VSH_OT_BOOL, .help = N_("shareable between domains") @@ -499,7 +503,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom = NULL; const char *source = NULL, *target = NULL, *driver = NULL, *subdriver = NULL, *type = NULL, *mode = NULL, -*cache = NULL, *serial = NULL, *straddr = NULL; +*cache = NULL, *serial = NULL, *straddr = NULL, +*wwn = NULL; struct DiskAddress diskAddr; bool isFile = false, functionReturn = false; int ret; @@ -538,6 +543,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0 || vshCommandOptStringReq(ctl, cmd, "cache", &cache) < 0 || vshCommandOptStringReq(ctl, cmd, "serial", &serial) < 0 || +vshCommandOptStringReq(ctl, cmd, "wwn", &wwn) < 0 || vshCommandOptStringReq(ctl, cmd, "address", &straddr) < 0 || vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0) goto cleanup; @@ -564,6 +570,9 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) } } +if (wwn && !virValidateWWN(wwn)) +goto cleanup; + /* Make XML of disk */ virBufferAsprintf(&buf, "%s\n", serial); +if (wwn) +virBufferAsprintf(&buf, " %s\n", wwn); + if (vshCommandOptBool(cmd, "shareable")) virBufferAddLit(&buf, " \n"); diff --git a/tools/virsh.pod b/tools/virsh.pod index 047c241..69c290f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1849,8 +1849,8 @@ expected. [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] [I<--driver driver>] [I<--subdriver subdriver>] [I<--cache cache>] [I<--type type>] [I<--mode mode>] [I<--config>] [I<--sourcetype soucetype>] -[I<--serial serial>] [I<--shareable>] [I<--rawio>] [I<--address address>] -[I<--multifunction>] [I<--print-xml>] +[I<--serial serial>] [I<--wwn wwn>] [I<--shareable>] [I<--rawio>] +[I<--address address>] [I<--multifunction>] [I<--print-xml>] Attach a new disk device to the domain. I is path for the files and devices. I controls the bus or @@ -1870,8 +1870,8 @@ I can specify the two specific mode I or I. I can indicate the type of source (block|file) I can be one of "default", "none", "writethrough", "writeback", "directsync" or "unsafe". -I is the serial of disk device. I indicates the disk device -is shareable between domains. +I is the serial of disk device. I is the wwn of disk device. +I indicates the disk device is shareable between domains. I indicates the disk needs rawio capability. I is the address of disk device in the form of pci:domain.bus.slot.function, scsi:controller.bus.unit or ide:controller.bus.unit. -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list