Re: [libvirt] how to control usb read write perm
On 04/25/2013 11:15 AM, yue wrote: hi,all i have 2 questions. both is related to spice usb redirect. 1. if i can control RW perm of usb device which is producted via spice ? in order to control user's rw operation in guestVM . AFAIK currently, there is no such control on Read and Write permission. But you can use the following redirection filter to limit usb devices usbdev is a whitelist of filter policies. redirfilter usbdev class='0x08' vendor='0x1234' product='0xbeef' version='2.00' allow='yes'/ usbdev allow='no'/ /redirfilter 2.if user have several usbs, and plug them at the same time when using spice client. if i can control which usb(s) can be accessed by user in guestVM? The virtual machine need to add one or more USB redirection channelsredirdev first in to domain XML, and these filter policies apply to them globally. For example: devices ... redirdev bus='usb' type='spicevmc' address type='usb' bus='0' port='2'/ /redirdev redirfilter usbdev vendor='0x1234' product='0xbeef' allow='yes'/ /redirfilter ... /devices About details see :http://www.libvirt.org/formatdomain.html#elementsRedir Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 1/1] qemu: Add command line builder and parser for NVRAM.
From: Li Zhang zhlci...@linux.vnet.ibm.com
This patch is to add command line builder and parser
for NVRAM device, and add test cases.
Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
v6 - v5:
* Add test cases data files.
src/qemu/qemu_capabilities.c | 3 +
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_command.c| 88 +-
tests/qemuargv2xmltest.c | 2 +
.../qemuxml2argv-pseries-nvram.args| 5 ++
.../qemuxml2argv-pseries-nvram.xml | 23 ++
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmltest.c| 2 +
8 files changed, 123 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ef291c0..1d54477 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -220,6 +220,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
machine-usb-opt,
tpm-passthrough,
tpm-tis,
+
+ nvram, /* 140 */
);
struct _virQEMUCaps {
@@ -1347,6 +1349,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ virtio-rng-ccw, QEMU_CAPS_DEVICE_VIRTIO_RNG },
{ rng-random, QEMU_CAPS_OBJECT_RNG_RANDOM },
{ rng-egd, QEMU_CAPS_OBJECT_RNG_EGD },
+{ spapr-nvram, QEMU_CAPS_DEVICE_NVRAM },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 4e76799..85f47c4 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -179,6 +179,8 @@ enum virQEMUCapsFlags {
QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 138, /* -tpmdev passthrough */
QEMU_CAPS_DEVICE_TPM_TIS = 139, /* -device tpm_tis */
+QEMU_CAPS_DEVICE_NVRAM = 140, /*-global spapr-nvram.reg=*/
+
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 009d42d..41b8d78 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -53,6 +53,10 @@
#define VIR_FROM_THIS VIR_FROM_QEMU
+#define VIO_ADDR_NET 0x1000ul
+#define VIO_ADDR_SCSI 0x2000ul
+#define VIO_ADDR_SERIAL 0x3000ul
+#define VIO_ADDR_NVRAM 0x3000ul
VIR_ENUM_DECL(virDomainDiskQEMUBus)
VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST,
@@ -1148,7 +1152,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
STREQ(def-nets[i]-model, spapr-vlan))
def-nets[i]-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
if (qemuAssignSpaprVIOAddress(def, def-nets[i]-info,
- 0x1000ul) 0)
+ VIO_ADDR_NET) 0)
goto cleanup;
}
@@ -1163,7 +1167,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
def-controllers[i]-info.type =
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
if (qemuAssignSpaprVIOAddress(def, def-controllers[i]-info,
- 0x2000ul) 0)
+ VIO_ADDR_SCSI) 0)
goto cleanup;
}
@@ -1173,7 +1177,16 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr
def,
STREQ(def-os.machine, pseries))
def-serials[i]-info.type =
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
if (qemuAssignSpaprVIOAddress(def, def-serials[i]-info,
- 0x3000ul) 0)
+ VIO_ADDR_SERIAL) 0)
+goto cleanup;
+}
+
+if (def-nvram) {
+if (def-os.arch == VIR_ARCH_PPC64
+STREQ(def-os.machine, pseries))
+def-nvram-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
+if (qemuAssignSpaprVIOAddress(def, def-nvram-info,
+ VIO_ADDR_NVRAM) 0)
goto cleanup;
}
@@ -3969,6 +3982,32 @@ error:
return NULL;
}
+static char *
+qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+if (dev-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO
+dev-info.addr.spaprvio.has_reg) {
+virBufferAsprintf(buf, spapr-nvram.reg=0x%llx,
+ dev-info.addr.spaprvio.reg);
+} else {
+virReportError(VIR_ERR_XML_ERROR,
+ %s, _(NVRAM address only can be spaprvio
currently.\n));
+goto error;
+}
+
+if (virBufferError(buf)) {
+virReportOOMError();
+goto error;
+}
+
+return virBufferContentAndReset(buf);
+
+error:
+
Re: [libvirt] [PATCHv2] doc: Clarify usage of SELinux baselabel
On 04/25/13 00:22, Eric Blake wrote: On 04/24/2013 12:02 PM, Peter Krempa wrote: State what fields are used when generating SELinux labels from a baselabel. --- Notes: Version 2: - add reference to example docs/formatdomain.html.in | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) ACK. Pushed. Thanks. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 1/1] qemu: Add command line builder and parser for NVRAM.
On 25/04/13 16:46, Li Zhang wrote: From: Li Zhangzhlci...@linux.vnet.ibm.com This patch is to add command line builder and parser for NVRAM device, and add test cases. Signed-off-by: Li Zhangzhlci...@linux.vnet.ibm.com --- v6 - v5: * Add test cases data files. Pushed with 1/2 together, with the attached diffs squashed in, one would be glad if you can apply diff yourself in though. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Virtual machine using the encrypted image fail to migrate by libvirt
The qemu-kvm and libvirt migration process is as follows:
e.g: migrate from VM1 to VM2
(1)virtual machine migration process by qemu-kvm
(qemu-kvm-0.12.1.2-2.209.el6)is as follows:
==VM1
1.set password
2.continue
==VM2
3.start and wait for migration(--incoming)
==VM1
4.migrating
5.migrate finish
6.close VM1
==VM2
7.clear password
8.set password
9.continue
(2)virtual machine migration process by libvirt is as follows:
==VM1
1.set password
2.continue
==VM2
3.start (--incoming)
4.set password
5.wait for migration
==VM1
6.migrating
7.migrate finish
8.close VM1
==VM2
9.clear password (here we lost the password !!)
10.continue
The migration process by libvirt will cause the migrate fail, because qemu-kvm
will clear password before it continues. So, I move step 4 just before step 10.
Here is the patch:
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6ad1c30..8cb8fdc 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -169,6 +169,10 @@ struct _qemuMigrationCookie {
qemuMigrationCookieNBDPtr nbd;
};
+extern int qemuProcessInitPasswords(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ virDomainObjPtr vm);
+
static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr
grap)
{
if (!grap)
@@ -4002,6 +4006,10 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
* = 0.10.6 to work properly. This isn't strictly necessary on
* older qemu's, but it also doesn't hurt anything there
*/
+/* we will set password here */
+if (qemuProcessInitPasswords(dconn, driver, vm) 0)
+virReportError(VIR_ERR_INTERNAL_ERROR,%s, _(init passwords
failed));
+
if (qemuProcessStartCPUs(driver, vm, dconn,
VIR_DOMAIN_RUNNING_MIGRATED,
QEMU_ASYNC_JOB_MIGRATION_IN) 0) {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 925939d..093e638 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -73,6 +73,11 @@
#define ATTACH_POSTFIX : attaching\n
#define SHUTDOWN_POSTFIX : shutting down\n
+int qemuProcessInitPasswords(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ virDomainObjPtr vm);
+
+
/**
* qemuProcessRemoveDomainStatus
*
@@ -1981,8 +1986,7 @@ qemuProcessSetEmulatorAffinities(virConnectPtr conn
ATTRIBUTE_UNUSED,
return ret;
}
-static int
-qemuProcessInitPasswords(virConnectPtr conn,
+int qemuProcessInitPasswords(virConnectPtr conn,
virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
@@ -3828,8 +3832,11 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
VIR_DEBUG(Setting any required VM passwords);
-if (qemuProcessInitPasswords(conn, driver, vm) 0)
-goto cleanup;
+/* if it is migration , we will not set password here */
+if (!migrateFrom){
+if (qemuProcessInitPasswords(conn, driver, vm) 0)
+ goto cleanup;
+ }
/* If we have -device, then addresses are assigned explicitly.
* If not, then we have to detect dynamic ones here */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 5/5] qemu: auto-add bridges and allow using them
On 04/25/2013 02:39 AM, Eric Blake wrote:
On 04/23/2013 06:47 AM, Ján Tomko wrote:
@@ -1326,15 +1368,53 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
qemuDomainObjPrivatePtr priv = NULL;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
+int max_idx = -1;
So let's trace what happens if I have XML with no controller but I do
use 33 PCI devices and have a capable qemu:
max_idx starts at -1,
int nbuses = 0;
int i;
+int rv;
for (i = 0; i def-ncontrollers; i++) {
-if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI)
-nbuses++;
+if (def-controllers[i]-type ==
VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
+if (def-controllers[i]-idx max_idx)
+max_idx = def-controllers[i]-idx;
+}
+}
If no controllers were specified, it is still at -1,
+
+nbuses = max_idx + 1;
so nbuses is now 0,
+
+if (nbuses 0
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
therefore we skip this if,
+virDomainDeviceInfo info;
+/* 1st pass to figure out how many PCI bridges we need */
+if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, true)))
+goto cleanup;
+if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) 0)
+goto cleanup;
+/* Reserve 1 extra slot for a (potential) bridge */
+if (qemuDomainPCIAddressSetNextAddr(addrs, info) 0)
+goto cleanup;
+
+for (i = 1; i addrs-nbuses; i++) {
+if ((rv = virDomainDefMaybeAddController(
+def, VIR_DOMAIN_CONTROLLER_TYPE_PCI,
+i, VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE)) 0)
+goto cleanup;
+/* If we added a new bridge, we will need one more address
*/
+if (rv 0 qemuDomainPCIAddressSetNextAddr(addrs, info)
0)
+goto cleanup;
+}
+nbuses = addrs-nbuses;
+qemuDomainPCIAddressSetFree(addrs);
+addrs = NULL;
+
+} else if (max_idx 0) {
we don't error out,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(PCI bridges are not supported
+ by this QEMU binary));
+goto cleanup;
}
but we also didn't auto-instantiate any bridges, even if the capability
is supported. Is that a problem?
No, if the machine has no buses, we would have no place to put the
bridge in. (Unless it's a PCI express machine, but libvirt doesn't know
how to use those yet)
And we'll error out anyway with:
error: XML error: No PCI buses available
either in GetNextSlot called by AssignPCISlots for devices without an
address, or in PCIAddressValidate called by CollectPCIAddress for
explicitly specified PCI addresses.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 1/1] qemu: Add command line builder and parser for NVRAM.
On 2013年04月25日 16:59, Osier Yang wrote: On 25/04/13 16:46, Li Zhang wrote: From: Li Zhangzhlci...@linux.vnet.ibm.com This patch is to add command line builder and parser for NVRAM device, and add test cases. Signed-off-by: Li Zhangzhlci...@linux.vnet.ibm.com --- v6 - v5: * Add test cases data files. Pushed with 1/2 together, with the attached diffs squashed in, one would be glad if you can apply diff yourself in though. Thanks a lot, Osier. :) Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test: Add JSON test for query-tpm-types
On 04/22/2013 08:53 AM, Stefan Berger wrote: Add a test case for query-tpm-models QMP command. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com Any comments on this patch? Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/5] conf: add PCI controllers
On 04/22/2013 10:11 PM, Laine Stump wrote: On 04/22/2013 02:43 PM, Ján Tomko wrote: --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2124,7 +2124,7 @@ p Each controller has a mandatory attribute codetype/code, which must be one of ide, fdc, scsi, sata, usb, - ccid, or virtio-serial, and a mandatory + ccid, virtio-serial or pci, and a mandatory attribute codeindex/code which is the decimal integer describing in which order the bus controller is encountered (for use in codecontroller/code attributes @@ -2177,6 +2177,26 @@ lt;/devicesgt; .../pre +p + PCI controllers have an optional codemodel/code attribute with + possible values codepci-root/code or codepci-bridge/code. + For machine types which provide an implicit pci bus, the pci-root + controller with index=0 is auto-added and required to use PCI devices. + PCI root has no address. + PCI bridges are auto-added if there are too many devices to fit on + the one bus provided by pci-root, or a PCI bus number greater than zero + was specified. (span class=sincesince 1.0.5/span) Just so that it's clear that it's not automatic-only, you should also say something like a pci-bridge device can be manually added in the domain's configuration, but care should be taken to not have any gaps in the sequence of index attributes when there are multiple pci controllers. Gaps in the indexes might work, as long as the bridges don't reference unspecified buses. I'll be squashing this in before pushing: diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index bd4b77c..0c0506b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2189,7 +2189,12 @@ PCI root has no address. PCI bridges are auto-added if there are too many devices to fit on the one bus provided by pci-root, or a PCI bus number greater than zero - was specified. (span class=sincesince 1.0.5/span) + was specified. + PCI bridges can also be specified manually, but their addresses should + only refer to PCI buses provided by already specified PCI controllers. + Leaving gaps in the PCI controller indexes might lead to an invalid + configuration. + (span class=sincesince 1.0.5/span) /p pre ... ACK. Thanks, Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/5] qemu: auto-add pci-root controller for pc machine types
On 04/22/2013 10:37 PM, Laine Stump wrote:
On 04/22/2013 02:43 PM, Ján Tomko wrote:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a7aabdf..ab99538 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -673,6 +673,37 @@ qemuDomainDefPostParse(virDomainDefPtr def,
!(def-emulator = virDomainDefGetDefaultEmulator(def, caps)))
return -1;
+/* Add implicit PCI root controller if the machine has one */
+switch (def-os.arch) {
+case VIR_ARCH_I686:
+case VIR_ARCH_X86_64:
+if (!def-os.machine)
+break;
+if (STRPREFIX(def-os.machine, pc-q35) ||
+STREQ(def-os.machine, q35) ||
+STREQ(def-os.machine, isapc))
+break;
+if (!STRPREFIX(def-os.machine, pc-0.)
+!STRPREFIX(def-os.machine, pc-1.)
+!STREQ(def-os.machine, pc)
+!STRPREFIX(def-os.machine, rhel))
+break;
If you're going to fall through to a different case like this, you
should put a comment in the code something like this:
/* FALL THROUGH TO NEXT CASE */
just so people don't have to think too hard :-)
However, I think it would be more easily expandable in the future if you
had a straight switch statement with all the cases, and just set a
needsPCIRoot boolean for those cases that need it, then an if
(needsPCIRoot) at the end. In the future when we want to add other
implicit devices, each case can be a mix of the appropriate needsThis
and needsThat, with the actual additions at the end.
...
ACK, with the addition of the FALLTHROUGH comment, or restructuring it
is as I suggested.
I'm squashing this in before pushing:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ab99538..98ac56f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -668,6 +668,8 @@ qemuDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps,
void *opaque ATTRIBUTE_UNUSED)
{
+bool addPCIRoot = false;
+
/* check for emulator and create a default one if needed */
if (!def-emulator
!(def-emulator = virDomainDefGetDefaultEmulator(def, caps)))
@@ -688,6 +690,8 @@ qemuDomainDefPostParse(virDomainDefPtr def,
!STREQ(def-os.machine, pc)
!STRPREFIX(def-os.machine, rhel))
break;
+addPCIRoot = true;
+break;
case VIR_ARCH_ALPHA:
case VIR_ARCH_PPC:
@@ -695,15 +699,18 @@ qemuDomainDefPostParse(virDomainDefPtr def,
case VIR_ARCH_PPCEMB:
case VIR_ARCH_SH4:
case VIR_ARCH_SH4EB:
-if (virDomainDefMaybeAddController(
-def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
-VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) 0)
-return -1;
+addPCIRoot = true;
break;
default:
break;
}
+if (addPCIRoot
+virDomainDefMaybeAddController(
+def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
+VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) 0)
+return -1;
+
return 0;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix usb master startport parsing
When all usb controllers connected to the same bus have master
startport='x'/ specified, none of them have 'id=usb' assigned and
thus qemu fails due to invalid masterport specification (we use 'usb'
for that purpose). Adding a check that at least one of the
controllers is specified without master startport='x'/ and in case
this happens, don't error out, just emit a warning and fix it within
the first such controller found. This makes UX better by not forcing
them to fix controller definition after removing the only non-master
usb controller.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c | 17 ++---
.../qemuxml2argv-usb-ich9-no-companion.args | 6 ++
.../qemuxml2argv-usb-ich9-no-companion.xml | 21 +
tests/qemuxml2argvtest.c| 3 +++
4 files changed, 44 insertions(+), 3 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 958b77b..a948cfc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9912,7 +9912,8 @@ virDomainDefParseXML(xmlDocPtr xml,
virHashTablePtr bootHash = NULL;
xmlNodePtr cur;
bool usb_none = false;
-bool usb_other = false;
+virDomainControllerDefPtr usb_first = NULL;
+bool usb_master = false;
bool primaryVideo = false;
if (VIR_ALLOC(def) 0) {
@@ -10855,7 +10856,7 @@ virDomainDefParseXML(xmlDocPtr xml,
/* sanitize handling of none usb controller */
if (controller-type == VIR_DOMAIN_CONTROLLER_TYPE_USB) {
if (controller-model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
-if (usb_other || usb_none) {
+if (usb_first || usb_none) {
virDomainControllerDefFree(controller);
virReportError(VIR_ERR_XML_DETAIL, %s,
_(Can't add another USB controller:
@@ -10871,14 +10872,24 @@ virDomainDefParseXML(xmlDocPtr xml,
USB is disabled for this domain));
goto error;
}
-usb_other = true;
+usb_first = controller;
}
+
+if (controller-info.mastertype ==
VIR_DOMAIN_CONTROLLER_MASTER_NONE)
+usb_master = true;
}
virDomainControllerInsertPreAlloced(def, controller);
}
VIR_FREE(nodes);
+if (usb_first !usb_master) {
+VIR_WARN(No usb controller specified without master startport,
+ omitting startport in first USB controller);
+usb_first-info.master.usb.startport = 0;
+usb_first-info.mastertype = VIR_DOMAIN_CONTROLLER_MASTER_NONE;
+}
+
if (def-virtType == VIR_DOMAIN_VIRT_QEMU ||
def-virtType == VIR_DOMAIN_VIRT_KQEMU ||
def-virtType == VIR_DOMAIN_VIRT_KVM)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.args
b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.args
new file mode 100644
index 000..c97a6d3
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.args
@@ -0,0 +1,6 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M
pc \
+-m 214 -smp 1 -nographic -nodefconfig -nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \
+-device ich9-usb-uhci2,id=usb,bus=pci.0,addr=0x4 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml
b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml
new file mode 100644
index 000..895d932
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml
@@ -0,0 +1,21 @@
+domain type='qemu'
+ nameQEMUGuest1/name
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+ memory unit='KiB'219136/memory
+ currentMemory unit='KiB'219136/currentMemory
+ vcpu placement='static'1/vcpu
+ os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+ /os
+ devices
+emulator/usr/bin/qemu/emulator
+controller type='usb' index='0' model='ich9-uhci2'
+ master startport='2'/
+ address type='pci' domain='0x' bus='0x00' slot='0x04' function='0'/
+/controller
+memballoon model='virtio'
+ address type='pci' domain='0x' bus='0x00' slot='0x03' function='0'/
+/memballoon
+ /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f40d002..e485a70 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -768,6 +768,9 @@ mymain(void)
DO_TEST(usb-ich9-companion,
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
Re: [libvirt] [PATCH] test: Add JSON test for query-tpm-types
On Mon, Apr 22, 2013 at 08:53:24AM -0400, Stefan Berger wrote:
Add a test case for query-tpm-models QMP command.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
tests/qemumonitorjsontest.c | 55
1 file changed, 55 insertions(+)
Index: libvirt/tests/qemumonitorjsontest.c
===
--- libvirt.orig/tests/qemumonitorjsontest.c
+++ libvirt/tests/qemumonitorjsontest.c
@@ -25,6 +25,7 @@
#include qemu/qemu_conf.h
#include virthread.h
#include virerror.h
+#include virstring.h
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -440,6 +441,59 @@ cleanup:
static int
+testQemuMonitorJSONGetTPMModels(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
Don't cast 'void *'
+qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt);
+int ret = -1;
+char **tpmmodels = NULL;
+int ntpmmodels = 0;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddItem(test, query-tpm-models,
+ {
+ \return\: [
+ \passthrough\
+ ]
+ }) 0)
+goto cleanup;
+
+if ((ntpmmodels =
qemuMonitorGetTPMModels(qemuMonitorTestGetMonitor(test),
Whitespace damaged patch
+ tpmmodels)) 0)
+goto cleanup;
+
+if (ntpmmodels != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ ntpmmodels %d is not 1, ntpmmodels);
+goto cleanup;
+}
+
+#define CHECK(i, wantname) \
+do {\
+if (STRNEQ(tpmmodels[i], (wantname))) {
\
More damage
+ virReportError(VIR_ERR_INTERNAL_ERROR, \
+ name %s is not %s, \
+ tpmmodels[i], (wantname));
\
More damage
+goto cleanup; \
+ } \
+} while (0)
+
+CHECK(0, passthrough);
+
+#undef CHECK
+
+ret = 0;
+
+cleanup:
+qemuMonitorTestFree(test);
+virStringFreeList(tpmmodels);
+return ret;
+}
Please use git-send email in future - your mail program is trashing
the formatting which makes reviewing more painful.
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 usb master startport parsing
On Thu, Apr 25, 2013 at 01:05:49PM +0200, Martin Kletzander wrote: When all usb controllers connected to the same bus have master startport='x'/ specified, none of them have 'id=usb' assigned and thus qemu fails due to invalid masterport specification (we use 'usb' for that purpose). Adding a check that at least one of the controllers is specified without master startport='x'/ and in case this happens, don't error out, just emit a warning and fix it within the first such controller found. This makes UX better by not forcing them to fix controller definition after removing the only non-master usb controller. No, using VIR_WARN in the parser is a really bad. This is a broken configuration and should be reported as such. It is not the responsibility of libvirt to workaround apps generating broken configs. if the user removes the master controller, then the app should be removing any non-master controllers that depend on it. new file mode 100644 index 000..895d932 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml @@ -0,0 +1,21 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + devices +emulator/usr/bin/qemu/emulator +controller type='usb' index='0' model='ich9-uhci2' + master startport='2'/ + address type='pci' domain='0x' bus='0x00' slot='0x04' function='0'/ +/controller This is just a broken configuration since there is no master controller to assocated with the companion with. 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] qemu_conf: Don't discard strdup OOM error
After 78d7c3c5 we are strdup()-ing path to qemu-bridge-helper. However, the check for its return value is missing. So it is possible we've ignored the OOM error silently. --- src/qemu/qemu_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e9a3407..7c3f317 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -241,7 +241,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) } } #endif -cfg-bridgeHelperName = strdup(/usr/libexec/qemu-bridge-helper); +if (!(cfg-bridgeHelperName = strdup(/usr/libexec/qemu-bridge-helper))) +goto no_memory; cfg-clearEmulatorCapabilities = true; -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: reject controllers with duplicate indexes
Reject multiple controllers with the same index,
except for USB controllers.
Multi-function USB controllers can have the same index.
---
src/conf/domain_conf.c | 59 ++
1 file changed, 59 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cb69178..b6323fd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2594,6 +2594,63 @@ virDomainDeviceInfoIterate(virDomainDefPtr def,
static int
+virDomainDefRejectDuplicateControllers(virDomainDefPtr def)
+{
+int max_idx[VIR_DOMAIN_CONTROLLER_TYPE_LAST];
+virBitmapPtr bitmaps[VIR_DOMAIN_CONTROLLER_TYPE_LAST] = { NULL };
+virDomainControllerDefPtr cont;
+size_t nbitmaps = 0;
+int ret = -1;
+bool b;
+int i;
+
+memset(max_idx, -1, sizeof(max_idx)/sizeof(max_idx[0]));
+
+for (i = 0; i def-ncontrollers; i++) {
+cont = def-controllers[i];
+if (cont-idx max_idx[cont-type])
+max_idx[cont-type] = cont-idx;
+}
+
+/* multiple USB controllers with the same index are allowed */
+max_idx[VIR_DOMAIN_CONTROLLER_TYPE_USB] = -1;
+
+for (i = 0; i VIR_DOMAIN_CONTROLLER_TYPE_LAST; i++) {
+if (max_idx[i] = 0 !(bitmaps[i] = virBitmapNew(max_idx[i] + 1)))
+goto no_memory;
+nbitmaps++;
+}
+
+for (i = 0; i def-ncontrollers; i++) {
+cont = def-controllers[i];
+
+if (max_idx[cont-type] == -1)
+continue;
+
+ignore_value(virBitmapGetBit(bitmaps[cont-type], cont-idx, b));
+if (b) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Multiple '%s' controllers with index '%d'),
+ virDomainControllerTypeToString(cont-type),
+ cont-idx);
+goto cleanup;
+}
+ignore_value(virBitmapSetBit(bitmaps[cont-type], cont-idx));
+}
+
+ret = 0;
+cleanup:
+for (i = 0; i nbitmaps; i++)
+virBitmapFree(bitmaps[i]);
+return ret;
+
+no_memory:
+virReportOOMError();
+goto cleanup;
+}
+
+
+static int
virDomainDefPostParseInternal(virDomainDefPtr def,
virCapsPtr caps ATTRIBUTE_UNUSED)
{
@@ -2673,6 +2730,8 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
}
}
+if (virDomainDefRejectDuplicateControllers(def) 0)
+return -1;
return 0;
no_memory:
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] qemu: add PCI bridge support
On 04/22/2013 08:43 PM, Ján Tomko wrote: Add new 'pci' controller type with two models: pci-root - auto-added to machines with implicit pci bus pci-bridge - auto-added if the devices would not leave at least one slot empty on bus 0 or bus 0 is specified v3: moved the implicit PCI root addition to qemu's post parse callback, added an xml - xml test and schema validation rewrote implicit controller removal and search for free slots check for multiple pci controllers with the same index added documentation Ján Tomko (4): qemu: call post-parse callbacks when parsing command line too conf: add PCI controllers qemu: auto-add pci-root controller for pc machine types qemu: auto-add bridges and allow using them liguang (1): qemu: build command line for pci-bridge device I've pushed patches 1-5 and sent another version of patch 6 - conf: reject controllers with duplicate indexes separately. Thank you for your reviews. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_conf: Don't discard strdup OOM error
On 25/04/13 19:37, Michal Privoznik wrote: After 78d7c3c5 we are strdup()-ing path to qemu-bridge-helper. However, the check for its return value is missing. So it is possible we've ignored the OOM error silently. --- src/qemu/qemu_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e9a3407..7c3f317 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -241,7 +241,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) } } #endif -cfg-bridgeHelperName = strdup(/usr/libexec/qemu-bridge-helper); +if (!(cfg-bridgeHelperName = strdup(/usr/libexec/qemu-bridge-helper))) +goto no_memory; cfg-clearEmulatorCapabilities = true; ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Build failed in Jenkins: libvirt-syntax-check #905
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/905/
--
Started by upstream project libvirt-build build number 1011
Building in workspace
http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/ws/
[workspace] $ /bin/sh -xe /tmp/hudson3421895490898657872.sh
+ make syntax-check
GENbracket-spacing-check
src/qemu/qemu_command.c:1637: for ( ; a.slot
QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
maint.mk: incorrect whitespace around brackets, see HACKING for rules
make: *** [bracket-spacing-check] Error 1
Build step 'Execute shell' marked build as failure
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu_command.c: Fix whitespacing within for()
After 9d6e56db the syntax-check was unhappy due to wrong whitespacing:
src/qemu/qemu_command.c:1637: for ( ; a.slot QEMU_PCI_ADDRESS_SLOT_LAST;
a.slot++) {
maint.mk: incorrect whitespace around brackets, see HACKING for rules
make: *** [bracket-spacing-check] Error 1
---
Pushed under trivial rule.
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 50281be..1c48cbb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1634,7 +1634,7 @@
qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
/* Start the search at the last used bus and slot */
for (a.slot++; a.bus addrs-nbuses; a.bus++) {
-for ( ; a.slot QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
+for (; a.slot QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
if (!qemuDomainPCIAddressSlotInUse(addrs, a))
goto success;
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Fix usb master startport parsing
When all usb controllers connected to the same bus have master
startport='x'/ specified, none of them have 'id=usb' assigned and
thus qemu fails due to invalid masterport specification (we use 'usb'
for that purpose). Adding a check that at least one of the
controllers is specified without master startport='x'/ and in case
this happens, error out due to invalid configuration.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
v2:
- Don't fix user/app errors, just error out.
---
src/conf/domain_conf.c | 10 ++
.../qemuxml2argv-usb-ich9-no-companion.xml | 21 +
tests/qemuxml2argvtest.c| 3 +++
3 files changed, 34 insertions(+)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 958b77b..99d3232 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9913,6 +9913,7 @@ virDomainDefParseXML(xmlDocPtr xml,
xmlNodePtr cur;
bool usb_none = false;
bool usb_other = false;
+bool usb_master = false;
bool primaryVideo = false;
if (VIR_ALLOC(def) 0) {
@@ -10873,12 +10874,21 @@ virDomainDefParseXML(xmlDocPtr xml,
}
usb_other = true;
}
+
+if (controller-info.mastertype ==
VIR_DOMAIN_CONTROLLER_MASTER_NONE)
+usb_master = true;
}
virDomainControllerInsertPreAlloced(def, controller);
}
VIR_FREE(nodes);
+if (usb_first !usb_master) {
+virReportError(VIR_ERR_XML_DETAIL, %s,
+ _(No master USB controller specified));
+goto error;
+}
+
if (def-virtType == VIR_DOMAIN_VIRT_QEMU ||
def-virtType == VIR_DOMAIN_VIRT_KQEMU ||
def-virtType == VIR_DOMAIN_VIRT_KVM)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml
b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml
new file mode 100644
index 000..895d932
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml
@@ -0,0 +1,21 @@
+domain type='qemu'
+ nameQEMUGuest1/name
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+ memory unit='KiB'219136/memory
+ currentMemory unit='KiB'219136/currentMemory
+ vcpu placement='static'1/vcpu
+ os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+ /os
+ devices
+emulator/usr/bin/qemu/emulator
+controller type='usb' index='0' model='ich9-uhci2'
+ master startport='2'/
+ address type='pci' domain='0x' bus='0x00' slot='0x04' function='0'/
+/controller
+memballoon model='virtio'
+ address type='pci' domain='0x' bus='0x00' slot='0x03' function='0'/
+/memballoon
+ /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f40d002..abb3b27 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -768,6 +768,9 @@ mymain(void)
DO_TEST(usb-ich9-companion,
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1);
+DO_TEST_FAILURE(usb-ich9-no-companion,
+QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
+QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1);
DO_TEST(usb-hub,
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_USB_HUB,
QEMU_CAPS_NODEFCONFIG);
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 5/5] qemu: auto-add bridges and allow using them
On 22.04.2013 20:43, Ján Tomko wrote:
Add a dry run address allocation to figure out how many bridges
will be needed for all the devices without explicit addresses.
Auto-add just enough bridges to put all the devices on, or up to the
bridge with the largest specified index.
---
src/conf/domain_conf.c | 26 ++-
src/qemu/qemu_command.c| 211
+
src/qemu/qemu_command.h| 4 +-
tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml | 210
tests/qemuxml2xmltest.c| 1 +
5 files changed, 411 insertions(+), 41 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3787ff1..ec7d0e6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1519,41 +1609,58 @@ void
qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs)
VIR_FREE(addrs);
}
-
static int
qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
virDevicePCIAddressPtr next_addr)
{
-virDevicePCIAddress tmp_addr = addrs-lastaddr;
-int i;
-char *addr;
+virDevicePCIAddress a = addrs-lastaddr;
-tmp_addr.slot++;
-for (i = 0; i QEMU_PCI_ADDRESS_SLOT_LAST; i++, tmp_addr.slot++) {
-if (QEMU_PCI_ADDRESS_SLOT_LAST = tmp_addr.slot) {
-tmp_addr.slot = 0;
-}
+if (addrs-nbuses == 0) {
+virReportError(VIR_ERR_XML_ERROR, %s, _(No PCI buses available));
+return -1;
+}
-if (!(addr = qemuPCIAddressAsString(tmp_addr)))
-return -1;
+/* Start the search at the last used bus and slot */
+for (a.slot++; a.bus addrs-nbuses; a.bus++) {
+for ( ; a.slot QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
I know I am too late, but this causes 'make syntax-check' fail. I've
just pushed trivial patch for that.
+if (!qemuDomainPCIAddressSlotInUse(addrs, a))
+goto success;
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] qemu: Split out code to generate VNC command line
On Tue, Apr 23, 2013 at 03:46:09PM +0200, Peter Krempa wrote:
Decrease size of qemuBuildGraphicsCommandLine() by splitting out
spice-related code into qemuBuildGraphicsVNCCommandLine().
I guess you mean VNC-related code here.
Christophe
This patch also fixes 2 possible memory leaks on error path in the code
that was split-out. The buffer containing the already generated options
and a listen address string could be leaked.
Also break a few very long lines and reflow code that fits now.
---
src/qemu/qemu_command.c | 244
+---
1 file changed, 125 insertions(+), 119 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d6d782c..66b2ba8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5419,6 +5419,130 @@ cleanup:
static int
+qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
+virCommandPtr cmd,
+virDomainDefPtr def,
+virQEMUCapsPtr qemuCaps,
+virDomainGraphicsDefPtr graphics)
+{
+virBuffer opt = VIR_BUFFER_INITIALIZER;
+const char *listenNetwork;
+const char *listenAddr = NULL;
+char *netAddr = NULL;
+bool escapeAddr;
+int ret;
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(vnc graphics are not supported with this QEMU));
+goto error;
+}
+
+if (graphics-data.vnc.socket || cfg-vncAutoUnixSocket) {
+if (!graphics-data.vnc.socket
+virAsprintf(graphics-data.vnc.socket,
+%s/%s.vnc, cfg-libDir, def-name) == -1)
+goto no_memory;
+
+virBufferAsprintf(opt, unix:%s, graphics-data.vnc.socket);
+
+} else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
+switch (virDomainGraphicsListenGetType(graphics, 0)) {
+case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
+listenAddr = virDomainGraphicsListenGetAddress(graphics, 0);
+break;
+
+case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
+listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0);
+if (!listenNetwork)
+break;
+ret = networkGetNetworkAddress(listenNetwork, netAddr);
+if (ret = -2) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s, _(network-based listen not possible,
+ network driver not present));
+goto error;
+}
+if (ret 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(listen network '%s' had no usable
address),
+ listenNetwork);
+goto error;
+}
+listenAddr = netAddr;
+/* store the address we found in the graphics element so it
will
+ * show up in status. */
+if (virDomainGraphicsListenSetAddress(graphics, 0,
+ listenAddr, -1, false) 0)
+ goto error;
+break;
+}
+
+if (!listenAddr)
+listenAddr = cfg-vncListen;
+
+escapeAddr = strchr(listenAddr, ':') != NULL;
+if (escapeAddr)
+virBufferAsprintf(opt, [%s], listenAddr);
+else
+virBufferAdd(opt, listenAddr, -1);
+virBufferAsprintf(opt, :%d,
+ graphics-data.vnc.port - 5900);
+
+VIR_FREE(netAddr);
+} else {
+virBufferAsprintf(opt, %d,
+ graphics-data.vnc.port - 5900);
+}
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
+if (graphics-data.vnc.auth.passwd || cfg-vncPassword)
+virBufferAddLit(opt, ,password);
+
+if (cfg-vncTLS) {
+virBufferAddLit(opt, ,tls);
+if (cfg-vncTLSx509verify)
+virBufferAsprintf(opt, ,x509verify=%s,
cfg-vncTLSx509certdir);
+else
+virBufferAsprintf(opt, ,x509=%s, cfg-vncTLSx509certdir);
+}
+
+if (cfg-vncSASL) {
+virBufferAddLit(opt, ,sasl);
+
+if (cfg-vncSASLdir)
+virCommandAddEnvPair(cmd, SASL_CONF_DIR, cfg-vncSASLdir);
+
+/* TODO: Support ACLs later */
+}
+}
+
+virCommandAddArg(cmd, -vnc);
+virCommandAddArgBuffer(cmd, opt);
+if (graphics-data.vnc.keymap)
+virCommandAddArgList(cmd, -k, graphics-data.vnc.keymap, NULL);
+
+/* Unless user requested it, set the audio backend to none, to
+ * prevent it opening the host OS audio devices, since that causes
+ * security issues and might not work when using VNC.
+ */
[libvirt] Jenkins build is back to normal : libvirt-syntax-check #906
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/906/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] fix typo introduced by 90430791
From: Bamvor Jian Zhang bamv2...@gmail.com
Signed-off-by: Bamvor Jian Zhang bjzh...@suse.com
---
src/node_device/node_device_hal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index 4a430dc..63245a9 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -764,7 +764,7 @@ static int nodeDeviceClose(virConnectPtr conn
ATTRIBUTE_UNUSED)
static virNodeDeviceDriver halNodeDeviceDriver = {
.name = halNodeDeviceDriver,
.nodeDeviceOpen = nodeDeviceOpen, /* 0.5.0 */
-.nodDeviceClose = nodDeviceClose, /* 0.5.0 */
+.nodeDeviceClose = nodeDeviceClose, /* 0.5.0 */
.nodeNumOfDevices = nodeNumOfDevices, /* 0.5.0 */
.nodeListDevices = nodeListDevices, /* 0.5.0 */
.connectListAllNodeDevices = nodeListAllNodeDevices, /* 0.10.2 */
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] New CPU hotplug APIs
Hi upstream, I'd like to discuss the design of CPU modification related APIs before I start working on them. Qemu recently added support for modification of the state of the cpu using the guest agent and is also doing work on hot plug of cpus and possibly even hot unplug. This stuff will require us to introduce new APIs to take full advantage of the new code for qemu. I'm imagining adding 3 new API functions: 1 universal getter function and 2 specific setters: one for agent based modifications and one for classic (ACPI) cpu hotplug. 1) The getter function: int virDomainGetVCPUMap(virDomainPtr dom, const char **cpumap, unsigned int flags); With no flags, this function would return the map of online CPUs in the guest. The flags will allow us then to do: VIR_DOMAIN_VCPU_MAP_AGENT_ONLINE - map of online cpus as the guest agent sees it VIR_DOMAIN_VCPU_MAP_AGENT_OFFLINABLE - map of online cpus that can be turned off VIR_DOMAIN_VCPU_MAP_AGENT_ONLINABLE - map of offline cpus that can be turned on VIR_DOMAIN_VCPU_MAP_AGENT_POSSIBLE - all vcpus as the guest agent sees them (_AGENT_OFFLINE probably isn't useful And similarly for offline processors: VIR_DOMAIN_VCPU_MAP_ONLINE VIR_DOMAIN_VCPU_MAP_ONLINABLE VIR_DOMAIN_VCPU_MAP_POSSIBLE (no idea if offlinable makes sense here) The universal nature of this function would be documented right away and would save us having separate getters for agent based hotplug and classic one. The returned map would be automatically allocated and the length of it returned as the return value. This getter will allow us representing (possibly) sparse allocation of the cpu IDs. 2) Setters int virDomainSetVCPUState(virDomainPtr dom, int id, bool state, unsigned int flags); for classic CPU hotplug and: virDomainSetGuestVCPUState(virDomainPtr dom, int id, bool state, unsigned int flags); for agent based cpu offlining. This will represent the setter functions with similar semantics. I've gone for two to absolutely differentiate the agent based stuff from the classic one, but they could be merged into a single one with appropriate flags). These will allow modification of state of single CPUs so that errors can be handled gracefully. The id corresponds to position of the bit in the cpumap requested by the getter func described above. Thanks in advance for your input on this design stuff. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] qemu: Split out code to generate VNC command line
On 04/25/13 12:34, Christophe Fergeau wrote: On Tue, Apr 23, 2013 at 03:46:09PM +0200, Peter Krempa wrote: Decrease size of qemuBuildGraphicsCommandLine() by splitting out spice-related code into qemuBuildGraphicsVNCCommandLine(). I guess you mean VNC-related code here. Ah, yes. I borrowed the commit message from the patch moving the spice code and forgot to update that. Unfortunately I already pushed this patch. Christophe This patch also fixes 2 possible memory leaks on error path in the code that was split-out. The buffer containing the already generated options and a listen address string could be leaked. Also break a few very long lines and reflow code that fits now. --- src/qemu/qemu_command.c | 244 +--- 1 file changed, 125 insertions(+), 119 deletions(-) Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Fix usb master startport parsing
On Thu, Apr 25, 2013 at 01:59:04PM +0200, Martin Kletzander wrote: When all usb controllers connected to the same bus have master startport='x'/ specified, none of them have 'id=usb' assigned and thus qemu fails due to invalid masterport specification (we use 'usb' for that purpose). Adding a check that at least one of the controllers is specified without master startport='x'/ and in case this happens, error out due to invalid configuration. Signed-off-by: Martin Kletzander mklet...@redhat.com --- v2: - Don't fix user/app errors, just error out. --- src/conf/domain_conf.c | 10 ++ .../qemuxml2argv-usb-ich9-no-companion.xml | 21 + tests/qemuxml2argvtest.c| 3 +++ 3 files changed, 34 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml 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
Re: [libvirt] [PATCH] test: Add JSON test for query-tpm-types
On 04/25/2013 07:06 AM, Daniel P. Berrange wrote:
On Mon, Apr 22, 2013 at 08:53:24AM -0400, Stefan Berger wrote:
Add a test case for query-tpm-models QMP command.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
tests/qemumonitorjsontest.c | 55
1 file changed, 55 insertions(+)
Index: libvirt/tests/qemumonitorjsontest.c
===
--- libvirt.orig/tests/qemumonitorjsontest.c
+++ libvirt/tests/qemumonitorjsontest.c
@@ -25,6 +25,7 @@
#include qemu/qemu_conf.h
#include virthread.h
#include virerror.h
+#include virstring.h
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -440,6 +441,59 @@ cleanup:
static int
+testQemuMonitorJSONGetTPMModels(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
Don't cast 'void *'
I has to be casted due to the const. Adding const before
virDomainXMLOptionPtr xmlopt doesn't help it. Also, it's cp.
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Fix usb master startport parsing
On 04/25/2013 02:45 PM, Daniel P. Berrange wrote: On Thu, Apr 25, 2013 at 01:59:04PM +0200, Martin Kletzander wrote: When all usb controllers connected to the same bus have master startport='x'/ specified, none of them have 'id=usb' assigned and thus qemu fails due to invalid masterport specification (we use 'usb' for that purpose). Adding a check that at least one of the controllers is specified without master startport='x'/ and in case this happens, error out due to invalid configuration. Signed-off-by: Martin Kletzander mklet...@redhat.com --- v2: - Don't fix user/app errors, just error out. --- src/conf/domain_conf.c | 10 ++ .../qemuxml2argv-usb-ich9-no-companion.xml | 21 + tests/qemuxml2argvtest.c| 3 +++ 3 files changed, 34 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-no-companion.xml ACK Daniel Thanks, I've fixed up just a nit to make it build check properly and pushed. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test: Add JSON test for query-tpm-types
On Thu, Apr 25, 2013 at 08:52:40AM -0400, Stefan Berger wrote:
On 04/25/2013 07:06 AM, Daniel P. Berrange wrote:
On Mon, Apr 22, 2013 at 08:53:24AM -0400, Stefan Berger wrote:
Add a test case for query-tpm-models QMP command.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
tests/qemumonitorjsontest.c | 55
1 file changed, 55 insertions(+)
Index: libvirt/tests/qemumonitorjsontest.c
===
--- libvirt.orig/tests/qemumonitorjsontest.c
+++ libvirt/tests/qemumonitorjsontest.c
@@ -25,6 +25,7 @@
#include qemu/qemu_conf.h
#include virthread.h
#include virerror.h
+#include virstring.h
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -440,6 +441,59 @@ cleanup:
static int
+testQemuMonitorJSONGetTPMModels(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
Don't cast 'void *'
I has to be casted due to the const. Adding const before
virDomainXMLOptionPtr xmlopt doesn't help it. Also, it's cp.
Ah, ok then.
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] qemu: auto-add pci-root to 'pc-i440*' machines too
Commit b33eb0d missed this machine type. --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 98ac56f..4e88eaf 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -687,6 +687,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, break; if (!STRPREFIX(def-os.machine, pc-0.) !STRPREFIX(def-os.machine, pc-1.) +!STRPREFIX(def-os.machine, pc-i440) !STREQ(def-os.machine, pc) !STRPREFIX(def-os.machine, rhel)) break; -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: New XML to disable memory merge at guest startup
On 04/17/2013 08:54 AM, Eric Blake wrote: On 04/17/2013 08:22 AM, Osier Yang wrote: QEMU introduced command line -mem-merge=on|off (defaults to on) to enable/disable the memory merge (KSM) at guest startup. This exposes it by new XML: memoryBacking nosharepages/ /memoryBacking The XML tag is same with what we used internally for old RHEL. Good - that means that RHEL 6 (and any other downstream distro that was already borrowing the RHEL extension) will not break when rebasing to pick up this change from upstream in place of their downstream extension (RHEL will actually want to add a followup patch on top of this that _also_ tries the older -redhat-disable-KSM downstream spelling of the option, but that's a problem for RHEL and not this list). +if (strstr(help, -mem-merge)) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_MEM_MERGE); This would only scrape the existence of -mem-merge in qemu 1.2 and earlier, since we don't read -help in 1.3 and later. But qemu 1.2 doesn't have -mem-merge, so this bit will never get set. You need to instead populate the new capability based on a QMP probe, not string scraping. But I don't know offhand what that probe would be; you may need to ask on the qemu list. And we have a winner. We are getting query-command-line-options for qemu 1.5, and it looks easy enough that distros may be able to backport it into earlier qemu (again, I'll leave it up to Red Hat internal lists on whether it will be backported to RHEL). https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg05040.html Looking forward to v2. Do you need help writing the src/qemu/qemu_monitor_json.c changes needed to utilize the new query-command-line-options QMP command, since I kind of spearheaded the design review on the qemu list? -- 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] fix typo introduced by 90430791
On 04/25/2013 06:17 AM, Bamvor Jian Zhang wrote:
From: Bamvor Jian Zhang bamv2...@gmail.com
Signed-off-by: Bamvor Jian Zhang bjzh...@suse.com
---
src/node_device/node_device_hal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK and pushed.
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index 4a430dc..63245a9 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -764,7 +764,7 @@ static int nodeDeviceClose(virConnectPtr conn
ATTRIBUTE_UNUSED)
static virNodeDeviceDriver halNodeDeviceDriver = {
.name = halNodeDeviceDriver,
.nodeDeviceOpen = nodeDeviceOpen, /* 0.5.0 */
-.nodDeviceClose = nodDeviceClose, /* 0.5.0 */
+.nodeDeviceClose = nodeDeviceClose, /* 0.5.0 */
.nodeNumOfDevices = nodeNumOfDevices, /* 0.5.0 */
.nodeListDevices = nodeListDevices, /* 0.5.0 */
.connectListAllNodeDevices = nodeListAllNodeDevices, /* 0.10.2 */
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: auto-add pci-root to 'pc-i440*' machines too
On 04/25/2013 07:25 AM, Ján Tomko wrote: Commit b33eb0d missed this machine type. --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+) ACK. diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 98ac56f..4e88eaf 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -687,6 +687,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, break; if (!STRPREFIX(def-os.machine, pc-0.) !STRPREFIX(def-os.machine, pc-1.) +!STRPREFIX(def-os.machine, pc-i440) !STREQ(def-os.machine, pc) !STRPREFIX(def-os.machine, rhel)) break; -- 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] conf: reject controllers with duplicate indexes
On 04/25/2013 05:41 AM, Ján Tomko wrote:
Reject multiple controllers with the same index,
except for USB controllers.
Multi-function USB controllers can have the same index.
---
Compared to the earlier version you posted as patch 6/5 on the pci
patches, you changed to a stack allocation of the bitmaps.
src/conf/domain_conf.c | 59
++
1 file changed, 59 insertions(+)
+virDomainDefRejectDuplicateControllers(virDomainDefPtr def)
+{
+int max_idx[VIR_DOMAIN_CONTROLLER_TYPE_LAST];
+virBitmapPtr bitmaps[VIR_DOMAIN_CONTROLLER_TYPE_LAST] = { NULL };
+virDomainControllerDefPtr cont;
+size_t nbitmaps = 0;
+int ret = -1;
+bool b;
+int i;
+
+memset(max_idx, -1, sizeof(max_idx)/sizeof(max_idx[0]));
That's too small. If VIR_DOMAIN_CONTROLLER_TYPE_LAST is 8, it only sets
2 entries and leaves the last 6 uninitialized. You want:
memset(max_idx, -1, sizeof(max_idx));
ACK with the fixed initialization.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: New XML to disable memory merge at guest startup
On 25/04/13 22:14, Eric Blake wrote: On 04/17/2013 08:54 AM, Eric Blake wrote: On 04/17/2013 08:22 AM, Osier Yang wrote: QEMU introduced command line -mem-merge=on|off (defaults to on) to enable/disable the memory merge (KSM) at guest startup. This exposes it by new XML: memoryBacking nosharepages/ /memoryBacking The XML tag is same with what we used internally for old RHEL. Good - that means that RHEL 6 (and any other downstream distro that was already borrowing the RHEL extension) will not break when rebasing to pick up this change from upstream in place of their downstream extension (RHEL will actually want to add a followup patch on top of this that _also_ tries the older -redhat-disable-KSM downstream spelling of the option, but that's a problem for RHEL and not this list). +if (strstr(help, -mem-merge)) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_MEM_MERGE); This would only scrape the existence of -mem-merge in qemu 1.2 and earlier, since we don't read -help in 1.3 and later. But qemu 1.2 doesn't have -mem-merge, so this bit will never get set. You need to instead populate the new capability based on a QMP probe, not string scraping. But I don't know offhand what that probe would be; you may need to ask on the qemu list. And we have a winner. We are getting query-command-line-options for qemu 1.5, and it looks easy enough that distros may be able to backport it into earlier qemu (again, I'll leave it up to Red Hat internal lists on whether it will be backported to RHEL). https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg05040.html Looking forward to v2. Do you need help writing the src/qemu/qemu_monitor_json.c changes needed to utilize the new query-command-line-options QMP command, since I kind of spearheaded the design review on the qemu list? I don't have much time before the beginning of May. Appreciated if you can do it. Or I can take it if it's delayed. :-) Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: auto-add pci-root to 'pc-i440*' machines too
On 04/25/2013 04:19 PM, Eric Blake wrote: On 04/25/2013 07:25 AM, Ján Tomko wrote: Commit b33eb0d missed this machine type. --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+) ACK. Thank you, I've pushed it now. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: reject controllers with duplicate indexes
On 04/25/2013 04:36 PM, Eric Blake wrote:
On 04/25/2013 05:41 AM, Ján Tomko wrote:
Reject multiple controllers with the same index,
except for USB controllers.
Multi-function USB controllers can have the same index.
---
Compared to the earlier version you posted as patch 6/5 on the pci
patches, you changed to a stack allocation of the bitmaps.
src/conf/domain_conf.c | 59
++
1 file changed, 59 insertions(+)
+virDomainDefRejectDuplicateControllers(virDomainDefPtr def)
+{
+int max_idx[VIR_DOMAIN_CONTROLLER_TYPE_LAST];
+virBitmapPtr bitmaps[VIR_DOMAIN_CONTROLLER_TYPE_LAST] = { NULL };
+virDomainControllerDefPtr cont;
+size_t nbitmaps = 0;
+int ret = -1;
+bool b;
+int i;
+
+memset(max_idx, -1, sizeof(max_idx)/sizeof(max_idx[0]));
That's too small. If VIR_DOMAIN_CONTROLLER_TYPE_LAST is 8, it only sets
2 entries and leaves the last 6 uninitialized. You want:
memset(max_idx, -1, sizeof(max_idx));
ACK with the fixed initialization.
Thanks, I've fixed it and pushed it.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvirt_lxc start problem when selinux enbale
Hi，all： the problem came out when selinux was enforced in targeted+MCS I start lxc through virsh――“virsh -c lxc:/// start instance-4bd6” 1. When selinux is Permissive，lxc start is ok The result of “Ps auxZ” is： system_u:system_r:virtd_lxc_t:s0-s0:c0.c1023 root 19218 0.0 0.0 47624 1244 ? Ss 15:26 0:00 /usr/libexec/libvirt_lxc --name system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19219 0.3 0.0 19276 1532 ? Ss 15:26 0:00 /sbin/init system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19406 0.0 0.0 177444 1332 ? Sl 15:26 0:00 /sbin/rsyslogd -i /var/run/sysl system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19420 0.0 0.0 64120 1144 ? Ss 15:26 0:00 /usr/sbin/sshd system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19427 0.0 0.0 22136 956 ? Ss 15:26 0:00 xinetd -stayalive -pidfile /var system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19434 0.0 0.0 64316 832 ? Ss 15:26 0:00 /usr/sbin/saslauthd -m /var/run system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19435 0.0 0.0 64316 600 ? S15:26 0:00 /usr/sbin/saslauthd -m /var/run system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19450 0.0 0.0 82388 2392 ? Ss 15:26 0:00 sendmail: rejecting new message system_u:system_r:svirt_lxc_net_t:s0:c192,c392 51 19459 0.0 0.0 78116 2016 ? Ss 15:26 0:00 sendmail: Queue runner@01:00:00 system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19467 0.0 0.0 175528 3672 ? Ss 15:26 0:00 /usr/sbin/httpd system_u:system_r:svirt_lxc_net_t:s0:c192,c392 48 19470 0.0 0.0 175528 2204 ? S15:26 0:00 /usr/sbin/httpd system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19475 0.0 0.0 117212 1348 ? Ss 15:26 0:00 crond system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19491 0.0 0.0 4108 600 pts/0 Ss+ 15:26 0:00 /sbin/mingetty /dev/tty1 We can get into the lxc through “ssh” 2. When selinux is Enforcing，lxc start bad Th result of “ps auxZ” is: system_u:system_r:virtd_lxc_t:s0-s0:c0.c1023 root 20624 0.0 0.0 47624 1244 ? Ss 15:29 0:00 /usr/libexec/libvirt_lxc --name system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 20625 0.0 0.0 17172 1036 pts/0 Ss+ 15:29 0:00 /sbin/init Only /sbin/init process started, no else. This is the real problem There is avc error messages in dmesg、/var/log/messages、/var/log/secure, and the same with selinux is Permissive Can anybody give some hints? Here are some system information: Kernel version 3.3.4 Libvirt version 0.9.13 Lxc guest image Centos 6.3 Lxc xml info is: !-- WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE OVERWRITTEN AND LOST. Changes to this xml configuration should be made using: virsh edit instance-4bd6 or other application using the libvirt API. -- domain type='lxc' nameinstance-4bd6/name uuid96eada0e-7ea0-4865-8271-3565811c8eb0/uuid memory unit='KiB'524288/memory currentMemory unit='KiB'524288/currentMemory vcpu placement='static'1/vcpu os type arch='x86_64'exe/type init/sbin/init/init cmdlineconsole=ttyS0/cmdline /os clock offset='utc'/ on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashdestroy/on_crash devices emulator/usr/libexec/libvirt_lxc/emulator filesystem type='mount' accessmode='passthrough' source dir='/home/stack/nova_state/instances/instance-4bd6/rootfs'/ target dir='/'/ /filesystem interface type='bridge' mac address='fa:16:3e:09:00:a2'/ source bridge='br100'/ filterref filter='nova-instance-instance-4bd6-fa163e0900a2' parameter name='DHCPSERVER' value='10.0.0.1'/ parameter name='IP' value='10.0.0.11'/ parameter name='PROJMASK' value='255.255.254.0'/ parameter name='PROJNET' value='10.0.0.0'/ /filterref /interface console type='pty' target type='lxc' port='0'/ /console /devices seclabel type='static' model='selinux' relabel='yes' labelsystem_u:system_r:svirt_lxc_net_t:s0:c192,c392/label /seclabel /domain Best Regard Huangchaochang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] 答复: libvirt_lxc start problem when selinux enbale
Sorry “There is avc error messages in dmesg ……” ――should be “There is no avc error……” 发件人: Huang,Chaochang 发送时间: 2013年4月25日 15:41 收件人: 'libvir-list@redhat.com'; 'libvirt-us...@redhat.com' 主题: libvirt_lxc start problem when selinux enbale Hi，all： the problem came out when selinux was enforced in targeted+MCS I start lxc through virsh――“virsh -c lxc:/// start instance-4bd6” 1. When selinux is Permissive，lxc start is ok The result of “Ps auxZ” is： system_u:system_r:virtd_lxc_t:s0-s0:c0.c1023 root 19218 0.0 0.0 47624 1244 ? Ss 15:26 0:00 /usr/libexec/libvirt_lxc --name system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19219 0.3 0.0 19276 1532 ? Ss 15:26 0:00 /sbin/init system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19406 0.0 0.0 177444 1332 ? Sl 15:26 0:00 /sbin/rsyslogd -i /var/run/sysl system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19420 0.0 0.0 64120 1144 ? Ss 15:26 0:00 /usr/sbin/sshd system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19427 0.0 0.0 22136 956 ? Ss 15:26 0:00 xinetd -stayalive -pidfile /var system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19434 0.0 0.0 64316 832 ? Ss 15:26 0:00 /usr/sbin/saslauthd -m /var/run system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19435 0.0 0.0 64316 600 ? S15:26 0:00 /usr/sbin/saslauthd -m /var/run system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19450 0.0 0.0 82388 2392 ? Ss 15:26 0:00 sendmail: rejecting new message system_u:system_r:svirt_lxc_net_t:s0:c192,c392 51 19459 0.0 0.0 78116 2016 ? Ss 15:26 0:00 sendmail: Queue runner@01:00:00 system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19467 0.0 0.0 175528 3672 ? Ss 15:26 0:00 /usr/sbin/httpd system_u:system_r:svirt_lxc_net_t:s0:c192,c392 48 19470 0.0 0.0 175528 2204 ? S15:26 0:00 /usr/sbin/httpd system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19475 0.0 0.0 117212 1348 ? Ss 15:26 0:00 crond system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 19491 0.0 0.0 4108 600 pts/0 Ss+ 15:26 0:00 /sbin/mingetty /dev/tty1 We can get into the lxc through “ssh” 2. When selinux is Enforcing，lxc start bad Th result of “ps auxZ” is: system_u:system_r:virtd_lxc_t:s0-s0:c0.c1023 root 20624 0.0 0.0 47624 1244 ? Ss 15:29 0:00 /usr/libexec/libvirt_lxc --name system_u:system_r:svirt_lxc_net_t:s0:c192,c392 root 20625 0.0 0.0 17172 1036 pts/0 Ss+ 15:29 0:00 /sbin/init Only /sbin/init process started, no else. This is the real problem There is avc error messages in dmesg、/var/log/messages、/var/log/secure, and the same with selinux is Permissive Can anybody give some hints? Here are some system information: Kernel version 3.3.4 Libvirt version 0.9.13 Lxc guest image Centos 6.3 Lxc xml info is: !-- WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE OVERWRITTEN AND LOST. Changes to this xml configuration should be made using: virsh edit instance-4bd6 or other application using the libvirt API. -- domain type='lxc' nameinstance-4bd6/name uuid96eada0e-7ea0-4865-8271-3565811c8eb0/uuid memory unit='KiB'524288/memory currentMemory unit='KiB'524288/currentMemory vcpu placement='static'1/vcpu os type arch='x86_64'exe/type init/sbin/init/init cmdlineconsole=ttyS0/cmdline /os clock offset='utc'/ on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashdestroy/on_crash devices emulator/usr/libexec/libvirt_lxc/emulator filesystem type='mount' accessmode='passthrough' source dir='/home/stack/nova_state/instances/instance-4bd6/rootfs'/ target dir='/'/ /filesystem interface type='bridge' mac address='fa:16:3e:09:00:a2'/ source bridge='br100'/ filterref filter='nova-instance-instance-4bd6-fa163e0900a2' parameter name='DHCPSERVER' value='10.0.0.1'/ parameter name='IP' value='10.0.0.11'/ parameter name='PROJMASK' value='255.255.254.0'/ parameter name='PROJNET' value='10.0.0.0'/ /filterref /interface console type='pty' target type='lxc' port='0'/ /console /devices seclabel type='static' model='selinux' relabel='yes' labelsystem_u:system_r:svirt_lxc_net_t:s0:c192,c392/label /seclabel /domain Best Regard Huangchaochang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Error in documentation about memory ballon
Hello, In Memory Ballon ( http://www.libvirt.org/formatdomain.html#elementsMemBalloon ) section, the second XML exemple has an error. Effectively, the documentation shows the following : ... devices watchdog model='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/ /devices /domain but it should be ... devices memballoon model='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/ /memballoon /devices /domain Best regards, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: fix memballoon examples
Use a pair of 'memballoon' tags instead of single 'watchdog' one. Add a few missing colons. --- Pushed under the trivial rule. docs/formatdomain.html.in | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0c0506b..8d43303 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4355,7 +4355,7 @@ qemu-kvm -net nic,model=? /dev/null /p p - Example automatically added device with KVM + Example: automatically added device with KVM /p pre ... @@ -4365,13 +4365,14 @@ qemu-kvm -net nic,model=? /dev/null .../pre p - Example manually added device with static PCI slot 2 requested + Example: manually added device with static PCI slot 2 requested /p pre ... lt;devicesgt; -lt;watchdog model='virtio'/gt; -lt;address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/gt; +lt;memballoon model='virtio'gt; + lt;address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/gt; +lt;/memballoongt; lt;/devicesgt; lt;/domaingt;/pre -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix build error with older glibc
Jim Fehlig reported on IRC that older glibc triggers this warning:
cc1: warnings being treated as errors
qemu/qemu_domain.c: In function 'qemuDomainDefFormatBuf':
qemu/qemu_domain.c:1297: error: declaration of 'remove' shadows a global
declaration [-Wshadow]
/usr/include/stdio.h:157: error: shadowed declaration is here [-Wshadow]
make[3]: *** [libvirt_driver_qemu_impl_la-qemu_domain.lo] Error 1
Fix it like we have done in the past (such as commit 2e6322a).
* src/qemu/qemu_domain.c (qemuDomainDefFormatBuf): Avoid shadowing
a function name.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Pushing under the build-breaker rule.
src/qemu/qemu_domain.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 98ac56f..0ef79cd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1293,7 +1293,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
if ((flags VIR_DOMAIN_XML_MIGRATABLE)) {
int i;
-int remove = 0;
+int toremove = 0;
virDomainControllerDefPtr usb = NULL, pci = NULL;
/* If only the default USB controller is present, we can remove it
@@ -1313,7 +1313,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
if (usb usb-idx == 0 usb-model == -1) {
VIR_DEBUG(Removing default USB controller from domain '%s'
for migration compatibility, def-name);
-remove++;
+toremove++;
} else {
usb = NULL;
}
@@ -1334,15 +1334,15 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
pci-model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
VIR_DEBUG(Removing default 'pci-root' from domain '%s'
for migration compatibility, def-name);
-remove++;
+toremove++;
} else {
pci = NULL;
}
-if (remove) {
+if (toremove) {
controllers = def-controllers;
ncontrollers = def-ncontrollers;
-if (VIR_ALLOC_N(def-controllers, ncontrollers - remove) 0) {
+if (VIR_ALLOC_N(def-controllers, ncontrollers - toremove) 0) {
controllers = NULL;
virReportOOMError();
goto cleanup;
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Error in documentation about memory ballon
On 04/25/2013 11:39 AM, Alexandre Laurent wrote: Hello, In Memory Ballon ( http://www.libvirt.org/formatdomain.html#elementsMemBalloon ) section, the second XML exemple has an error. Effectively, the documentation shows the following : ... devices watchdog model='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/ /devices /domain but it should be ... devices memballoon model='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/ /memballoon /devices /domain Best regards, It should be fixed now, thank you for your report. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/8] qemu: set qemu process' RLIMIT_MEMLOCK when VFIO is used
VFIO requires all of the guest's memory and IO space to be lockable in
RAM. The domain's max_balloon is the maximum amount of memory the
domain can have (in KiB). We add a generous 1GiB to that for IO space
(still much better than KVM device assignment, where the KVM module
actually *ignores* the process limits and locks everything anyway),
and convert from KiB to bytes.
In the case of hotplug, we are changing the limit for the already
existing qemu process (prlimit() is used under the hood), and for
regular commandline additions of vfio devices, we schedule a call to
setrlimit() that will happen after the qemu process is forked.
---
src/qemu/qemu_command.c | 22 +++---
src/qemu/qemu_hotplug.c | 24 +---
2 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 761291c..80a0dec 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7906,13 +7906,21 @@ qemuBuildCommandLine(virConnectPtr conn,
if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS
hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
{
-if ((hostdev-source.subsys.u.pci.backend
- == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO)
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(VFIO PCI device assignment is not supported
by
- this version of qemu));
-goto error;
+if (hostdev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(VFIO PCI device assignment is not
+ supported by this version of qemu));
+goto error;
+}
+/* VFIO requires all of the guest's memory to be
+ * locked resident, plus some amount for IO
+ * space. Alex Williamson suggested adding 1GiB for IO
+ * space just to be safe (some finer tuning might be
+ * nice, though).
+ */
+virCommandSetMaxMemLock(cmd, ((unsigned long
long)def-mem.max_balloon + (1024 * 1024)) * 1024);
}
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 30c8bda..af0b8a6 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -999,13 +999,23 @@ int qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver,
hostdev, 1) 0)
return -1;
-if ((hostdev-source.subsys.u.pci.backend
- == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO)
-!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(VFIO PCI device assignment is not supported by
- this version of qemu));
-goto error;
+if (hostdev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(VFIO PCI device assignment is not
+ supported by this version of qemu));
+goto error;
+}
+/* VFIO requires all of the guest's memory to be locked
+ * resident, plus some amount for IO space. Alex Williamson
+ * suggested adding 1GiB for IO space just to be safe (some
+ * finer tuning might be nice, though).
+ * In this case, the guest's memory may already be locked, but
+ * it doesn't hurt to change the limit to the same value.
+ */
+virSetMaxMemLock(vm-pid,
+ ((unsigned long long)vm-def-mem.max_balloon + (1024
* 1024)) * 1024);
}
if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/8] security: update hostdev labelling functions for VFIO
Legacy kvm style pci device assignment requires changes to the
labelling of several sysfs files for each device, but for vfio device
assignment, the only thing that needs to be relabelled/chowned is the
group device for the group that contains the device to be assigned.
---
src/security/security_apparmor.c | 12 +++-
src/security/security_dac.c | 27 ---
src/security/security_selinux.c | 24 ++--
3 files changed, 57 insertions(+), 6 deletions(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 122edd4..0aff794 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -831,7 +831,17 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
if (!pci)
goto done;
-ret = virPCIDeviceFileIterate(pci, AppArmorSetSecurityPCILabel, ptr);
+if (dev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+char *vfioGroupDev = virPCIDeviceGetVFIOGroupDev(pci);
+
+if (!vfioGroupDev)
+goto done;
+ret = AppArmorSetSecurityPCILabel(pci, vfioGroupDev, ptr);
+VIR_FREE(vfioGroupDev);
+} else {
+ret = virPCIDeviceFileIterate(pci, AppArmorSetSecurityPCILabel,
ptr);
+}
virPCIDeviceFree(pci);
break;
}
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 8576081..5e00112 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -516,8 +516,19 @@
virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
if (!pci)
goto done;
-ret = virPCIDeviceFileIterate(pci, virSecurityDACSetSecurityPCILabel,
- params);
+if (dev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+char *vfioGroupDev = virPCIDeviceGetVFIOGroupDev(pci);
+
+if (!vfioGroupDev)
+goto done;
+ret = virSecurityDACSetSecurityPCILabel(pci, vfioGroupDev, params);
+VIR_FREE(vfioGroupDev);
+} else {
+ret = virPCIDeviceFileIterate(pci,
virSecurityDACSetSecurityPCILabel,
+ params);
+}
+
virPCIDeviceFree(pci);
break;
@@ -596,7 +607,17 @@
virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr,
if (!pci)
goto done;
-ret = virPCIDeviceFileIterate(pci,
virSecurityDACRestoreSecurityPCILabel, mgr);
+if (dev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+char *vfioGroupDev = virPCIDeviceGetVFIOGroupDev(pci);
+
+if (!vfioGroupDev)
+goto done;
+ret = virSecurityDACRestoreSecurityPCILabel(pci, vfioGroupDev,
mgr);
+VIR_FREE(vfioGroupDev);
+} else {
+ret = virPCIDeviceFileIterate(pci,
virSecurityDACRestoreSecurityPCILabel, mgr);
+}
virPCIDeviceFree(pci);
break;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index a8b74ee..a5b54cb 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1342,7 +1342,17 @@
virSecuritySELinuxSetSecurityHostdevSubsysLabel(virDomainDefPtr def,
if (!pci)
goto done;
-ret = virPCIDeviceFileIterate(pci,
virSecuritySELinuxSetSecurityPCILabel, def);
+if (dev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+char *vfioGroupDev = virPCIDeviceGetVFIOGroupDev(pci);
+
+if (!vfioGroupDev)
+goto done;
+ret = virSecuritySELinuxSetSecurityPCILabel(pci, vfioGroupDev,
def);
+VIR_FREE(vfioGroupDev);
+} else {
+ret = virPCIDeviceFileIterate(pci,
virSecuritySELinuxSetSecurityPCILabel, def);
+}
virPCIDeviceFree(pci);
break;
@@ -1504,7 +1514,17 @@
virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr,
if (!pci)
goto done;
-ret = virPCIDeviceFileIterate(pci,
virSecuritySELinuxRestoreSecurityPCILabel, mgr);
+if (dev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+char *vfioGroupDev = virPCIDeviceGetVFIOGroupDev(pci);
+
+if (!vfioGroupDev)
+goto done;
+ret = virSecuritySELinuxRestoreSecurityPCILabel(pci, vfioGroupDev,
mgr);
+VIR_FREE(vfioGroupDev);
+} else {
+ret = virPCIDeviceFileIterate(pci,
virSecuritySELinuxRestoreSecurityPCILabel, mgr);
+}
virPCIDeviceFree(pci);
break;
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
[libvirt] [PATCH 5/8] qemu: use vfio-pci on commandline when appropriate
The device option for vfio-pci is nearly identical to that for
pci-assign - only the configfd parameter isn't supported (or needed).
Checking for presence of the bootindex parameter is done separately
from constructing the commandline, similar to how it is done for
pci-assign.
---
src/qemu/qemu_command.c | 48 ++--
src/qemu/qemu_hotplug.c | 13 -
2 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4f14dff..761291c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4345,14 +4345,19 @@ qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev,
const char *configfd,
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
-virBufferAddLit(buf, pci-assign);
+if (dev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+virBufferAddLit(buf, vfio-pci);
+} else {
+virBufferAddLit(buf, pci-assign);
+if (configfd *configfd)
+virBufferAsprintf(buf, ,configfd=%s, configfd);
+}
virBufferAsprintf(buf, ,host=%.2x:%.2x.%.1x,
dev-source.subsys.u.pci.addr.bus,
dev-source.subsys.u.pci.addr.slot,
dev-source.subsys.u.pci.addr.function);
virBufferAsprintf(buf, ,id=%s, dev-info-alias);
-if (configfd *configfd)
-virBufferAsprintf(buf, ,configfd=%s, configfd);
if (dev-info-bootIndex)
virBufferAsprintf(buf, ,bootindex=%d, dev-info-bootIndex);
if (qemuBuildDeviceAddressStr(buf, dev-info, qemuCaps) 0)
@@ -7850,12 +7855,23 @@ qemuBuildCommandLine(virConnectPtr conn,
supported for PCI and USB devices));
goto error;
} else {
-if (hostdev-source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_BOOTINDEX)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(booting from assigned PCI devices is not
- supported with this version of qemu));
-goto error;
+if (hostdev-source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+if (hostdev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+if (!virQEMUCapsGet(qemuCaps,
QEMU_CAPS_VFIO_PCI_BOOTINDEX)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(booting from PCI devices
assigned with VFIO
+ is not supported with this
version of qemu));
+goto error;
+}
+} else {
+if (!virQEMUCapsGet(qemuCaps,
QEMU_CAPS_PCI_BOOTINDEX)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(booting from assigned PCI
devices is not
+ supported with this version of
qemu));
+goto error;
+}
+}
}
if (hostdev-source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB
!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_HOST_BOOTINDEX)) {
@@ -7889,9 +7905,21 @@ qemuBuildCommandLine(virConnectPtr conn,
/* PCI */
if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS
hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
{
+
+if ((hostdev-source.subsys.u.pci.backend
+ == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO)
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(VFIO PCI device assignment is not supported
by
+ this version of qemu));
+goto error;
+}
+
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
char *configfd_name = NULL;
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) {
+if ((hostdev-source.subsys.u.pci.backend
+ != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO)
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) {
int configfd = qemuOpenPCIConfig(hostdev);
if (configfd = 0) {
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f6b2fc8..30c8bda 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -999,13 +999,24 @@ int qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver,
hostdev, 1) 0)
return -1;
+if
[libvirt] [PATCH 0/8] VDIO support part 3
This *almost* completes VFIO support. I still need to add a couple of xml/args test cases. Laine Stump (8): util: new function virPCIDeviceGetVFIOGroupDev security: update hostdev labelling functions for VFIO qemu: add VFIO devices to cgroup ACL qemu: add QEMU_CAPS_VFIO_PCI_BOOTINDEX qemu: use vfio-pci on commandline when appropriate util: new virCommandSetMax(MemLock|Processes|Files) qemu: use new virCommandSetMax(Processes|Files) qemu: set qemu process' RLIMIT_MEMLOCK when VFIO is used configure.ac | 2 +- src/libvirt_private.syms | 7 ++ src/qemu/qemu_capabilities.c | 7 ++ src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_cgroup.c | 11 +++ src/qemu/qemu_command.c | 56 --- src/qemu/qemu_hotplug.c | 23 +- src/qemu/qemu_process.c | 38 +- src/security/security_apparmor.c | 12 +++- src/security/security_dac.c | 27 +++- src/security/security_selinux.c | 24 ++- src/util/vircommand.c| 38 ++ src/util/vircommand.h| 4 ++ src/util/virpci.c| 34 + src/util/virpci.h| 2 + src/util/virutil.c | 146 +++ src/util/virutil.h | 4 ++ 17 files changed, 382 insertions(+), 55 deletions(-) -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/8] qemu: use new virCommandSetMax(Processes|Files)
These were previously being set in a custom hook function, but now
that virCommand directly supports setting them, we can eliminate that
part of the hook and call the APIs directly.
---
src/qemu/qemu_process.c | 38 ++
1 file changed, 2 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 925939d..f12d7d5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -25,8 +25,6 @@
#include unistd.h
#include signal.h
#include sys/stat.h
-#include sys/time.h
-#include sys/resource.h
#if defined(__linux__)
# include linux/capability.h
#elif defined(__FreeBSD__)
@@ -2453,37 +2451,6 @@ qemuProcessPrepareChardevDevice(virDomainDefPtr def
ATTRIBUTE_UNUSED,
}
-static int
-qemuProcessLimits(virQEMUDriverConfigPtr cfg)
-{
-struct rlimit rlim;
-
-if (cfg-maxProcesses 0) {
-rlim.rlim_cur = rlim.rlim_max = cfg-maxProcesses;
-if (setrlimit(RLIMIT_NPROC, rlim) 0) {
-virReportSystemError(errno,
- _(cannot limit number of processes to %d),
- cfg-maxProcesses);
-return -1;
-}
-}
-
-if (cfg-maxFiles 0) {
-/* Max number of opened files is one greater than
- * actual limit. See man setrlimit */
-rlim.rlim_cur = rlim.rlim_max = cfg-maxFiles + 1;
-if (setrlimit(RLIMIT_NOFILE, rlim) 0) {
-virReportSystemError(errno,
- _(cannot set max opened files to %d),
- cfg-maxFiles);
-return -1;
-}
-}
-
-return 0;
-}
-
-
struct qemuProcessHookData {
virConnectPtr conn;
virDomainObjPtr vm;
@@ -2526,9 +2493,6 @@ static int qemuProcessHook(void *data)
if (virSecurityManagerClearSocketLabel(h-driver-securityManager,
h-vm-def) 0)
goto cleanup;
-if (qemuProcessLimits(h-cfg) 0)
-goto cleanup;
-
/* This must take place before exec(), so that all QEMU
* memory allocation is on the correct NUMA node
*/
@@ -3697,6 +3661,8 @@ int qemuProcessStart(virConnectPtr conn,
}
virCommandSetPreExecHook(cmd, qemuProcessHook, hookData);
+virCommandSetMaxProcesses(cmd, cfg-maxProcesses);
+virCommandSetMaxFiles(cmd, cfg-maxFiles);
VIR_DEBUG(Setting up security labelling);
if (virSecurityManagerSetChildProcessLabel(driver-securityManager,
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/8] qemu: add VFIO devices to cgroup ACL
We don't know exactly the names of the VFIO devices that will be
needed (and due to hotplug, we can't ever assume we won't need them at
all), so we just add an ACL to allow any vfio device - they all have
the major number 244 (/dev/vfio/vfio is 244,0, and the /dev/vfio/n
devices are up from there).
---
src/qemu/qemu_cgroup.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 891984a..ad2027d 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -44,6 +44,7 @@ static const char *const defaultDeviceACL[] = {
};
#define DEVICE_PTY_MAJOR 136
#define DEVICE_SND_MAJOR 116
+#define DEVICE_VFIO_MAJOR 244
static int
qemuSetupDiskPathAllow(virDomainDiskDefPtr disk,
@@ -388,6 +389,16 @@ int qemuSetupCgroup(virQEMUDriverPtr driver,
}
}
+rc = virCgroupAllowDeviceMajor(priv-cgroup, 'c', DEVICE_VFIO_MAJOR,
+ VIR_CGROUP_DEVICE_RW);
+virDomainAuditCgroupMajor(vm, priv-cgroup, allow, DEVICE_VFIO_MAJOR,
+ vfio, rw, rc == 0);
+if (rc != 0) {
+virReportSystemError(-rc, %s,
+ _(unable to allow /dev/vfio/ devices));
+goto cleanup;
+}
+
for (i = 0; deviceACL[i] != NULL ; i++) {
if (access(deviceACL[i], F_OK) 0) {
VIR_DEBUG(Ignoring non-existant device %s,
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/8] util: new function virPCIDeviceGetVFIOGroupDev
Given a virPCIDevice, this function returns the path for the device
that controls the vfio group the device belongs to,
e.g. /dev/vfio/15.
---
src/libvirt_private.syms | 1 +
src/util/virpci.c| 34 ++
src/util/virpci.h| 2 ++
3 files changed, 37 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 33ec379..2a2c40e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1609,6 +1609,7 @@ virPCIDeviceGetReprobe;
virPCIDeviceGetStubDriver;
virPCIDeviceGetUnbindFromStub;
virPCIDeviceGetUsedBy;
+virPCIDeviceGetVFIOGroupDev;
virPCIDeviceIsAssignable;
virPCIDeviceListAdd;
virPCIDeviceListCount;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 73f36d0..673568f 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1726,6 +1726,40 @@ cleanup:
return ret;
}
+/* virPCIDeviceGetVFIOGroupDev - return the name of the device used to
+ * control this PCI device's group (e.g. /dev/vfio/15)
+ */
+char *
+virPCIDeviceGetVFIOGroupDev(virPCIDevicePtr dev)
+{
+char *devPath = NULL;
+char *groupPath = NULL;
+char *groupDev = NULL;
+
+if (virPCIFile(devPath, dev-name, iommu_group) 0)
+goto cleanup;
+if (virFileIsLink(devPath) != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Invalid device %s iommu_group file %s is not a
symlink),
+ dev-name, devPath);
+goto cleanup;
+}
+if (virFileResolveLink(devPath, groupPath) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unable to resolve device %s iommu_group symlink %s),
+ dev-name, devPath);
+goto cleanup;
+}
+if (virAsprintf(groupDev, /dev/vfio/%s, basename(groupPath)) 0) {
+virReportOOMError();
+goto cleanup;
+}
+cleanup:
+VIR_FREE(devPath);
+VIR_FREE(groupPath);
+return groupDev;
+}
+
static int
virPCIDeviceDownstreamLacksACS(virPCIDevicePtr dev)
{
diff --git a/src/util/virpci.h b/src/util/virpci.h
index db0be35..3911b72 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -111,6 +111,8 @@ typedef int (*virPCIDeviceFileActor)(virPCIDevicePtr dev,
int virPCIDeviceFileIterate(virPCIDevicePtr dev,
virPCIDeviceFileActor actor,
void *opaque);
+char *
+virPCIDeviceGetVFIOGroupDev(virPCIDevicePtr dev);
int virPCIDeviceIsAssignable(virPCIDevicePtr dev,
int strict_acs_check);
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/8] util: new virCommandSetMax(MemLock|Processes|Files)
This patch adds two sets of functions:
1) lower level functions that will immediately set the
RLIMIT_MEMLOCK. RLIMIT_NPROC, or RLIMIT_NOFILE of either the current
process (using setrlimit()) or any other process (using prlimit()).
2) functions for virCommand* that will setup a virCommand object to
set those limits at a later time just after it has forked a new
process, but before it execs the new program.
configure.ac has prlimit and setrlimit added to the list of functions
to check for, and the low level functions are hopefully properly
unsupported error) on all platforms.
You'll notice some ugly typecasting around pid_t; this is all an
attempt to follow the formula given to me by Eric for getting it to
compile without warnings on all platforms.
---
configure.ac | 2 +-
src/libvirt_private.syms | 6 ++
src/util/vircommand.c| 38
src/util/vircommand.h| 4 ++
src/util/virutil.c | 146 +++
src/util/virutil.h | 4 ++
6 files changed, 199 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 89dae3d..23c24d2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -194,7 +194,7 @@ dnl Availability of various common functions (non-fatal if
missing),
dnl and various less common threadsafe functions
AC_CHECK_FUNCS_ONCE([cfmakeraw geteuid getgid getgrnam_r getmntent_r \
getpwuid_r getuid initgroups kill mmap newlocale posix_fallocate \
- posix_memalign regexec sched_getaffinity setns symlink])
+ posix_memalign prlimit regexec sched_getaffinity setns setrlimit symlink])
dnl Availability of pthread functions (if missing, win32 threading is
dnl assumed). Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE.
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2a2c40e..c988c21 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1185,6 +1185,9 @@ virCommandSetErrorFD;
virCommandSetGID;
virCommandSetInputBuffer;
virCommandSetInputFD;
+virCommandSetMaxFiles;
+virCommandSetMaxMemLock;
+virCommandSetMaxProcesses;
virCommandSetOutputBuffer;
virCommandSetOutputFD;
virCommandSetPidFile;
@@ -1904,6 +1907,9 @@ virSetBlocking;
virSetCloseExec;
virSetDeviceUnprivSGIO;
virSetInherit;
+virSetMaxFiles;
+virSetMaxMemLock;
+virSetMaxProcesses;
virSetNonBlock;
virSetUIDGID;
virSetUIDGIDWithCaps;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index ac56a63..d0def8c 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -107,6 +107,10 @@ struct _virCommand {
char *pidfile;
bool reap;
+unsigned long long maxMemLock;
+unsigned int maxProcesses;
+unsigned int maxFiles;
+
uid_t uid;
gid_t gid;
unsigned long long capabilities;
@@ -598,6 +602,13 @@ virExec(virCommandPtr cmd)
goto fork_error;
}
+if (virSetMaxMemLock(-1, cmd-maxMemLock) 0)
+goto fork_error;
+if (virSetMaxProcesses(-1, cmd-maxProcesses) 0)
+goto fork_error;
+if (virSetMaxFiles(-1, cmd-maxFiles) 0)
+goto fork_error;
+
if (cmd-hook) {
VIR_DEBUG(Run hook %p %p, cmd-hook, cmd-opaque);
ret = cmd-hook(cmd-opaque);
@@ -958,6 +969,33 @@ virCommandSetUID(virCommandPtr cmd, uid_t uid)
cmd-uid = uid;
}
+void
+virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-maxMemLock = bytes;
+}
+
+void
+virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-maxProcesses = procs;
+}
+
+void
+virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-maxFiles = files;
+}
+
/**
* virCommandClearCaps:
* @cmd: the command to modify
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 6c13795..18568fe 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -65,6 +65,10 @@ void virCommandSetGID(virCommandPtr cmd, gid_t gid);
void virCommandSetUID(virCommandPtr cmd, uid_t uid);
+void virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes);
+void virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs);
+void virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files);
+
void virCommandClearCaps(virCommandPtr cmd);
void virCommandAllowCap(virCommandPtr cmd,
diff --git a/src/util/virutil.c b/src/util/virutil.c
index b9de33c..058a069 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -38,6 +38,10 @@
#include sys/types.h
#include sys/ioctl.h
#include sys/wait.h
+#if HAVE_SETRLIMIT
+# include sys/time.h
+# include sys/resource.h
+#endif
#if HAVE_MMAP
# include sys/mman.h
#endif
@@ -2992,6 +2996,148 @@ virGetGroupName(gid_t gid ATTRIBUTE_UNUSED)
}
#endif /* HAVE_GETPWUID_R */
+#if HAVE_SETRLIMIT
+
+# if HAVE_PRLIMIT
+static int
+virPrLimit(pid_t pid, int resource, struct rlimit *rlim)
+{
+
[libvirt] [PATCH 4/8] qemu: add QEMU_CAPS_VFIO_PCI_BOOTINDEX
For some reason, the bootindex parameter wasn't included in early
versions of vfio support (qemu 1.4) so we have to check for it
separately from vfio itself.
---
src/qemu/qemu_capabilities.c | 7 +++
src/qemu/qemu_capabilities.h | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d5df4b3..2acf535 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -224,6 +224,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
nvram, /* 140 */
pci-bridge, /* 141 */
vfio-pci, /* 142 */
+ vfio-pci.bootindex, /* 143 */
);
struct _virQEMUCaps {
@@ -1376,6 +1377,10 @@ static struct virQEMUCapsStringFlags
virQEMUCapsObjectPropsPciAssign[] = {
{ bootindex, QEMU_CAPS_PCI_BOOTINDEX },
};
+static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVfioPci[] = {
+{ bootindex, QEMU_CAPS_VFIO_PCI_BOOTINDEX },
+};
+
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsScsiDisk[] = {
{ channel, QEMU_CAPS_SCSI_DISK_CHANNEL },
{ wwn, QEMU_CAPS_SCSI_DISK_WWN },
@@ -1422,6 +1427,8 @@ static struct virQEMUCapsObjectTypeProps
virQEMUCapsObjectProps[] = {
ARRAY_CARDINALITY(virQEMUCapsObjectPropsPciAssign) },
{ kvm-pci-assign, virQEMUCapsObjectPropsPciAssign,
ARRAY_CARDINALITY(virQEMUCapsObjectPropsPciAssign) },
+{ vfio-pci, virQEMUCapsObjectPropsVfioPci,
+ ARRAY_CARDINALITY(virQEMUCapsObjectPropsVfioPci) },
{ scsi-disk, virQEMUCapsObjectPropsScsiDisk,
ARRAY_CARDINALITY(virQEMUCapsObjectPropsScsiDisk) },
{ ide-drive, virQEMUCapsObjectPropsIDEDrive,
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 49ee505..213f63c 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -181,7 +181,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_DEVICE_NVRAM = 140, /* -global spapr-nvram.reg= */
QEMU_CAPS_DEVICE_PCI_BRIDGE = 141, /* -device pci-bridge */
QEMU_CAPS_DEVICE_VFIO_PCI= 142, /* -device vfio-pci */
-
+QEMU_CAPS_VFIO_PCI_BOOTINDEX = 143, /* bootindex param for vfio-pci device
*/
QEMU_CAPS_LAST, /* this must always be the last item */
};
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 9/8] qemu: xml/args tests for VFIO hostdev and interface type='hostdev'/
These should be squashed in with the patch that adds commandline handling of vfio (they would fail at any earlier time). --- .../qemuxml2argv-hostdev-vfio.args | 5 +++ .../qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml | 33 + .../qemuxml2argv-net-hostdev-vfio.args | 6 .../qemuxml2argv-net-hostdev-vfio.xml | 41 ++ tests/qemuxml2argvtest.c | 6 tests/qemuxml2xmltest.c| 2 ++ 6 files changed, 93 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-vfio.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-vfio.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.args new file mode 100644 index 000..e6e42de --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +/dev/HostVG/QEMUGuest2 -device vfio-pci,host=06:12.5,id=hostdev0,\ +bus=pci.0,addr=0x3 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml new file mode 100644 index 000..8daa53a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml @@ -0,0 +1,33 @@ +domain type='qemu' + nameQEMUGuest2/name + uuidc7a5fdbd-edaf-9466-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest2'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='pci' index='0' model='pci-root'/ +hostdev mode='subsystem' type='pci' managed='yes' + driver name='vfio'/ + source +address domain='0x' bus='0x06' slot='0x12' function='0x5'/ + /source +/hostdev +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-vfio.args b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-vfio.args new file mode 100644 index 000..da5886e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-vfio.args @@ -0,0 +1,6 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S \ +-M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-usb -hda /dev/HostVG/QEMUGuest1 \ +-device vfio-pci,host=03:07.1,id=hostdev0,bus=pci.0,addr=0x3 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-vfio.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-vfio.xml new file mode 100644 index 000..90419a4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-vfio.xml @@ -0,0 +1,41 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='pci' index='0' model='pci-root'/ +interface type='hostdev' managed='yes' + mac address='00:11:22:33:44:55'/ + driver name='vfio'/ + source +address type='pci' domain='0x0002' bus='0x03' slot='0x07' function='0x1'/ + /source + vlan +tag id='42'/ + /vlan + virtualport type='802.1Qbg' +parameters managerid='11' typeid='1193047' typeidversion='2' instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/ + /virtualport + model type='rtl8139'/ +/interface +memballoon model='virtio'/ + /devices +/domain diff --git
Re: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
On 04/22/2013 11:59 AM, Laine Stump wrote: address should be optional unless prefix or netmask is non-0, although I've now noticed that won't be handled properly due to virSocketAddrGetIpPrefix returning -1 when there is no address or prefix or netmask (I'm fixing that before I push that patch, so you can just toss your 1/2 patch, rebase, and assume it's fixed). I have most of the stuff reworked except for the address, gateway, netmask, and prefix code. Getting all of those balanced so they work correctly is a bit tricky.. 1. For route, I am requiring that both address= and gateway= be specified with address='0.0.0.0' and address='::' being valid addresses. For IPv4, netmask='0.0.0.0' works correctly but prefix=0 does not. For IPv4, address='0.0.0.0' results in a default route. I am not sure what all these extra default routes are going to do to things but lets not get in the way of the experimenter. For IPv6, this address='::', prefix='0' is a slightly different matter as default routes are usually handled differently. I am going to go ahead and implement it but I am not sure it is a good idea. Normally, if you do not specify a prefix for IPv6, the default is 64. But if you do specify one, then it will be used. It is getting real close and it should be ready real soon now ;)) Gene -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] util: new virCommandSetMax(MemLock|Processes|Files)
On Thu, Apr 25, 2013 at 01:57:56PM -0400, Laine Stump wrote:
diff --git a/src/util/virutil.c b/src/util/virutil.c
index b9de33c..058a069 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -38,6 +38,10 @@
#include sys/types.h
#include sys/ioctl.h
#include sys/wait.h
+#if HAVE_SETRLIMIT
+# include sys/time.h
+# include sys/resource.h
+#endif
#if HAVE_MMAP
# include sys/mman.h
#endif
@@ -2992,6 +2996,148 @@ virGetGroupName(gid_t gid ATTRIBUTE_UNUSED)
}
#endif /* HAVE_GETPWUID_R */
+#if HAVE_SETRLIMIT
+
+# if HAVE_PRLIMIT
+static int
+virPrLimit(pid_t pid, int resource, struct rlimit *rlim)
+{
+return prlimit(pid, resource, rlim, NULL);
+}
+# else
+static int
+virPrLimit(pid_t pid ATTRIBUTE_UNUSED,
+ int resource ATTRIBUTE_UNUSED,
+ struct rlimit *rlim ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS, %s, _(Not supported on this platform));
+return -1;
+}
+# endif
+
+int
+virSetMaxMemLock(pid_t pid, unsigned long long bytes)
+{
+struct rlimit rlim;
+
+if (bytes == 0)
+return 0;
+
+rlim.rlim_cur = rlim.rlim_max = bytes;
+if (pid == (pid_t)-1) {
+if (setrlimit(RLIMIT_MEMLOCK, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit locked memory to %llu),
+ bytes);
+return -1;
+}
+} else {
+if (virPrLimit(pid, RLIMIT_MEMLOCK, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit locked memory
+ of process %lld to %llu),
+ (long long int)pid, bytes);
+return -1;
+}
+}
+return 0;
+}
+
+int
+virSetMaxProcesses(pid_t pid, unsigned int procs)
+{
+struct rlimit rlim;
+
+if (procs == 0)
+return 0;
+
+rlim.rlim_cur = rlim.rlim_max = procs;
+if (pid == (pid_t)-1) {
+if (setrlimit(RLIMIT_NPROC, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit number of subprocesses to
%u),
+ procs);
+return -1;
+}
+} else {
+if (virPrLimit(pid, RLIMIT_NPROC, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit number of subprocesses
+ of process %lld to %u),
+ (long long int)pid, procs);
+return -1;
+}
+}
+return 0;
+}
+
+int
+virSetMaxFiles(pid_t pid, unsigned int files)
+{
+struct rlimit rlim;
+
+if (files == 0)
+return 0;
+
+ /* Max number of opened files is one greater than actual limit. See
+* man setrlimit.
+*
+* NB: That indicates to me that we would want the following code
+* to say files - 1, but the original of this code in
+* qemu_process.c also had files + 1, so this preserves current
+* behavior.
+*/
+rlim.rlim_cur = rlim.rlim_max = files + 1;
+if (pid == (pid_t)-1) {
+if (setrlimit(RLIMIT_NOFILE, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit number of open files to
%u),
+ files);
+return -1;
+}
+} else {
+if (virPrLimit(pid, RLIMIT_NOFILE, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit number of open files
+ of process %lld to %u),
+ (long long int)pid, files);
+return -1;
+}
+}
+return 0;
+}
+#else
+int
+virSetMaxMemLock(pid_t pid ATTRIBUTE_UNUSED, unsigned long long bytes)
+{
+if (bytes == 0)
+return 0;
+
+virReportSystemError(ENOSYS, %s, _(Not supported on this platform));
+return -1;
+}
+
+int
+virSetMaxProcesses(pid_t pid ATTRIBUTE_UNUSED, unsigned int procs)
+{
+if (procs == 0)
+return 0;
+
+virReportSystemError(ENOSYS, %s, _(Not supported on this platform));
+return -1;
+}
+
+int
+virSetMaxFiles(pid_t pid ATTRIBUTE_UNUSED, unsigned int files)
+{
+if (files == 0)
+return 0;
+
+virReportSystemError(ENOSYS, %s, _(Not supported on this platform));
+return -1;
+}
+#endif
Since these functions all take pid_t as their first arg,
they should all be named virProcessX and be located
in virprocess.{c,h} rather than virutil.{c,h}
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 :|
--
[libvirt] [PATCH 1/2] Improve /domainsnapshot/disks/disk@snapshot doc
The previous description was a bit confusing. --- docs/formatsnapshot.html.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 8fcc04c..b1e259f 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -146,8 +146,9 @@ the a href=formatdomain.html#elementsDisksdisk devices/a specified for the domain at the time of the snapshot. The attribute codesnapshot/code is -optional, and has the same values of the disk device -element for a domain +optional, and the possible values are the same as the +codesnapshotcode attribute for + a href=formatdomain.html#elementsDisksdisk devices/a (codeno/code, codeinternal/code, or codeexternal/code). Some hypervisors like ESX require that if specified, the snapshot mode must not -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Two trivial doc patches
Hey, I've just pushed these 2 patches under the trivial/doc rule. Christophe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] util: new function virPCIDeviceGetVFIOGroupDev
On 04/25/2013 11:57 AM, Laine Stump wrote:
Given a virPCIDevice, this function returns the path for the device
that controls the vfio group the device belongs to,
e.g. /dev/vfio/15.
---
+if (virAsprintf(groupDev, /dev/vfio/%s, basename(groupPath)) 0) {
We shouldn't be using basename() from libgen.h - it has horrible
portability problems, and is not thread-safe. Instead, use
last_component() from gnulib's dirname.h. [I really ought to add a
syntax-check to cfg.mk and clean up the existing offenders].
ACK with that fixed.
--
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 6/8] util: new virCommandSetMax(MemLock|Processes|Files)
On 04/25/2013 03:27 PM, Daniel P. Berrange wrote:
Since these functions all take pid_t as their first arg, they should
all be named virProcessX and be located in virprocess.{c,h} rather
than virutil.{c,h}
Yes, of course! The thought Move it somewhere else! was trying to come
to my mind while I was writing it, but I didn't give myself time enough
to think.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
On 04/25/2013 03:13 PM, Gene Czarcinski wrote: On 04/22/2013 11:59 AM, Laine Stump wrote: address should be optional unless prefix or netmask is non-0, although I've now noticed that won't be handled properly due to virSocketAddrGetIpPrefix returning -1 when there is no address or prefix or netmask (I'm fixing that before I push that patch, so you can just toss your 1/2 patch, rebase, and assume it's fixed). I have most of the stuff reworked except for the address, gateway, netmask, and prefix code. Getting all of those balanced so they work correctly is a bit tricky.. 1. For route, I am requiring that both address= and gateway= be specified with address='0.0.0.0' and address='::' being valid addresses. For IPv4, netmask='0.0.0.0' works correctly but prefix=0 does not. For IPv4, address='0.0.0.0' results in a default route. I am not sure what all these extra default routes are going to do to things but lets not get in the way of the experimenter. For IPv6, this address='::', prefix='0' is a slightly different matter as default routes are usually handled differently. I am going to go ahead and implement it but I am not sure it is a good idea. Normally, if you do not specify a prefix for IPv6, the default is 64. But if you do specify one, then it will be used. It is getting real close and it should be ready real soon now ;)) AAGH With IPv4 using address='0.0.0.0' and netmask='0.0.0.0' things work just fine but with prefix not so much. The problem is that with prefix=0, it is not in the xml which then results it it defaulting at a later time. This is an extreme corner case. Usually a zero prefix is just ignored. Gene -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: avoid unsafe functions in libgen.h
POSIX says that both basename() and dirname() may return static
storage (aka they are not thread-safe); and that they may but
not must modify their input argument. Furthermore, libgen.h
is not available on all platforms. For these reasons, you should
never use these functions in a multi-threaded library.
Gnulib instead recommends a way to avoid the portability nightmare:
gnulib's dirname.h provides useful counterparts. The obvious
dir_name() and base_name() are GPL (because they malloc(), but call
exit() on failure) so we can't use them; but the LGPL variants
mdir_name() (malloc's or returns NULL) and last_component (always
points into the incoming string without modifying it, differing
from basename semantics only on corner cases like the empty string
that we shouldn't be hitting in the first place) are already in use
in libvirt. This finishes the swap over to the safe functions.
* cfg.mk (sc_prohibit_libgen): New rule.
* src/util/vircgroup.c: Fix offenders.
* src/parallels/parallels_storage.c (parallelsPoolAddByDomain):
Likewise.
* src/parallels/parallels_network.c (parallelsGetBridgedNetInfo):
Likewise.
* src/node_device/node_device_udev.c (udevProcessSCSIHost)
(udevProcessSCSIDevice): Likewise.
* src/storage/storage_backend_disk.c
(virStorageBackendDiskDeleteVol): Likewise.
* src/util/virpci.c (virPCIGetDeviceAddressFromSysfsLink):
Likewise.
* src/util/virstoragefile.h (_virStorageFileMetadata): Avoid false
positive.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Spotted during a review of Laine's pending work.
cfg.mk | 6 ++
src/node_device/node_device_udev.c | 7 ---
src/parallels/parallels_network.c | 4 +++-
src/parallels/parallels_storage.c | 8
src/storage/storage_backend_disk.c | 5 +++--
src/util/vircgroup.c | 3 +--
src/util/virpci.c | 3 ++-
src/util/virstoragefile.h | 2 +-
8 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index ebb6613..f0f78f5 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -493,6 +493,12 @@ sc_prohibit_gethostby:
halt='use getaddrinfo, not gethostby*' \
$(_sc_search_regexp)
+# dirname and basename from libgen.h are not required to be thread-safe
+sc_prohibit_libgen:
+ @prohibit='( (base|dir)name *\(|include .libgen\.h)'\
+ halt='use functions from gnulib dirname.h, not libgen.h'\
+ $(_sc_search_regexp)
+
# raw xmlGetProp requires some nasty casts
sc_prohibit_xmlGetProp:
@prohibit='\xmlGetProp *\('\
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 92292be..f98841e 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1,7 +1,7 @@
/*
* node_device_udev.c: node device enumeration - libudev implementation
*
- * Copyright (C) 2009-2012 Red Hat, Inc.
+ * Copyright (C) 2009-2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -26,6 +26,7 @@
#include scsi/scsi.h
#include c-ctype.h
+#include dirname.h
#include node_device_udev.h
#include virerror.h
#include node_device_conf.h
@@ -653,7 +654,7 @@ static int udevProcessSCSIHost(struct udev_device *device
ATTRIBUTE_UNUSED,
union _virNodeDevCapData *data = def-caps-data;
char *filename = NULL;
-filename = basename(def-sysfs_path);
+filename = last_component(def-sysfs_path);
if (!STRPREFIX(filename, host)) {
VIR_ERROR(_(SCSI host found, but its udev name '%s' does
@@ -774,7 +775,7 @@ static int udevProcessSCSIDevice(struct udev_device *device
ATTRIBUTE_UNUSED,
union _virNodeDevCapData *data = def-caps-data;
char *filename = NULL, *p = NULL;
-filename = basename(def-sysfs_path);
+filename = last_component(def-sysfs_path);
if (udevStrToLong_ui(filename, p, 10, data-scsi.host) == -1) {
goto out;
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index c61e280..7a9436f 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -2,6 +2,7 @@
* parallels_storage.c: core privconn functions for managing
* Parallels Cloud Server hosts
*
+ * Copyright (C) 2013 Red Hat, Inc.
* Copyright (C) 2012 Parallels, Inc.
*
* This library is free software; you can redistribute it and/or
@@ -23,6 +24,7 @@
#include config.h
#include datatypes.h
+#include dirname.h
#include viralloc.h
#include virerror.h
#include md5.h
@@ -64,7 +66,7 @@ static int parallelsGetBridgedNetInfo(virNetworkDefPtr def,
virJSONValuePtr jobj
goto cleanup;
}
-if (!(def-bridge = strdup(basename(bridgePath {
+if (!(def-bridge = strdup(last_component(bridgePath {
virReportOOMError();
goto cleanup;
}
diff --git a/src/parallels/parallels_storage.c
Re: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
On 04/25/2013 02:13 PM, Gene Czarcinski wrote: It is getting real close and it should be ready real soon now ;)) AAGH With IPv4 using address='0.0.0.0' and netmask='0.0.0.0' things work just fine but with prefix not so much. The problem is that with prefix=0, it is not in the xml which then results it it defaulting at a later time. Is it getting defaulted back to 0? In the past, if we have XML elements that can validly be 0, but where 0 is not the default, it works to add a witness field (has_prefix); if has_prefix is true, then use prefix instead of defaulting to a non-zero prefix that we normally use. Yeah, it's a bit of a pain, but it's not the first time. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
On 04/25/2013 04:13 PM, Gene Czarcinski wrote: On 04/25/2013 03:13 PM, Gene Czarcinski wrote: On 04/22/2013 11:59 AM, Laine Stump wrote: address should be optional unless prefix or netmask is non-0, although I've now noticed that won't be handled properly due to virSocketAddrGetIpPrefix returning -1 when there is no address or prefix or netmask (I'm fixing that before I push that patch, so you can just toss your 1/2 patch, rebase, and assume it's fixed). I have most of the stuff reworked except for the address, gateway, netmask, and prefix code. Getting all of those balanced so they work correctly is a bit tricky.. 1. For route, I am requiring that both address= and gateway= be specified with address='0.0.0.0' and address='::' being valid addresses. For IPv4, netmask='0.0.0.0' works correctly but prefix=0 does not. For IPv4, address='0.0.0.0' results in a default route. I am not sure what all these extra default routes are going to do to things but lets not get in the way of the experimenter. For IPv6, this address='::', prefix='0' is a slightly different matter as default routes are usually handled differently. I am going to go ahead and implement it but I am not sure it is a good idea. Normally, if you do not specify a prefix for IPv6, the default is 64. But if you do specify one, then it will be used. It is getting real close and it should be ready real soon now ;)) AAGH With IPv4 using address='0.0.0.0' and netmask='0.0.0.0' things work just fine but with prefix not so much. The problem is that with prefix=0, it is not in the xml which then results it it defaulting at a later time. This is an extreme corner case. Usually a zero prefix is just ignored. Defaulting to what? I thought that when I pushed the utility function for that, I modified it to return a prefix of 0 if the address was 0.0.0.0 and neither netmask nor prefix was set. I guess it might be problematic if address was *not* 0 and you wanted an explicit 0 prefix, but I don't think that would ever be useful. If you really want prefix to show up in the xml if someone explicitly puts prefix='0' in there, you can add a bool prefix_specified; to the object, and set that when you see a prefix, even if it's 0. Then in the formatter you'll know that you should write out the value of prefix, even if it's 0. There are a few examples of doing this in either the network or domain xml parser/formatter - just search for occurrences of the word _specified in src/conf/*.[ch] and you'll find them. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid unsafe functions in libgen.h
On 04/25/2013 04:30 PM, Eric Blake wrote:
POSIX says that both basename() and dirname() may return static
storage (aka they are not thread-safe); and that they may but
not must modify their input argument. Furthermore, libgen.h
is not available on all platforms. For these reasons, you should
never use these functions in a multi-threaded library.
Gnulib instead recommends a way to avoid the portability nightmare:
gnulib's dirname.h provides useful counterparts. The obvious
dir_name() and base_name() are GPL (because they malloc(), but call
exit() on failure) so we can't use them; but the LGPL variants
mdir_name() (malloc's or returns NULL) and last_component (always
points into the incoming string without modifying it, differing
from basename semantics only on corner cases like the empty string
that we shouldn't be hitting in the first place) are already in use
in libvirt. This finishes the swap over to the safe functions.
* cfg.mk (sc_prohibit_libgen): New rule.
* src/util/vircgroup.c: Fix offenders.
* src/parallels/parallels_storage.c (parallelsPoolAddByDomain):
Likewise.
* src/parallels/parallels_network.c (parallelsGetBridgedNetInfo):
Likewise.
* src/node_device/node_device_udev.c (udevProcessSCSIHost)
(udevProcessSCSIDevice): Likewise.
* src/storage/storage_backend_disk.c
(virStorageBackendDiskDeleteVol): Likewise.
* src/util/virpci.c (virPCIGetDeviceAddressFromSysfsLink):
Likewise.
* src/util/virstoragefile.h (_virStorageFileMetadata): Avoid false
positive.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Spotted during a review of Laine's pending work.
cfg.mk | 6 ++
src/node_device/node_device_udev.c | 7 ---
src/parallels/parallels_network.c | 4 +++-
src/parallels/parallels_storage.c | 8
src/storage/storage_backend_disk.c | 5 +++--
src/util/vircgroup.c | 3 +--
src/util/virpci.c | 3 ++-
src/util/virstoragefile.h | 2 +-
8 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index ebb6613..f0f78f5 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -493,6 +493,12 @@ sc_prohibit_gethostby:
halt='use getaddrinfo, not gethostby*' \
$(_sc_search_regexp)
+# dirname and basename from libgen.h are not required to be thread-safe
+sc_prohibit_libgen:
+ @prohibit='( (base|dir)name *\(|include .libgen\.h)'\
+ halt='use functions from gnulib dirname.h, not libgen.h'\
+ $(_sc_search_regexp)
+
# raw xmlGetProp requires some nasty casts
sc_prohibit_xmlGetProp:
@prohibit='\xmlGetProp *\('\
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 92292be..f98841e 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1,7 +1,7 @@
/*
* node_device_udev.c: node device enumeration - libudev implementation
*
- * Copyright (C) 2009-2012 Red Hat, Inc.
+ * Copyright (C) 2009-2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -26,6 +26,7 @@
#include scsi/scsi.h
#include c-ctype.h
+#include dirname.h
#include node_device_udev.h
#include virerror.h
#include node_device_conf.h
@@ -653,7 +654,7 @@ static int udevProcessSCSIHost(struct udev_device *device
ATTRIBUTE_UNUSED,
union _virNodeDevCapData *data = def-caps-data;
char *filename = NULL;
-filename = basename(def-sysfs_path);
+filename = last_component(def-sysfs_path);
if (!STRPREFIX(filename, host)) {
VIR_ERROR(_(SCSI host found, but its udev name '%s' does
@@ -774,7 +775,7 @@ static int udevProcessSCSIDevice(struct udev_device
*device ATTRIBUTE_UNUSED,
union _virNodeDevCapData *data = def-caps-data;
char *filename = NULL, *p = NULL;
-filename = basename(def-sysfs_path);
+filename = last_component(def-sysfs_path);
if (udevStrToLong_ui(filename, p, 10, data-scsi.host) == -1) {
goto out;
diff --git a/src/parallels/parallels_network.c
b/src/parallels/parallels_network.c
index c61e280..7a9436f 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -2,6 +2,7 @@
* parallels_storage.c: core privconn functions for managing
* Parallels Cloud Server hosts
*
+ * Copyright (C) 2013 Red Hat, Inc.
* Copyright (C) 2012 Parallels, Inc.
*
* This library is free software; you can redistribute it and/or
@@ -23,6 +24,7 @@
#include config.h
#include datatypes.h
+#include dirname.h
#include viralloc.h
#include virerror.h
#include md5.h
@@ -64,7 +66,7 @@ static int parallelsGetBridgedNetInfo(virNetworkDefPtr def,
virJSONValuePtr jobj
goto cleanup;
}
-if (!(def-bridge = strdup(basename(bridgePath {
+if (!(def-bridge =
Re: [libvirt] [PATCH 3/8] qemu: add VFIO devices to cgroup ACL
On 04/25/2013 11:57 AM, Laine Stump wrote:
We don't know exactly the names of the VFIO devices that will be
needed (and due to hotplug, we can't ever assume we won't need them at
all), so we just add an ACL to allow any vfio device - they all have
the major number 244 (/dev/vfio/vfio is 244,0, and the /dev/vfio/n
devices are up from there).
---
src/qemu/qemu_cgroup.c | 11 +++
1 file changed, 11 insertions(+)
@@ -388,6 +389,16 @@ int qemuSetupCgroup(virQEMUDriverPtr driver,
}
}
+rc = virCgroupAllowDeviceMajor(priv-cgroup, 'c', DEVICE_VFIO_MAJOR,
+ VIR_CGROUP_DEVICE_RW);
+virDomainAuditCgroupMajor(vm, priv-cgroup, allow,
DEVICE_VFIO_MAJOR,
+ vfio, rw, rc == 0);
+if (rc != 0) {
+virReportSystemError(-rc, %s,
+ _(unable to allow /dev/vfio/ devices));
+goto cleanup;
+}
This matches what we have done for ptys and sound; but it's rather broad
in that the access is now ALWAYS allowed. For ptys, always being
enabled makes sense (you seldom start a guest without a pty, and even if
we blindly grant the privilege to a guest that doesn't use a pty there's
not much they could break by a rogue guest opening a pty device); but
for vfio, it's tied to something that we don't want to allow rogue
access to, when it can be avoided.
Do we want to be a bit smarter and only add it to the file at the point
when we start a guest with a vfio device and/or hotplug a vfio device,
then revoke the cgroup privileges any time we hot-unplug the last vfio
device? If so, it would be similar to how we add cgroup device ACL
privileges for block disks at hotplug time, except that we'd need some
domain tracking to count how many vfio devices are plugged at once, so
we aren't revoking access until the last hot-unplug.
I can live with this as-is (we at least audit that it happens), but
wonder if it is worth respinning this patch to be smarter about when to
allow vfio in the guest.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] qemu: add QEMU_CAPS_VFIO_PCI_BOOTINDEX
On 04/25/2013 11:57 AM, Laine Stump wrote: For some reason, the bootindex parameter wasn't included in early versions of vfio support (qemu 1.4) so we have to check for it separately from vfio itself. --- src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) 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 5/8] qemu: use vfio-pci on commandline when appropriate
On 04/25/2013 11:57 AM, Laine Stump wrote:
The device option for vfio-pci is nearly identical to that for
pci-assign - only the configfd parameter isn't supported (or needed).
Checking for presence of the bootindex parameter is done separately
from constructing the commandline, similar to how it is done for
pci-assign.
---
src/qemu/qemu_command.c | 48 ++--
src/qemu/qemu_hotplug.c | 13 -
2 files changed, 50 insertions(+), 11 deletions(-)
@@ -7850,12 +7855,23 @@ qemuBuildCommandLine(virConnectPtr conn,
supported for PCI and USB devices));
goto error;
} else {
-if (hostdev-source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_BOOTINDEX)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(booting from assigned PCI devices is
not
- supported with this version of
qemu));
-goto error;
+if (hostdev-source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+if (hostdev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+if (!virQEMUCapsGet(qemuCaps,
QEMU_CAPS_VFIO_PCI_BOOTINDEX)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(booting from PCI devices
assigned with VFIO
+ is not supported with this
version of qemu));
Line break after space...
+goto error;
+}
+} else {
+if (!virQEMUCapsGet(qemuCaps,
QEMU_CAPS_PCI_BOOTINDEX)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(booting from assigned PCI
devices is not
+ supported with this version
of qemu));
...line break before space. Looks a bit inconsistent, but the end
result is the same.
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] build: avoid unsafe functions in libgen.h
On 04/25/2013 02:41 PM, Laine Stump wrote: On 04/25/2013 04:30 PM, Eric Blake wrote: POSIX says that both basename() and dirname() may return static storage (aka they are not thread-safe); and that they may but not must modify their input argument. Furthermore, libgen.h is not available on all platforms. For these reasons, you should never use these functions in a multi-threaded library. Gnulib instead recommends a way to avoid the portability nightmare: gnulib's dirname.h provides useful counterparts. The obvious dir_name() and base_name() are GPL (because they malloc(), but call exit() on failure) so we can't use them; but the LGPL variants mdir_name() (malloc's or returns NULL) and last_component (always points into the incoming string without modifying it, differing from basename semantics only on corner cases like the empty string that we shouldn't be hitting in the first place) are already in use in libvirt. This finishes the swap over to the safe functions. ACK. Thanks! Now pushed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/8] security: update hostdev labelling functions for VFIO
On 04/25/2013 11:57 AM, Laine Stump wrote: Legacy kvm style pci device assignment requires changes to the labelling of several sysfs files for each device, but for vfio device assignment, the only thing that needs to be relabelled/chowned is the group device for the group that contains the device to be assigned. --- src/security/security_apparmor.c | 12 +++- src/security/security_dac.c | 27 --- src/security/security_selinux.c | 24 ++-- 3 files changed, 57 insertions(+), 6 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 6/8] util: new virCommandSetMax(MemLock|Processes|Files)
On 04/25/2013 11:57 AM, Laine Stump wrote:
This patch adds two sets of functions:
1) lower level functions that will immediately set the
RLIMIT_MEMLOCK. RLIMIT_NPROC, or RLIMIT_NOFILE of either the current
process (using setrlimit()) or any other process (using prlimit()).
2) functions for virCommand* that will setup a virCommand object to
set those limits at a later time just after it has forked a new
process, but before it execs the new program.
configure.ac has prlimit and setrlimit added to the list of functions
to check for, and the low level functions are hopefully properly
unsupported error) on all platforms.
You'll notice some ugly typecasting around pid_t; this is all an
attempt to follow the formula given to me by Eric for getting it to
compile without warnings on all platforms.
---
configure.ac | 2 +-
src/libvirt_private.syms | 6 ++
src/util/vircommand.c| 38
src/util/vircommand.h| 4 ++
src/util/virutil.c | 146
+++
src/util/virutil.h | 4 ++
6 files changed, 199 insertions(+), 1 deletion(-)
@@ -598,6 +602,13 @@ virExec(virCommandPtr cmd)
goto fork_error;
}
+if (virSetMaxMemLock(-1, cmd-maxMemLock) 0)
+goto fork_error;
+if (virSetMaxProcesses(-1, cmd-maxProcesses) 0)
+goto fork_error;
+if (virSetMaxFiles(-1, cmd-maxFiles) 0)
+goto fork_error;
Hmm. uid_t and gid_t can validly be 0, hence the sentinel has to be -1.
But pid_t can never be 0 (it starts with 1), so it is easier if you let
0 be the sentinel that means current process.
+#if HAVE_SETRLIMIT
+
+# if HAVE_PRLIMIT
+static int
+virPrLimit(pid_t pid, int resource, struct rlimit *rlim)
+{
+return prlimit(pid, resource, rlim, NULL);
This doesn't report errors;
+}
+# else
+static int
+virPrLimit(pid_t pid ATTRIBUTE_UNUSED,
+ int resource ATTRIBUTE_UNUSED,
+ struct rlimit *rlim ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS, %s, _(Not supported on this platform));
+return -1;
while this does. Either the caller should always be responsible for
reporting the errors, or you should fix the HAVE_PRLIMIT version to call
virReportSystemError() as needed. Looking below at your callers, I go
for the latter - simplify the non-prlimit variant to just be:
errno = ENOSYS;
return -1;
[1]...
+}
+# endif
+
+int
+virSetMaxMemLock(pid_t pid, unsigned long long bytes)
+{
+struct rlimit rlim;
+
+if (bytes == 0)
+return 0;
+
+rlim.rlim_cur = rlim.rlim_max = bytes;
+if (pid == (pid_t)-1) {
Overkill; since there is never a process id of 0, that makes a sentinel
that is much easier to check for. (You did get the cast right, per our
IRC conversation; only I didn't realize why you needed the cast in the
first place - pid_t -1 is the process id that represents the entire
process group of the init(1) process).
+if (setrlimit(RLIMIT_MEMLOCK, rlim) 0) {
RLIMIT_MEMLOCK is not portable. POSIX lists only the following:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/setrlimit.html
RLIMIT_CORE
RLIMIT_CPU
RLIMIT_DATA
RLIMIT_FSIZE
RLIMIT_NOFILE
RLIMIT_STACK
RLIMIT_AS
The rest are extensions, so you'll need to wrap this code in #ifdef
RLIMIT_MEMLOCK and provide a fallback when it is not present. You need
#ifdef instead of #if (POSIX guarantees that all RLIMIT_* extensions in
sys/resource.h will be macros, but does not guarantee which, if any,
of those macros will have the value 0), but at least you don't need to
touch configure.ac for this particular probe.
+virReportSystemError(errno,
+ _(cannot limit locked memory to %llu),
+ bytes);
+return -1;
+}
+} else {
+if (virPrLimit(pid, RLIMIT_MEMLOCK, rlim) 0) {
prlimit(0, ...) is identical to prlimit(getpid(), ...), which in turn is
identical to setrlimit(...). Then again, since setrlimit() is more
portable than prlimit, I agree with you doing the differentiation on the
sentinel pid yourself instead of blindly trying virPrLimit().
+virReportSystemError(errno,
+ _(cannot limit locked memory
+ of process %lld to %llu),
+ (long long int)pid, bytes);
+return -1;
...[1] since here we always report any failure.
+}
+}
+return 0;
+}
+
+int
+virSetMaxProcesses(pid_t pid, unsigned int procs)
+{
+struct rlimit rlim;
+
+if (procs == 0)
+return 0;
+
+rlim.rlim_cur = rlim.rlim_max = procs;
+if (pid == (pid_t)-1) {
+if (setrlimit(RLIMIT_NPROC, rlim) 0) {
Another non-portable limit, needing #ifdef treatment.
...
+rlim.rlim_cur = rlim.rlim_max = files + 1;
+if (pid == (pid_t)-1) {
+if
Re: [libvirt] [PATCH 7/8] qemu: use new virCommandSetMax(Processes|Files)
On 04/25/2013 11:57 AM, Laine Stump wrote: These were previously being set in a custom hook function, but now that virCommand directly supports setting them, we can eliminate that part of the hook and call the APIs directly. --- src/qemu/qemu_process.c | 38 ++ 1 file changed, 2 insertions(+), 36 deletions(-) fun diffstat! 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 8/8] qemu: set qemu process' RLIMIT_MEMLOCK when VFIO is used
On 04/25/2013 11:57 AM, Laine Stump wrote: VFIO requires all of the guest's memory and IO space to be lockable in RAM. The domain's max_balloon is the maximum amount of memory the domain can have (in KiB). We add a generous 1GiB to that for IO space (still much better than KVM device assignment, where the KVM module actually *ignores* the process limits and locks everything anyway), and convert from KiB to bytes. In the case of hotplug, we are changing the limit for the already existing qemu process (prlimit() is used under the hood), and for regular commandline additions of vfio devices, we schedule a call to setrlimit() that will happen after the qemu process is forked. --- src/qemu/qemu_command.c | 22 +++--- src/qemu/qemu_hotplug.c | 24 +--- 2 files changed, 32 insertions(+), 14 deletions(-) +/* VFIO requires all of the guest's memory to be + * locked resident, plus some amount for IO + * space. Alex Williamson suggested adding 1GiB for IO + * space just to be safe (some finer tuning might be + * nice, though). + */ +virCommandSetMaxMemLock(cmd, ((unsigned long long)def-mem.max_balloon + (1024 * 1024)) * 1024); Worth wrapping the long line, perhaps at the + operator? Or even using a temporary: unsigned long long mem_kbytes = def-mem.max_balloon + (1024 * 1024); virCommandSetMaxMemLock(cmd, mem_kbytes * 1024); +virSetMaxMemLock(vm-pid, + ((unsigned long long)vm-def-mem.max_balloon + (1024 * 1024)) * 1024); and again. Line wrapping is trivial, though, so: 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 9/8] qemu: xml/args tests for VFIO hostdev and interface type='hostdev'/
On 04/25/2013 12:37 PM, Laine Stump wrote: These should be squashed in with the patch that adds commandline handling of vfio (they would fail at any earlier time). --- .../qemuxml2argv-hostdev-vfio.args | 5 +++ .../qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml | 33 + .../qemuxml2argv-net-hostdev-vfio.args | 6 .../qemuxml2argv-net-hostdev-vfio.xml | 41 ++ tests/qemuxml2argvtest.c | 6 tests/qemuxml2xmltest.c| 2 ++ 6 files changed, 93 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-vfio.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-vfio.xml 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 1/8] util: new function virPCIDeviceGetVFIOGroupDev
On 04/25/2013 04:06 PM, Eric Blake wrote:
On 04/25/2013 11:57 AM, Laine Stump wrote:
Given a virPCIDevice, this function returns the path for the device
that controls the vfio group the device belongs to,
e.g. /dev/vfio/15.
---
+if (virAsprintf(groupDev, /dev/vfio/%s, basename(groupPath)) 0) {
We shouldn't be using basename() from libgen.h - it has horrible
portability problems, and is not thread-safe. Instead, use
last_component() from gnulib's dirname.h. [I really ought to add a
syntax-check to cfg.mk and clean up the existing offenders].
ACK with that fixed.
Okay. I fixed that. I'm going to send one message for each of the three
series when I push them.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/8] qemu: use vfio-pci on commandline when appropriate
On 04/25/2013 05:07 PM, Eric Blake wrote:
On 04/25/2013 11:57 AM, Laine Stump wrote:
The device option for vfio-pci is nearly identical to that for
pci-assign - only the configfd parameter isn't supported (or needed).
Checking for presence of the bootindex parameter is done separately
from constructing the commandline, similar to how it is done for
pci-assign.
---
src/qemu/qemu_command.c | 48
++--
src/qemu/qemu_hotplug.c | 13 -
2 files changed, 50 insertions(+), 11 deletions(-)
@@ -7850,12 +7855,23 @@ qemuBuildCommandLine(virConnectPtr conn,
supported for PCI and USB devices));
goto error;
} else {
-if (hostdev-source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_BOOTINDEX)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(booting from assigned PCI devices is
not
- supported with this version of
qemu));
-goto error;
+if (hostdev-source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+if (hostdev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+if (!virQEMUCapsGet(qemuCaps,
QEMU_CAPS_VFIO_PCI_BOOTINDEX)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(booting from PCI devices
assigned with VFIO
+ is not supported with this
version of qemu));
Line break after space...
+goto error;
+}
+} else {
+if (!virQEMUCapsGet(qemuCaps,
QEMU_CAPS_PCI_BOOTINDEX)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(booting from assigned PCI
devices is not
+ supported with this version
of qemu));
...line break before space. Looks a bit inconsistent, but the end
result is the same.
I fixed those all up so they have the space before the line break.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/8] qemu: set qemu process' RLIMIT_MEMLOCK when VFIO is used
On 04/25/2013 06:14 PM, Eric Blake wrote: On 04/25/2013 11:57 AM, Laine Stump wrote: VFIO requires all of the guest's memory and IO space to be lockable in RAM. The domain's max_balloon is the maximum amount of memory the domain can have (in KiB). We add a generous 1GiB to that for IO space (still much better than KVM device assignment, where the KVM module actually *ignores* the process limits and locks everything anyway), and convert from KiB to bytes. In the case of hotplug, we are changing the limit for the already existing qemu process (prlimit() is used under the hood), and for regular commandline additions of vfio devices, we schedule a call to setrlimit() that will happen after the qemu process is forked. --- src/qemu/qemu_command.c | 22 +++--- src/qemu/qemu_hotplug.c | 24 +--- 2 files changed, 32 insertions(+), 14 deletions(-) +/* VFIO requires all of the guest's memory to be + * locked resident, plus some amount for IO + * space. Alex Williamson suggested adding 1GiB for IO + * space just to be safe (some finer tuning might be + * nice, though). + */ +virCommandSetMaxMemLock(cmd, ((unsigned long long)def-mem.max_balloon + (1024 * 1024)) * 1024); Worth wrapping the long line, perhaps at the + operator? Or even using a temporary: unsigned long long mem_kbytes = def-mem.max_balloon + (1024 * 1024); virCommandSetMaxMemLock(cmd, mem_kbytes * 1024); I did the latter. +virSetMaxMemLock(vm-pid, + ((unsigned long long)vm-def-mem.max_balloon + (1024 * 1024)) * 1024); and again. Same here. Line wrapping is trivial, though, so: ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] VFIO support part 3
On 04/25/2013 01:57 PM, Laine Stump wrote: This *almost* completes VFIO support. I still need to add a couple of xml/args test cases. I actually added the test cases as a separate patch in this series, but then squashed them into the earlier patch that constructs a qemu commandline with vfio support before pushing Laine Stump (8): util: new function virPCIDeviceGetVFIOGroupDev security: update hostdev labelling functions for VFIO qemu: add QEMU_CAPS_VFIO_PCI_BOOTINDEX qemu: use vfio-pci on commandline when appropriate I pushed the above 4 patches, but didn't push the ones below [PATCH 3/8] qemu: add VFIO devices to cgroup ACL Eric questioned whether 3/8 was doing the right thing, and I'd like to get danpb's opinion. [PATCH 6/8] util: new virCommandSetMax(MemLock|Processes|Files) [PATCH 7/8] qemu: use new virCommandSetMax(Processes|Files) [PATCH 8/8] qemu: set qemu process' RLIMIT_MEMLOCK when VFIO is used Eric pointed out several problems with 6/8, which I've addressed, and 7/8 and 8/8 depend on 6/8 so I can't push them yet. To make things easier, I'm going to close out this thread and repost a new thread with just these 4 remaining patches. If they are ACKed and DV wants to freeze before I wake up, please feel free to push them. Thanks for all the quick reviews! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] qemu: add VFIO devices to cgroup ACL
We don't know exactly the names of the VFIO devices that will be
needed (and due to hotplug, we can't ever assume we won't need them at
all), so we just add an ACL to allow any vfio device - they all have
the major number 244 (/dev/vfio/vfio is 244,0, and the /dev/vfio/n
devices are up from there).
---
src/qemu/qemu_cgroup.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 891984a..ad2027d 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -44,6 +44,7 @@ static const char *const defaultDeviceACL[] = {
};
#define DEVICE_PTY_MAJOR 136
#define DEVICE_SND_MAJOR 116
+#define DEVICE_VFIO_MAJOR 244
static int
qemuSetupDiskPathAllow(virDomainDiskDefPtr disk,
@@ -388,6 +389,16 @@ int qemuSetupCgroup(virQEMUDriverPtr driver,
}
}
+rc = virCgroupAllowDeviceMajor(priv-cgroup, 'c', DEVICE_VFIO_MAJOR,
+ VIR_CGROUP_DEVICE_RW);
+virDomainAuditCgroupMajor(vm, priv-cgroup, allow, DEVICE_VFIO_MAJOR,
+ vfio, rw, rc == 0);
+if (rc != 0) {
+virReportSystemError(-rc, %s,
+ _(unable to allow /dev/vfio/ devices));
+goto cleanup;
+}
+
for (i = 0; deviceACL[i] != NULL ; i++) {
if (access(deviceACL[i], F_OK) 0) {
VIR_DEBUG(Ignoring non-existant device %s,
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] qemu: use new virCommandSetMax(Processes|Files)
These were previously being set in a custom hook function, but now
that virCommand directly supports setting them, we can eliminate that
part of the hook and call the APIs directly.
---
src/qemu/qemu_process.c | 38 ++
1 file changed, 2 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 925939d..f12d7d5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -25,8 +25,6 @@
#include unistd.h
#include signal.h
#include sys/stat.h
-#include sys/time.h
-#include sys/resource.h
#if defined(__linux__)
# include linux/capability.h
#elif defined(__FreeBSD__)
@@ -2453,37 +2451,6 @@ qemuProcessPrepareChardevDevice(virDomainDefPtr def
ATTRIBUTE_UNUSED,
}
-static int
-qemuProcessLimits(virQEMUDriverConfigPtr cfg)
-{
-struct rlimit rlim;
-
-if (cfg-maxProcesses 0) {
-rlim.rlim_cur = rlim.rlim_max = cfg-maxProcesses;
-if (setrlimit(RLIMIT_NPROC, rlim) 0) {
-virReportSystemError(errno,
- _(cannot limit number of processes to %d),
- cfg-maxProcesses);
-return -1;
-}
-}
-
-if (cfg-maxFiles 0) {
-/* Max number of opened files is one greater than
- * actual limit. See man setrlimit */
-rlim.rlim_cur = rlim.rlim_max = cfg-maxFiles + 1;
-if (setrlimit(RLIMIT_NOFILE, rlim) 0) {
-virReportSystemError(errno,
- _(cannot set max opened files to %d),
- cfg-maxFiles);
-return -1;
-}
-}
-
-return 0;
-}
-
-
struct qemuProcessHookData {
virConnectPtr conn;
virDomainObjPtr vm;
@@ -2526,9 +2493,6 @@ static int qemuProcessHook(void *data)
if (virSecurityManagerClearSocketLabel(h-driver-securityManager,
h-vm-def) 0)
goto cleanup;
-if (qemuProcessLimits(h-cfg) 0)
-goto cleanup;
-
/* This must take place before exec(), so that all QEMU
* memory allocation is on the correct NUMA node
*/
@@ -3697,6 +3661,8 @@ int qemuProcessStart(virConnectPtr conn,
}
virCommandSetPreExecHook(cmd, qemuProcessHook, hookData);
+virCommandSetMaxProcesses(cmd, cfg-maxProcesses);
+virCommandSetMaxFiles(cmd, cfg-maxFiles);
VIR_DEBUG(Setting up security labelling);
if (virSecurityManagerSetChildProcessLabel(driver-securityManager,
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Remaining patches from VFIO series
I've pushed everything else from all 3 VFIO series. Patch 1/4 in this series had questions from Eric about whether it is the right way to go, or if we want to do something more limited: https://www.redhat.com/archives/libvir-list/2013-April/msg01864.html Eric and danpb had both raised issues with Patch 2/4, so I redid it addressing all the points they brought up, and it's now ready for review: https://www.redhat.com/archives/libvir-list/2013-April/msg01853.html https://www.redhat.com/archives/libvir-list/2013-April/msg01869.html 3/4 and 4/4 were ACKed, but depend on 2/4, so I couldn't push them. If these patches are ACKed and DV wants to make the RC1 snapshot before I wake up, anyone else feel free to push these 4 patches as they are (assuming they're ACKed, of course :-) Laine Stump (4): util: new virCommandSetMax(MemLock|Processes|Files) qemu: use new virCommandSetMax(Processes|Files) qemu: set qemu process' RLIMIT_MEMLOCK when VFIO is used qemu: add VFIO devices to cgroup ACL configure.ac | 2 +- src/libvirt_private.syms | 6 ++ src/qemu/qemu_cgroup.c | 11 src/qemu/qemu_command.c | 25 +--- src/qemu/qemu_hotplug.c | 27 ++--- src/qemu/qemu_process.c | 38 +--- src/util/vircommand.c| 38 src/util/vircommand.h| 4 ++ src/util/virprocess.c| 152 ++- src/util/virprocess.h| 5 +- 10 files changed, 255 insertions(+), 53 deletions(-) -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] qemu: set qemu process' RLIMIT_MEMLOCK when VFIO is used
VFIO requires all of the guest's memory and IO space to be lockable in
RAM. The domain's max_balloon is the maximum amount of memory the
domain can have (in KiB). We add a generous 1GiB to that for IO space
(still much better than KVM device assignment, where the KVM module
actually *ignores* the process limits and locks everything anyway),
and convert from KiB to bytes.
In the case of hotplug, we are changing the limit for the already
existing qemu process (prlimit() is used under the hood), and for
regular commandline additions of vfio devices, we schedule a call to
setrlimit() that will happen after the qemu process is forked.
---
src/qemu/qemu_command.c | 25 ++---
src/qemu/qemu_hotplug.c | 27 ---
2 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6f6b61b..2ce0672 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7906,13 +7906,24 @@ qemuBuildCommandLine(virConnectPtr conn,
if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS
hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
{
-if ((hostdev-source.subsys.u.pci.backend
- == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO)
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(VFIO PCI device assignment is not supported
by
- this version of qemu));
-goto error;
+if (hostdev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+unsigned long long memKB;
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(VFIO PCI device assignment is not
+ supported by this version of qemu));
+goto error;
+}
+/* VFIO requires all of the guest's memory to be
+ * locked resident, plus some amount for IO
+ * space. Alex Williamson suggested adding 1GiB for IO
+ * space just to be safe (some finer tuning might be
+ * nice, though).
+ */
+memKB = def-mem.max_balloon + (1024 * 1024);
+virCommandSetMaxMemLock(cmd, memKB * 1024);
}
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f608ec4..15cca08 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -38,6 +38,7 @@
#include viralloc.h
#include virpci.h
#include virfile.h
+#include virprocess.h
#include qemu_cgroup.h
#include locking/domain_lock.h
#include network/bridge_driver.h
@@ -999,13 +1000,25 @@ int qemuDomainAttachHostPciDevice(virQEMUDriverPtr
driver,
hostdev, 1) 0)
return -1;
-if ((hostdev-source.subsys.u.pci.backend
- == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO)
-!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(VFIO PCI device assignment is not supported by
- this version of qemu));
-goto error;
+if (hostdev-source.subsys.u.pci.backend
+== VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_VFIO) {
+unsigned long long memKB;
+
+if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(VFIO PCI device assignment is not
+ supported by this version of qemu));
+goto error;
+}
+/* VFIO requires all of the guest's memory to be locked
+ * resident, plus some amount for IO space. Alex Williamson
+ * suggested adding 1GiB for IO space just to be safe (some
+ * finer tuning might be nice, though).
+ * In this case, the guest's memory may already be locked, but
+ * it doesn't hurt to change the limit to the same value.
+ */
+memKB = vm-def-mem.max_balloon + (1024 * 1024);
+virProcessSetMaxMemLock(vm-pid, memKB * 1024);
}
if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] util: new virCommandSetMax(MemLock|Processes|Files)
This patch adds two sets of functions:
1) lower level virProcessSet*() functions that will immediately set
the RLIMIT_MEMLOCK. RLIMIT_NPROC, or RLIMIT_NOFILE of either the
current process (using setrlimit()) or any other process (using
prlimit()). current process is indicated by passing a 0 for pid.
2) functions for virCommand* that will setup a virCommand object to
set those limits at a later time just after it has forked a new
process, but before it execs the new program.
configure.ac has prlimit and setrlimit added to the list of functions
to check for, and the low level functions log an unsupported error)
on platforms that don't support those functions.
---
configure.ac | 2 +-
src/libvirt_private.syms | 6 ++
src/util/vircommand.c| 38
src/util/vircommand.h| 4 ++
src/util/virprocess.c| 152 ++-
src/util/virprocess.h| 5 +-
6 files changed, 204 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac
index 89dae3d..23c24d2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -194,7 +194,7 @@ dnl Availability of various common functions (non-fatal if
missing),
dnl and various less common threadsafe functions
AC_CHECK_FUNCS_ONCE([cfmakeraw geteuid getgid getgrnam_r getmntent_r \
getpwuid_r getuid initgroups kill mmap newlocale posix_fallocate \
- posix_memalign regexec sched_getaffinity setns symlink])
+ posix_memalign prlimit regexec sched_getaffinity setns setrlimit symlink])
dnl Availability of pthread functions (if missing, win32 threading is
dnl assumed). Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE.
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2a2c40e..0bb6f5f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1185,6 +1185,9 @@ virCommandSetErrorFD;
virCommandSetGID;
virCommandSetInputBuffer;
virCommandSetInputFD;
+virCommandSetMaxFiles;
+virCommandSetMaxMemLock;
+virCommandSetMaxProcesses;
virCommandSetOutputBuffer;
virCommandSetOutputFD;
virCommandSetPidFile;
@@ -1668,6 +1671,9 @@ virProcessGetNamespaces;
virProcessKill;
virProcessKillPainfully;
virProcessSetAffinity;
+virProcessSetMaxFiles;
+virProcessSetMaxMemLock;
+virProcessSetMaxProcesses;
virProcessSetNamespaces;
virProcessTranslateStatus;
virProcessWait;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index ac56a63..98521ec 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -107,6 +107,10 @@ struct _virCommand {
char *pidfile;
bool reap;
+unsigned long long maxMemLock;
+unsigned int maxProcesses;
+unsigned int maxFiles;
+
uid_t uid;
gid_t gid;
unsigned long long capabilities;
@@ -598,6 +602,13 @@ virExec(virCommandPtr cmd)
goto fork_error;
}
+if (virProcessSetMaxMemLock(0, cmd-maxMemLock) 0)
+goto fork_error;
+if (virProcessSetMaxProcesses(0, cmd-maxProcesses) 0)
+goto fork_error;
+if (virProcessSetMaxFiles(0, cmd-maxFiles) 0)
+goto fork_error;
+
if (cmd-hook) {
VIR_DEBUG(Run hook %p %p, cmd-hook, cmd-opaque);
ret = cmd-hook(cmd-opaque);
@@ -958,6 +969,33 @@ virCommandSetUID(virCommandPtr cmd, uid_t uid)
cmd-uid = uid;
}
+void
+virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-maxMemLock = bytes;
+}
+
+void
+virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-maxProcesses = procs;
+}
+
+void
+virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-maxFiles = files;
+}
+
/**
* virCommandClearCaps:
* @cmd: the command to modify
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 6c13795..18568fe 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -65,6 +65,10 @@ void virCommandSetGID(virCommandPtr cmd, gid_t gid);
void virCommandSetUID(virCommandPtr cmd, uid_t uid);
+void virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes);
+void virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs);
+void virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files);
+
void virCommandClearCaps(virCommandPtr cmd);
void virCommandAllowCap(virCommandPtr cmd,
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index a492bd1..fb81805 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1,7 +1,7 @@
/*
* virprocess.c: interaction with processes
*
- * Copyright (C) 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2010-2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -27,6 +27,10 @@
#include signal.h
#include errno.h
#include sys/wait.h
+#if HAVE_SETRLIMIT
+# include sys/time.h
+# include sys/resource.h
+#endif
Re: [libvirt] [PATCH 0/7] VFIO support part 2
On 04/24/2013 03:26 PM, Laine Stump wrote: More patches to go on top of the 3 patches yesterday. These patches add a new API that I just learned was necessary - virNodeDeviceDetachFlags() (explanation in patches), implement it for xen and qemu, and use it in virsh. Laine Stump (7): pci: keep a stubDriver in each virPCIDevice qemu: bind/unbind stub driver according to config driver name='x'/ hypervisor api: new virNodeDeviceDetachFlags hypervisor api: implement RPC calls for virNodeDeviceDetachFlags qemu: implement virNodeDeviceDetachFlags backend xen: implement virNodeDeviceDetachFlags backend virsh: use new virNodeDeviceDetachFlags I addressed Eric's concerns and pushed these 7 patches. Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] start of patches to support VFIO device assignment
On 04/23/2013 12:45 PM, Laine Stump wrote: This work isn't finished, but since there's a freeze coming up in a few days, I thought I should send these patches that *are* finished to get them out of the way before the last minute rush. Patch 1 is trivial. Patch 2 is longer, but completely mechanical (NOTE: there may be some uses of that struct living in code that I lack the environment to compile for, so test builds by people with odd platforms would be appreciated!) Patch 3 is standard parser/formatter/RNG/docs for the new XML. I haven't added any test cases, because lacking backend functionality I could only test xml-to-xml, which would require adding yet another test xml file, and I'd rather wait until the backend commandline-building code is in place, then simply add the new element to a few existing test XML files. In email awhile ago, danpb had suggested driver name='vfio|qemu'/ I've used driver name='kvm'/ because, unlike vhost-net where driver name='qemu'/ indicates that the driver used is in the usermode qemu process (and *not* in the kernel), in this case driver name='kvm'/ indicates that the driver used is in the kernel (and I've seen it referenced in several places as being done by KVM, but never as done by QEMU (makes sense, since qemu is all in userland) Laine Stump (3): qemu: detect vfio-pci device assignment support conf: put hostdev pci address in a struct conf: formatter/parser/RNG/docs for hostdev driver name='kvm|vfio'/ I pushed these three patches after making the small changes Eric requested. Thanks for the reviews. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-perl][PATCH] Fix typo in domain help page
The help page of $dom-set_metadata have a typo,this patch fix it. --- lib/Sys/Virt/Domain.pm |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm index a502029..053f127 100644 --- a/lib/Sys/Virt/Domain.pm +++ b/lib/Sys/Virt/Domain.pm @@ -112,7 +112,7 @@ C$flags parameter defaults to zero. Sets the metadata element of type C$type to hold the value C$val. If C$type is CSys::Virt::Domain::METADATA_ELEMENT then the C$key and C$uri elements specify an XML namespace -to use, otherwise they should both be Cnudef. The optional +to use, otherwise they should both be Cundef. The optional C$flags parameter defaults to zero. =item $dom-is_active() -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [question] Why doesn't virsh command support an optional parameter
hi guys, It's hard for me to understand why doesn't virsh command support an optional parameter because I think omitting a parameter and offering a default value sometimes is quite convenient. For example: $ virsh shutdown guest --mode acpi The option --mode only accept string acpi, agent, initctl and signal, assuming that acpi is the most frequently used parameter when --mode is specified, why not just using $ virsh shutdown guest --mode acpi instead. Maybe this example is not very precise, but what I mean is if there is an option which only accepts several candidate values and one of the value is always used when it is specified, then we can make it the default parameter of this option. This would be convenient and save some typing. Of course, this can also result in ambiguity to people who is not familiar with the command. But with the help, it won't trap one very long, will it? So, why doesn't virsh support this and what do you think of adding this feature? Thanks Zhang Xiaohe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [question] Why doesn't virsh command support an optional parameter
On 04/25/2013 09:19 PM, Zhang Xiaohe wrote: hi guys, It's hard for me to understand why doesn't virsh command support an optional parameter because I think omitting a parameter and offering a default value sometimes is quite convenient. For example: $ virsh shutdown guest --mode acpi Did you mean 'virsh shutdown guest --mode' here, with a missing trailing argument to the --mode option? The option --mode only accept string acpi, agent, initctl and signal, assuming that acpi is the most frequently used parameter when --mode is specified, why not just using $ virsh shutdown guest --mode acpi instead. The default is to not specify --mode at all; if you are worried about a mode, then there is no way for virsh to know what mode is best for you. In this particular example, while acpi works for qemu, it does not work for lxc; there, the best default is more likely to be initctl. Since we can't make a different default per hypervisor without repeating all the logic of a hypervisor in virsh, this seems like a change that's not worth making. Also, consider what it would take to implement optional option parsing. Existing scripts that used --mode=acpi are immune, but scripts that used '--mode acpi' as two arguments now have a problem - how do we know that the user didn't intend '--mode' with its default setting, and 'acpi' as a separate argument? Maybe this example is not very precise, but what I mean is if there is an option which only accepts several candidate values and one of the value is always used when it is specified, then we can make it the default parameter of this option. This would be convenient and save some typing. Of course, this can also result in ambiguity to people who is not familiar with the command. But with the help, it won't trap one very long, will it? I'm unwilling to make any current option that requires an argument switch to becoming an option with an optional argument (because that in itself introduces parser ambiguities that may break scrips written against an older virsh). However, there IS a request to add user-defined alias support to virsh. If you find yourself frequently typing a given command, it would make sense to have virsh have a way to let you define an alias that shortens the amount of typing you have to do to get that alias. It wouldn't be default out of the box, but may be a compromise that could help the situation you are worried about. For virsh aliases to work, someone would have to submit patches - is that something you are interested in writing? -- 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] [question] Why doesn't virsh command support an optional parameter
于 2013年04月26日 11:35, Eric Blake 写道: On 04/25/2013 09:19 PM, Zhang Xiaohe wrote: hi guys, It's hard for me to understand why doesn't virsh command support an optional parameter because I think omitting a parameter and offering a default value sometimes is quite convenient. For example: $ virsh shutdown guest --mode acpi Did you mean 'virsh shutdown guest --mode' here, with a missing trailing argument to the --mode option? Sorry, I intended to say that use 'virsh shutdown guest --mode' instead of 'virsh shutdown guest --mode acpi' The option --mode only accept string acpi, agent, initctl and signal, assuming that acpi is the most frequently used parameter when --mode is specified, why not just using $ virsh shutdown guest --mode acpi instead. The default is to not specify --mode at all; if you are worried about a mode, then there is no way for virsh to know what mode is best for you. In this particular example, while acpi works for qemu, it does not work for lxc; there, the best default is more likely to be initctl. Since we can't make a different default per hypervisor without repeating all the logic of a hypervisor in virsh, this seems like a change that's not worth making. I use this example to describe only the format, so just ignore what mode really means. Also, consider what it would take to implement optional option parsing. Existing scripts that used --mode=acpi are immune, but scripts that used '--mode acpi' as two arguments now have a problem - how do we know that the user didn't intend '--mode' with its default setting, and 'acpi' as a separate argument? This does be a problem. Maybe this example is not very precise, but what I mean is if there is an option which only accepts several candidate values and one of the value is always used when it is specified, then we can make it the default parameter of this option. This would be convenient and save some typing. Of course, this can also result in ambiguity to people who is not familiar with the command. But with the help, it won't trap one very long, will it? I'm unwilling to make any current option that requires an argument switch to becoming an option with an optional argument (because that in itself introduces parser ambiguities that may break scrips written against an older virsh). OK, I see. However, there IS a request to add user-defined alias support to virsh. If you find yourself frequently typing a given command, it would make sense to have virsh have a way to let you define an alias that shortens the amount of typing you have to do to get that alias. It wouldn't be default out of the box, but may be a compromise that could help the situation you are worried about. For virsh aliases to work, someone would have to submit patches - is that something you are interested in writing? My members and I currently are writing a new feature for qemu and this needs to add an option to one virsh command. We are discussing whether a default value can be offered with an optional parameter because one of the values is always valid while the others are not. But now we'll try another way. Thanks, Zhang Xiaohe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
