[libvirt] [PATCH v5 0/9] Add native TLS encrypted chardev TCP support
by John Ferlan
v4: http://www.redhat.com/archives/libvir-list/2016-June/msg01709.html
Since I have it on a branch and have been updating, I figured I'd post
the most recent stuff. Patches 1-4 were "partially" ACK'd in v2 of this
series, but there's been changes to the conf handling upstream. Patch 5
adds a new secret type 'tls'. Previous incarnations of these changes
borrowed a common secret type, but this one is specific. It's more or
less what got removed for LUKS with the names changed to protect the
innocent (reference Dragnet). Patches 6-9 is what was mostly missing
in the earlier series.
Differences to v4... mostly updates/merges with the numerous changes
to master since that time.
I lost track of whether the desire was to have /etc/pki/libvirt-%s or
/etc/pki/qemu-%s directories... I think we've been using the libvirt-%s
for vnc/spice so far, so I just followed that for chardev although there
was a comment at one time to use qemu-chardev during review of patch 2
of the v2 series:
http://www.redhat.com/archives/libvir-list/2016-June/msg01072.html
John Ferlan (9):
conf: Add new default TLS X.509 certificate default directory
conf: Introduce chartcp_tls_x509_cert_dir
qemu: Add support for TLS X.509 path to TCP chardev backend
qemu: Add the ability to hotplug the TLS X.509 environment
conf: Add new secret type "tls"
conf: Add new secret element for tcp chardev
qemu: Introduce qemuDomainChardevPrivatePtr
qemu: Add a secret object to/for a chardev tcp with secret
qemu: Add the ability to hotplug a secret object for TCP chardev TLS
docs/aclpolkit.html.in | 4 +
docs/formatdomain.html.in | 29 +++++
docs/formatsecret.html.in | 59 ++++++++-
docs/schemas/domaincommon.rng | 21 +++
docs/schemas/secret.rng | 10 ++
include/libvirt/libvirt-secret.h | 1 +
src/access/viraccessdriverpolkit.c | 13 ++
src/conf/domain_conf.c | 64 ++++++++--
src/conf/domain_conf.h | 8 +-
src/conf/secret_conf.c | 23 +++-
src/conf/secret_conf.h | 1 +
src/conf/virsecretobj.c | 5 +
src/libxl/libxl_domain.c | 2 +-
src/lxc/lxc_native.c | 2 +-
src/qemu/libvirtd_qemu.aug | 11 +-
src/qemu/qemu.conf | 83 +++++++++---
src/qemu/qemu_alias.c | 16 +++
src/qemu/qemu_alias.h | 3 +
src/qemu/qemu_command.c | 141 ++++++++++++++++++++-
src/qemu/qemu_command.h | 9 ++
src/qemu/qemu_conf.c | 57 ++++++++-
src/qemu/qemu_conf.h | 7 +
src/qemu/qemu_domain.c | 124 +++++++++++++++++-
src/qemu/qemu_domain.h | 22 ++++
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_hotplug.c | 104 ++++++++++++++-
src/qemu/qemu_hotplug.h | 3 +-
src/qemu/qemu_monitor_json.c | 9 ++
src/qemu/qemu_parse_command.c | 4 +-
src/qemu/qemu_process.c | 2 +-
src/qemu/test_libvirtd_qemu.aug.in | 5 +
src/vz/vz_sdk.c | 2 +-
src/xenconfig/xen_sxpr.c | 2 +-
tests/qemuhotplugtest.c | 2 +-
.../qemuxml2argv-serial-tcp-tlsx509-chardev.args | 33 +++++
.../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 41 ++++++
...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 ++++++
...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 51 ++++++++
tests/qemuxml2argvtest.c | 21 +++
.../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 50 ++++++++
...ml2xmlout-serial-tcp-tlsx509-secret-chardev.xml | 1 +
tests/qemuxml2xmltest.c | 2 +
tests/secretxml2xmlin/usage-tls.xml | 7 +
tests/secretxml2xmltest.c | 1 +
44 files changed, 1038 insertions(+), 57 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-secret-chardev.xml
create mode 100644 tests/secretxml2xmlin/usage-tls.xml
--
2.7.4
8 years, 1 month
Re: [libvirt] [PATCH v7 0/4] Add Mediated device support
by Alex Williamson
Hi folks,
At KVM Forum we had a BoF session primarily around the mediated device
sysfs interface. I'd like to share what I think we agreed on and the
"problem areas" that still need some work so we can get the thoughts
and ideas from those who weren't able to attend.
DanPB expressed some concern about the mdev_supported_types sysfs
interface, which exposes a flat csv file with fields like "type",
"number of instance", "vendor string", and then a bunch of type
specific fields like "framebuffer size", "resolution", "frame rate
limit", etc. This is not entirely machine parsing friendly and sort of
abuses the sysfs concept of one value per file. Example output taken
from Neo's libvirt RFC:
cat /sys/bus/pci/devices/0000:86:00.0/mdev_supported_types
# vgpu_type_id, vgpu_type, max_instance, num_heads, frl_config, framebuffer,
max_resolution
11 ,"GRID M60-0B", 16, 2, 45, 512M, 2560x1600
12 ,"GRID M60-0Q", 16, 2, 60, 512M, 2560x1600
13 ,"GRID M60-1B", 8, 2, 45, 1024M, 2560x1600
14 ,"GRID M60-1Q", 8, 2, 60, 1024M, 2560x1600
15 ,"GRID M60-2B", 4, 2, 45, 2048M, 2560x1600
16 ,"GRID M60-2Q", 4, 4, 60, 2048M, 2560x1600
17 ,"GRID M60-4Q", 2, 4, 60, 4096M, 3840x2160
18 ,"GRID M60-8Q", 1, 4, 60, 8192M, 3840x2160
The create/destroy then looks like this:
echo "$mdev_UUID:vendor_specific_argument_list" >
/sys/bus/pci/devices/.../mdev_create
echo "$mdev_UUID:vendor_specific_argument_list" >
/sys/bus/pci/devices/.../mdev_destroy
"vendor_specific_argument_list" is nebulous.
So the idea to fix this is to explode this into a directory structure,
something like:
├── mdev_destroy
└── mdev_supported_types
├── 11
│ ├── create
│ ├── description
│ └── max_instances
├── 12
│ ├── create
│ ├── description
│ └── max_instances
└── 13
├── create
├── description
└── max_instances
Note that I'm only exposing the minimal attributes here for simplicity,
the other attributes would be included in separate files and we would
require vendors to create standard attributes for common device classes.
For vGPUs like NVIDIA where we don't support multiple types
concurrently, this directory structure would update as mdev devices are
created, removing no longer available types. I carried forward
max_instances here, but perhaps we really want to copy SR-IOV and
report a max and current allocation. Creation and deletion is
simplified as we can simply "echo $UUID > create" per type. I don't
understand why destroy had a parameter list, so here I imagine we can
simply do the same... in fact, I'd actually rather see a "remove" sysfs
entry under each mdev device, so we remove it at the device rather than
in some central location (any objections?).
We discussed how this might look with Intel devices which do allow
mixed vGPU types concurrently. We believe, but need confirmation, that
the vendor driver could still make a finite set of supported types,
perhaps with additional module options to the vendor driver to enable
more "exotic" types. So for instance if IGD vGPUs are based on
power-of-2 portions of the framebuffer size, then the vendor driver
could list types with 32MB, 64MB, 128MB, etc in useful and popular
sizes. As vGPUs are allocated, the larger sizes may become unavailable.
We still don't have any way for the admin to learn in advance how the
available supported types will change once mdev devices start to be
created. I'm not sure how we can create a specification for this, so
probing by creating devices may be the most flexible model.
The other issue is the start/stop requirement, which was revealed to
setup peer-to-peer resources between vGPUs which is a limited hardware
resource. We'd really like to have these happen automatically on the
first open of a vfio mdev device file and final release. So we
brainstormed how the open/release callbacks could know the other mdev
devices for a given user. This is where the instance number came into
play previously. This is an area that needs work.
There was a thought that perhaps on open() the vendor driver could look
at the user pid and use that to associate with other devices, but the
problem here is that we open and begin access to each device, so
devices do this discovery serially rather than in parallel as desired.
(we might not fault in mmio space yet though, so I wonder if open()
could set the association of mdev to pid, then the first mmio fault
would trigger the resource allocation? Then all the "magic" would live
in the vendor driver. open() could fail if the pid already has running
mdev devices and the vendor driver chooses not to support hotplug)
One comment was that for a GPU that only supports homogeneous vGPUs,
libvirt may choose to create all the vGPUs in advance and handle them
as we do SR-IOV VFs. The UUID+instance model would preclude such a use
case.
We also considered whether iommu groups could be (ab)used for this use
case, peer-to-peer would in fact be an iommu grouping constraint
afterall. This would have the same UUID+instance constraint as above
though and would require some sort of sysfs interface for the user to
be able to create multiple mdevs within a group.
Everyone was given homework to think about this on their flights home,
so I expect plenty of ideas by now ;)
Overall I think mediated devices were well received by the community,
so let's keep up the development and discussion to bring it to
fruition. Thanks,
Alex
8 years, 1 month
[libvirt] [PATCH] usb: allow host devices to be specified by port
by Thomas Hebb
Previously, only VID/PID and bus/device matching were supported. Neither
of these provide a stable and persistent way of assigning a guest a
specific host device out of several with the same VID and PID, as device
numbers change on every enumeration.
Add a third method of matching, bus/port, which allows a specific port on
the host to be specified using the dotted notation found in Linux's
"devpath" sysfs attribute.
---
src/conf/domain_conf.c | 36 ++++-
src/conf/domain_conf.h | 2 +
src/libvirt_private.syms | 2 -
src/util/virhostdev.c | 126 ++++++++--------
src/util/virusb.c | 160 ++++++++-------------
src/util/virusb.h | 23 ++-
tests/virusbtest.c | 124 ++++++++++------
.../sys_bus_usb/devices/1-1.5.3.1/devpath | 1 +
.../sys_bus_usb/devices/1-1.5.3.3/devpath | 1 +
.../sys_bus_usb/devices/1-1.5.3/devpath | 1 +
.../sys_bus_usb/devices/1-1.5.4/devpath | 1 +
.../sys_bus_usb/devices/1-1.5.5/devpath | 1 +
.../sys_bus_usb/devices/1-1.5.6/devpath | 1 +
.../sys_bus_usb/devices/1-1.5/devpath | 1 +
.../sys_bus_usb/devices/1-1.6/devpath | 1 +
.../virusbtestdata/sys_bus_usb/devices/1-1/devpath | 1 +
.../sys_bus_usb/devices/2-1.2/devpath | 1 +
.../virusbtestdata/sys_bus_usb/devices/2-1/devpath | 1 +
.../sys_bus_usb/devices/usb1/devpath | 1 +
.../sys_bus_usb/devices/usb2/devpath | 1 +
.../sys_bus_usb/devices/usb3/devpath | 1 +
.../sys_bus_usb/devices/usb4/devpath | 1 +
22 files changed, 266 insertions(+), 222 deletions(-)
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath
create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9f7b906..a587cc2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5531,7 +5531,8 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
goto out;
}
} else if (xmlStrEqual(cur->name, BAD_CAST "address")) {
- char *bus, *device;
+ char *bus, *device, *port;
+ bool dev_or_port = false;
bus = virXMLPropString(cur, "bus");
if (bus) {
@@ -5557,10 +5558,24 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
VIR_FREE(device);
goto out;
}
+ dev_or_port = true;
VIR_FREE(device);
- } else {
+ }
+
+ port = virXMLPropString(cur, "port");
+ if (port) {
+ if (*port) {
+ usbsrc->port = port;
+ dev_or_port = true;
+ } else {
+ VIR_FREE(port);
+ }
+ }
+
+ if (!dev_or_port) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("usb address needs device id"));
+ _("usb address needs either device id "
+ "or port"));
goto out;
}
} else {
@@ -20326,10 +20341,17 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n", usbsrc->vendor);
virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", usbsrc->product);
}
- if (usbsrc->bus || usbsrc->device) {
- virBufferAsprintf(buf, "<address %sbus='%d' device='%d'/>\n",
- includeTypeInAddr ? "type='usb' " : "",
- usbsrc->bus, usbsrc->device);
+ if (usbsrc->bus || usbsrc->device || usbsrc->port) {
+ virBufferAddLit(buf, "<address");
+ if (includeTypeInAddr)
+ virBufferAddLit(buf, " type='usb'");
+ if (usbsrc->bus)
+ virBufferAsprintf(buf, " bus='%u'", usbsrc->bus);
+ if (usbsrc->device)
+ virBufferAsprintf(buf, " device='%u'", usbsrc->device);
+ if (usbsrc->port)
+ virBufferAsprintf(buf, " port='%s'", usbsrc->port);
+ virBufferAddLit(buf, "/>\n");
}
break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ba0ad5f..91bd621 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -317,6 +317,8 @@ VIR_ENUM_DECL(virDomainHostdevSubsysSCSIProtocol)
typedef struct _virDomainHostdevSubsysUSB virDomainHostdevSubsysUSB;
typedef virDomainHostdevSubsysUSB *virDomainHostdevSubsysUSBPtr;
struct _virDomainHostdevSubsysUSB {
+ char *port;
+
bool autoAddress; /* bus/device were filled automatically based
on vendor/product */
unsigned bus;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ccb4c5e..d412eac 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2511,8 +2511,6 @@ virURIResolveAlias;
# util/virusb.h
virUSBDeviceFileIterate;
virUSBDeviceFind;
-virUSBDeviceFindByBus;
-virUSBDeviceFindByVendor;
virUSBDeviceFree;
virUSBDeviceGetBus;
virUSBDeviceGetDevno;
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 9b5ca6f..e809c97 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1179,89 +1179,95 @@ virHostdevMarkUSBDevices(virHostdevManagerPtr mgr,
static int
+virHostdevFindUSBDeviceWithFlags(virDomainHostdevDefPtr hostdev,
+ bool mandatory,
+ unsigned int flags,
+ virUSBDevicePtr *usb)
+{
+ virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
+ unsigned vendor = usbsrc->vendor;
+ unsigned product = usbsrc->product;
+ unsigned bus = usbsrc->bus;
+ char *port = usbsrc->port;
+ unsigned device = usbsrc->device;
+ virUSBDeviceListPtr devs;
+ int rc;
+
+ rc = virUSBDeviceFind(vendor, product, bus, device, port, NULL,
+ mandatory, flags, &devs);
+ if (rc < 0)
+ return -1;
+
+ if (rc == 1) {
+ *usb = virUSBDeviceListGet(devs, 0);
+ virUSBDeviceListSteal(devs, *usb);
+ }
+ virObjectUnref(devs);
+
+ if (rc > 1) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Multiple USB devices for %x:%x, "
+ "use <address> to specify one"),
+ vendor, product);
+ return -1;
+ }
+
+ return rc;
+}
+
+static int
virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
bool mandatory,
virUSBDevicePtr *usb)
{
virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
unsigned vendor = usbsrc->vendor;
- unsigned product = usbsrc->product;
unsigned bus = usbsrc->bus;
unsigned device = usbsrc->device;
+ char *port = usbsrc->port;
bool autoAddress = usbsrc->autoAddress;
+ unsigned int flags = 0;
int rc;
*usb = NULL;
- if (vendor && bus) {
- rc = virUSBDeviceFind(vendor, product, bus, device,
- NULL,
- autoAddress ? false : mandatory,
- usb);
- if (rc < 0) {
- return -1;
- } else if (!autoAddress) {
- goto out;
- } else {
- VIR_INFO("USB device %x:%x could not be found at previous"
- " address (bus:%u device:%u)",
- vendor, product, bus, device);
- }
- }
+ /* First attempt, matching on all known fields. */
+ if (vendor)
+ flags |= USB_DEVICE_FIND_BY_VENDOR;
+ if (device)
+ flags |= USB_DEVICE_FIND_BY_DEVICE;
+ if (port)
+ flags |= USB_DEVICE_FIND_BY_PORT;
+
+ rc = virHostdevFindUSBDeviceWithFlags(hostdev,
+ autoAddress ? false : mandatory,
+ flags, usb);
+ if (rc < 0)
+ return -1;
- /* When vendor is specified, its USB address is either unspecified or the
- * device could not be found at the USB device where it had been
- * automatically found before.
- */
- if (vendor) {
- virUSBDeviceListPtr devs;
+ if (rc != 1 && autoAddress) {
+ VIR_INFO("USB device could not be found at previous address "
+ "(bus:%u device:%u)", bus, device);
- rc = virUSBDeviceFindByVendor(vendor, product, NULL, mandatory, &devs);
- if (rc < 0)
- return -1;
+ /* Second attempt, for when the device number has changed. */
+ flags &= ~((unsigned int) USB_DEVICE_FIND_BY_DEVICE);
+ usbsrc->device = 0;
- if (rc == 1) {
- *usb = virUSBDeviceListGet(devs, 0);
- virUSBDeviceListSteal(devs, *usb);
- }
- virObjectUnref(devs);
-
- if (rc == 0) {
- goto out;
- } else if (rc > 1) {
- if (autoAddress) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Multiple USB devices for %x:%x were found,"
- " but none of them is at bus:%u device:%u"),
- vendor, product, bus, device);
- } else {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Multiple USB devices for %x:%x, "
- "use <address> to specify one"),
- vendor, product);
- }
+ rc = virHostdevFindUSBDeviceWithFlags(hostdev, mandatory,
+ flags, usb);
+
+ if (rc < 0)
return -1;
- }
+ }
+ if (!*usb) {
+ hostdev->missing = true;
+ } else if (!usbsrc->bus || !usbsrc->device) {
usbsrc->bus = virUSBDeviceGetBus(*usb);
usbsrc->device = virUSBDeviceGetDevno(*usb);
usbsrc->autoAddress = true;
-
- if (autoAddress) {
- VIR_INFO("USB device %x:%x found at bus:%u device:%u (moved"
- " from bus:%u device:%u)",
- vendor, product,
- usbsrc->bus, usbsrc->device,
- bus, device);
- }
- } else if (!vendor && bus) {
- if (virUSBDeviceFindByBus(bus, device, NULL, mandatory, usb) < 0)
- return -1;
}
- out:
- if (!*usb)
- hostdev->missing = true;
return 0;
}
diff --git a/src/util/virusb.c b/src/util/virusb.c
index 6a001a7..2e15849 100644
--- a/src/util/virusb.c
+++ b/src/util/virusb.c
@@ -68,12 +68,6 @@ struct _virUSBDeviceList {
virUSBDevicePtr *devs;
};
-typedef enum {
- USB_DEVICE_ALL = 0,
- USB_DEVICE_FIND_BY_VENDOR = 1 << 0,
- USB_DEVICE_FIND_BY_BUS = 1 << 1,
-} virUSBDeviceFindFlags;
-
static virClassPtr virUSBDeviceListClass;
static void virUSBDeviceListDispose(void *obj);
@@ -119,11 +113,33 @@ static int virUSBSysReadFile(const char *f_name, const char *d_name,
return ret;
}
+static int virUSBSysReadFileStr(const char *f_name, const char *d_name,
+ char **value)
+{
+ int ret = -1, tmp;
+ char *buf = NULL;
+ char *filename = NULL;
+
+ tmp = virAsprintf(&filename, USB_SYSFS "/devices/%s/%s", d_name, f_name);
+ if (tmp < 0)
+ goto cleanup;
+
+ if (virFileReadAll(filename, 1024, &buf) < 0)
+ goto cleanup;
+
+ *value = buf;
+ ret = 0;
+ cleanup:
+ VIR_FREE(filename);
+ return ret;
+}
+
static virUSBDeviceListPtr
virUSBDeviceSearch(unsigned int vendor,
unsigned int product,
unsigned int bus,
unsigned int devno,
+ const char *port,
const char *vroot,
unsigned int flags)
{
@@ -143,6 +159,8 @@ virUSBDeviceSearch(unsigned int vendor,
while ((direrr = virDirRead(dir, &de, USB_SYSFS "/devices")) > 0) {
unsigned int found_prod, found_vend, found_bus, found_devno;
+ char *found_port;
+ bool port_matches;
char *tmpstr = de->d_name;
if (strchr(de->d_name, ':'))
@@ -170,16 +188,31 @@ virUSBDeviceSearch(unsigned int vendor,
10, &found_devno) < 0)
goto cleanup;
+ if (virUSBSysReadFileStr("devpath", de->d_name,
+ &found_port) < 0) {
+ goto cleanup;
+ } else {
+ found_port[strlen(found_port) - 1] = '\0'; /* remove newline */
+ port_matches = STREQ_NULLABLE(found_port, port);
+ VIR_FREE(found_port);
+ }
+
if ((flags & USB_DEVICE_FIND_BY_VENDOR) &&
(found_prod != product || found_vend != vendor))
continue;
- if (flags & USB_DEVICE_FIND_BY_BUS) {
+ if (flags & USB_DEVICE_FIND_BY_DEVICE) {
if (found_bus != bus || found_devno != devno)
continue;
found = true;
}
+ if (flags & USB_DEVICE_FIND_BY_PORT) {
+ if (found_bus != bus || !port_matches)
+ continue;
+ found = true;
+ }
+
usb = virUSBDeviceNew(found_bus, found_devno, vroot);
if (!usb)
goto cleanup;
@@ -204,36 +237,43 @@ virUSBDeviceSearch(unsigned int vendor,
}
int
-virUSBDeviceFindByVendor(unsigned int vendor,
- unsigned int product,
- const char *vroot,
- bool mandatory,
- virUSBDeviceListPtr *devices)
+virUSBDeviceFind(unsigned int vendor,
+ unsigned int product,
+ unsigned int bus,
+ unsigned int devno,
+ const char *port,
+ const char *vroot,
+ bool mandatory,
+ unsigned int flags,
+ virUSBDeviceListPtr *devices)
{
virUSBDeviceListPtr list;
int count;
- if (!(list = virUSBDeviceSearch(vendor, product, 0, 0,
- vroot,
- USB_DEVICE_FIND_BY_VENDOR)))
+ if (!(list = virUSBDeviceSearch(vendor, product, bus, devno, port,
+ vroot, flags)))
return -1;
- if (list->count == 0) {
+ count = list->count;
+ if (count == 0) {
virObjectUnref(list);
if (!mandatory) {
- VIR_DEBUG("Did not find USB device %x:%x",
- vendor, product);
if (devices)
*devices = NULL;
return 0;
}
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Did not find USB device %x:%x"), vendor, product);
+ _("Did not find a matching USB device (matching "
+ "on%s%s%s all others ignored): vid:%04x, pid:%04x, "
+ "bus:%u, device:%u, port:%s"),
+ flags & USB_DEVICE_FIND_BY_VENDOR ? " vid/pid," : "",
+ flags & USB_DEVICE_FIND_BY_DEVICE ? " bus/device," : "",
+ flags & USB_DEVICE_FIND_BY_PORT ? " bus/port," : "",
+ vendor, product, bus, devno, port ? port : "(null)");
return -1;
}
- count = list->count;
if (devices)
*devices = list;
else
@@ -242,86 +282,6 @@ virUSBDeviceFindByVendor(unsigned int vendor,
return count;
}
-int
-virUSBDeviceFindByBus(unsigned int bus,
- unsigned int devno,
- const char *vroot,
- bool mandatory,
- virUSBDevicePtr *usb)
-{
- virUSBDeviceListPtr list;
-
- if (!(list = virUSBDeviceSearch(0, 0, bus, devno,
- vroot,
- USB_DEVICE_FIND_BY_BUS)))
- return -1;
-
- if (list->count == 0) {
- virObjectUnref(list);
- if (!mandatory) {
- VIR_DEBUG("Did not find USB device bus:%u device:%u",
- bus, devno);
- if (usb)
- *usb = NULL;
- return 0;
- }
-
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Did not find USB device bus:%u device:%u"),
- bus, devno);
- return -1;
- }
-
- if (usb) {
- *usb = virUSBDeviceListGet(list, 0);
- virUSBDeviceListSteal(list, *usb);
- }
- virObjectUnref(list);
-
- return 0;
-}
-
-int
-virUSBDeviceFind(unsigned int vendor,
- unsigned int product,
- unsigned int bus,
- unsigned int devno,
- const char *vroot,
- bool mandatory,
- virUSBDevicePtr *usb)
-{
- virUSBDeviceListPtr list;
-
- unsigned int flags = USB_DEVICE_FIND_BY_VENDOR|USB_DEVICE_FIND_BY_BUS;
- if (!(list = virUSBDeviceSearch(vendor, product, bus, devno,
- vroot, flags)))
- return -1;
-
- if (list->count == 0) {
- virObjectUnref(list);
- if (!mandatory) {
- VIR_DEBUG("Did not find USB device %x:%x bus:%u device:%u",
- vendor, product, bus, devno);
- if (usb)
- *usb = NULL;
- return 0;
- }
-
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Did not find USB device %x:%x bus:%u device:%u"),
- vendor, product, bus, devno);
- return -1;
- }
-
- if (usb) {
- *usb = virUSBDeviceListGet(list, 0);
- virUSBDeviceListSteal(list, *usb);
- }
- virObjectUnref(list);
-
- return 0;
-}
-
virUSBDevicePtr
virUSBDeviceNew(unsigned int bus,
unsigned int devno,
diff --git a/src/util/virusb.h b/src/util/virusb.h
index f98ea21..0834cd4 100644
--- a/src/util/virusb.h
+++ b/src/util/virusb.h
@@ -35,29 +35,26 @@ typedef virUSBDevice *virUSBDevicePtr;
typedef struct _virUSBDeviceList virUSBDeviceList;
typedef virUSBDeviceList *virUSBDeviceListPtr;
+typedef enum {
+ USB_DEVICE_ALL = 0,
+ USB_DEVICE_FIND_BY_VENDOR = 1 << 0,
+ USB_DEVICE_FIND_BY_DEVICE = 1 << 1,
+ USB_DEVICE_FIND_BY_PORT = 1 << 2,
+} virUSBDeviceFindFlags;
+
virUSBDevicePtr virUSBDeviceNew(unsigned int bus,
unsigned int devno,
const char *vroot);
-int virUSBDeviceFindByBus(unsigned int bus,
- unsigned int devno,
- const char *vroot,
- bool mandatory,
- virUSBDevicePtr *usb);
-
-int virUSBDeviceFindByVendor(unsigned int vendor,
- unsigned int product,
- const char *vroot,
- bool mandatory,
- virUSBDeviceListPtr *devices);
-
int virUSBDeviceFind(unsigned int vendor,
unsigned int product,
unsigned int bus,
unsigned int devno,
+ const char *port,
const char *vroot,
bool mandatory,
- virUSBDevicePtr *usb);
+ unsigned int flags,
+ virUSBDeviceListPtr *devices);
void virUSBDeviceFree(virUSBDevicePtr dev);
int virUSBDeviceSetUsedBy(virUSBDevicePtr dev,
diff --git a/tests/virusbtest.c b/tests/virusbtest.c
index 4bbfe4a..0782b73 100644
--- a/tests/virusbtest.c
+++ b/tests/virusbtest.c
@@ -33,7 +33,8 @@
typedef enum {
FIND_BY_ALL,
FIND_BY_VENDOR,
- FIND_BY_BUS
+ FIND_BY_DEVICE,
+ FIND_BY_PORT
} testUSBFindFlags;
struct findTestInfo {
@@ -42,6 +43,7 @@ struct findTestInfo {
unsigned int product;
unsigned int bus;
unsigned int devno;
+ const char *port;
const char *vroot;
bool mandatory;
int how;
@@ -78,25 +80,31 @@ static int testDeviceFind(const void *opaque)
virUSBDeviceListPtr devs = NULL;
int rv = 0;
size_t i, ndevs = 0;
+ unsigned int flags = 0;
switch (info->how) {
case FIND_BY_ALL:
- rv = virUSBDeviceFind(info->vendor, info->product,
- info->bus, info->devno,
- info->vroot, info->mandatory, &dev);
+ flags = USB_DEVICE_FIND_BY_VENDOR |
+ USB_DEVICE_FIND_BY_DEVICE |
+ USB_DEVICE_FIND_BY_PORT;
break;
case FIND_BY_VENDOR:
- rv = virUSBDeviceFindByVendor(info->vendor, info->product,
- info->vroot, info->mandatory, &devs);
+ flags = USB_DEVICE_FIND_BY_VENDOR;
break;
- case FIND_BY_BUS:
- rv = virUSBDeviceFindByBus(info->bus, info->devno,
- info->vroot, info->mandatory, &dev);
+ case FIND_BY_DEVICE:
+ flags = USB_DEVICE_FIND_BY_DEVICE;
+ break;
+ case FIND_BY_PORT:
+ flags = USB_DEVICE_FIND_BY_PORT;
break;
}
+ rv = virUSBDeviceFind(info->vendor, info->product,
+ info->bus, info->devno, info->port,
+ info->vroot, info->mandatory, flags, &devs);
+
if (info->expectFailure) {
- if (rv == 0) {
+ if (rv >= 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
"unexpected success");
} else {
@@ -107,9 +115,18 @@ static int testDeviceFind(const void *opaque)
goto cleanup;
}
+ if (info->how != FIND_BY_VENDOR) {
+ if (rv == 1) {
+ dev = virUSBDeviceListGet(devs, 0);
+ virUSBDeviceListSteal(devs, dev);
+ } else {
+ goto cleanup;
+ }
+ }
+
switch (info->how) {
case FIND_BY_ALL:
- case FIND_BY_BUS:
+ case FIND_BY_DEVICE:
if (virUSBDeviceFileIterate(dev, testDeviceFileActor, NULL) < 0)
goto cleanup;
break;
@@ -155,14 +172,17 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED)
virUSBDeviceListPtr list = NULL;
virUSBDeviceListPtr devlist = NULL;
virUSBDevicePtr dev = NULL;
+ virUSBDeviceListPtr devs = NULL;
int ret = -1;
+ int rv;
size_t i, ndevs;
if (!(list = virUSBDeviceListNew()))
goto cleanup;
#define EXPECTED_NDEVS_ONE 3
- if (virUSBDeviceFindByVendor(0x1d6b, 0x0002, NULL, true, &devlist) < 0)
+ if (virUSBDeviceFind(0x1d6b, 0x0002, 0, 0, NULL, NULL, true,
+ USB_DEVICE_FIND_BY_VENDOR, &devlist) < 0)
goto cleanup;
ndevs = virUSBDeviceListCount(devlist);
@@ -186,7 +206,8 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED)
goto cleanup;
#define EXPECTED_NDEVS_TWO 3
- if (virUSBDeviceFindByVendor(0x18d1, 0x4e22, NULL, true, &devlist) < 0)
+ if (virUSBDeviceFind(0x18d1, 0x4e22, 0, 0, NULL, NULL, true,
+ USB_DEVICE_FIND_BY_VENDOR, &devlist) < 0)
goto cleanup;
ndevs = virUSBDeviceListCount(devlist);
@@ -206,8 +227,17 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED)
EXPECTED_NDEVS_ONE + EXPECTED_NDEVS_TWO) < 0)
goto cleanup;
- if (virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, NULL, true, &dev) < 0)
+ rv = virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, "1.5.6", NULL, true,
+ USB_DEVICE_FIND_BY_VENDOR |
+ USB_DEVICE_FIND_BY_DEVICE |
+ USB_DEVICE_FIND_BY_PORT, &devs);
+ if (rv != 1) {
goto cleanup;
+ } else {
+ dev = virUSBDeviceListGet(devs, 0);
+ virUSBDeviceListSteal(devs, dev);
+ }
+ virObjectUnref(devs);
if (!virUSBDeviceListFind(list, dev)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -239,49 +269,63 @@ mymain(void)
{
int rv = 0;
-#define DO_TEST_FIND_FULL(name, vend, prod, bus, devno, vroot, mand, how, fail) \
- do { \
- struct findTestInfo data = { name, vend, prod, bus, \
- devno, vroot, mand, how, fail \
- }; \
- if (virTestRun("USBDeviceFind " name, testDeviceFind, &data) < 0) \
- rv = -1; \
+#define DO_TEST_FIND_FULL(name, vend, prod, bus, devno, \
+ port, vroot, mand, how, fail) \
+ do { \
+ struct findTestInfo data = { name, vend, prod, bus, \
+ devno, port, vroot, mand, how, fail \
+ }; \
+ if (virTestRun("USBDeviceFind " name, testDeviceFind, &data) < 0) \
+ rv = -1; \
} while (0)
-#define DO_TEST_FIND(name, vend, prod, bus, devno) \
- DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, true, \
+#define DO_TEST_FIND(name, vend, prod, bus, devno, port) \
+ DO_TEST_FIND_FULL(name, vend, prod, bus, devno, port, NULL, true, \
FIND_BY_ALL, false)
-#define DO_TEST_FIND_FAIL(name, vend, prod, bus, devno) \
- DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, true, \
+#define DO_TEST_FIND_FAIL(name, vend, prod, bus, devno, port) \
+ DO_TEST_FIND_FULL(name, vend, prod, bus, devno, port, NULL, true, \
FIND_BY_ALL, true)
-#define DO_TEST_FIND_BY_BUS(name, bus, devno) \
- DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, true, \
- FIND_BY_BUS, false)
-#define DO_TEST_FIND_BY_BUS_FAIL(name, bus, devno) \
- DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, true, \
- FIND_BY_BUS, true)
+#define DO_TEST_FIND_BY_DEVICE(name, bus, devno) \
+ DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, NULL, true, \
+ FIND_BY_DEVICE, false)
+#define DO_TEST_FIND_BY_DEVICE_FAIL(name, bus, devno) \
+ DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, NULL, true, \
+ FIND_BY_DEVICE, true)
#define DO_TEST_FIND_BY_VENDOR(name, vend, prod) \
- DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, true, \
+ DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, NULL, true, \
FIND_BY_VENDOR, false)
#define DO_TEST_FIND_BY_VENDOR_FAIL(name, vend, prod) \
- DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, true, \
+ DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, NULL, true, \
FIND_BY_VENDOR, true)
- DO_TEST_FIND("Nexus", 0x18d1, 0x4e22, 1, 20);
- DO_TEST_FIND_FAIL("Nexus wrong devnum", 0x18d1, 0x4e22, 1, 25);
- DO_TEST_FIND_FAIL("Bogus", 0xf00d, 0xbeef, 1024, 768);
+#define DO_TEST_FIND_BY_PORT(name, bus, port) \
+ DO_TEST_FIND_FULL(name, 101, 202, bus, 456, port, NULL, true, \
+ FIND_BY_PORT, false)
+#define DO_TEST_FIND_BY_PORT_FAIL(name, bus, port) \
+ DO_TEST_FIND_FULL(name, 101, 202, bus, 456, port, NULL, true, \
+ FIND_BY_PORT, true)
+
+ DO_TEST_FIND("Nexus", 0x18d1, 0x4e22, 1, 20, "1.5.6");
+ DO_TEST_FIND_FAIL("Nexus wrong devnum", 0x18d1, 0x4e22, 1, 25, "1.5.6");
+ DO_TEST_FIND_FAIL("Nexus wrong port", 0x18d1, 0x4e22, 1, 25, "1.5.4");
+ DO_TEST_FIND_FAIL("Bogus", 0xf00d, 0xbeef, 1024, 768, "1.2.3.4");
- DO_TEST_FIND_BY_BUS("integrated camera", 1, 5);
- DO_TEST_FIND_BY_BUS_FAIL("wrong bus/devno combination", 2, 20);
- DO_TEST_FIND_BY_BUS_FAIL("missing bus", 5, 20);
- DO_TEST_FIND_BY_BUS_FAIL("missing devnum", 1, 158);
+ DO_TEST_FIND_BY_DEVICE("integrated camera", 1, 5);
+ DO_TEST_FIND_BY_DEVICE_FAIL("wrong bus/devno combination", 2, 20);
+ DO_TEST_FIND_BY_DEVICE_FAIL("missing bus", 5, 20);
+ DO_TEST_FIND_BY_DEVICE_FAIL("missing devnum", 1, 158);
DO_TEST_FIND_BY_VENDOR("Nexus (multiple results)", 0x18d1, 0x4e22);
DO_TEST_FIND_BY_VENDOR_FAIL("Bogus vendor and product", 0xf00d, 0xbeef);
DO_TEST_FIND_BY_VENDOR_FAIL("Valid vendor", 0x1d6b, 0xbeef);
+ DO_TEST_FIND_BY_PORT("Logitech mouse", 1, "1.5.3.3");
+ DO_TEST_FIND_BY_PORT_FAIL("wrong bus/port combination", 2, "1.5.3.3");
+ DO_TEST_FIND_BY_PORT_FAIL("missing bus", 5, "1.5.3.3");
+ DO_TEST_FIND_BY_PORT_FAIL("missing port", 1, "8.2.5");
+
if (virTestRun("USB List test", testUSBList, NULL) < 0)
rv = -1;
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath
new file mode 100644
index 0000000..02a7fbe
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath
@@ -0,0 +1 @@
+1.5.3.1
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath
new file mode 100644
index 0000000..23ca863
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath
@@ -0,0 +1 @@
+1.5.3.3
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath
new file mode 100644
index 0000000..8af85be
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath
@@ -0,0 +1 @@
+1.5.3
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath
new file mode 100644
index 0000000..94fe62c
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath
@@ -0,0 +1 @@
+1.5.4
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath
new file mode 100644
index 0000000..9075be4
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath
@@ -0,0 +1 @@
+1.5.5
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath
new file mode 100644
index 0000000..eac1e0a
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath
@@ -0,0 +1 @@
+1.5.6
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath
new file mode 100644
index 0000000..c239c60
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath
@@ -0,0 +1 @@
+1.5
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath
new file mode 100644
index 0000000..810ee4e
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath
@@ -0,0 +1 @@
+1.6
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath
@@ -0,0 +1 @@
+1
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath b/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath
new file mode 100644
index 0000000..5625e59
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath
@@ -0,0 +1 @@
+1.2
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath
@@ -0,0 +1 @@
+1
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath
@@ -0,0 +1 @@
+0
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath
@@ -0,0 +1 @@
+0
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath
@@ -0,0 +1 @@
+0
diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath b/tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath
@@ -0,0 +1 @@
+0
--
2.7.4
8 years, 1 month
[libvirt] [PATCH] Avoid segfault in virt-aa-helper when handling read-only mount filesystems
by rufo
This patch fixes a segfault in virt-aa-helper caused by attempting to modify a string literal in situ.
It is triggered when a domain has a <filesystem> with type='mount' configured readonly, and libvirt is using the AppArmor security driver for sVirt confinement.
---
src/security/virt-aa-helper.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 49e12b9..c22aa66 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -740,6 +740,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi
bool readonly = true;
bool explicit_deny_rule = true;
char *sub = NULL;
+ char *perms_new = strdup(perms);
if (path == NULL)
return rc;
@@ -764,12 +765,12 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi
return rc;
}
- if (strchr(perms, 'w') != NULL) {
+ if (strchr(perms_new, 'w') != NULL) {
readonly = false;
explicit_deny_rule = false;
}
- if ((sub = strchr(perms, 'R')) != NULL) {
+ if ((sub = strchr(perms_new, 'R')) != NULL) {
/* Don't write the invalid R permission, replace it with 'r' */
sub[0] = 'r';
explicit_deny_rule = false;
@@ -787,7 +788,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi
if (tmp[strlen(tmp) - 1] == '/')
tmp[strlen(tmp) - 1] = '\0';
- virBufferAsprintf(buf, " \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms);
+ virBufferAsprintf(buf, " \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms_new);
if (explicit_deny_rule) {
virBufferAddLit(buf, " # don't audit writes to readonly files\n");
virBufferAsprintf(buf, " deny \"%s%s\" w,\n", tmp, recursive ? "/**" : "");
--
2.9.3
8 years, 1 month
[libvirt] [PATCH] Delete extra wrap after vol-reisize error
by Yanqiu Zhang
This patch is to delete the extra wrap "\n" after failed vol-reisize error, both "Failed to change size of volume to" and "Failed to change size of volume by". For error with wrap, there will be an extra wrap between two errors, such as:
(1)# virsh vol-resize --pool default --vol vol-test 5M
error: Failed to change size of volume 'vol-test' to 5M
error: invalid argument: Can't shrink capacity below current capacity unless shrink flag explicitly specified
(2)# virsh vol-resize /var/lib/libvirt/images/volds --shrink --delta 10M
error: Failed to change size of volume 'volds' by 10M
error: invalid argument: can't shrink capacity below existing allocation
---
tools/virsh-volume.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index d35fee1..4b3b4d7 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -1128,8 +1128,8 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd)
ret = true;
} else {
vshError(ctl,
- delta ? _("Failed to change size of volume '%s' by %s\n")
- : _("Failed to change size of volume '%s' to %s\n"),
+ delta ? _("Failed to change size of volume '%s' by %s")
+ : _("Failed to change size of volume '%s' to %s"),
virStorageVolGetName(vol), capacityStr);
ret = false;
}
--
1.8.3.1
8 years, 1 month
[libvirt] [PATCH] qemu_driver: protect existed save image
by Chen Hanxiao
From: Chen Hanxiao <chenhanxiao(a)gmail.com>
We may overwrite existed save image by
'virsh save' command.
This patch will check qemu save headers
before trying to rewrite it.
Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com>
---
src/qemu/qemu_driver.c | 141 +++++++++++++++++++++++++++++++------------------
1 file changed, 89 insertions(+), 52 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a0ac2ef..56aa079 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2806,6 +2806,73 @@ bswap_header(virQEMUSaveHeaderPtr hdr)
}
+static int
+qemuSaveHeaderCheck(int fd,
+ virQEMUSaveHeaderPtr header,
+ const char *path,
+ bool unlink_corrupt)
+{
+ int ret = -1;
+
+ if (saferead(fd, header, sizeof(*header)) != sizeof(*header)) {
+ if (unlink_corrupt) {
+ if (VIR_CLOSE(fd) < 0 || unlink(path) < 0) {
+ virReportSystemError(errno,
+ _("cannot remove corrupt file: %s"),
+ path);
+ goto error;
+ }
+ return -3;
+ }
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ "%s", _("failed to read qemu header"));
+ goto error;
+ }
+
+ if (memcmp((*header).magic, QEMU_SAVE_MAGIC, sizeof((*header).magic)) != 0) {
+ const char *msg = _("image magic is incorrect");
+
+ if (memcmp((*header).magic, QEMU_SAVE_PARTIAL,
+ sizeof((*header).magic)) == 0) {
+ msg = _("save image is incomplete");
+ if (unlink_corrupt) {
+ if (VIR_CLOSE(fd) < 0 || unlink(path) < 0) {
+ virReportSystemError(errno,
+ _("cannot remove corrupt file: %s"),
+ path);
+ goto error;
+ }
+ return -3;
+ }
+ }
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s", msg);
+ goto error;
+ }
+
+ if ((*header).version > QEMU_SAVE_VERSION) {
+ /* convert endianess and try again */
+ bswap_header(header);
+ }
+
+ if ((*header).version > QEMU_SAVE_VERSION) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("image version is not supported (%d > %d)"),
+ (*header).version, QEMU_SAVE_VERSION);
+ goto error;
+ }
+
+ if ((*header).xml_len <= 0) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("invalid XML length: %d"), (*header).xml_len);
+ goto error;
+ }
+
+ ret = 0;
+
+ error:
+ return ret;
+}
+
/* return -errno on failure, or 0 on success */
static int
qemuDomainSaveHeader(int fd, const char *path, const char *xml,
@@ -2827,6 +2894,7 @@ qemuDomainSaveHeader(int fd, const char *path, const char *xml,
_("failed to write xml to '%s'"), path);
goto endjob;
}
+
endjob:
return ret;
}
@@ -3067,6 +3135,24 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
goto cleanup;
}
}
+
+ /* don't overwrite exsited save files */
+ if (virFileExists(path)) {
+ virQEMUSaveHeader header_tmp;
+ fd = qemuOpenFile(driver, NULL, path, O_RDONLY, NULL, NULL);
+ if (fd < 0)
+ goto cleanup;
+
+ if (qemuSaveHeaderCheck(fd, &header_tmp, path, false) >= 0) {
+ ret = -EEXIST;
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("failed to write existed image file '%s'"),
+ path);
+ goto cleanup;
+ }
+ VIR_FORCE_CLOSE(fd);
+ }
+
fd = qemuOpenFile(driver, vm, path,
O_WRONLY | O_TRUNC | O_CREAT | directFlag,
&needUnlink, &bypassSecurityDriver);
@@ -6327,6 +6413,7 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
bool unlink_corrupt)
{
int fd = -1;
+ int ret = -1;
virQEMUSaveHeader header;
char *xml = NULL;
virDomainDefPtr def = NULL;
@@ -6353,58 +6440,8 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
VIR_FILE_WRAPPER_BYPASS_CACHE)))
goto error;
- if (saferead(fd, &header, sizeof(header)) != sizeof(header)) {
- if (unlink_corrupt) {
- if (VIR_CLOSE(fd) < 0 || unlink(path) < 0) {
- virReportSystemError(errno,
- _("cannot remove corrupt file: %s"),
- path);
- goto error;
- }
- return -3;
- }
- virReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("failed to read qemu header"));
+ if ((ret = qemuSaveHeaderCheck(fd, &header, path, unlink_corrupt)) < 0)
goto error;
- }
-
- if (memcmp(header.magic, QEMU_SAVE_MAGIC, sizeof(header.magic)) != 0) {
- const char *msg = _("image magic is incorrect");
-
- if (memcmp(header.magic, QEMU_SAVE_PARTIAL,
- sizeof(header.magic)) == 0) {
- msg = _("save image is incomplete");
- if (unlink_corrupt) {
- if (VIR_CLOSE(fd) < 0 || unlink(path) < 0) {
- virReportSystemError(errno,
- _("cannot remove corrupt file: %s"),
- path);
- goto error;
- }
- return -3;
- }
- }
- virReportError(VIR_ERR_OPERATION_FAILED, "%s", msg);
- goto error;
- }
-
- if (header.version > QEMU_SAVE_VERSION) {
- /* convert endianess and try again */
- bswap_header(&header);
- }
-
- if (header.version > QEMU_SAVE_VERSION) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("image version is not supported (%d > %d)"),
- header.version, QEMU_SAVE_VERSION);
- goto error;
- }
-
- if (header.xml_len <= 0) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("invalid XML length: %d"), header.xml_len);
- goto error;
- }
if (VIR_ALLOC_N(xml, header.xml_len) < 0)
goto error;
@@ -6439,7 +6476,7 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
VIR_FORCE_CLOSE(fd);
virObjectUnref(caps);
- return -1;
+ return ret;
}
static int ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6)
--
1.8.3.1
8 years, 1 month
[libvirt] NPIV storage pools do not map to same LUN units across hosts.
by Nitesh Konkar
Link: http://wiki.libvirt.org/page/NPIV_in_libvirt
Topic: Virtual machine configuration change to use vHBA LUN
There is a NPIV storage pool defined on two hosts and pool contains a
total of 8 volumes, allocated from a storage device.
Source:
# virsh vol-list poolvhba0
Name Path
------------------------------------------------------------------------------
unit:0:0:0 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000366
unit:0:0:1 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000367
unit:0:0:2 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000368
unit:0:0:3 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000369
unit:0:0:4 /dev/disk/by-id/wwn-0x6005076802818bda300000000000036a
unit:0:0:5 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000380
unit:0:0:6 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000381
unit:0:0:7 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000382
--------------------------------------------------------------------
Destination:
--------------------------------------------------------------------
# virsh vol-list poolvhba0
Name Path
------------------------------------------------------------------------------
unit:0:0:0 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000380
unit:0:0:1 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000381
unit:0:0:2 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000382
unit:0:0:3 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000367
unit:0:0:4 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000368
unit:0:0:5 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000366
unit:0:0:6 /dev/disk/by-id/wwn-0x6005076802818bda300000000000036a
unit:0:0:7 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000369
--------------------------------------------------------------------
As you can see in the above output,the same set of eight LUNs from the
storage server have been mapped,
but the order that the LUNs are probed on each host is different,
resulting in different unit names
on the two different hosts .
If the the guest XMLs is referencing its storage by "unit" number then is
it safe to migrate such guests because the "unit number" is assigned by the
driver according to the specific way it probes the storage and hence
when you migrate
these guests , it results in different unit names on the destination hosts.
Thus the migrated guest gets mapped to the wrong LUNs and is given the
wrong disks.
The problem is that the LUN numbers on the destination host and source
host do not agree.
Example, LUN 0 on source_host, for example, may be LUN 5 on destination_host.
When the guest is given the wrong disk, it suffers a fatal I/O error. (This is
manifested as fatal I/O errors since the guest has no idea that its disks just
changed out under it.)The migration does not take into account that
the unit numbers do
match on on the source and destination sides.
So, should libvirt make sure that the guest domains reference NPIV
pool volumes by their
globally-unique wwn instead of by "unit" numbers?
The guest XML references its storage by "unit" number.
Eg:-
<disk type='volume' device='lun'>
<driver name='qemu' type='raw' cache='none'/>
<source pool='poolvhba0' volume='unit:0:0:0'/>
<backingStore/>
<target dev='vdb' bus='virtio'/>
<alias name='virtio-disk1'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x05'
function='0x0'/>
</disk>
I am planning to write a patch for it. Any comments on the above
observation/approach would be appreciated.
Thanks,
Nitesh.
8 years, 1 month
[libvirt] [PATCH] util: hostcpu: improve CPU freq code for FreeBSD
by Roman Bogorodskiy
Current implementation uses the dev.cpu.0.freq sysctl that is
provided by the cpufreq(4) framework and returns the actual
CPU frequency. However, there are environment where it's not available,
e.g. when running nested in KVM. In this case fall back to hw.clockrate
that reports CPU frequency at the boot time.
Resolves (hopefully):
https://bugzilla.redhat.com/show_bug.cgi?id=1369964
---
src/util/virhostcpu.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 0f03ff8..b5a37a8 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -994,9 +994,16 @@ virHostCPUGetInfo(virArch hostarch ATTRIBUTE_UNUSED,
*threads = 1;
# ifdef __FreeBSD__
+ /* dev.cpu.%d.freq reports current active CPU frequency. It is provided by
+ * the cpufreq(4) framework. However, it might be disabled or no driver
+ * available. In this case fallback to "hw.clockrate" which reports boot time
+ * CPU frequency. */
+
if (sysctlbyname("dev.cpu.0.freq", &cpu_freq, &cpu_freq_len, NULL, 0) < 0) {
- virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
- return -1;
+ if (sysctlbyname("hw.clockrate", &cpu_freq, &cpu_freq_len, NULL, 0) < 0) {
+ virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
+ return -1;
+ }
}
*mhz = cpu_freq;
--
2.7.4
8 years, 1 month
[libvirt] [PATCH 0/2] Improve handling of QEMU core dumping
by Daniel P. Berrange
Daniel P. Berrange (2):
qemu: add a max_core setting to qemu.conf for core dump size
qemu: allow turning off QEMU guest RAM dump globally
src/libvirt_private.syms | 2 ++
src/qemu/libvirtd_qemu.aug | 5 +++++
src/qemu/qemu.conf | 31 +++++++++++++++++++++++++++++++
src/qemu/qemu_command.c | 18 ++++++++++++------
src/qemu/qemu_conf.c | 20 ++++++++++++++++++++
src/qemu/qemu_conf.h | 2 ++
src/qemu/qemu_process.c | 1 +
src/qemu/test_libvirtd_qemu.aug.in | 2 ++
src/util/vircommand.c | 14 ++++++++++++++
src/util/vircommand.h | 1 +
src/util/virprocess.c | 36 ++++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 1 +
tests/qemuxml2argvtest.c | 4 ++++
13 files changed, 131 insertions(+), 6 deletions(-)
--
2.7.4
8 years, 1 month
[libvirt] [libvirt-glib/libvirt-gconfig 00/17] Graphics: Introduce the new Remote and Local classes (and also implement a few missing methods).
by Fabiano Fidêncio
While trying to use libvirt-gobject and libvirt-gconfig for accessing VMs
and looking at their config, instead of using libvirt and parsing XML
directly, I found out that a few methods have been missing and that
libvirt-gconfig is not exactly thought for the "reading their config" use
case (see more explanations on the 10th and 14th commits.
This series, unfortunately, introduces an ABI breakage.
Fabiano Fidêncio (17):
gconfig: Implement gvir_config_domain_graphics_vnc_get_autoport()
gconfig: Implement gvir_config_domain_graphics_spice_get_autoport()
gconfig: Implement gvir_config_domain_graphics_rdp_get_autoport()
gconfig: Implement gvir_config_domain_graphics_sdl_get_display()
gconfig: Implement gvir_config_domain_graphics_sdl_get_fullscreen()
gconfig: Implement gvir_config_domain_graphics_spice_get_tls_port()
gconfig: Implement gvir_config_domain_graphics_spice_{get,set}_host()
gconfig: Implement gvir_config_domain_graphics_vnc_{get,set}_host()
gconfig: Implement gvir_config_domain_graphics_rdp_{get,set}_host()
gconfig: Add GVirCofigDomainGraphicsRemote class
gconfig: Adapt GVirConfigDomainGraphicsSpice to
GVirConfigDomainGraphicsRemote
gconfig: Adapt GVirConfigDomainGraphicsRdp to
GVirConfigDomainGraphicsRemote
gconfig: Adapt GVirConfigDomainGraphicsVnc to
GVirConfigDomainGraphicsRemote
gconfig: Add GVirCofigDomainGraphicsLocal class
gconfig: Adapt GVirConfigDomainGraphicsSdl to
GVirConfigDomainGraphicsLocal
gconfig: Adapt GVirConfigDomainGraphicsDesktop to
GVirConfigDomainGraphicsLocal
gconfig,graphics: Avoid crash when gvir_config_object_new_from_xml()
returns NULL
libvirt-gconfig/Makefile.am | 4 +
.../libvirt-gconfig-domain-graphics-desktop.c | 14 ++-
.../libvirt-gconfig-domain-graphics-desktop.h | 4 +-
.../libvirt-gconfig-domain-graphics-local.c | 97 +++++++++++++++++++
.../libvirt-gconfig-domain-graphics-local.h | 68 ++++++++++++++
.../libvirt-gconfig-domain-graphics-rdp.c | 32 ++++++-
.../libvirt-gconfig-domain-graphics-rdp.h | 9 +-
.../libvirt-gconfig-domain-graphics-remote.c | 103 +++++++++++++++++++++
.../libvirt-gconfig-domain-graphics-remote.h | 70 ++++++++++++++
.../libvirt-gconfig-domain-graphics-sdl.c | 19 +++-
.../libvirt-gconfig-domain-graphics-sdl.h | 6 +-
.../libvirt-gconfig-domain-graphics-spice.c | 40 +++++++-
.../libvirt-gconfig-domain-graphics-spice.h | 10 +-
.../libvirt-gconfig-domain-graphics-vnc.c | 32 ++++++-
.../libvirt-gconfig-domain-graphics-vnc.h | 9 +-
libvirt-gconfig/libvirt-gconfig.h | 2 +
libvirt-gconfig/libvirt-gconfig.sym | 20 ++++
po/POTFILES.in | 2 +
18 files changed, 513 insertions(+), 28 deletions(-)
create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-local.c
create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-local.h
create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.c
create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.h
--
2.5.0
8 years, 1 month
