Re: [libvirt] [PATCH 00/48 v3] Atomic APIs to list objects
ping On 2012年08月03日 23:48, Osier Yang wrote: v2 - v3: * Various document fixes/improvements (Suggested by Laine and Eric) * Destroy virdomainlist.[ch], and folder all list helper funcs into its own *_conf.[ch] * Improve the helpers to get the object list. See https://www.redhat.com/archives/libvir-list/2012-July/msg01267.html * Rebased on top of commit 3df9626 by Peter and the virReportError series by Daniel. * Rebased on top of virsh split series. (I'm luck there is no change on those commands since v2). Except the already supported APIs for domain and domain snapshot, this series add the APIs for the left objects, including storage pool, storage vol, network, interface, node device, nwfilter, and secret. * Storage pool: - Support filtering the returned pool objects by active|inactive, persistent|transient, autostart|no-autostart, and pool types. - New options for virsh, --type to accept multiple pool types. * Storage vol: - Simply returns all the vol objects of a pool. * Network: - Support filtering the results using flags active|inactive, persistent|transient, autostart|no-autostart - New options for virsh. * Interface: - Support filtering the results using flags active|inactive. It's still O(n) underlying, as interface driver doesn't manage the objects itself, but using netcf lib instead. And netcf APIs don't support returning the struct yet. * Node Device: - Support filtering the results using capabilities type of the devices. - Extend --cap to accept multiple capability type. * Network Filter: - Simply returns all the objects. * Secret: - Simply returns all the objects. Osier Yang (48): Avoid strcase check for virsh-*.c Destroy virdomainlist.[ch] list: Define new API virStorageListALlStoragePools list: Add helpers for listing storage pool objects list: Implement the RPC calls for virConnectListAllStoragePools list: Implement listAllStoragePools for storage driver list: Implement listAllStoragePools for test driver list: Add helper to convert strings separated by ', ' to array virsh: Fix the wrong doc for pool-list list: Change MATCH for common use in virsh list: Use virConnectListAllStoragePools in virsh virsh: Use vshPrint instead of printf python: Expose virStorageListAllStoragePools to python binding list: Define new API virStoragePoolListAllVolumes list: Implemente RPC calls for virStoragePoolListAllVolumes list: Implement virStoragePoolListAllVolumes for storage driver list: Implement virStoragePoolListAllVolumes for test driver list: Use virStoragePoolListAllVolumes in virsh list: Expose virStoragePoolListAllVolumes to Python binding list: Define new API virConnectListAllNetworks list: Implement RPC calls for virConnectListAllNetworks list: Add helpers to list network objects list: Implement listAllNetworks for network driver list: Implement listAllNetworks for test driver list: Use virConnectListAllNetworks in virsh list: Expose virConnectListAllNetworks to Python binding list: Define new API virConnectListAllInterfaces list: Implemente RPC calls for virConnectListAllInterfaces list: Implement listAllInterfaces list: Use virConnectListAllInterfaces in virsh list: Expose virConnectListAllInterfaces to Python binding list: Define new API virConnectListAllNodeDevices list: Implemente RPC calls for virConnectListAllNodeDevices list: Add helpers for listing node devices list: Implement listAllNodeDevices list: Expose virConnectListAllNodeDevices to Python binding virsh: Fix a bug of nodedev-list list: Use virConnectListAllNodeDevices in virsh list: Define new API virConnectListAllNWFilters list: Implement RPC calls for virConnectListAllNWFilters list: Implement listAllNWFilters list: Use virConnectListAllNWFilters in virsh list: Expose virConnectListAllNWFilters to Python binding list: Define new API virConnectListAllSecrets list: Implement RPC calls for virConnectListAllSecrets list: Implement listAllSecrets list: Use virConnectListAllSecrets in virsh list: Expose virConnectListAllSecrets to Python binding cfg.mk|5 +- daemon/remote.c | 382 ++ include/libvirt/libvirt.h.in | 101 ++- python/generator.py | 11 +- python/libvirt-override-api.xml | 44 +++- python/libvirt-override-virConnect.py | 72 + python/libvirt-override-virStoragePool.py | 11 + python/libvirt-override.c | 337 src/Makefile.am | 125 src/conf/domain_conf.c| 189 +++- src/conf/domain_conf.h| 56 src/conf/network_conf.c |
[libvirt] [PATCH 0/2] Add lsi and virtio-scsi qemu caps
On qemu-kvm-0.15.1, it supports only lsi scsi controller model. On qemu-kvm-0.12.1.2, it supports only virtio-scsi-pci scsi model On qemu 1.1.50, it supports both. So, instead of using the lsilogic model by default, the patch tries to check which model the current QEMU supports, then choose it, lsi has the priority. If a scsi model is given in XML explicitly, we try to check if the underlying QEMU supports it or not, raise an error on checking failure. Guannan Ren(2) (1/2)qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers (2/2)test: add lsi and virtio-scsi qemu caps in testcases. src/qemu/qemu_capabilities.c |7 --- src/qemu/qemu_capabilities.h |2 - src/qemu/qemu_command.c | 88 +++-- src/qemu/qemu_command.h |3 +- tests/qemuhelptest.c | 10 +--- tests/qemuxml2argvtest.c | 16 +++- 6 files changed, 34 insertions(+), 92 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel.
When scsi model is given explicitly in XML(model 0) checking if the
underlying QEMU support it or not first, raise error on checking failure.
If the model is not given(mode = 0), return lsi or virtio-scsi
which is supported, lsi take the priority.
---
src/qemu/qemu_capabilities.c |7 +++
src/qemu/qemu_capabilities.h |2 +
src/qemu/qemu_command.c | 88 ++---
src/qemu/qemu_command.h |3 +-
4 files changed, 75 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 85c49a2..8282ad8 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -169,6 +169,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
virtio-s390,
balloon-event,
+ lsi, /*100*/
+ virtio-scsi-pci,
);
struct qemu_feature_flags {
@@ -1450,6 +1452,11 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr
flags)
strstr(str, name \virtio-serial-s390\))
qemuCapsSet(flags, QEMU_CAPS_VIRTIO_S390);
+if (strstr(str, name \lsi53c895a\))
+qemuCapsSet(flags, QEMU_CAPS_SCSI_LSI);
+if (strstr(str, name \virtio-scsi-pci\))
+qemuCapsSet(flags, QEMU_CAPS_VIRIO_SCSI_PCI);
+
/* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */
if (!qemuCapsGet(flags, QEMU_CAPS_CHARDEV_SPICEVMC)
strstr(str, name \spicevmc\))
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e8251dc..d2d68a9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -135,6 +135,8 @@ enum qemuCapsFlags {
QEMU_CAPS_NEC_USB_XHCI = 97, /* -device nec-usb-xhci */
QEMU_CAPS_VIRTIO_S390= 98, /* -device virtio-*-s390 */
QEMU_CAPS_BALLOON_EVENT = 99, /* Async event for balloon changes */
+QEMU_CAPS_SCSI_LSI = 100, /* -device lsi */
+QEMU_CAPS_VIRIO_SCSI_PCI = 101, /* -device virtio-scsi-pci */
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 6ad65a6..6a4578d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -469,19 +469,60 @@ static int
qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk)
}
static int
-qemuDefaultScsiControllerModel(virDomainDefPtr def) {
-if (STREQ(def-os.arch, ppc64)
-STREQ(def-os.machine, pseries)) {
-return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+qemuCheckScsiControllerModel(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
+ int *model)
+{
+if (*model 0) {
+switch (*model) {
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(This QEMU doesn't support
+ lsi scsi controller));
+return -1;
+}
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+if (!qemuCapsGet(qemuCaps, QEMU_CAPS_VIRIO_SCSI_PCI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(This QEMU doesn't support
+ virtio scsi controller));
+return -1;
+}
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
+/*TODO: need checking work here if necessary */
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unsupported controller model: %s),
+
virDomainControllerModelSCSITypeToString(*model));
+return -1;
+}
} else {
-return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+if (qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+*model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+} else if (qemuCapsGet(qemuCaps, QEMU_CAPS_VIRIO_SCSI_PCI)) {
+*model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
+} else if (STREQ(def-os.arch, ppc64)
+ STREQ(def-os.machine, pseries)) {
+*model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to determine model for scsi controller));
+return -1;
+}
}
+
+return 0;
}
/* Our custom -drive naming scheme used with id= */
static int
qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def,
-virDomainDiskDefPtr disk)
+virDomainDiskDefPtr
[libvirt] [PATCH 2/2] test: add lsi and virtio-scsi qemu caps in testcases.
--- tests/qemuhelptest.c | 10 +++--- tests/qemuxml2argvtest.c | 16 ++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 012ba26..62afabf 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -482,7 +482,8 @@ mymain(void) QEMU_CAPS_PCI_ROMBAR, QEMU_CAPS_NO_ACPI, QEMU_CAPS_VIRTIO_BLK_SG_IO, -QEMU_CAPS_CPU_HOST); +QEMU_CAPS_CPU_HOST, +QEMU_CAPS_SCSI_LSI); DO_TEST(qemu-kvm-0.12.1.2-rhel61, 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -677,7 +678,8 @@ mymain(void) QEMU_CAPS_FSDEV_WRITEOUT, QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_SCSI_CD, -QEMU_CAPS_IDE_CD); +QEMU_CAPS_IDE_CD, +QEMU_CAPS_SCSI_LSI); DO_TEST(qemu-1.1.0, 1001000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -754,7 +756,9 @@ mymain(void) QEMU_CAPS_IDE_CD, QEMU_CAPS_NO_USER_CONFIG, QEMU_CAPS_HDA_MICRO, -QEMU_CAPS_NEC_USB_XHCI); +QEMU_CAPS_NEC_USB_XHCI, +QEMU_CAPS_SCSI_LSI, +QEMU_CAPS_VIRIO_SCSI_PCI); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 39fcd9f..af996f3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -469,16 +469,19 @@ mymain(void) DO_TEST(disk-usb-device, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(disk-scsi-device, -QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, +QEMU_CAPS_SCSI_LSI); DO_TEST(disk-scsi-device-auto, -QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, +QEMU_CAPS_SCSI_LSI); DO_TEST(disk-scsi-disk-split, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, -QEMU_CAPS_SCSI_CD); +QEMU_CAPS_SCSI_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRIO_SCSI_PCI); DO_TEST(disk-scsi-vscsi, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(disk-scsi-virtio-scsi, -QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, +QEMU_CAPS_VIRIO_SCSI_PCI); DO_TEST(disk-sata-device, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_ICH9_AHCI); @@ -508,7 +511,8 @@ mymain(void) DO_TEST(disk-scsi-lun-passthrough, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, -QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_VIRTIO_BLK_SG_IO); +QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_VIRTIO_BLK_SG_IO, +QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRIO_SCSI_PCI); DO_TEST(graphics-vnc, NONE); DO_TEST(graphics-vnc-socket, NONE); @@ -768,7 +772,7 @@ mymain(void) DO_TEST(multifunction-pci-device, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, -QEMU_CAPS_PCI_MULTIFUNCTION); +QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_SCSI_LSI); DO_TEST(monitor-json, QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_MONITOR_JSON, QEMU_CAPS_NODEFCONFIG); -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/17] create a new cgroup and move all hypervisor threads to the new cgroup
On Mon, Aug 06, 2012 at 03:18:07PM -0600, Eric Blake wrote:
On 08/03/2012 12:36 AM, Hu Tao wrote:
From: Wen Congyang we...@cn.fujitsu.com
Create a new cgroup and move all hypervisor threads to the new cgroup.
And then we can do the other things:
1. limit only vcpu usage rather than the whole qemu
2. limit for hypervisor threads(include vhost-net threads)
Signed-off-by: Wen Congyang we...@cn.fujitsu.com
Signed-off-by: Tang Chen tangc...@cn.fujitsu.com
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
+int qemuSetupCgroupForHypervisor(struct qemud_driver *driver,
+ virDomainObjPtr vm)
+{
+
+for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
+if (!qemuCgroupControllerActive(driver, i)) {
+VIR_WARN(cgroup %d is not active, i);
+continue;
+}
Do we need to do this for every controller, or only for the cpu and
cpuacct controllers?
How about to add a third parameter to let the caller choose which cgroup
controller to set up?
--
Thanks,
Hu Tao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Avoid libvirtd crash in qemuDomainObjExitAgentInternal
* src/qemu/qemu_domain.c (qemuDomainObjExitAgentInternal): fix crashing
libvirtd due to derefing a NULL pointer.
For details, please see bug:
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=845966
Signed-off-by: Alex Jia a...@redhat.com
---
src/qemu/qemu_domain.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 86f0265..8667b6c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1136,12 +1136,14 @@ qemuDomainObjExitAgentInternal(struct qemud_driver
*driver,
virDomainObjPtr obj)
{
qemuDomainObjPrivatePtr priv = obj-privateData;
-int refs;
+int refs = -1;
-refs = qemuAgentUnref(priv-agent);
+if (priv-agent) {
+refs = qemuAgentUnref(priv-agent);
-if (refs 0)
-qemuAgentUnlock(priv-agent);
+if (refs 0)
+qemuAgentUnlock(priv-agent);
+}
if (driver_locked)
qemuDriverLock(driver);
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/48] list: Define new API virStorageListALlStoragePools
On Fri, Aug 03, 2012 at 11:48:06PM +0800, Osier Yang wrote:
This introduces a new API to list the storage pool objects,
4 groups of flags are provided to filter the returned pools:
Typo on the subject:
capital all in virStorageListALlStoragePools
^
Cheers,
-- Guido
* Active or not
* Autostarting or not
* Persistent or not
* And the pool type.
include/libvirt/libvirt.h.in: New enum virConnectListAllStoragePoolFlags;
Declare the API.
python/generator.py: Skip the generating
src/driver.h: (virDrvConnectListAllStoragePools)
src/libvirt.c: Implementation for the API.
src/libvirt_public.syms: Export the symbol.
---
include/libvirt/libvirt.h.in | 33
python/generator.py |5 +-
src/driver.h |5 ++
src/libvirt.c| 112
+++---
src/libvirt_public.syms |1 +
5 files changed, 146 insertions(+), 10 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index d21d029..e8f38ab 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2524,6 +2524,39 @@ int
virConnectListDefinedStoragePools(virConnectPtr conn,
int maxnames);
/*
+ * virConnectListAllStoragePoolsFlags:
+ *
+ * Flags used to tune pools returned by virConnectListAllStoragePools().
+ * Note that these flags come in groups; if all bits from a group are 0,
+ * then that group is not used to filter results.
+ */
+typedef enum {
+VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE = 1 0,
+VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE= 1 1,
+
+VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT= 1 2,
+VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT = 1 3,
+
+VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART = 1 4,
+VIR_CONNECT_LIST_STORAGE_POOLS_NO_AUTOSTART = 1 5,
+
+/* List pools by type */
+VIR_CONNECT_LIST_STORAGE_POOLS_DIR = 1 6,
+VIR_CONNECT_LIST_STORAGE_POOLS_FS= 1 7,
+VIR_CONNECT_LIST_STORAGE_POOLS_NETFS = 1 8,
+VIR_CONNECT_LIST_STORAGE_POOLS_LOGICAL = 1 9,
+VIR_CONNECT_LIST_STORAGE_POOLS_DISK = 1 10,
+VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI = 1 11,
+VIR_CONNECT_LIST_STORAGE_POOLS_SCSI = 1 12,
+VIR_CONNECT_LIST_STORAGE_POOLS_MPATH = 1 13,
+VIR_CONNECT_LIST_STORAGE_POOLS_RBD = 1 14,
+VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG = 1 15,
+} virConnectListAllStoragePoolsFlags;
+
+int virConnectListAllStoragePools(virConnectPtr conn,
+ virStoragePoolPtr
**pools,
+ unsigned int flags);
+/*
* Query a host for storage pools of a particular type
*/
char * virConnectFindStoragePoolSources(virConnectPtr conn,
diff --git a/python/generator.py b/python/generator.py
index 6559ece..6fab68c 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -337,7 +337,7 @@ foreign_encoding_args = (
#
###
-# Class methods which are written by hand in libvir.c but the Python-level
+# Class methods which are written by hand in libvirt.c but the Python-level
# code is still automatically generated (so they are not in skip_function()).
skip_impl = (
'virConnectGetVersion',
@@ -455,9 +455,10 @@ skip_function = (
'virConnectDomainEventDeregisterAny', # overridden in virConnect.py
'virSaveLastError', # We have our own python error wrapper
'virFreeError', # Only needed if we use virSaveLastError
-'virConnectListAllDomains', #overridden in virConnect.py
+'virConnectListAllDomains', # overridden in virConnect.py
'virDomainListAllSnapshots', # overridden in virDomain.py
'virDomainSnapshotListAllChildren', # overridden in virDomainSnapshot.py
+'virConnectListAllStoragePools', # overridden in virConnect.py
'virStreamRecvAll', # Pure python libvirt-override-virStream.py
'virStreamSendAll', # Pure python libvirt-override-virStream.py
diff --git a/src/driver.h b/src/driver.h
index aab9766..3c1ae3b 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1232,6 +1232,10 @@ typedef int
(*virDrvConnectListDefinedStoragePools) (virConnectPtr conn,
char **const names,
int maxnames);
+typedef int
+(*virDrvConnectListAllStoragePools) (virConnectPtr conn,
+ virStoragePoolPtr **pools,
+ unsigned int flags);
typedef char *
(*virDrvConnectFindStoragePoolSources)
Re: [libvirt] [PATCH V1] on special migration(domain defined, not started yet)
Hi, Eric
what do you think of this kludged way of
off-line migration?
在 2012-08-02四的 17:02 +0800，liguang写道：
From: liguang lig.f...@cn.fujitsu.com
a roughly way for offline-migrate (domain defined,
not started yet), now can do like this:
migrate --hard-migrate --xml dom.xml dom qemu+ssh://target/system
this patch will push dom.xml and all disk images to target
Signed-off-by: liguang lig.f...@cn.fujitsu.com
---
tools/virsh.c | 76
+
1 files changed, 76 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 53d1825..5793233 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7344,9 +7344,75 @@ static const vshCmdOptDef opts_migrate[] = {
{dname, VSH_OT_DATA, 0, N_(rename to new name during migration
(if supported))},
{timeout, VSH_OT_INT, 0, N_(force guest to suspend if live
migration exceeds timeout (in seconds))},
{xml, VSH_OT_STRING, 0, N_(filename containing updated XML for
the target)},
+{hard-migrate, VSH_OT_BOOL, 0, N_(migration when there's no
domain)},
{NULL, 0, 0, NULL}
};
+#define VIR_MIGRATE_HARD 1 10
+#define push_file(file) { \
+virAsprintf(topath, %s:%s, to, file);\
+cmd = virCommandNewArgList(scp, file, topath, NULL); \
+vshPrint(ctl, pushing %s to %s\n, file, to); \
+if (virCommandRunAsync(cmd, NULL) 0 ||\
+virCommandWait(cmd, NULL) 0) {\
+virshReportError(ctl); \
+goto cleanup; \
+} \
+}
+
+static void
+vshMigrateHard(vshControl *ctl, char *doc, char dst[])
+{
+xmlDocPtr xml = NULL;
+xmlXPathObjectPtr obj= NULL;
+xmlXPathContextPtr ctxt = NULL;
+xmlNodePtr *disks = NULL;
+virCommandPtr cmd;
+int i = 0, ret = 0;
+int outfd = STDOUT_FILENO;
+int errfd = STDERR_FILENO;
+char *src[] = {NULL}, *to, *topath;
+
+if (!vshConnectionUsability(ctl, ctl-conn))
+return;
+
+xml = virXMLParseFileCtxt(doc, ctxt);
+if (!xml) {
+vshError(NULL, %s, _(Fail to get domain information from));
+goto cleanup;
+}
+
+ret = virXPathNodeSet(./devices/disk, ctxt, disks);
+if (ret 0) {
+vshError(NULL, %s, _(Fail to get disk node));
+goto cleanup;
+}
+
+to = strtok(dst, /);
+to = strtok(NULL, /);
+virCommandSetInputFD(cmd, STDIN_FILENO);
+virCommandSetOutputFD(cmd, outfd);
+virCommandSetErrorFD(cmd, errfd);
+
+push_file(doc);
+
+for (i = 0 ; i ret ; i++) {
+ctxt-node = disks[i];
+src[i] = virXPathString(string(./source/@file
+|./source/@dir
+|./source/@name), ctxt);
+push_file(src[i]);
+}
+
+cleanup:
+xmlXPathFreeObject(obj);
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml);
+virCommandFree(cmd);
+if (src)
+VIR_FREE(src);return;
+}
+
static void
doMigrate (void *opaque)
{
@@ -7413,12 +7479,22 @@ doMigrate (void *opaque)
if (vshCommandOptBool(cmd, unsafe))
flags |= VIR_MIGRATE_UNSAFE;
+if (vshCommandOptBool(cmd, hard-migrate)) {
+flags |= VIR_MIGRATE_HARD;
+if (xmlfile == NULL)
+vshError(ctl, _(please specify xmlfile for
hard-migrate));
+}
if (xmlfile
virFileReadAll(xmlfile, 8192, xml) 0) {
vshError(ctl, _(file '%s' doesn't exist), xmlfile);
goto out;
}
+if (flags VIR_MIGRATE_HARD) {
+vshMigrateHard(ctl, (char *)xmlfile, (char *)desturi);
+goto out;
+}
+
if ((flags VIR_MIGRATE_PEER2PEER) ||
vshCommandOptBool(cmd, direct)) {
/* For peer2peer migration or direct migration we only expect
one URI
--
1.7.2.5
在 2012-08-01三的 06:13 -0600，Eric Blake写道：
On 07/31/2012 09:30 PM, liguang wrote:
Hi, All
If a VM domain defined, but not started yet,
and I want to migrate it to another server,
so that It can be started at there, what's
should i do? or is it rational?
Yes it is rational, and you're not the first to request it.
Unfortunately, we haven't yet taught 'virsh migrate' to support offline
migration, even though (or because?) it is dead simple to cobble
together yourself:
source$ virsh dumpxml $dom file.xml
dest$ virsh define file.xml
optionally followed by
source$ virsh undefine $dom
Patches are welcome to make 'virsh migrate' automatically handle an
offline migration.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] storage: support all file permissions
On 08/07/12 07:17, Laine Stump wrote: On 07/26/2012 04:52 AM, Ján Tomko wrote: sticky, setuid and setgid are no longer ignored. I'm always automatically wary of any code that allows setting the suid bit, in case it may allow some new security hole. I can't think of anything specific that could be allowed by setting the suid bit of a directory containing a disk image, but then again I haven't thought about it very hard :-) Can anyone convince me one way or the other? SUID on directories is ignored on most systems, but you could be able to create files belonging a group you're not a member of. But this patch enables it on files too, allowing everyone with access to privileged libvirtd to create SUID files. I don't know if it's possible to exploit this, but I don't like it, so NACK NACK NACK. It might help to learn why you want to be able to set these bits. libvirt is generally very specific about explicitly setting the uid of disk images properly as required at runtime... My issue was that vol-dumpxml reported wrong file permissions, as described in https://bugzilla.redhat.com/show_bug.cgi?id=839463 Writing the sticky bit seems harmless to me. Would it be safe to just read SGID and SUID without ever setting them? Or is it not worth the risk? Ján Tomko -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] block: Set cdrom device read only flag
Kevin Wolf kw...@redhat.com writes: Am 02.08.2012 09:20, schrieb Kevin Shanahan: On Thu, Aug 02, 2012 at 02:49:52PM +0930, Kevin Shanahan wrote: On Thu, Aug 02, 2012 at 11:46:13AM +0930, Kevin Shanahan wrote: On Thu, Aug 02, 2012 at 11:02:42AM +0930, Kevin Shanahan wrote: Set the block driver read_only flag for cdrom devices so that qmp_change_blockdev does not attempt to open cdrom files in read-write mode when changing media. Hrm, this fixes my simple test case using the kvm monitor directly but changing media via libvirt still has the same issue (fails for RO files, succeeds for writable files). $ virsh attach-disk --type cdrom --mode readonly test1 /srv/kvm/iso/ubuntu-12.04-server-amd64.iso hdc error: Failed to attach disk error: internal error unable to execute QEMU command 'change': Could not open '/srv/kvm/iso/ubuntu-12.04-server-amd64.iso' I'll keep looking into it. In the libvirt case, it seems libvirt is failing to add media=cdrom to the commandline, so in this case qemu is defaulting to media=disk and my proposed fix has no effect. Diving into libvirt now to see why no media=disk is getting added... Common test case has this xml (generated by virt-install): disk type='block' device='cdrom' driver name='qemu' type='raw'/ target dev='hdc' bus='ide'/ readonly/ alias name='ide0-1-0'/ address type='drive' controller='0' bus='1' target='0' unit='0'/ /disk Ok, looks like libvirt is intentionally leaving media=cdrom off the command line in the case that -device ide-cd,... is supported. Presumably by specifying the device this way, qemu is supposed to work out that the media type is cdrom automatically (but it doesn't, it defaults to disk). Libvirt wants to use: qemu-kvm ... \ -drive if=none,id=drive-ide0-1-0,readonly=on,format=raw \ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 ... If I hack qemu/qemu_command.c::qemuBuildDriveStr() to ignore the check for QEMU_CAPS_IDE_CD and always add media=cdrom, then (with my qemu as well patch) qemu will open cdrom media files read-only. There's probably a neater way to just get qemu to set the media type if -device ide-cd,... is used, but I haven't worked it out yet. Anyway, apologies for the rambling conversation with myself on your lists. Hope this is helpful in some way. Thanks, that's some good information. However, I don't think you should start from media=cdrom. libvirt already does specify readonly=on and that is the property you're really interested in. Since commit 528f7663 (released with 0.13) the 'change' command should keep the read-only flag for all kinds of media. Correct. Now I'm not sure where you lost the read-only flag. At least on git master it doesn't seem to reproduce for me. I can: $ qemu --enable-kvm -S -m 384 -vnc localhost:5500 -monitor stdio \ -drive if=none,id=cd,readonly=on,format=raw \ -device ide-cd,bus=ide.1,unit=0,drive=cd QEMU 1.1.50 monitor - type 'help' for more information (qemu) change cd r5.iso Could not open 'r5.iso' (qemu) q armbru@blackfin:~/work/images$ ls -l r5.iso -r--r--r--. 1 armbru armbru 2872639488 Mar 31 2011 r5.iso Looks like a QEMU bug to me. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH] block: Set cdrom device read only flag
Since pretty much every cdrom drive use scsi today, shouldnt the readonly/writeable flag for MMC devices rather be done down in the hw/scsi* code rather than the generic block code? If/when/ever I get enough time I would like to port the writeable dvd+r emulation I wrote for STGT to qemu. In STGT, MMC/DVD devices can be either read-only or read-write. If the filesize for the backing file is 0 bytes, it is assumed the file is an iso image and that the file is a read-only iso image. If filesize is ==0, then the file is opened read-write and is treated as a blank dvd+r disk that the initiator can write/burn to regards ronnie sahlberg On Tue, Aug 7, 2012 at 6:47 PM, Markus Armbruster arm...@redhat.com wrote: Kevin Wolf kw...@redhat.com writes: Am 02.08.2012 09:20, schrieb Kevin Shanahan: On Thu, Aug 02, 2012 at 02:49:52PM +0930, Kevin Shanahan wrote: On Thu, Aug 02, 2012 at 11:46:13AM +0930, Kevin Shanahan wrote: On Thu, Aug 02, 2012 at 11:02:42AM +0930, Kevin Shanahan wrote: Set the block driver read_only flag for cdrom devices so that qmp_change_blockdev does not attempt to open cdrom files in read-write mode when changing media. Hrm, this fixes my simple test case using the kvm monitor directly but changing media via libvirt still has the same issue (fails for RO files, succeeds for writable files). $ virsh attach-disk --type cdrom --mode readonly test1 /srv/kvm/iso/ubuntu-12.04-server-amd64.iso hdc error: Failed to attach disk error: internal error unable to execute QEMU command 'change': Could not open '/srv/kvm/iso/ubuntu-12.04-server-amd64.iso' I'll keep looking into it. In the libvirt case, it seems libvirt is failing to add media=cdrom to the commandline, so in this case qemu is defaulting to media=disk and my proposed fix has no effect. Diving into libvirt now to see why no media=disk is getting added... Common test case has this xml (generated by virt-install): disk type='block' device='cdrom' driver name='qemu' type='raw'/ target dev='hdc' bus='ide'/ readonly/ alias name='ide0-1-0'/ address type='drive' controller='0' bus='1' target='0' unit='0'/ /disk Ok, looks like libvirt is intentionally leaving media=cdrom off the command line in the case that -device ide-cd,... is supported. Presumably by specifying the device this way, qemu is supposed to work out that the media type is cdrom automatically (but it doesn't, it defaults to disk). Libvirt wants to use: qemu-kvm ... \ -drive if=none,id=drive-ide0-1-0,readonly=on,format=raw \ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 ... If I hack qemu/qemu_command.c::qemuBuildDriveStr() to ignore the check for QEMU_CAPS_IDE_CD and always add media=cdrom, then (with my qemu as well patch) qemu will open cdrom media files read-only. There's probably a neater way to just get qemu to set the media type if -device ide-cd,... is used, but I haven't worked it out yet. Anyway, apologies for the rambling conversation with myself on your lists. Hope this is helpful in some way. Thanks, that's some good information. However, I don't think you should start from media=cdrom. libvirt already does specify readonly=on and that is the property you're really interested in. Since commit 528f7663 (released with 0.13) the 'change' command should keep the read-only flag for all kinds of media. Correct. Now I'm not sure where you lost the read-only flag. At least on git master it doesn't seem to reproduce for me. I can: $ qemu --enable-kvm -S -m 384 -vnc localhost:5500 -monitor stdio \ -drive if=none,id=cd,readonly=on,format=raw \ -device ide-cd,bus=ide.1,unit=0,drive=cd QEMU 1.1.50 monitor - type 'help' for more information (qemu) change cd r5.iso Could not open 'r5.iso' (qemu) q armbru@blackfin:~/work/images$ ls -l r5.iso -r--r--r--. 1 armbru armbru 2872639488 Mar 31 2011 r5.iso Looks like a QEMU bug to me. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Don't check the 'connect' command in virsh-all test
From: Daniel P. Berrange berra...@redhat.com The 'virsh-all' test case will invoke each virsh command with no args. With the 'connect' command this causes virsh to try to connect to the default URI, which in turn tries to spawn libvirtd. This is not something we want todo in the test suite, so skip the 'connect' command. --- tests/virsh-all |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/virsh-all b/tests/virsh-all index 2650b86..d4e2633 100755 --- a/tests/virsh-all +++ b/tests/virsh-all @@ -25,7 +25,7 @@ fail=0 test_url=test:///default -$abs_top_builddir/tools/virsh -c $test_url help cmds || framework_failure +$abs_top_builddir/tools/virsh -c $test_url help | grep -v connect cmds || framework_failure cmds=$(sed -n 's/^\([^ ][^ ]*\) .*/\1/p' cmds) || framework_failure test -n $cmds || framework_failure -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Set LIBVIRT_AUTOSTART=0 when running test suites
From: Daniel P. Berrange berra...@redhat.com Occassionally some test cases will (accidentally) try to spawn libvirtd. Set the LIBVIRT_AUTOSTART=0 environment variable to ensure the remote driver never tries autostart. --- tests/Makefile.am |1 + 1 file changed, 1 insertion(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index 2fdaace..60d322d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -244,6 +244,7 @@ TESTS_ENVIRONMENT = \ PATH=$(path_add)$(PATH_SEPARATOR)$$PATH\ SHELL=$(SHELL) \ LIBVIRT_DRIVER_DIR=$(abs_top_builddir)/src/.libs \ + LIBVIRT_AUTOSTART=0 \ LC_ALL=C \ $(VG) -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add APIs for obtaining the unique ID of LVM SCSI volumes
On Fri, Aug 03, 2012 at 04:03:55PM +0100, Daniel P. Berrange wrote: On Thu, Aug 02, 2012 at 11:32:14PM -0500, Doug Goldstein wrote: On Thu, Aug 2, 2012 at 8:09 AM, Daniel P. Berrange berra...@redhat.com wrote: From: Daniel P. Berrange berra...@redhat.com Both LVM volumes and SCSI LUNs have a globally unique identifier associated with them. It is useful to be able to query this identifier to then perform disk locking, rather than try to figure out a stable pathname. -- Would it not be better to call methods from within libdevmapper and libblkid than relying on a callout? Its also worth noting that iSCSI provides a GUID for the volume as well. AFAICT, libblkid does not provide a way to get the SCSI WWN/Serial values. For iSCSI, I'm assuming that the devices just report the data using the regular kernel SCSI ioctls, so scsi_id should 'just work' already. Issuing SCSI ioctls would indeed be possible, but the code is not entirely trivial. I looked to see if sg3utils provided a nice API for this, and while they do have APIs for dealing with SCSI commands, all the code for actually interpreting the data is still in their command line tools under GPL license, so that's not good. Since we already invoke this scsi_id command from the storage drivers, what I have here is no worse. So I'm shelfing the idea of doing SCSI directly until I (or someone else) has more time That said, I think I might implement the SCSI lookup directly because I know remember previous discussions about the fact that the scsi_id command changed its supported command line arguments over time. I might try libdevmapper, if I can figure out its crazy API Well, the API is even more crazy that I expected, so I'm not going to change this patch now. Regards, 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] Add APIs for obtaining the unique ID of LVM SCSI volumes
On Mon, Aug 06, 2012 at 02:16:39PM -0600, Eric Blake wrote:
On 08/02/2012 07:09 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Both LVM volumes and SCSI LUNs have a globally unique
identifier associated with them. It is useful to be able
to query this identifier to then perform disk locking,
rather than try to figure out a stable pathname.
---
src/util/storage_file.c | 93
+
src/util/storage_file.h | 3 ++
2 files changed, 96 insertions(+)
+#ifdef HAVE_UDEV
+const char *virStorageFileGetSCSIKey(const char *path)
+{
+char *key = NULL;
+virCommandPtr cmd = virCommandNewArgList(
+/lib/udev/scsi_id,
Are we okay hard-coding the name of this utility, or should it be
learned during configure? At any rate, changing that can be an
incremental improvement later.
AFAIK, it is always in this location on all distros, so I think we're
safe for now. NB, this is the same as some code already present in
the storage driver source - I'll be sending a followup patch to
make the storage driver just call this method now.
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 v3 10/10] Turn virNetClient* into virObject instances
On Mon, Aug 06, 2012 at 01:56:33PM -0600, Eric Blake wrote:
On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Make all the virNetClient* objects use virObject APIs for
reference counting
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_probes.d |4 +-
src/lxc/lxc_monitor.c |4 +-
src/remote/remote_driver.c| 20 -
src/rpc/gendispatch.pl|4 +-
src/rpc/virnetclient.c| 96
+
src/rpc/virnetclient.h|4 +-
src/rpc/virnetclientprogram.c | 43 +-
src/rpc/virnetclientprogram.h |5 +--
src/rpc/virnetclientstream.c | 65 +---
src/rpc/virnetclientstream.h |5 +--
10 files changed, 110 insertions(+), 140 deletions(-)
+++ b/src/rpc/virnetclient.h
@@ -30,6 +30,7 @@
# endif
# include virnetclientprogram.h
# include virnetclientstream.h
+# include virobject.h
Same comments about .c instead of .h.
-void virNetClientProgramRef(virNetClientProgramPtr prog)
+void virNetClientProgramDispose(void *obj ATTRIBUTE_UNUSED)
{
-prog-refs++;
-}
-
-
-void virNetClientProgramFree(virNetClientProgramPtr prog)
-{
-if (!prog)
-return;
-
-prog-refs--;
-if (prog-refs 0)
-return;
-
-VIR_FREE(prog);
}
And another no-op dispose where you could use NULL instead.
ACK with this squashed in:
diff --git i/cfg.mk w/cfg.mk
index 64af1ee..c0457e7 100644
--- i/cfg.mk
+++ w/cfg.mk
@@ -145,9 +145,6 @@ useless_free_options =\
--name=virJSONValueFree\
--name=virLastErrFreeData \
--name=virNetMessageFree \
- --name=virNetClientFree \
- --name=virNetClientProgramFree\
- --name=virNetClientStreamFree \
--name=virNetServerMDNSFree \
--name=virNetServerMDNSEntryFree \
--name=virNetServerMDNSGroupFree \
diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms
index 0543005..79b4a18 100644
--- i/src/libvirt_private.syms
+++ w/src/libvirt_private.syms
@@ -1304,7 +1304,6 @@ virNetClientAddProgram;
virNetClientAddStream;
virNetClientClose;
virNetClientDupFD;
-virNetClientFree;
virNetClientGetFD;
virNetClientGetTLSKeySize;
virNetClientHasPassFD;
@@ -1318,7 +1317,6 @@ virNetClientNewExternal;
virNetClientNewSSH;
virNetClientNewTCP;
virNetClientNewUNIX;
-virNetClientRef;
virNetClientRemoteAddrString;
virNetClientRemoveStream;
virNetClientSendNoReply;
@@ -1333,12 +1331,10 @@ virNetClientSetTLSSession;
# virnetclientprogram.h
virNetClientProgramCall;
virNetClientProgramDispatch;
-virNetClientProgramFree;
virNetClientProgramGetProgram;
virNetClientProgramGetVersion;
virNetClientProgramMatches;
virNetClientProgramNew;
-virNetClientProgramRef;
# virnetclientstream.h
@@ -1346,13 +1342,11 @@ virNetClientStreamEOF;
virNetClientStreamEventAddCallback;
virNetClientStreamEventRemoveCallback;
virNetClientStreamEventUpdateCallback;
-virNetClientStreamFree;
virNetClientStreamMatches;
virNetClientStreamNew;
virNetClientStreamQueuePacket;
virNetClientStreamRaiseError;
virNetClientStreamRecvPacket;
-virNetClientStreamRef;
virNetClientStreamSendPacket;
virNetClientStreamSetError;
Thanks for all the reviews. This series is now merged, with the changes
you suggested for cfg.mk and libvirt_private.syms
Now we're in a position to consider removing locking from some places
which only use it for the purpose of protecting ref counts
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] qemu: Avoid libvirtd crash in qemuDomainObjExitAgentInternal
On Tue, Aug 07, 2012 at 03:18:38PM +0800, Alex Jia wrote:
* src/qemu/qemu_domain.c (qemuDomainObjExitAgentInternal): fix crashing
libvirtd due to derefing a NULL pointer.
For details, please see bug:
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=845966
Signed-off-by: Alex Jia a...@redhat.com
---
src/qemu/qemu_domain.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 86f0265..8667b6c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1136,12 +1136,14 @@ qemuDomainObjExitAgentInternal(struct qemud_driver
*driver,
virDomainObjPtr obj)
{
qemuDomainObjPrivatePtr priv = obj-privateData;
-int refs;
+int refs = -1;
-refs = qemuAgentUnref(priv-agent);
+if (priv-agent) {
+refs = qemuAgentUnref(priv-agent);
-if (refs 0)
-qemuAgentUnlock(priv-agent);
+if (refs 0)
+qemuAgentUnlock(priv-agent);
+}
if (driver_locked)
qemuDriverLock(driver);
I'm not convinced this is the right fix. The whole point of the Enter/ExitAgent
methods is to hold an extra reference on priv-agent, so that it is *not*
deleted while a agent command is run.
What is setting priv-agent to NULL while the command is still active ?
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] [test-API][PATCH] Delete the unused util param
The util is undefined and cause case run fail, it's with no use and should be deleted. Signed-off-by: Wayne Sun g...@redhat.com --- repos/interface/create.py |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/repos/interface/create.py b/repos/interface/create.py index 50d92d2..f5ef308 100644 --- a/repos/interface/create.py +++ b/repos/interface/create.py @@ -25,7 +25,7 @@ def display_current_interface(conn): logger.debug(current defined host interface list: %s \ % conn.listDefinedInterfaces()) -def check_create_interface(ifacename, util): +def check_create_interface(ifacename): Check creating interface result, it will can ping itself if create interface is successful. @@ -67,7 +67,7 @@ def create(params): ifaceobj.create(0) logger.info(create host interface %s % ifacename) display_current_interface(conn) -if check_create_interface(ifacename, util): +if check_create_interface(ifacename): logger.info(create host interface %s is successful % ifacename) else: logger.error(fail to check create interface) -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Add lsi and virtio-scsi qemu caps
On Tue, Aug 07, 2012 at 02:54:33PM +0800, Guannan Ren wrote: On qemu-kvm-0.15.1, it supports only lsi scsi controller model. On qemu-kvm-0.12.1.2, it supports only virtio-scsi-pci scsi model This doesn't make sense. virtio-scsi did not exist until long after qemu-kvm-0.12.1.2. On qemu 1.1.50, it supports both. So, instead of using the lsilogic model by default, the patch tries to check which model the current QEMU supports, then choose it, lsi has the priority. I'm not convinced we should ever be trying to use virtio scsi by default, given its wide lack of guest support. If LSI scsi does not exist, just report an error IMHO, and let virtio scsi require an explicit config choice. 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] util: include stderr in log message when an external command fails
On Mon, Aug 06, 2012 at 08:07:32PM -0400, Laine Stump wrote:
This patch is in response to:
https://bugzilla.redhat.com/show_bug.cgi?id=818467
If a caller to virCommandRun doesn't ask for the exitstatus of the
program it's running, the virCommand functions assume that they should
log an error message and return failure if the exit code isn't
0. However, only the commandline and exit status are logged, while
potentially useful information sent by the program to stderr is
discarded.
Fortunately, virCommandRun is already checking if the caller had asked
for stderr to be saved and, if not, sets things up to save it in
*cmd-errbuf. This makes it fairly simple for virCommandWait to
include *cmd-errbuf in the error log (there are still other callers
that don't setup errbuf, and even virCommandRun won't set it up if the
command is being daemonized, so we have to check that it's non-zero).
---
Note that the minor change to the first virReportError string was made
because I noticed that virCommandTranslateStatus already puts the word
status in its string. The new message is less awkward.
src/util/command.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c
index 334ca89..7755572 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -2269,7 +2269,7 @@ virPidWait(pid_t pid, int *exitstatus)
if (status != 0) {
char *st = virCommandTranslateStatus(status);
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Child process (%lld) status unexpected: %s),
+ _(Child process (%lld) unexpected %s),
(long long) pid, NULLSTR(st));
VIR_FREE(st);
return -1;
@@ -2327,9 +2327,13 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
if (status) {
char *str = virCommandToString(cmd);
char *st = virCommandTranslateStatus(status);
+bool haveErrMsg = cmd-errbuf *cmd-errbuf
(*cmd-errbuf)[0];
+
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Child process (%s) status unexpected: %s),
- str ? str : cmd-args[0], NULLSTR(st));
+ _(Child process (%s) unexpected %s%s%s),
+ str ? str : cmd-args[0], NULLSTR(st),
+ haveErrMsg ? : : ,
+ haveErrMsg ? *cmd-errbuf : );
VIR_FREE(str);
VIR_FREE(st);
return -1;
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 0/2] Add lsi and virtio-scsi qemu caps
On 08/07/2012 07:37 PM, Daniel P. Berrange wrote: On Tue, Aug 07, 2012 at 02:54:33PM +0800, Guannan Ren wrote: On qemu-kvm-0.15.1, it supports only lsi scsi controller model. On qemu-kvm-0.12.1.2, it supports only virtio-scsi-pci scsi model This doesn't make sense. virtio-scsi did not exist until long after qemu-kvm-0.12.1.2. This is the version on my RHEL6.3 , maybe not upstream version code. # rpm -qa|grep kvm qemu-kvm-0.12.1.2-2.302.el6.x86_64 # /usr/libexec/qemu-kvm --version QEMU PC emulator version 0.12.1 (qemu-kvm-0.12.1.2) On qemu 1.1.50, it supports both. So, instead of using the lsilogic model by default, the patch tries to check which model the current QEMU supports, then choose it, lsi has the priority. I'm not convinced we should ever be trying to use virtio scsi by default, given its wide lack of guest support. If LSI scsi does not exist, just report an error IMHO, and let virtio scsi require an explicit config choice. Get it. the virtio-scsi is not used widely. Thanks for the review. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH] block: Set cdrom device read only flag
ronnie sahlberg ronniesahlb...@gmail.com writes:
Since pretty much every cdrom drive use scsi today, shouldnt the
readonly/writeable flag for MMC devices rather
be done down in the hw/scsi* code rather than the generic block code?
There are two separate things that can be read-only: device models and
BlockDriverStates. -drive's parameter readonly applies to the top
BlockDriverState. Some device models default their read-only-ness to
their BlockDriverState's.
Examples:
* Device models ide-cd and scsi-cd are always read-only. They don't
care whether the BlockDriverState that backs them is.
* Device model ide-hd is always read-write. It fails initialization
when its BlockDriverState is read-only.
* Device model scsi-hd supports both read-only and read-write. It's
read-only iff its BlockDriverState is.
* -drive if={ide,scsi},media=cdrom implies readonly=on, and creates an
{ide,scsi}-cd device.
* -drive if={ide,scsi},media=disk creates an {ide,scsi}-hd device
(media=disk is the default).
* -drive without readonly=on fails when the image isn't writable.
If/when/ever I get enough time I would like to port the writeable
dvd+r emulation I wrote for STGT to qemu.
In STGT, MMC/DVD devices can be either read-only or read-write.
If the filesize for the backing file is 0 bytes, it is assumed the
file is an iso image and that the file is a read-only iso image.
If filesize is ==0, then the file is opened read-write and is treated
as a blank dvd+r disk that the initiator can write/burn to
I doubt keying on the backing file size is a good idea, too obscure.
Why would keying on the BlockDriverState's read-only-ness not work?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Memory Ballooning for VM Issue
Hi, I have some problems with qemu ballooning. I saw the qemu docs. I found the qemu monitor command balloon that can request VM to change its memory allocation to value(in MB). it requested qemu command line with -balloon virtio, then I start qemu into monitor mode with memory 512MB. first show balloon information. (qemu) info balloon (qemu)balloon: actual=512 the command above if like this, I think the balloon device worked fine. (qemu) balloon 400 there was no error when I executed this command. but when I executed info balloon, the result was still 512. Is there something wrong? please give me some ideas. thanks. qemu-kvm version: 1.0.1 linux kernel version: 3.1.6 xuanmao_001-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH] Delete the unused util param
On 08/07/2012 07:31 PM, Wayne Sun wrote: The util is undefined and cause case run fail, it's with no use and should be deleted. Signed-off-by: Wayne Sun g...@redhat.com --- repos/interface/create.py |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/repos/interface/create.py b/repos/interface/create.py index 50d92d2..f5ef308 100644 --- a/repos/interface/create.py +++ b/repos/interface/create.py @@ -25,7 +25,7 @@ def display_current_interface(conn): logger.debug(current defined host interface list: %s \ % conn.listDefinedInterfaces()) -def check_create_interface(ifacename, util): +def check_create_interface(ifacename): Check creating interface result, it will can ping itself if create interface is successful. @@ -67,7 +67,7 @@ def create(params): ifaceobj.create(0) logger.info(create host interface %s % ifacename) display_current_interface(conn) -if check_create_interface(ifacename, util): +if check_create_interface(ifacename): logger.info(create host interface %s is successful % ifacename) else: logger.error(fail to check create interface) ACK, thanks and pushed. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH 2/2] Replace env.conf with global.conf in env_parser
On 08/06/2012 03:46 PM, Wayne Sun wrote: Only update the expection info when parsing. Signed-off-by: Wayne Sun g...@redhat.com --- src/env_parser.py | 22 +++--- 1 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/env_parser.py b/src/env_parser.py index f02af57..ddda727 100644 --- a/src/env_parser.py +++ b/src/env_parser.py @@ -30,7 +30,7 @@ class Envparser(object): self.cfg.read(configfile) else: raise exception.FileDoesNotExist( -env.conf is not a regular file or nonexist) +global.conf is not a regular file or nonexist) def has_section(self, section): if self.cfg.has_section(section): @@ -46,7 +46,7 @@ class Envparser(object): return False else: raise exception.SectionDoesNotExist( -In env.conf, the section %s is nonexist % section) +In global.conf, the section %s is nonexist % section) def sections_list(self): return self.cfg.sections() @@ -56,7 +56,7 @@ class Envparser(object): return self.cfg.options(section) else: raise exception.SectionDoesNotExist( -In env.conf, the section %s is nonexist % section) +In global.conf, the section %s is nonexist % section) def get_value(self, section, option): if self.has_section: @@ -64,17 +64,17 @@ class Envparser(object): return self.cfg.get(section, option) else: raise exception.OptionDoesNotExist( -In env.conf, the option %s is nonexist % option) +In global.conf, the option %s is nonexist % option) else: raise exception.SectionDoesNotExist( -In env.conf, the section %s is nonexist % section) +In global.conf, the section %s is nonexist % section) def get_items(self, section): if self.has_section: return self.cfg.items(section) else: raise exception.SectionDoesNotExist( -In env.conf, the section %s is nonexist % section) +In global.conf, the section %s is nonexist % section) def add_section(self, section): if self.has_section: @@ -91,10 +91,10 @@ class Envparser(object): return True else: raise exception.OptionDoesNotExist( -In env.conf, the option %s is nonexist % option) +In global.conf, the option %s is nonexist % option) else: raise exception.SectionDoesNotExist( -In env.conf, the section %s is nonexist % section) +In global.conf, the section %s is nonexist % section) def remove_section(self, section): if self.has_section: @@ -102,7 +102,7 @@ class Envparser(object): return True else: raise exception.SectionDoesNotExist( -In env.conf, the section %s is nonexist % section) +In global.conf, the section %s is nonexist % section) def set_value(self, section, option, value): if self.has_section: @@ -111,6 +111,6 @@ class Envparser(object): return True else: raise exception.OptionDoesNotExist( -In env.conf, the option %s is nonexist % option) +In global.conf, the option %s is nonexist % option) raise exception.SectionDoesNotExist( -In env.conf, the section %s is nonexist % section) +In global.conf, the section %s is nonexist % section) It should be global.cfg, but dont' worry let me change them. ACK and pushed. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH 1/2] Reconnct libvirt after libvirtd restart
On 08/06/2012 03:46 PM, Wayne Sun wrote: In domain_nfs_start case, libvirtd will be restarted during test, which broke existing connection. User need re-init connection in test case, for this: * Using sharedmod data dictionary to store Envparser class in generator. * Do not clear data dictionary in env_inspect, user can update it or framework release it at last. * Using sharemod_init in env_inspect to re-init conn in domain_nfs_start. For this case, it's better not to use the connection object offered by framework. The case could create its own connection through case options. Signed-off-by: Wayne Sun g...@redhat.com --- repos/sVirt/domain_nfs_start.py | 11 +-- src/env_inspect.py |1 - src/generator.py|2 ++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/repos/sVirt/domain_nfs_start.py b/repos/sVirt/domain_nfs_start.py index 88d349c..4d48d97 100644 --- a/repos/sVirt/domain_nfs_start.py +++ b/repos/sVirt/domain_nfs_start.py @@ -12,7 +12,7 @@ import sys import libvirt from libvirt import libvirtError - +from src import env_inspect The env_inspect is framework module. It is not recommanded to use directly in testcase. from src import sharedmod from utils import utils from shutil import copy @@ -163,6 +163,8 @@ def domain_nfs_start(params): logger.error(Error: fail to get domain %s xml % guestname) return 1 +conn.close() + # set env logger.info(prepare the environment) ret = prepare_env(dynamic_ownership, virt_use_nfs, guestname, \ @@ -171,6 +173,11 @@ def domain_nfs_start(params): logger.error(failed to prepare the environment) return 1 +# reconnect libvirt +env = sharedmod.data['env'] +env_inspect.sharemod_init(env, logger) +conn = sharedmod.libvirtobj['conn'] you could create own connection rather than use the connection object offerred by framework. + domobj = conn.lookupByName(guestname) logger.info(begin to test start domain from nfs storage) @@ -283,7 +290,7 @@ def domain_nfs_start(params): logger.error(Error: fail to get domain %s state % guestname) return 1 -if state != shutoff: +if state != libvirt.VIR_DOMAIN_SHUTOFF: logger.info(shut down the domain %s % guestname) try: domobj.destroy() diff --git a/src/env_inspect.py b/src/env_inspect.py index b260ff8..a6dc4b1 100644 --- a/src/env_inspect.py +++ b/src/env_inspect.py @@ -98,7 +98,6 @@ def sharemod_init(env_parser, logger): # initialize conn object in sharedmod sharedmod.libvirtobj.clear() -sharedmod.data.clear() sharedmod.libvirtobj['conn'] = conn return 0 diff --git a/src/generator.py b/src/generator.py index 0cdc9de..f01f2fb 100644 --- a/src/generator.py +++ b/src/generator.py @@ -30,6 +30,7 @@ from testcasexml import xml_file_to_str import env_parser import env_inspect import format +import sharedmod class FuncGen(object): To generate a callable testcase @@ -56,6 +57,7 @@ class FuncGen(object): self.__case_info_save(activity, testrunid) self.env = env_parser.Envparser(global.cfg) +sharedmod.data['env'] = self.env mapper_obj = mapper.Mapper(activity) case_list = mapper_obj.module_casename_func_map() -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V1] on special migration(domain defined, not started yet)
On 08/07/2012 02:00 AM, liguang wrote:
Hi, Eric
what do you think of this kludged way of
off-line migration?
在 2012-08-02四的 17:02 +0800，liguang写道：
From: liguang lig.f...@cn.fujitsu.com
a roughly way for offline-migrate (domain defined,
not started yet), now can do like this:
migrate --hard-migrate --xml dom.xml dom qemu+ssh://target/system
this patch will push dom.xml and all disk images to target
Having virsh be able to do this makes it possible to port to older
libvirtd, but I'd really like to first see support for offline migration
directly in the virDomainMigrate* APIs.
Signed-off-by: liguang lig.f...@cn.fujitsu.com
---
tools/virsh.c | 76
+
1 files changed, 76 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 53d1825..5793233 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7344,9 +7344,75 @@ static const vshCmdOptDef opts_migrate[] = {
{dname, VSH_OT_DATA, 0, N_(rename to new name during migration
(if supported))},
{timeout, VSH_OT_INT, 0, N_(force guest to suspend if live
migration exceeds timeout (in seconds))},
{xml, VSH_OT_STRING, 0, N_(filename containing updated XML for
the target)},
+{hard-migrate, VSH_OT_BOOL, 0, N_(migration when there's no
domain)},
I'd rather see a name like --offline.
{NULL, 0, 0, NULL}
};
+#define VIR_MIGRATE_HARD 1 10
+#define push_file(file) { \
+virAsprintf(topath, %s:%s, to, file);\
+cmd = virCommandNewArgList(scp, file, topath, NULL); \
+vshPrint(ctl, pushing %s to %s\n, file, to); \
+if (virCommandRunAsync(cmd, NULL) 0 ||\
+virCommandWait(cmd, NULL) 0) {\
If you're just going to wait for a file, then virCommandRun() is better
than virCommandRunAsync/virCommandWait.
+virshReportError(ctl); \
+goto cleanup; \
+} \
+}
+
+static void
+vshMigrateHard(vshControl *ctl, char *doc, char dst[])
const char *, if this function isn't going to alter 'doc' or 'dst'.
+{
+xmlDocPtr xml = NULL;
+xmlXPathObjectPtr obj= NULL;
+xmlXPathContextPtr ctxt = NULL;
+xmlNodePtr *disks = NULL;
+virCommandPtr cmd;
+int i = 0, ret = 0;
+int outfd = STDOUT_FILENO;
+int errfd = STDERR_FILENO;
+char *src[] = {NULL}, *to, *topath;
+
+if (!vshConnectionUsability(ctl, ctl-conn))
+return;
+
+xml = virXMLParseFileCtxt(doc, ctxt);
+if (!xml) {
+vshError(NULL, %s, _(Fail to get domain information from));
+goto cleanup;
+}
+
+ret = virXPathNodeSet(./devices/disk, ctxt, disks);
+if (ret 0) {
+vshError(NULL, %s, _(Fail to get disk node));
+goto cleanup;
+}
+
+to = strtok(dst, /);
+to = strtok(NULL, /);
strtok() is not thread-safe, and may not be used in libvirt sources.
Run 'make syntax-check' to flag things like this.
+virCommandSetInputFD(cmd, STDIN_FILENO);
+virCommandSetOutputFD(cmd, outfd);
+virCommandSetErrorFD(cmd, errfd);
+
+push_file(doc);
+
+for (i = 0 ; i ret ; i++) {
+ctxt-node = disks[i];
+src[i] = virXPathString(string(./source/@file
+|./source/@dir
+|./source/@name), ctxt);
+push_file(src[i]);
Using scp to copy disk images does not seem like the right approach - it
doesn't scale to non-ssh connections. Rather, we should be using
libvirt API, like virStorageVolUpload().
+}
+
+cleanup:
+xmlXPathFreeObject(obj);
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml);
+virCommandFree(cmd);
+if (src)
+VIR_FREE(src);return;
Formatting is off.
+}
+
static void
doMigrate (void *opaque)
{
@@ -7413,12 +7479,22 @@ doMigrate (void *opaque)
if (vshCommandOptBool(cmd, unsafe))
flags |= VIR_MIGRATE_UNSAFE;
+if (vshCommandOptBool(cmd, hard-migrate)) {
+flags |= VIR_MIGRATE_HARD;
If you aren't calling the libvirt API, then don't stick a random bit in
the 'flags' parameter destined for that API. Instead, use a new bool
variable to track whether the user is requesting offline migration.
+if (xmlfile == NULL)
+vshError(ctl, _(please specify xmlfile for
hard-migrate));
+}
if (xmlfile
virFileReadAll(xmlfile, 8192, xml) 0) {
vshError(ctl, _(file '%s' doesn't exist), xmlfile);
goto out;
}
+if (flags VIR_MIGRATE_HARD) {
+vshMigrateHard(ctl, (char *)xmlfile, (char *)desturi);
+goto out;
+}
+
if ((flags VIR_MIGRATE_PEER2PEER) ||
vshCommandOptBool(cmd, direct)) {
/* For peer2peer migration or direct
Re: [libvirt] [PATCH] Don't check the 'connect' command in virsh-all test
On 08/07/2012 05:03 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The 'virsh-all' test case will invoke each virsh command with no args. With the 'connect' command this causes virsh to try to connect to the default URI, which in turn tries to spawn libvirtd. This is not something we want todo in the test suite, so skip the 'connect' command. --- tests/virsh-all |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. I've mentioned the issue before, and even wondered whether it would make sense to alter the semantics of the 'connect' command with no arguments to instead reconnect to the current URI instead of reconnecting to the default URI, but never got around to polishing a patch into something I like. So this is a decent compromise of just avoiding the problem altogether. -- Eric Blake ebl...@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] Set LIBVIRT_AUTOSTART=0 when running test suites
On 08/07/2012 05:03 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Occassionally some test cases will (accidentally) try to spawn s/Occassionally/Occasionally/ libvirtd. Set the LIBVIRT_AUTOSTART=0 environment variable to ensure the remote driver never tries autostart. --- tests/Makefile.am |1 + 1 file changed, 1 insertion(+) ACK. -- Eric Blake ebl...@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 v2] storage: support all file permissions
On Tue, Aug 07, 2012 at 10:27:15AM +0200, Ján Tomko wrote: On 08/07/12 07:17, Laine Stump wrote: On 07/26/2012 04:52 AM, Ján Tomko wrote: sticky, setuid and setgid are no longer ignored. I'm always automatically wary of any code that allows setting the suid bit, in case it may allow some new security hole. I can't think of anything specific that could be allowed by setting the suid bit of a directory containing a disk image, but then again I haven't thought about it very hard :-) Can anyone convince me one way or the other? SUID on directories is ignored on most systems, but you could be able to create files belonging a group you're not a member of. But this patch enables it on files too, allowing everyone with access to privileged libvirtd to create SUID files. I don't know if it's possible to exploit this, but I don't like it, so NACK NACK NACK. It might help to learn why you want to be able to set these bits. libvirt is generally very specific about explicitly setting the uid of disk images properly as required at runtime... My issue was that vol-dumpxml reported wrong file permissions, as described in https://bugzilla.redhat.com/show_bug.cgi?id=839463 Writing the sticky bit seems harmless to me. Would it be safe to just read SGID and SUID without ever setting them? Or is it not worth the risk? IMO we should preserve the existing behavior of not modifying the bits, but we should report them correctly, although I don't feel all that strongly about it. Dave Ján Tomko -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/17] Supports for hypervisor-pin and hypervisor-bandwidth
On Fri, Aug 03, 2012 at 02:36:02PM +0800, Hu Tao wrote: This series is a merge of 1) Support hypervisor-threads-pin in vcpupin (https://www.redhat.com/archives/libvir-list/2012-July/msg01361.html) 2) support to set cpu bandwidth for hypervisor threads (https://www.redhat.com/archives/libvir-list/2012-June/msg01161.html) to make life easier because of the two share some patches. This series is really focusing on pinning threads associated with the emulator element, rather than the hypervisor. The hypervisor is a separate entity that is shared. So I'm thinking that this entire patch series could replace 'hypervisor' with 'emulator' everywhere. Any one has agree or disagree ? 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 09/17] Introduce virDomainPinHypervisorFlags and virDomainGetHypervisorPinInfo functions.
On Mon, Aug 06, 2012 at 05:31:23PM -0600, Eric Blake wrote: On 08/03/2012 12:36 AM, Hu Tao wrote: From: Tang Chen tangc...@cn.fujitsu.com Introduce 2 APIs for client to use. 1) virDomainPinHypervisorFlags: call remote driver api, such as remoteDomainPinHypervisorFlags. 2) virDomainGetHypervisorPinInfo: call remote driver api, such as remoteDomainGetHypervisorPinInfo. Signed-off-by: Tang Chen tangc...@cn.fujitsu.com Signed-off-by: Hu Tao hu...@cn.fujitsu.com --- include/libvirt/libvirt.h.in | 10 +++ src/driver.h | 13 +++- src/libvirt.c| 147 ++ src/libvirt_public.syms |2 + 4 files changed, 171 insertions(+), 1 deletion(-) Here's where I start to have questions on whether this approach makes sense. We already have a function for doing pinning, so why not add a new flag and reuse the existing function instead of adding a new one? That is, it might be nicer to change virDomainPinVcpuFlags (side note, why did we name it with 'Flags' in the name? In retrospect, that was a dumb naming choice) to have the 'vcpu' argument become signed, with -1 now being a valid value for all hypervisor threads not tied to a vcpu thread. Since the function has extern C linkage, it would not be an ABI break (it _would_ be a minor API break, but the only code that would fail to recompile is code that uses a function pointer to virDomainPinVcpuFlags, since most code would just get C's implicit casting rules). That is, instead of adding a new function, why can't: virDomainPinVcpuFlags(dom, -1, map, len, flags) serve as a way to pin all non-vcpu threads? Or if changing from 'unsigned int vcpu' to 'int vcpu' in the pinning function is unpalatable, then we could use: virDomainPinVcpuFlags(dom, 0, map, len, flags | VIR_DOMAIN_PIN_VCPU_HYPERVISOR) And for querying the hypervisor pinning, how about: virDomainGetVcpuPinInfo(dom, 1, map, len, flags | VIR_DOMAIN_GET_VCPU_PIN_HYPERVISOR) While what you describe could be made to work, I tend to prefer the idea of adding in new APIs specifically for this, and dealing with code duplication inside the drivers instead of at the public API level. 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 v7 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
This patch adds support that enables passing of file descriptors
to the QEMU monitor where they will be stored in specified file
descriptor sets.
A file descriptor set can be used by a client like libvirt to
store file descriptors for the same file. This allows the
client to open a file with different access modes (O_RDWR,
O_WRONLY, O_RDONLY) and add/remove the passed fds to/from an fd
set as needed. This will allow QEMU to (in a later patch in this
series) open and reopen the same file by dup()ing the fd in
the fd set that corresponds to the file, where the fd has the
matching access mode flag that QEMU requests.
The new QMP commands are:
add-fd: Add a file descriptor to an fd set
remove-fd: Remove a file descriptor from an fd set
query-fdsets: Return information describing all fd sets
Note: These commands are not compatible with the existing getfd
and closefd QMP commands.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v5:
-This patch is new in v5 and replaces the pass-fd QMP command
from v4.
-By grouping fds in fd sets, we ease managability with an fd
set per file, addressing concerns raised in v4 about handling
reopens and preventing fd leakage. (ebl...@redhat.com,
kw...@redhat.com, dberra...@redhat.com)
v6
-Make @fd optional for remove-fd (ebl...@redhat.com)
-Make @fdset-id optional for add-fd (ebl...@redhat.com)
v7:
-Share fd sets among all monitor connections (kw...@redhat.com)
-Added mon_refcount to keep track of monitor connection count.
monitor.c| 167 ++
qapi-schema.json | 110 +++
qerror.c |4 ++
qerror.h |3 +
qmp-commands.hx | 131 ++
5 files changed, 415 insertions(+)
diff --git a/monitor.c b/monitor.c
index 49dccfe..04b86b7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -140,6 +140,23 @@ struct mon_fd_t {
QLIST_ENTRY(mon_fd_t) next;
};
+/* file descriptor associated with a file descriptor set */
+typedef struct mon_fdset_fd_t mon_fdset_fd_t;
+struct mon_fdset_fd_t {
+int fd;
+bool removed;
+QLIST_ENTRY(mon_fdset_fd_t) next;
+};
+
+/* file descriptor set containing fds passed via SCM_RIGHTS */
+typedef struct mon_fdset_t mon_fdset_t;
+struct mon_fdset_t {
+int64_t id;
+int refcount;
+QLIST_HEAD(, mon_fdset_fd_t) fds;
+QLIST_ENTRY(mon_fdset_t) next;
+};
+
typedef struct MonitorControl {
QObject *id;
JSONMessageParser parser;
@@ -211,6 +228,8 @@ static inline int mon_print_count_get(const Monitor *mon) {
return 0; }
#define QMP_ACCEPT_UNKNOWNS 1
static QLIST_HEAD(mon_list, Monitor) mon_list;
+static QLIST_HEAD(mon_fdsets, mon_fdset_t) mon_fdsets;
+static int mon_refcount;
static mon_cmd_t mon_cmds[];
static mon_cmd_t info_cmds[];
@@ -2389,6 +2408,154 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
return -1;
}
+static void monitor_fdset_cleanup(mon_fdset_t *mon_fdset)
+{
+mon_fdset_fd_t *mon_fdset_fd;
+mon_fdset_fd_t *mon_fdset_fd_next;
+
+if (mon_fdset-refcount != 0) {
+return;
+}
+
+QLIST_FOREACH_SAFE(mon_fdset_fd, mon_fdset-fds, next, mon_fdset_fd_next)
{
+if (mon_refcount == 0 || mon_fdset_fd-removed) {
+close(mon_fdset_fd-fd);
+QLIST_REMOVE(mon_fdset_fd, next);
+g_free(mon_fdset_fd);
+}
+}
+
+if (QLIST_EMPTY(mon_fdset-fds)) {
+QLIST_REMOVE(mon_fdset, next);
+g_free(mon_fdset);
+}
+}
+
+AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, Error **errp)
+{
+int fd;
+Monitor *mon = cur_mon;
+mon_fdset_t *mon_fdset;
+mon_fdset_fd_t *mon_fdset_fd;
+AddfdInfo *fdinfo;
+
+fd = qemu_chr_fe_get_msgfd(mon-chr);
+if (fd == -1) {
+qerror_report(QERR_FD_NOT_SUPPLIED);
+return NULL;
+}
+
+if (has_fdset_id) {
+QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+if (mon_fdset-id == fdset_id) {
+break;
+}
+}
+if (mon_fdset == NULL) {
+qerror_report(QERR_FDSET_NOT_FOUND, fdset_id);
+return NULL;
+}
+} else {
+int64_t fdset_id_prev = -1;
+mon_fdset_t *mon_fdset_cur = QLIST_FIRST(mon_fdsets);
+
+/* Use first available fdset ID */
+QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+mon_fdset_cur = mon_fdset;
+if (fdset_id_prev == mon_fdset_cur-id - 1) {
+fdset_id_prev = mon_fdset_cur-id;
+continue;
+}
+break;
+}
+
+mon_fdset = g_malloc0(sizeof(*mon_fdset));
+mon_fdset-id = fdset_id_prev + 1;
+mon_fdset-refcount = 0;
+
+/* The fdset list is ordered by fdset ID */
+if (mon_fdset-id == 0) {
+QLIST_INSERT_HEAD(mon_fdsets, mon_fdset, next);
+} else if (mon_fdset-id mon_fdset_cur-id) {
+
[libvirt] [PATCH v7 4/6] block: Convert open calls to qemu_open
This patch converts all block layer open calls to qemu_open.
Note that this adds the O_CLOEXEC flag to the changed open paths
when the O_CLOEXEC macro is defined.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v2:
-Convert calls to qemu_open instead of file_open (kw...@redhat.com)
-Mention introduction of O_CLOEXEC (kw...@redhat.com)
v3-v7:
-No changes
block/raw-posix.c | 18 +-
block/raw-win32.c |4 ++--
block/vdi.c |5 +++--
block/vmdk.c | 21 +
block/vpc.c |2 +-
block/vvfat.c |4 ++--
6 files changed, 26 insertions(+), 28 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 0dce089..7408a42 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -572,8 +572,8 @@ static int raw_create(const char *filename,
QEMUOptionParameter *options)
options++;
}
-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
- 0644);
+fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+ 0644);
if (fd 0) {
result = -errno;
} else {
@@ -846,7 +846,7 @@ static int hdev_open(BlockDriverState *bs, const char
*filename, int flags)
if ( bsdPath[ 0 ] != '\0' ) {
strcat(bsdPath,s0);
/* some CDs don't have a partition 0 */
-fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
+fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
if (fd 0) {
bsdPath[strlen(bsdPath)-1] = '1';
} else {
@@ -903,7 +903,7 @@ static int fd_open(BlockDriverState *bs)
#endif
return -EIO;
}
-s-fd = open(bs-filename, s-open_flags ~O_NONBLOCK);
+s-fd = qemu_open(bs-filename, s-open_flags ~O_NONBLOCK);
if (s-fd 0) {
s-fd_error_time = get_clock();
s-fd_got_error = 1;
@@ -977,7 +977,7 @@ static int hdev_create(const char *filename,
QEMUOptionParameter *options)
options++;
}
-fd = open(filename, O_WRONLY | O_BINARY);
+fd = qemu_open(filename, O_WRONLY | O_BINARY);
if (fd 0)
return -errno;
@@ -1055,7 +1055,7 @@ static int floppy_probe_device(const char *filename)
if (strstart(filename, /dev/fd, NULL))
prio = 50;
-fd = open(filename, O_RDONLY | O_NONBLOCK);
+fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
if (fd 0) {
goto out;
}
@@ -1108,7 +1108,7 @@ static void floppy_eject(BlockDriverState *bs, bool
eject_flag)
close(s-fd);
s-fd = -1;
}
-fd = open(bs-filename, s-open_flags | O_NONBLOCK);
+fd = qemu_open(bs-filename, s-open_flags | O_NONBLOCK);
if (fd = 0) {
if (ioctl(fd, FDEJECT, 0) 0)
perror(FDEJECT);
@@ -1158,7 +1158,7 @@ static int cdrom_probe_device(const char *filename)
int prio = 0;
struct stat st;
-fd = open(filename, O_RDONLY | O_NONBLOCK);
+fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
if (fd 0) {
goto out;
}
@@ -1282,7 +1282,7 @@ static int cdrom_reopen(BlockDriverState *bs)
*/
if (s-fd = 0)
close(s-fd);
-fd = open(bs-filename, s-open_flags, 0644);
+fd = qemu_open(bs-filename, s-open_flags, 0644);
if (fd 0) {
s-fd = -1;
return -EIO;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index e4b0b75..8d7838d 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -255,8 +255,8 @@ static int raw_create(const char *filename,
QEMUOptionParameter *options)
options++;
}
-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
- 0644);
+fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+ 0644);
if (fd 0)
return -EIO;
set_sparse(fd);
diff --git a/block/vdi.c b/block/vdi.c
index 57325d6..c4f1529 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -653,8 +653,9 @@ static int vdi_create(const char *filename,
QEMUOptionParameter *options)
options++;
}
-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
- 0644);
+fd = qemu_open(filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
if (fd 0) {
return -errno;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index 18e9b4c..557dc1b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1161,10 +1161,9 @@ static int vmdk_create_extent(const char *filename,
int64_t filesize,
VMDK4Header header;
uint32_t tmp, magic, grains, gd_size, gt_size, gt_count;
-fd = open(
-filename,
-O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
-0644);
+fd = qemu_open(filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
if (fd 0) {
return -errno;
}
@@ -1484,15 +1483,13 @@
[libvirt] [PATCH] Add support for firewalld (new version)
* bridge_driver, nwfilter_driver: new dbus filters to get FirewallD1.Reloaded
signal and DBus.NameOwnerChanged on org.fedoraproject.FirewallD1
* iptables, ebtables, nwfilter_ebiptables_driver: use firewall-cmd direct
passthrough interface
* spec file changed as requested
---
configure.ac | 8 ++
libvirt.spec.in | 11
src/Makefile.am | 8 +++---
src/network/bridge_driver.c | 46 +++
src/nwfilter/nwfilter_driver.c| 46 +++
src/nwfilter/nwfilter_ebiptables_driver.c | 32 +
src/util/ebtables.c | 35 +++
src/util/iptables.c | 21 --
8 files changed, 201 insertions(+), 6 deletions(-)
diff --git a/configure.ac b/configure.ac
index 8a04d91..7142450 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1282,6 +1282,14 @@ AM_CONDITIONAL([HAVE_POLKIT1], [test x$with_polkit1 =
xyes])
AC_SUBST([POLKIT_CFLAGS])
AC_SUBST([POLKIT_LIBS])
+dnl firewalld
+AC_ARG_WITH([firewalld],
+ AC_HELP_STRING([--with-firewalld], [enable firewalld support]))
+if test x$with_firewalld = xyes ; then
+ AC_DEFINE_UNQUOTED([HAVE_FIREWALLD], [1], [whether firewalld support is
enabled])
+fi
+AM_CONDITIONAL([HAVE_FIREWALLD], [test x$with_firewalld = xyes])
+
dnl Avahi library
AC_ARG_WITH([avahi],
AC_HELP_STRING([--with-avahi], [use avahi to advertise remote daemon
@:@default=check@:@]),
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 67b955a..ea2fd88 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -106,6 +106,7 @@
%define with_sanlock 0%{!?_without_sanlock:0}
%define with_systemd 0%{!?_without_systemd:0}
%define with_numad 0%{!?_without_numad:0}
+%define with_firewalld 0%{!?_without_firewalld:0}
# Non-server/HV driver defaults which are always enabled
%define with_python0%{!?_without_python:1}
@@ -146,6 +147,11 @@
%define with_systemd 1
%endif
+# Fedora 18 / RHEL-7 are first where firewalld support is enabled
+%if 0%{?fedora} = 17 || 0%{?rhel} = 7
+%define with_firewalld 1
+%endif
+
# RHEL-5 has restricted QEMU to x86_64 only and is too old for LXC
%if 0%{?rhel} == 5
%define with_qemu_tcg 0
@@ -1182,6 +1188,10 @@ of recent versions of Linux (and other OSes).
%define _without_driver_modules --without-driver-modules
%endif
+%if %{with_firewalld}
+%define _with_firewalld --with-firewalld
+%endif
+
%define when %(date +%%F-%%T)
%define where %(hostname)
%define who %{?packager}%{!?packager:Unknown}
@@ -1240,6 +1250,7 @@ autoreconf -if
%{?_without_audit} \
%{?_without_dtrace} \
%{?_without_driver_modules} \
+ %{?_with_firewalld} \
%{with_packager} \
%{with_packager_version} \
--with-qemu-user=%{qemu_user} \
diff --git a/src/Makefile.am b/src/Makefile.am
index 6ed4a41..f3f4731 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -989,7 +989,7 @@ libvirt_driver_network_la_SOURCES =
libvirt_driver_network_la_LIBADD = libvirt_driver_network_impl.la
if WITH_DRIVER_MODULES
mod_LTLIBRARIES += libvirt_driver_network.la
-libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la $(LIBNL_LIBS)
+libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la $(LIBNL_LIBS)
$(DBUS_LIBS)
libvirt_driver_network_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
else
noinst_LTLIBRARIES += libvirt_driver_network.la
@@ -999,7 +999,7 @@ endif
libvirt_driver_network_impl_la_CFLAGS = \
$(LIBNL_CFLAGS) \
- -I$(top_srcdir)/src/conf $(AM_CFLAGS)
+ -I$(top_srcdir)/src/conf $(AM_CFLAGS) $(DBUS_CFLAGS)
libvirt_driver_network_impl_la_SOURCES = $(NETWORK_DRIVER_SOURCES)
endif
EXTRA_DIST += network/default.xml
@@ -1149,9 +1149,9 @@ noinst_LTLIBRARIES += libvirt_driver_nwfilter.la
#libvirt_la_BUILT_LIBADD += libvirt_driver_nwfilter.la
endif
libvirt_driver_nwfilter_la_CFLAGS = $(LIBPCAP_CFLAGS) \
- -I$(top_srcdir)/src/conf $(LIBNL_CFLAGS) $(AM_CFLAGS)
+ -I$(top_srcdir)/src/conf $(LIBNL_CFLAGS) $(AM_CFLAGS)
$(DBUS_CFLAGS)
libvirt_driver_nwfilter_la_LDFLAGS = $(LD_AMFLAGS)
-libvirt_driver_nwfilter_la_LIBADD = $(LIBPCAP_LIBS) $(LIBNL_LIBS)
+libvirt_driver_nwfilter_la_LIBADD = $(LIBPCAP_LIBS) $(LIBNL_LIBS) $(DBUS_LIBS)
if WITH_DRIVER_MODULES
libvirt_driver_nwfilter_la_LIBADD += ../gnulib/lib/libgnu.la
libvirt_driver_nwfilter_la_LDFLAGS += -module -avoid-version
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index a5046f1..86b178e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -61,6 +61,7 @@
#include virnetdev.h
#include virnetdevbridge.h
#include virnetdevtap.h
+#include virdbus.h
#define NETWORK_PID_DIR LOCALSTATEDIR /run/libvirt/network
#define NETWORK_STATE_DIR LOCALSTATEDIR /lib/libvirt/network
@@
[libvirt] [PATCH v7 6/6] block: Enable qemu_open/close to work with fd sets
When qemu_open is passed a filename of the /dev/fdset/nnn
format (where nnn is the fdset ID), an fd with matching access
mode flags will be searched for within the specified monitor
fd set. If the fd is found, a dup of the fd will be returned
from qemu_open.
Each fd set has a reference count. The purpose of the reference
count is to determine if an fd set contains file descriptors that
have open dup() references that have not yet been closed. It is
incremented on qemu_open and decremented on qemu_close. It is
not until the refcount is zero that file desriptors in an fd set
can be closed. If an fd set has dup() references open, then we
must keep the other fds in the fd set open in case a reopen
of the file occurs that requires an fd with a different access
mode.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v2:
-Get rid of file_open and move dup code to qemu_open
(kw...@redhat.com)
-Use strtol wrapper instead of atoi (kw...@redhat.com)
v3:
-Add note about fd leakage (ebl...@redhat.com)
v4
-Moved patch to be later in series (lcapitul...@redhat.com)
-Update qemu_open to check access mode flags and set flags that
can be set (ebl...@redhat.com, kw...@redhat.com)
v5:
-This patch was overhauled quite a bit in this version, with
the addition of fd set and refcount support.
-Use qemu_set_cloexec() on dup'd fd (ebl...@redhat.com)
-Modify flags set by fcntl on dup'd fd (ebl...@redhat.com)
-Reduce syscalls when setting flags for dup'd fd (ebl...@redhat.com)
-Fix O_RDWR, O_RDONLY, O_WRONLY checks (ebl...@redhat.com)
v6:
-Pass only the fd to qemu_close() and keep track of dup fds per fd
set. (kw...@redhat.com, ebl...@redhat.com)
-Handle refcount incr/decr in new dup_fd_add/remove fd functions.
-Use qemu_set_cloexec() appropriately in qemu_dup() (kw...@redhat.com)
-Simplify setting of setfl_flags in qemu_dup() (kw...@redhat.com)
-Add preprocessor checks for F_DUPFD_CLOEXEC (ebl...@redhat.com)
-Simplify flag checking in monitor_fdset_get_fd() (kw...@redhat.com)
v7:
-Minor updates to reference global mon_fdsets, and to remove
default_mon usage in osdep.c. (kw...@redhat.com)
cutils.c |5 +++
monitor.c | 87
monitor.h |5 +++
osdep.c | 112 +
qemu-common.h |1 +
qemu-tool.c | 20 +++
6 files changed, 230 insertions(+)
diff --git a/cutils.c b/cutils.c
index 9d4c570..8b0d2bb 100644
--- a/cutils.c
+++ b/cutils.c
@@ -382,3 +382,8 @@ int qemu_parse_fd(const char *param)
}
return fd;
}
+
+int qemu_parse_fdset(const char *param)
+{
+return qemu_parse_fd(param);
+}
diff --git a/monitor.c b/monitor.c
index 84eade8..a16d48b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -154,6 +154,7 @@ struct mon_fdset_t {
int64_t id;
int refcount;
QLIST_HEAD(, mon_fdset_fd_t) fds;
+QLIST_HEAD(, mon_fdset_fd_t) dup_fds;
QLIST_ENTRY(mon_fdset_t) next;
};
@@ -2566,6 +2567,92 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
return fdset_list;
}
+int monitor_fdset_get_fd(int64_t fdset_id, int flags)
+{
+mon_fdset_t *mon_fdset;
+mon_fdset_fd_t *mon_fdset_fd;
+int mon_fd_flags;
+
+QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+if (mon_fdset-id != fdset_id) {
+continue;
+}
+QLIST_FOREACH(mon_fdset_fd, mon_fdset-fds, next) {
+if (mon_fdset_fd-removed) {
+continue;
+}
+
+mon_fd_flags = fcntl(mon_fdset_fd-fd, F_GETFL);
+if (mon_fd_flags == -1) {
+return -1;
+}
+
+if ((flags O_ACCMODE) == (mon_fd_flags O_ACCMODE)) {
+return mon_fdset_fd-fd;
+}
+}
+errno = EACCES;
+return -1;
+}
+errno = ENOENT;
+return -1;
+}
+
+int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
+{
+mon_fdset_t *mon_fdset;
+mon_fdset_fd_t *mon_fdset_fd_dup;
+
+QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+if (mon_fdset-id != fdset_id) {
+continue;
+}
+QLIST_FOREACH(mon_fdset_fd_dup, mon_fdset-dup_fds, next) {
+if (mon_fdset_fd_dup-fd == dup_fd) {
+return -1;
+}
+}
+mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup));
+mon_fdset_fd_dup-fd = dup_fd;
+QLIST_INSERT_HEAD(mon_fdset-dup_fds, mon_fdset_fd_dup, next);
+mon_fdset-refcount++;
+return 0;
+}
+return -1;
+}
+
+static int _monitor_fdset_dup_fd_find(int dup_fd, bool remove)
+{
+mon_fdset_t *mon_fdset;
+mon_fdset_fd_t *mon_fdset_fd_dup;
+
+QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+QLIST_FOREACH(mon_fdset_fd_dup, mon_fdset-dup_fds, next) {
+if (mon_fdset_fd_dup-fd == dup_fd) {
+if (remove) {
+QLIST_REMOVE(mon_fdset_fd_dup, next);
+
Re: [libvirt] [PATCH 00/17] Supports for hypervisor-pin and hypervisor-bandwidth
On 08/07/2012 09:47 AM, Daniel P. Berrange wrote: On Fri, Aug 03, 2012 at 02:36:02PM +0800, Hu Tao wrote: This series is a merge of 1) Support hypervisor-threads-pin in vcpupin (https://www.redhat.com/archives/libvir-list/2012-July/msg01361.html) 2) support to set cpu bandwidth for hypervisor threads (https://www.redhat.com/archives/libvir-list/2012-June/msg01161.html) to make life easier because of the two share some patches. This series is really focusing on pinning threads associated with the emulator element, rather than the hypervisor. The hypervisor is a separate entity that is shared. So I'm thinking that this entire patch series could replace 'hypervisor' with 'emulator' everywhere. Any one has agree or disagree ? I definitely agree - when I hear 'hypervisor', I think 'qemu:///system', which is the technology used to run multiple guests, but when I hear 'emulator', I think of a subset of a domain, namely the specific qemu pid_t running a given guest. Also, we're not pinning all of the hypervisor's threads, but just the threads that are associated with emulation but not a specific vcpu. That is, marking up your comment in 1/17: cgroup mount point +--libvirt= setting up a namespace (*) +--qemu= hypervisor level +--domain name = domain level +--vcpu0 = vcpu level ... +--vcpuN +--hypervisor = emulator so a domain really is made up of an 'emulator' and 'vcpu' threads, and a 'hypervisor' contains domains, rather than making up a portion of a domain. -- Eric Blake ebl...@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
[libvirt] [PATCH v7 1/6] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg
Set the close-on-exec flag for the file descriptor received
via SCM_RIGHTS.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v4
-This patch is new in v4 (ebl...@redhat.com)
v5
-Fallback to FD_CLOEXEC if MSG_CMSG_CLOEXEC is not available
(ebl...@redhat.com, stefa...@linux.vnet.ibm.com)
v6
-Set cloexec on correct fd (ebl...@redhat.com)
v7
-No changes
qemu-char.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index c2aaaee..ab4a928 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2238,6 +2238,9 @@ static void unix_process_msgfd(CharDriverState *chr,
struct msghdr *msg)
if (fd 0)
continue;
+#ifndef MSG_CMSG_CLOEXEC
+qemu_set_cloexec(fd);
+#endif
if (s-msgfd != -1)
close(s-msgfd);
s-msgfd = fd;
@@ -2253,6 +2256,7 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char
*buf, size_t len)
struct cmsghdr cmsg;
char control[CMSG_SPACE(sizeof(int))];
} msg_control;
+int flags = 0;
ssize_t ret;
iov[0].iov_base = buf;
@@ -2263,9 +2267,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char
*buf, size_t len)
msg.msg_control = msg_control;
msg.msg_controllen = sizeof(msg_control);
-ret = recvmsg(s-fd, msg, 0);
-if (ret 0 s-is_unix)
+#ifdef MSG_CMSG_CLOEXEC
+flags |= MSG_CMSG_CLOEXEC;
+#endif
+ret = recvmsg(s-fd, msg, flags);
+if (ret 0 s-is_unix) {
unix_process_msgfd(chr, msg);
+}
return ret;
}
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 5/6] block: Convert close calls to qemu_close
This patch converts all block layer close calls, that correspond
to qemu_open calls, to qemu_close.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v5:
-This patch is new in v5. (kw...@redhat.com, ebl...@redhat.com)
v6-v7:
-No changes
block/raw-posix.c | 24
block/raw-win32.c |2 +-
block/vmdk.c |4 ++--
block/vpc.c |2 +-
block/vvfat.c | 12 ++--
osdep.c |5 +
qemu-common.h |1 +
savevm.c |4 ++--
8 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 7408a42..a172de3 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -271,7 +271,7 @@ static int raw_open_common(BlockDriverState *bs, const char
*filename,
out_free_buf:
qemu_vfree(s-aligned_buf);
out_close:
-close(fd);
+qemu_close(fd);
return -errno;
}
@@ -376,7 +376,7 @@ static void raw_close(BlockDriverState *bs)
{
BDRVRawState *s = bs-opaque;
if (s-fd = 0) {
-close(s-fd);
+qemu_close(s-fd);
s-fd = -1;
if (s-aligned_buf != NULL)
qemu_vfree(s-aligned_buf);
@@ -580,7 +580,7 @@ static int raw_create(const char *filename,
QEMUOptionParameter *options)
if (ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) {
result = -errno;
}
-if (close(fd) != 0) {
+if (qemu_close(fd) != 0) {
result = -errno;
}
}
@@ -850,7 +850,7 @@ static int hdev_open(BlockDriverState *bs, const char
*filename, int flags)
if (fd 0) {
bsdPath[strlen(bsdPath)-1] = '1';
} else {
-close(fd);
+qemu_close(fd);
}
filename = bsdPath;
}
@@ -889,7 +889,7 @@ static int fd_open(BlockDriverState *bs)
last_media_present = (s-fd = 0);
if (s-fd = 0
(get_clock() - s-fd_open_time) = FD_OPEN_TIMEOUT) {
-close(s-fd);
+qemu_close(s-fd);
s-fd = -1;
#ifdef DEBUG_FLOPPY
printf(Floppy closed\n);
@@ -988,7 +988,7 @@ static int hdev_create(const char *filename,
QEMUOptionParameter *options)
else if (lseek(fd, 0, SEEK_END) total_size * BDRV_SECTOR_SIZE)
ret = -ENOSPC;
-close(fd);
+qemu_close(fd);
return ret;
}
@@ -1038,7 +1038,7 @@ static int floppy_open(BlockDriverState *bs, const char
*filename, int flags)
return ret;
/* close fd so that we can reopen it as needed */
-close(s-fd);
+qemu_close(s-fd);
s-fd = -1;
s-fd_media_changed = 1;
@@ -1070,7 +1070,7 @@ static int floppy_probe_device(const char *filename)
prio = 100;
outc:
-close(fd);
+qemu_close(fd);
out:
return prio;
}
@@ -1105,14 +1105,14 @@ static void floppy_eject(BlockDriverState *bs, bool
eject_flag)
int fd;
if (s-fd = 0) {
-close(s-fd);
+qemu_close(s-fd);
s-fd = -1;
}
fd = qemu_open(bs-filename, s-open_flags | O_NONBLOCK);
if (fd = 0) {
if (ioctl(fd, FDEJECT, 0) 0)
perror(FDEJECT);
-close(fd);
+qemu_close(fd);
}
}
@@ -1173,7 +1173,7 @@ static int cdrom_probe_device(const char *filename)
prio = 100;
outc:
-close(fd);
+qemu_close(fd);
out:
return prio;
}
@@ -1281,7 +1281,7 @@ static int cdrom_reopen(BlockDriverState *bs)
* FreeBSD seems to not notice sometimes...
*/
if (s-fd = 0)
-close(s-fd);
+qemu_close(s-fd);
fd = qemu_open(bs-filename, s-open_flags, 0644);
if (fd 0) {
s-fd = -1;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 8d7838d..c56bf83 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -261,7 +261,7 @@ static int raw_create(const char *filename,
QEMUOptionParameter *options)
return -EIO;
set_sparse(fd);
ftruncate(fd, total_size * 512);
-close(fd);
+qemu_close(fd);
return 0;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index 557dc1b..daee426 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1258,7 +1258,7 @@ static int vmdk_create_extent(const char *filename,
int64_t filesize,
ret = 0;
exit:
-close(fd);
+qemu_close(fd);
return ret;
}
@@ -1506,7 +1506,7 @@ static int vmdk_create(const char *filename,
QEMUOptionParameter *options)
}
ret = 0;
exit:
-close(fd);
+qemu_close(fd);
return ret;
}
diff --git a/block/vpc.c b/block/vpc.c
index 60ebf5a..c0b82c4 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -744,7 +744,7 @@ static int vpc_create(const char *filename,
QEMUOptionParameter *options)
}
fail:
-close(fd);
+qemu_close(fd);
return ret;
}
diff --git a/block/vvfat.c b/block/vvfat.c
index 22b586a..59d3c5b 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1105,7 +1105,7 @@ static inline void
vvfat_close_current_file(BDRVVVFATState *s)
if(s-current_mapping)
[libvirt] [PATCH v7 0/6] file descriptor passing using fd sets
libvirt's sVirt security driver provides SELinux MAC isolation for Qemu guest processes and their corresponding image files. In other words, sVirt uses SELinux to prevent a QEMU process from opening files that do not belong to it. sVirt provides this support by labeling guests and resources with security labels that are stored in file system extended attributes. Some file systems, such as NFS, do not support the extended attribute security namespace, and therefore cannot support sVirt isolation. A solution to this problem is to provide fd passing support, where libvirt opens files and passes file descriptors to QEMU. This, along with SELinux policy to prevent QEMU from opening files, can provide image file isolation for NFS files stored on the same NFS mount. This patch series adds the add-fd, remove-fd, and query-fdsets QMP monitor commands, which allow file descriptors to be passed via SCM_RIGHTS, and assigned to specified fd sets. This allows fd sets to be created per file with fds having, for example, different access rights. When QEMU needs to reopen a file with different access rights, it can search for a matching fd in the fd set. Fd sets also allow for easy tracking of fds per file, helping to prevent fd leaks. Support is also added to the block layer to allow QEMU to dup an fd from an fdset when the filename is of the /dev/fdset/nnn format, where nnn is the fd set ID. No new SELinux policy is required to prevent open of NFS files (files with type nfs_t). The virt_use_nfs boolean type simply needs to be set to false, and open will be prevented (and dup will be allowed). For example: # setsebool virt_use_nfs 0 # getsebool virt_use_nfs virt_use_nfs -- off Corey Bryant (6): qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg qapi: Introduce add-fd, remove-fd, query-fdsets monitor: Clean up fd sets on monitor disconnect block: Convert open calls to qemu_open block: Convert close calls to qemu_close block: Enable qemu_open/close to work with fd sets block/raw-posix.c | 42 - block/raw-win32.c |6 +- block/vdi.c |5 +- block/vmdk.c | 25 +++-- block/vpc.c |4 +- block/vvfat.c | 16 ++-- cutils.c |5 + monitor.c | 273 + monitor.h |5 + osdep.c | 117 +++ qapi-schema.json | 110 + qemu-char.c | 12 ++- qemu-common.h |2 + qemu-tool.c | 20 qerror.c |4 + qerror.h |3 + qmp-commands.hx | 131 + savevm.c |4 +- 18 files changed, 730 insertions(+), 54 deletions(-) -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On 08/07/2012 09:58 AM, Corey Bryant wrote:
This patch adds support that enables passing of file descriptors
to the QEMU monitor where they will be stored in specified file
descriptor sets.
A file descriptor set can be used by a client like libvirt to
store file descriptors for the same file. This allows the
client to open a file with different access modes (O_RDWR,
O_WRONLY, O_RDONLY) and add/remove the passed fds to/from an fd
set as needed. This will allow QEMU to (in a later patch in this
series) open and reopen the same file by dup()ing the fd in
the fd set that corresponds to the file, where the fd has the
matching access mode flag that QEMU requests.
The new QMP commands are:
add-fd: Add a file descriptor to an fd set
remove-fd: Remove a file descriptor from an fd set
query-fdsets: Return information describing all fd sets
+
+# @AddfdInfo:
+#
+# Information about a file descriptor that was added to an fd set.
+#
+# @fdset_id: The ID of the fd set that @fd was added to.
+#
+# @fd: The file descriptor that was received via SCM rights and
+# added to the fd set.
+#
+# Since: 1.2.0
We're not very consistent on '1.2' vs. '1.2.0' in since listings, but
that's probably worth a global cleanup closer to hard freeze.
+##
+{ 'type': 'AddfdInfo', 'data': {'fdset_id': 'int', 'fd': 'int'} }
This is a new command, so s/fdset_id/fdset-id/
+
+##
+# @add-fd:
+#
+# Add a file descriptor, that was passed via SCM rights, to an fd set.
+#
+# @fdset_id: #optional The ID of the fd set to add the file descriptor to.
+#
+# Returns: @AddfdInfo on success
+# If file descriptor was not received, FdNotSupplied
+# If @fdset_id does not exist, FdSetNotFound
+#
+# Notes: The list of fd sets is shared by all monitor connections.
+#
+#If @fdset_id is not specified, a new fd set will be created.
+#
+# Since: 1.2.0
+##
+{ 'command': 'add-fd', 'data': {'*fdset_id': 'int'},
Again, s/fdset_id/fdset-id/
+ 'returns': 'AddfdInfo' }
+
+##
+# @remove-fd:
+#
+# Remove a file descriptor from an fd set.
+#
+# @fdset_id: The ID of the fd set that the file descriptor belongs to.
+#
+# @fd: #optional The file descriptor that is to be removed.
+#
+# Returns: Nothing on success
+# If @fdset_id or @fd is not found, FdNotFound
+#
+# Since: 1.2.0
+#
+# Notes: The list of fd sets is shared by all monitor connections.
+#
+#File descriptors that are removed:
+#o will not be closed until the reference count corresponding
+# to @fdset_id reaches zero.
+#o will not be available for use after successful completion
+# of the remove-fd command.
+#
+#If @fd is not specified, all file descriptors in @fdset_id
+#will be removed.
+##
+{ 'command': 'remove-fd', 'data': {'fdset_id': 'int', '*fd': 'int'} }
And again.
+
+##
+# @FdsetFdInfo:
+#
+# Information about a file descriptor that belongs to an fd set.
+#
+# @fd: The file descriptor value.
+#
+# @removed: If true, the remove-fd command has been issued for this fd.
+#
+# Since: 1.2.0
+##
+{ 'type': 'FdsetFdInfo', 'data': {'fd': 'int', 'removed': 'bool'} }
Is it worth providing any additional information? For example, knowing
whether the fd is O_RDONLY, O_WRONLY, or O_RDWR might be beneficial to
management apps trying to discover what fds are already present after a
reconnection, in order to decide whether to close them without having to
resort to /proc/$qemupid/fdinfo/nnn lookups. It might even be worth
marking such information optional, present only when 'removed':false.
+
+##
+# @FdsetInfo:
+#
+# Information about an fd set.
+#
+# @fdset_id: The ID of the fd set.
+#
+# @refcount: A count of the number of outstanding dup() references to
+#this fd set.
+#
+# @in_use: If true, a monitor is connected and has access to this fd set.
+#
+# @fds: A list of file descriptors that belong to this fd set.
+#
+# Since: 1.2.0
+##
+{ 'type': 'FdsetInfo',
+ 'data': {'fdset_id': 'int', 'refcount': 'int', 'in_use': 'bool',
+ 'fds': ['FdsetFdInfo']} }
s/fdset_id/fdset-id/; s/in_use/in-use/
+
+##
+# @query-fdsets:
+#
+# Return information describing all fd sets.
+#
+# Returns: A list of @FdsetInfo
+#
+# Since: 1.2.0
+#
+# Notes: The list of fd sets is shared by all monitor connections.
+#
+#File descriptors are not closed until @refcount is zero,
+#and either @in_use is false or @removed is true.
+#
+##
+{ 'command': 'query-fdsets', 'returns': ['FdsetInfo'] }
diff --git a/qerror.c b/qerror.c
index 92c4eff..63a0aa1 100644
--- a/qerror.c
+++ b/qerror.c
@@ -148,6 +148,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = No file descriptor supplied via SCM_RIGHTS,
},
{
+.error_fmt = QERR_FDSET_NOT_FOUND,
+.desc = File descriptor set with ID '%(id)' not found,
+},
Conflicts with
Re: [libvirt] [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets
On 08/06/2012 10:15 AM, Corey Bryant wrote:
On 08/06/2012 09:51 AM, Kevin Wolf wrote:
Am 06.08.2012 15:32, schrieb Corey Bryant:
On 08/06/2012 05:15 AM, Kevin Wolf wrote:
Am 03.08.2012 00:21, schrieb Corey Bryant:
@@ -84,6 +158,36 @@ int qemu_open(const char *name, int flags, ...)
int ret;
int mode = 0;
+#ifndef _WIN32
+const char *fdset_id_str;
+
+/* Attempt dup of fd from fd set */
+if (strstart(name, /dev/fdset/, fdset_id_str)) {
+int64_t fdset_id;
+int fd, dupfd;
+
+fdset_id = qemu_parse_fdset(fdset_id_str);
+if (fdset_id == -1) {
+errno = EINVAL;
+return -1;
+}
+
+fd = monitor_fdset_get_fd(default_mon, fdset_id, flags);
I know that use of default_mon in this patch is not correct, but I
wanted to get these patches out for review. I used default_mon for
testing because cur_mon wasn't pointing to the monitor I'd added
fd sets
to. I need to figure out why.
Does it make sense to use default_mon here? After digging into this
some more, I'm thinking it makes sense, and I'll explain why.
It looks like cur_mon can't be used. cur_mon will point to the
monitor
object for the duration of a command, and be reset to old_mon (NULL in
my case) after the command completes.
qemu_open() and qemu_close() are frequently called long after a
monitor
command has come and gone, so cur_mon won't work. For example,
drive_add will cause qemu_open() to be called, but after the
command has
completed, the file will keep getting opened/closed during normal QEMU
operation. I'm not sure why, I've just noticed this behavior.
Does anyone have any thoughts on this? It would require fd sets to be
added to the default monitor only.
I think we have two design options that would make sense:
1. Make the file descriptors global instead of per-monitor. Is there a
reason why each monitor has its own set of fds? (Also I'm
wondering
if they survive a monitor disconnect this way?)
I'd prefer to have them associated with a monitor object so that we can
more easily keep track of whether or not they're in use by a monitor
connection.
Hm, I see.
2. Save a monitor reference with the fdset information.
Are you saying that each monitor would have the same copy of fdset
information?
This one doesn't really make sense indeed...
Allowing to send file descriptors on every monitor, but making only
those of the default monitor actually usable, sounds like a bad choice
to me.
What if we also allow them to be added only to the default monitor?
Would get you some kind of consistency at least, even though I don't
like that secondary monitors can't use the functionality.
Can't we make the fdset information global, so that a qemu_open/close()
searches all of them, but let it have a Monitor* owner for keeping track
whether it's in use?
I think global fdsets might work (sorry I didn't think it through enough
on my first reply). I think I'll need to drop the in_use flag and tie
monitor references into the refcount. (I know I know, you suggested
that a while back.. :). I'll give it a shot and see how it goes.
I just submitted the v7 patch series which makes fd sets global, rather
than each Monitor object having an fd set. This allows the list of fd
sets to be shared among all monitor connections.
--
Regards,
Corey
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/17] Introduce remoteDomainPinHypervisorFlags and remoteDomainGetHypervisorPinInfo functions in remote driver.
On 08/03/2012 12:36 AM, Hu Tao wrote:
From: Tang Chen tangc...@cn.fujitsu.com
Again, a long subject line. Given Dan's rename suggestion, I propose:
remote: introduce emulator pinning RPCs
static int
+remoteDispatchDomainPinHypervisorFlags(virNetServerPtr server
ATTRIBUTE_UNUSED,
No need for the 'Flags' suffix in the name.
+
+static int
+remoteDispatchDomainGetHypervisorPinInfo(virNetServerPtr server
ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client
ATTRIBUTE_UNUSED,
+ virNetMessagePtr msg
ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+
remote_domain_get_hypervisor_pin_info_args *args,
+
remote_domain_get_hypervisor_pin_info_ret *ret)
+{
+virDomainPtr dom = NULL;
+unsigned char *cpumaps = NULL;
+int num;
+int rv = -1;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv-conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not
open));
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv-conn, args-dom)))
+goto cleanup;
+
+/* There is only one cpumap struct for all hypervisor threads */
+if (args-ncpumaps != 1) {
If that's true, then we don't need args-ncpumaps in the first place.
That's a tweak to make in the .x file.
+++ b/src/remote/remote_driver.c
@@ -1841,6 +1841,106 @@ done:
}
static int
+remoteDomainPinHypervisorFlags (virDomainPtr dom,
+unsigned char *cpumap,
+int cpumaplen,
+unsigned int flags)
+{
+int rv = -1;
+struct private_data *priv = dom-conn-privateData;
+remote_domain_pin_hypervisor_flags_args args;
+
+remoteDriverLock(priv);
+
+if (cpumaplen REMOTE_CPUMAP_MAX) {
+virReportError(VIR_ERR_RPC,
+ _(%s length greater than maximum: %d %d),
+ cpumap, (int)cpumaplen, REMOTE_CPUMAP_MAX);
Why are we casting cpumaplen, which is already an int?
+static int
+remoteDomainGetHypervisorPinInfo (virDomainPtr domain,
+ unsigned char *cpumaps,
+ int maplen,
+ unsigned int flags)
+{
+int rv = -1;
+int i;
+remote_domain_get_hypervisor_pin_info_args args;
+remote_domain_get_hypervisor_pin_info_ret ret;
+struct private_data *priv = domain-conn-privateData;
+
+remoteDriverLock(priv);
+
+/* There is only one cpumap for all hypervisor threads */
+if (INT_MULTIPLY_OVERFLOW(1, maplen) ||
This expression is always false ('1 * anything' cannot overflow), and
therefore dead code worth simplifying.
@@ -5254,6 +5354,8 @@ static virDriver remote_driver = {
.domainPinVcpu = remoteDomainPinVcpu, /* 0.3.0 */
.domainPinVcpuFlags = remoteDomainPinVcpuFlags, /* 0.9.3 */
.domainGetVcpuPinInfo = remoteDomainGetVcpuPinInfo, /* 0.9.3 */
+.domainPinHypervisorFlags = remoteDomainPinHypervisorFlags, /* 0.9.13 */
+.domainGetHypervisorPinInfo = remoteDomainGetHypervisorPinInfo, /*
0.9.13 */
0.10.0
+++ b/src/remote/remote_protocol.x
@@ -1054,6 +1054,25 @@ struct remote_domain_get_vcpu_pin_info_ret {
int num;
};
+struct remote_domain_pin_hypervisor_flags_args {
Again, no need for '_flags' in the name.
+remote_nonnull_domain dom;
+unsigned int vcpu;
No need for a wasted 'vcpu' arg.
+opaque cpumapREMOTE_CPUMAP_MAX; /* (unsigned char *) */
+unsigned int flags;
+};
+
+struct remote_domain_get_hypervisor_pin_info_args {
+remote_nonnull_domain dom;
+int ncpumaps;
No need for an 'ncpumaps' arg.
+int maplen;
+unsigned int flags;
+};
+
+struct remote_domain_get_hypervisor_pin_info_ret {
+opaque cpumapsREMOTE_CPUMAPS_MAX;
+int num;
'num' is a bit misleading for a name, and probably not necessary. The
return value of virDomainGetHypervisorPinInfo is either 0 (no pinning
present) or 1 (pinning present and details returned) (or negative for
error, but that doesn't go through this RPC struct). If RPC allows for
0-length cpumaps, then you don't need num; otherwise, I'd rename num
to 'ret', since it is recording the return value of 0 or 1.
--
Eric Blake ebl...@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
[libvirt] [PATCH 3/3] test: add lsi and virtio-scsi qemu caps in testcases.
--- tests/qemuhelptest.c | 10 +++--- tests/qemuxml2argvtest.c | 16 ++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 75c818c..28d742e 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -482,7 +482,8 @@ mymain(void) QEMU_CAPS_PCI_ROMBAR, QEMU_CAPS_NO_ACPI, QEMU_CAPS_VIRTIO_BLK_SG_IO, -QEMU_CAPS_CPU_HOST); +QEMU_CAPS_CPU_HOST, +QEMU_CAPS_SCSI_LSI); DO_TEST(qemu-kvm-0.12.1.2-rhel61, 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -677,7 +678,8 @@ mymain(void) QEMU_CAPS_FSDEV_WRITEOUT, QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_SCSI_CD, -QEMU_CAPS_IDE_CD); +QEMU_CAPS_IDE_CD, +QEMU_CAPS_SCSI_LSI); DO_TEST(qemu-1.1.0, 1001000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -755,7 +757,9 @@ mymain(void) QEMU_CAPS_NO_USER_CONFIG, QEMU_CAPS_HDA_MICRO, QEMU_CAPS_NEC_USB_XHCI, -QEMU_CAPS_NETDEV_BRIDGE); +QEMU_CAPS_NETDEV_BRIDGE, +QEMU_CAPS_SCSI_LSI, +QEMU_CAPS_VIRIO_SCSI_PCI); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1adba78..2094984 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -458,16 +458,19 @@ mymain(void) DO_TEST(disk-usb-device, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(disk-scsi-device, -QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, +QEMU_CAPS_SCSI_LSI); DO_TEST(disk-scsi-device-auto, -QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, +QEMU_CAPS_SCSI_LSI); DO_TEST(disk-scsi-disk-split, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, -QEMU_CAPS_SCSI_CD); +QEMU_CAPS_SCSI_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRIO_SCSI_PCI); DO_TEST(disk-scsi-vscsi, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(disk-scsi-virtio-scsi, -QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, +QEMU_CAPS_VIRIO_SCSI_PCI); DO_TEST(disk-sata-device, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_ICH9_AHCI); @@ -497,7 +500,8 @@ mymain(void) DO_TEST(disk-scsi-lun-passthrough, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, -QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_VIRTIO_BLK_SG_IO); +QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_VIRTIO_BLK_SG_IO, +QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRIO_SCSI_PCI); DO_TEST(graphics-vnc, NONE); DO_TEST(graphics-vnc-socket, NONE); @@ -757,7 +761,7 @@ mymain(void) DO_TEST(multifunction-pci-device, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, -QEMU_CAPS_PCI_MULTIFUNCTION); +QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_SCSI_LSI); DO_TEST(monitor-json, QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_MONITOR_JSON, QEMU_CAPS_NODEFCONFIG); -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: add capabilities flags related to scsi controller
QEMU_CAPS_SCSI_LSI
set the flag when lsi53c895a, bus PCI, alias lsi in
the output of qemu -device ?
-device lsi in qemu command line
QEMU_CAPS_VIRIO_SCSI_PCI
set the flag when name virtio-scsi-pci, bus PCI in
the output of qemu devices query.
-device virtio-scsi-pci in qemu command line
---
src/qemu/qemu_capabilities.c |7 +++
src/qemu/qemu_capabilities.h |2 ++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 82a2870..ac90162 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -170,6 +170,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
balloon-event,
bridge, /* 100 */
+ lsi,
+ virtio-scsi-pci,
);
@@ -1455,6 +1457,11 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr
flags)
strstr(str, name \virtio-serial-s390\))
qemuCapsSet(flags, QEMU_CAPS_VIRTIO_S390);
+if (strstr(str, name \lsi53c895a\))
+qemuCapsSet(flags, QEMU_CAPS_SCSI_LSI);
+if (strstr(str, name \virtio-scsi-pci\))
+qemuCapsSet(flags, QEMU_CAPS_VIRIO_SCSI_PCI);
+
/* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */
if (!qemuCapsGet(flags, QEMU_CAPS_CHARDEV_SPICEVMC)
strstr(str, name \spicevmc\))
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c1b67a6..05e942e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -136,6 +136,8 @@ enum qemuCapsFlags {
QEMU_CAPS_VIRTIO_S390= 98, /* -device virtio-*-s390 */
QEMU_CAPS_BALLOON_EVENT = 99, /* Async event for balloon changes */
QEMU_CAPS_NETDEV_BRIDGE = 100, /* bridge helper support */
+QEMU_CAPS_SCSI_LSI = 101, /* -device lsi */
+QEMU_CAPS_VIRIO_SCSI_PCI = 102, /* -device virtio-scsi-pci */
QEMU_CAPS_LAST, /* this must always be the last item */
};
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel.
When scsi model is given explicitly in XML(model 0) checking if the
underlying QEMU supports it or not first, raise an error on checking
failure.
When the model is not given(mode = 0), return LSI by default, if
the QEMU doesn't support it, raise an error.
---
src/qemu/qemu_command.c | 106 +++---
src/qemu/qemu_command.h |3 +-
src/qemu/qemu_process.c |9 +++-
3 files changed, 88 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a05..d4791c6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -469,19 +469,58 @@ static int
qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk)
}
static int
-qemuDefaultScsiControllerModel(virDomainDefPtr def) {
-if (STREQ(def-os.arch, ppc64)
-STREQ(def-os.machine, pseries)) {
-return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+qemuCheckScsiControllerModel(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
+ int *model)
+{
+if (*model 0) {
+switch (*model) {
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(This QEMU doesn't support
+ lsi scsi controller));
+return -1;
+}
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+if (!qemuCapsGet(qemuCaps, QEMU_CAPS_VIRIO_SCSI_PCI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(This QEMU doesn't support
+ virtio scsi controller));
+return -1;
+}
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
+/*TODO: need checking work here if necessary */
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unsupported controller model: %s),
+ virDomainControllerModelSCSITypeToString(*model));
+return -1;
+}
} else {
-return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+if (STREQ(def-os.arch, ppc64)
+STREQ(def-os.machine, pseries)) {
+*model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+} else if (qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+*model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to determine model for scsi controller));
+return -1;
+}
}
+
+return 0;
}
/* Our custom -drive naming scheme used with id= */
static int
qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def,
-virDomainDiskDefPtr disk)
+virDomainDiskDefPtr disk,
+virBitmapPtr qemuCaps)
{
const char *prefix = virDomainDiskBusTypeToString(disk-bus);
int controllerModel = -1;
@@ -491,11 +530,10 @@ qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def,
controllerModel =
virDomainDiskFindControllerModel(def, disk,
VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
-}
-if (controllerModel == -1 ||
-controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO)
-controllerModel = qemuDefaultScsiControllerModel(def);
+if ((qemuCheckScsiControllerModel(def, qemuCaps,
controllerModel)) 0)
+return -1;
+}
if (disk-bus != VIR_DOMAIN_DISK_BUS_SCSI ||
controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
@@ -533,7 +571,7 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef,
{
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE))
-return qemuAssignDeviceDiskAliasCustom(vmdef, def);
+return qemuAssignDeviceDiskAliasCustom(vmdef, def, qemuCaps);
else
return qemuAssignDeviceDiskAliasFixed(def);
} else {
@@ -850,13 +888,24 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def,
virDomainDeviceInfoPtr info,
return 0;
}
-int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)
+int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps)
{
int i, rc;
int model;
+virBitmapPtr localCaps = NULL;
/* Default values match QEMU. See spapr_(llan|vscsi|vty).c */
+if (!qemuCaps) {
+/* need to get information from real environment */
+if (qemuCapsExtractVersionInfo(def-emulator, def-os.arch,
+
[libvirt] [PATCH 0/3 v2] Add lsi and virtio-scsi qemu caps
Add two capabilities flags for the model of scsi controller LSI and virtio-scsi. Use the lsilogic model by default, if current QEMU supports doesn't it, raise an error. When scsi model is given in XML explicitly, we try to check if the underlying QEMU supports it or not, raise an error on checking failure. Guannan Ren(3) qemu: add capabilities flags related to scsi controller qemu: add two qemu caps for lsi and virtio-scsi SCSI add lsi and virtio-scsi qemu caps in testcases. src/qemu/qemu_capabilities.c |7 +++ src/qemu/qemu_capabilities.h |2 + src/qemu/qemu_command.c | 106 +++-- src/qemu/qemu_command.h |3 +- src/qemu/qemu_process.c |9 ++- tests/qemuhelptest.c | 10 +++- tests/qemuxml2argvtest.c | 16 -- 7 files changed, 114 insertions(+), 39 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/17] Supports for hypervisor-pin and hypervisor-bandwidth
On Tue, Aug 07, 2012 at 10:13:19AM -0600, Eric Blake wrote: On 08/07/2012 09:47 AM, Daniel P. Berrange wrote: On Fri, Aug 03, 2012 at 02:36:02PM +0800, Hu Tao wrote: This series is a merge of 1) Support hypervisor-threads-pin in vcpupin (https://www.redhat.com/archives/libvir-list/2012-July/msg01361.html) 2) support to set cpu bandwidth for hypervisor threads (https://www.redhat.com/archives/libvir-list/2012-June/msg01161.html) to make life easier because of the two share some patches. This series is really focusing on pinning threads associated with the emulator element, rather than the hypervisor. The hypervisor is a separate entity that is shared. So I'm thinking that this entire patch series could replace 'hypervisor' with 'emulator' everywhere. Any one has agree or disagree ? I definitely agree - when I hear 'hypervisor', I think 'qemu:///system', which is the technology used to run multiple guests, but when I hear 'emulator', I think of a subset of a domain, namely the specific qemu pid_t running a given guest. Also, we're not pinning all of the hypervisor's threads, but just the threads that are associated with emulation but not a specific vcpu. That is, marking up your comment in 1/17: cgroup mount point +--libvirt= setting up a namespace (*) +--qemu= hypervisor level +--domain name = domain level +--vcpu0 = vcpu level ... +--vcpuN +--hypervisor = emulator so a domain really is made up of an 'emulator' and 'vcpu' threads, and a 'hypervisor' contains domains, rather than making up a portion of a domain. Also note that LXC has an emulator /usr/libexec/libvirt_lxc for which all these new APIs apply, but the term 'hypervisor' is meaningless for container based virt. 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 v7 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On 08/07/2012 12:49 PM, Eric Blake wrote:
On 08/07/2012 09:58 AM, Corey Bryant wrote:
This patch adds support that enables passing of file descriptors
to the QEMU monitor where they will be stored in specified file
descriptor sets.
A file descriptor set can be used by a client like libvirt to
store file descriptors for the same file. This allows the
client to open a file with different access modes (O_RDWR,
O_WRONLY, O_RDONLY) and add/remove the passed fds to/from an fd
set as needed. This will allow QEMU to (in a later patch in this
series) open and reopen the same file by dup()ing the fd in
the fd set that corresponds to the file, where the fd has the
matching access mode flag that QEMU requests.
The new QMP commands are:
add-fd: Add a file descriptor to an fd set
remove-fd: Remove a file descriptor from an fd set
query-fdsets: Return information describing all fd sets
+
+# @AddfdInfo:
+#
+# Information about a file descriptor that was added to an fd set.
+#
+# @fdset_id: The ID of the fd set that @fd was added to.
+#
+# @fd: The file descriptor that was received via SCM rights and
+# added to the fd set.
+#
+# Since: 1.2.0
We're not very consistent on '1.2' vs. '1.2.0' in since listings, but
that's probably worth a global cleanup closer to hard freeze.
I'll make a note of it. Or does Luiz usually do a cleanup?
+##
+{ 'type': 'AddfdInfo', 'data': {'fdset_id': 'int', 'fd': 'int'} }
This is a new command, so s/fdset_id/fdset-id/
Ok
+
+##
+# @add-fd:
+#
+# Add a file descriptor, that was passed via SCM rights, to an fd set.
+#
+# @fdset_id: #optional The ID of the fd set to add the file descriptor to.
+#
+# Returns: @AddfdInfo on success
+# If file descriptor was not received, FdNotSupplied
+# If @fdset_id does not exist, FdSetNotFound
+#
+# Notes: The list of fd sets is shared by all monitor connections.
+#
+#If @fdset_id is not specified, a new fd set will be created.
+#
+# Since: 1.2.0
+##
+{ 'command': 'add-fd', 'data': {'*fdset_id': 'int'},
Again, s/fdset_id/fdset-id/
Ok
+ 'returns': 'AddfdInfo' }
+
+##
+# @remove-fd:
+#
+# Remove a file descriptor from an fd set.
+#
+# @fdset_id: The ID of the fd set that the file descriptor belongs to.
+#
+# @fd: #optional The file descriptor that is to be removed.
+#
+# Returns: Nothing on success
+# If @fdset_id or @fd is not found, FdNotFound
+#
+# Since: 1.2.0
+#
+# Notes: The list of fd sets is shared by all monitor connections.
+#
+#File descriptors that are removed:
+#o will not be closed until the reference count corresponding
+# to @fdset_id reaches zero.
+#o will not be available for use after successful completion
+# of the remove-fd command.
+#
+#If @fd is not specified, all file descriptors in @fdset_id
+#will be removed.
+##
+{ 'command': 'remove-fd', 'data': {'fdset_id': 'int', '*fd': 'int'} }
And again.
Ok
+
+##
+# @FdsetFdInfo:
+#
+# Information about a file descriptor that belongs to an fd set.
+#
+# @fd: The file descriptor value.
+#
+# @removed: If true, the remove-fd command has been issued for this fd.
+#
+# Since: 1.2.0
+##
+{ 'type': 'FdsetFdInfo', 'data': {'fd': 'int', 'removed': 'bool'} }
Is it worth providing any additional information? For example, knowing
whether the fd is O_RDONLY, O_WRONLY, or O_RDWR might be beneficial to
management apps trying to discover what fds are already present after a
reconnection, in order to decide whether to close them without having to
resort to /proc/$qemupid/fdinfo/nnn lookups. It might even be worth
marking such information optional, present only when 'removed':false.
It makes sense but I'd like to limit the new functionality at this point
so that I can get this support into QEMU 1.2. Can this be added as a
follow up patch?
+
+##
+# @FdsetInfo:
+#
+# Information about an fd set.
+#
+# @fdset_id: The ID of the fd set.
+#
+# @refcount: A count of the number of outstanding dup() references to
+#this fd set.
+#
+# @in_use: If true, a monitor is connected and has access to this fd set.
+#
+# @fds: A list of file descriptors that belong to this fd set.
+#
+# Since: 1.2.0
+##
+{ 'type': 'FdsetInfo',
+ 'data': {'fdset_id': 'int', 'refcount': 'int', 'in_use': 'bool',
+ 'fds': ['FdsetFdInfo']} }
s/fdset_id/fdset-id/; s/in_use/in-use/
Ok
+
+##
+# @query-fdsets:
+#
+# Return information describing all fd sets.
+#
+# Returns: A list of @FdsetInfo
+#
+# Since: 1.2.0
+#
+# Notes: The list of fd sets is shared by all monitor connections.
+#
+#File descriptors are not closed until @refcount is zero,
+#and either @in_use is false or @removed is true.
+#
+##
+{ 'command': 'query-fdsets', 'returns': ['FdsetInfo'] }
diff --git a/qerror.c b/qerror.c
index 92c4eff..63a0aa1 100644
--- a/qerror.c
+++ b/qerror.c
@@ -148,6 +148,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = No file descriptor
[libvirt] PF interface brought down if guest using a VF in hostdev mode under 802.1Qbh
Hi, In 802.1Qbh case where exists a SR-IOV capable network device, if any of the virtual functions of this device is used in hostdev mode for the guest, port-profile disassociate will also cause physical function interface to go down. This appears to be a bug, but wanted to find if this was done intentionally for some reasons. To be more specific, if a network device supports SRIOV and its VF is being used in pci passthrough mode, when the guest is shutdown or destroyed following happens - qemuProcessStop \_ qemuDomainReAttachHostdevDevices(hostdevs) \_ qemuDomainHostdevNetConfigRestore(hostdev) \_ virNetDevPortProfileDisassociate(linkdev) \_ virNetDevSetOnline(linkdev, false) In above, qemuDomainHostdevNetConfigRestore() finds out the PF for provided hostdev (which is VF) and passes it to virNetDevPortProfileDisassociate() as linkdev. Later, linkdev gets passed to virNetDevSetOnline() where the interface is brought down by clearing IFF_UP flag. However, in macvtap emulation mode, virNetDevMacVLanDeleteWithVPortProfile() passes VF as linkdev (and -1 as vf) to virNetDevPortProfileDisassociate(), hence, not affecting PF at all. Bringing down a PF, when only VF is being brought down is not expected behavior (unless, I'm missing something here). A way to get around it would be to check if there exists vf (=0) in virNetDevPortProfileDisassociate(), and if so, it should only pass the interface name of this VF rather than PF itself to virNetDevSetOnline(). If it sounds reasonable, I'll send out a fix. Thanks, Nishank -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/17] Improve vcpupin to support hypervisorpin dynamically.
On 08/03/2012 12:36 AM, Hu Tao wrote:
From: Tang Chen tangc...@cn.fujitsu.com
Modify vcpupin command to support hypervisor threads pin.
1) add --hypervisor option to get hypervisor threads info.
2) add --hypervisor cpuset to set hypervisor threads to specified
cpuset.
Dan's comment of s/hypervisor/emulator/ applies here as well.
Signed-off-by: Tang Chen tangc...@cn.fujitsu.com
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
tests/vcpupin|6 +--
tools/virsh-domain.c | 147
+-
tools/virsh.pod | 12 +++--
3 files changed, 109 insertions(+), 56 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index f1fb038..e55806e 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -31,16 +31,16 @@ fi
fail=0
# Invalid syntax.
-$abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1
out 21
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test a
--vcpu 0,1 out 21
Why did you have to change this example invocation? If you broke
back-compat with existing clients, then we need to rethink how to add
the new functionality into virsh. Since this was a negative test (where
the command used to fail), if the command now succeeds where it used to
fail, and adding '--vcpu' preserves the negative test aspect, I could
understand it; but that doesn't appear to be the case here.
test $? = 1 || fail=1
cat \EOF exp || fail=1
-error: vcpupin: Invalid or missing vCPU number.
+error: vcpupin: Invalid or missing vCPU number, or missing --hypervisor
option.
Changing an error message is okay, though.
EOF
compare exp out || fail=1
# An out-of-range vCPU number deserves a diagnostic, too.
-$abs_top_builddir/tools/virsh --connect test:///default vcpupin test 100 0,1
out 21
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test --vcpu
100 0,1 out 21
Again, you shouldn't be needing to modify this test.
test $? = 1 || fail=1
cat \EOF exp || fail=1
error: vcpupin: Invalid vCPU number.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 33b1727..74603e8 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4373,14 +4373,15 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd)
* vcpupin command
*/
static const vshCmdInfo info_vcpupin[] = {
-{help, N_(control or query domain vcpu affinity)},
-{desc, N_(Pin domain VCPUs to host physical CPUs.)},
+{help, N_(control or query domain vcpu or hypervisor threads
affinities)},
+{desc, N_(Pin domain VCPUs or hypervisor threads to host physical
CPUs.)},
{NULL, NULL}
};
static const vshCmdOptDef opts_vcpupin[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
-{vcpu, VSH_OT_INT, 0, N_(vcpu number)},
+{vcpu, VSH_OT_INT, VSH_OFLAG_REQ_OPT, N_(vcpu number)},
This is the broken change - you can't require existing users to start
typing --vcpu.
+{hypervisor, VSH_OT_BOOL, VSH_OFLAG_REQ_OPT, N_(pin hypervisor
threads)},
bool options cannot require an option, so using VSH_OFLAG_REQ_OPT is
wrong here (hmm, virsh.c's self-check validation should probably be
erroring out on this combination).
I think what you want to end up with is:
vcpupin dom
= query ALL pinnings
vcpupin dom --emulator
= query just the emulator pinning
vcpupin dom [--vcpu] number
= query just vcpuN pinning
vcpupin dom --emulator cpulist
= set the emulator pinning to cpulist
vcpupin dom [--vcpu] number cpulist
= set the vcpuN pinning to cpulist
Or in virsh.pod syntax:
vcpupin domain [ { [--vcpu] number | --emulator } [cpulist] ]
[--config] [--live] [--current]
such that either --emulator or an integer but not both must be present
if cpulist is also present; and that an integer can optionally be
prefixed with --vcpu. There's no way to write that directly in
opts_vcpupin, so it requires listing both options as optional, then
manual checks in the function body that exactly one of the two options
was present.
Hmm, I see a potential gotcha: the parser would treat:
vcpupin dom --emulator 0
as assigning '0' to --vcpu, instead of to --cpulist, so users of
--emulator would always have to spell out:
vcpupin dom --emulator --cpulist 0
but since that's a new command, at least we wouldn't be breaking
back-compat.
Maybe a solution would be to take --vcpu -1 as the special case of
requesting the emulator pinning, instead of adding a new flag
--emulator. That makes sense if we do the same overloading at the
libvirt API level.
Or maybe the solution is to add a new command 'virsh emulatorpin' for
emulator pinning, instead of trying to overload 'virsh vcpupin',
especially, since you have proposed separate libvirt APIs. That does
mean you can't list all pinnings in one command, but that's not too
tough of a restriction.
I'm still thinking about the best way to resolve this UI issue, and I
think part of the answer is determined by the
[libvirt] [ANNOUNCE] libvirt-glib 0.1.1 release
I am pleased to announce that a new release of the libvirt-glib package, version 0.1.1 is now available from ftp://libvirt.org/libvirt/glib/ The packages are GPG signed with Key fingerprint: DE11 B48B FDC1 6349 4312 0860 FCAC ABEE 62D1 1E4C (2048R) New in this release: - Add bindings for virDomainSnapshotCreate*(). - Add bindings for virDomainRestore*(). - Add GVir.DomainShutdownFlags() binding. - Add GVir.DomainXMLFlags binding. libvirt-glib comprises three distinct libraries: - libvirt-glib- Integrate with the GLib event loop and error handling - libvirt-gconfig - Representation of libvirt XML documents as GObjects - libvirt-gobject - Mapping of libvirt APIs into the GObject type system NB: While libvirt aims to be API/ABI stable forever, with libvirt-glib we are not yet guaranteeing that libvirt-glib libraries are API/ABI permanently stable. As of the 0.0.8 release, we have tentatively frozen the API/ABI with the intent of being longterm stable hereafter, but there is still a small chance we might find flaws requiring an API/ABI change. The likelihood of this is low, however, and we will strive to avoid it. Follow up comments about libvirt-glib should be directed to the regular libvir-list redhat com development list. Thanks to all the people involved in contributing to this release. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v6 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On Fri, Aug 3, 2012 at 6:28 PM, Corey Bryant cor...@linux.vnet.ibm.com wrote:
diff --git a/monitor.c b/monitor.c
index 49dccfe..9aa9f7e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -140,6 +140,24 @@ struct mon_fd_t {
QLIST_ENTRY(mon_fd_t) next;
};
+/* file descriptor associated with a file descriptor set */
+typedef struct mon_fdset_fd_t mon_fdset_fd_t;
+struct mon_fdset_fd_t {
QEMU coding style is:
typedef struct MonFdsetFd MonFdsetFd;
struct MonFdsetFd {
See ./CODING_STYLE for more info.
+int fd;
+bool removed;
+QLIST_ENTRY(mon_fdset_fd_t) next;
+};
+
+/* file descriptor set containing fds passed via SCM_RIGHTS */
+typedef struct mon_fdset_t mon_fdset_t;
+struct mon_fdset_t {
+int64_t id;
+int refcount;
+bool in_use;
+QLIST_HEAD(, mon_fdset_fd_t) fds;
+QLIST_ENTRY(mon_fdset_t) next;
+};
At this point in the patch series it's not clear to me whether the
removed and refcount/in_use fields are a clean and correct solution.
Exposing these fields via QMP is also something I'm going to carefully
review because they seem like internals.
+
typedef struct MonitorControl {
QObject *id;
JSONMessageParser parser;
@@ -176,7 +194,8 @@ struct Monitor {
int print_calls_nr;
#endif
QError *error;
-QLIST_HEAD(,mon_fd_t) fds;
+QLIST_HEAD(, mon_fd_t) fds;
+QLIST_HEAD(, mon_fdset_t) fdsets;
QLIST_ENTRY(Monitor) entry;
};
@@ -2389,6 +2408,157 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
return -1;
}
+static void monitor_fdset_cleanup(mon_fdset_t *mon_fdset)
+{
+mon_fdset_fd_t *mon_fdset_fd;
+mon_fdset_fd_t *mon_fdset_fd_next;
+
+if (mon_fdset-refcount != 0) {
+return;
+}
+
+QLIST_FOREACH_SAFE(mon_fdset_fd, mon_fdset-fds, next,
mon_fdset_fd_next) {
+if (!mon_fdset-in_use || mon_fdset_fd-removed) {
+close(mon_fdset_fd-fd);
+QLIST_REMOVE(mon_fdset_fd, next);
+g_free(mon_fdset_fd);
+}
+}
+
+if (QLIST_EMPTY(mon_fdset-fds)) {
+QLIST_REMOVE(mon_fdset, next);
+g_free(mon_fdset);
+}
+}
+
+AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, Error **errp)
+{
+int fd;
+Monitor *mon = cur_mon;
+mon_fdset_t *mon_fdset;
+mon_fdset_fd_t *mon_fdset_fd;
+AddfdInfo *fdinfo;
+
+fd = qemu_chr_fe_get_msgfd(mon-chr);
+if (fd == -1) {
+qerror_report(QERR_FD_NOT_SUPPLIED);
+return NULL;
+}
+
+if (has_fdset_id) {
+QLIST_FOREACH(mon_fdset, mon-fdsets, next) {
+if (mon_fdset-id == fdset_id) {
+break;
+}
+}
+if (mon_fdset == NULL) {
+qerror_report(QERR_FDSET_NOT_FOUND, fdset_id);
+return NULL;
fd is leaked?
+}
+} else {
+int64_t fdset_id_prev = -1;
+mon_fdset_t *mon_fdset_cur = QLIST_FIRST(mon-fdsets);
+
+/* Use first available fdset ID */
+QLIST_FOREACH(mon_fdset, mon-fdsets, next) {
+mon_fdset_cur = mon_fdset;
+if (fdset_id_prev == mon_fdset_cur-id - 1) {
+fdset_id_prev = mon_fdset_cur-id;
+continue;
+}
+break;
+}
+
+mon_fdset = g_malloc0(sizeof(*mon_fdset));
+mon_fdset-id = fdset_id_prev + 1;
+mon_fdset-refcount = 0;
+mon_fdset-in_use = true;
+
+/* The fdset list is ordered by fdset ID */
+if (mon_fdset-id == 0) {
+QLIST_INSERT_HEAD(mon-fdsets, mon_fdset, next);
+} else if (mon_fdset-id mon_fdset_cur-id) {
+QLIST_INSERT_BEFORE(mon_fdset_cur, mon_fdset, next);
+} else {
+QLIST_INSERT_AFTER(mon_fdset_cur, mon_fdset, next);
+}
+}
+
+mon_fdset_fd = g_malloc0(sizeof(*mon_fdset_fd));
+mon_fdset_fd-fd = fd;
+mon_fdset_fd-removed = false;
+QLIST_INSERT_HEAD(mon_fdset-fds, mon_fdset_fd, next);
+
+fdinfo = g_malloc0(sizeof(*fdinfo));
+fdinfo-fdset_id = mon_fdset-id;
+fdinfo-fd = mon_fdset_fd-fd;
+
+return fdinfo;
+}
+
+void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
+{
+Monitor *mon = cur_mon;
+mon_fdset_t *mon_fdset;
+mon_fdset_fd_t *mon_fdset_fd;
+char fd_str[20];
+
+QLIST_FOREACH(mon_fdset, mon-fdsets, next) {
+if (mon_fdset-id != fdset_id) {
+continue;
+}
+QLIST_FOREACH(mon_fdset_fd, mon_fdset-fds, next) {
+if (has_fd mon_fdset_fd-fd != fd) {
+continue;
+}
+mon_fdset_fd-removed = true;
+if (has_fd) {
+break;
+}
+}
+monitor_fdset_cleanup(mon_fdset);
+return;
+}
+snprintf(fd_str, sizeof(fd_str), %ld, fd);
+qerror_report(QERR_FD_NOT_FOUND, fd_str);
Why use an fd_str instead
Re: [libvirt] [Qemu-devel] [PATCH v6 3/6] monitor: Clean up fd sets on monitor disconnect
On Fri, Aug 03, 2012 at 01:28:06PM -0400, Corey Bryant wrote: Each fd set has a boolean that keeps track of whether or not the fd set is in use by a monitor connection. When a monitor disconnects, all fds that are members of an fd set with refcount of zero are closed. This prevents any fd leakage associated with a client disconnect prior to using a passed fd. v5: -This patch is new in v5. -This support addresses concerns from v4 regarding fd leakage if the client disconnects unexpectedly. (ebl...@redhat.com, kw...@redhat.com, dberra...@redhat.com) v6: -No changes Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- monitor.c | 15 +++ 1 file changed, 15 insertions(+) The lifecycle of fdsets and fds isn't clear to me. It seems like just a refcount in fdset should handle this without extra fields like in_use. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v7 3/6] monitor: Clean up fd sets on monitor disconnect
Fd sets are shared by all monitor connections. Fd sets are considered
to be in use while at least one monitor is connected. When the last
monitor disconnects, all fds that are members of an fd set with
refcount of zero are closed. This prevents any fd leakage associated
with a client disconnect prior to using a passed fd.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v5:
-This patch is new in v5.
-This support addresses concerns from v4 regarding fd leakage
if the client disconnects unexpectedly. (ebl...@redhat.com,
kw...@redhat.com, dberra...@redhat.com)
v6:
-No changes
v7:
-Removed monitor_fdsets_set_in_use() function since we now use
mon_refcount to determine if fdsets are in use.
-Added monitor_fdsets_cleanup() function, and increment/decrement
of mon_refcount when monitor connects/disconnects.
monitor.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/monitor.c b/monitor.c
index 04b86b7..84eade8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2431,6 +2431,16 @@ static void monitor_fdset_cleanup(mon_fdset_t *mon_fdset)
}
}
+static void monitor_fdsets_cleanup(void)
+{
+mon_fdset_t *mon_fdset;
+mon_fdset_t *mon_fdset_next;
+
+QLIST_FOREACH_SAFE(mon_fdset, mon_fdsets, next, mon_fdset_next) {
+monitor_fdset_cleanup(mon_fdset);
+}
+}
+
AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, Error **errp)
{
int fd;
@@ -4760,9 +4770,12 @@ static void monitor_control_event(void *opaque, int
event)
data = get_qmp_greeting();
monitor_json_emitter(mon, data);
qobject_decref(data);
+mon_refcount++;
break;
case CHR_EVENT_CLOSED:
json_message_parser_destroy(mon-mc-parser);
+mon_refcount--;
+monitor_fdsets_cleanup();
break;
}
}
@@ -4803,6 +4816,12 @@ static void monitor_event(void *opaque, int event)
readline_show_prompt(mon-rs);
}
mon-reset_seen = 1;
+mon_refcount++;
+break;
+
+case CHR_EVENT_CLOSED:
+mon_refcount--;
+monitor_fdsets_cleanup();
break;
}
}
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/17] Update XML Schema for new entries
On 08/03/2012 12:36 AM, Hu Tao wrote: From: Wen Congyang we...@cn.fujitsu.com --- docs/schemas/domaincommon.rng | 10 ++ 1 file changed, 10 insertions(+) Missing counterpart documentation in docs/formatdomain.html.in, and a corresponding test case somewhere in tests. I would consider squashing this in with 15/17 - the new schema doesn't do us any good without also being able to parse it. -- Eric Blake ebl...@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 13/17] limit cpu bandwidth only for vcpus
On 08/03/2012 12:36 AM, Hu Tao wrote: This patch changes the behaviour of xml element cputune.period and cputune.quota to limit cpu bandwidth only for vcpus, and no longer limit cpu bandwidth for the whole guest. The reasons to do this are: - This matches docs of cputune.period and cputune.quota. - The other parts excepting vcpus are treated as hypervisor, and there are seperate period/quota settings for hypervisor s/hypervisor/emulator/ throughout, s/seperate/separate/ Is it worth being able to put a separate quota on the entire domain (emulator+vcpus), independently from the smaller quotas on a per-vcpu or per-emulator basis? While I do think this is an appropriate fix (normally, what we are trying to rate limit is the guest itself, and that means run the emulator as fast as possible to let the guest keep up), I do worry that it means someone that was previously using this to limit the entire domain would now be oversubscribed, unless we also have a way to preserve the ability to limit the entire domain. -- Eric Blake ebl...@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 15/17] qemu: Implement hypervisor's period and quota tunable XML configuration and parsing
On 08/03/2012 12:36 AM, Hu Tao wrote: From: Wen Congyang we...@cn.fujitsu.com subject is long. I'd suggest: qemu: introduce period/quota tuning for emulator -- Eric Blake ebl...@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] util: include stderr in log message when an external command fails
On 08/07/2012 07:38 AM, Daniel P. Berrange wrote: On Mon, Aug 06, 2012 at 08:07:32PM -0400, Laine Stump wrote: This patch is in response to: https://bugzilla.redhat.com/show_bug.cgi?id=818467 If a caller to virCommandRun doesn't ask for the exitstatus of the program it's running, the virCommand functions assume that they should log an error message and return failure if the exit code isn't 0. However, only the commandline and exit status are logged, while potentially useful information sent by the program to stderr is discarded. Fortunately, virCommandRun is already checking if the caller had asked for stderr to be saved and, if not, sets things up to save it in *cmd-errbuf. This makes it fairly simple for virCommandWait to include *cmd-errbuf in the error log (there are still other callers that don't setup errbuf, and even virCommandRun won't set it up if the command is being daemonized, so we have to check that it's non-zero). ACK Pushed. Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v6 3/6] monitor: Clean up fd sets on monitor disconnect
On 08/07/2012 01:32 PM, Stefan Hajnoczi wrote: On Fri, Aug 03, 2012 at 01:28:06PM -0400, Corey Bryant wrote: Each fd set has a boolean that keeps track of whether or not the fd set is in use by a monitor connection. When a monitor disconnects, all fds that are members of an fd set with refcount of zero are closed. This prevents any fd leakage associated with a client disconnect prior to using a passed fd. v5: -This patch is new in v5. -This support addresses concerns from v4 regarding fd leakage if the client disconnects unexpectedly. (ebl...@redhat.com, kw...@redhat.com, dberra...@redhat.com) v6: -No changes Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- monitor.c | 15 +++ 1 file changed, 15 insertions(+) The lifecycle of fdsets and fds isn't clear to me. It seems like just a refcount in fdset should handle this without extra fields like in_use. The lifecycle of fdsets and fds starts with add-fd. I'll explain the lifecycle end of fdsets and fds below. To follow along with the code, this cleanup occurs in monitor_fdset_cleanup(). Fds are closed and removed from an fdset when there are no more open dup() fds (refcount == 0) for the fd set, and there are either no monitor connections (!in-use) or the fd has been removed with remove-fd. In other words fds get cleaned up when: (refcount == 0 (!in-use || removed)) Let me explain each variable: (1) refcount is incremented when qemu_open() dup()s an fd from an fd set and is decremented when qemu_close() closes a dup()d fd. We don't want to close any fds in an fd set if refcount 0, because this file could be reopened with different access mode flags, which would require dup() of another fd from the fdset. (2) in-use is used to prevent fd leakage if a monitor disconnects and abandons fds. If libvirt adds fds and then disconnects without issuing a command that references the fds, then refcount will be zero, and in-use will be false, and the fds will be closed and removed from the fd set. When the monitor connection is restored, the query-fdsets command can be used to see what fd sets and fds are available. (3) If the remove-fd command is issued, the fd is marked for removal. It won't be closed until there are no more outstanding dup() references on the fd set, for similar reasons to why we don't close the fd in (1). fdsets simply get cleaned up once all fds from the set have been closed and removed. Hopefully this clears things up a bit more. Please also take a look at the v7 series that I sent out today. Fd sets are now stored globally, rather than one per Monitor object. This simplifies things a bit. -- Regards, Corey -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 16/17] qemu: Implement hypervisor_period and hypervisor_quota's modification
On 08/03/2012 12:36 AM, Hu Tao wrote:
From: Wen Congyang we...@cn.fujitsu.com
allow the user change/get hypervisor's period and quota when the vm is
running.
---
include/libvirt/libvirt.h.in | 16 +
src/qemu/qemu_driver.c | 133
+-
2 files changed, 148 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 15c08c1..dd34295 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -692,6 +692,22 @@ typedef virTypedParameter *virTypedParameterPtr;
#define VIR_DOMAIN_SCHEDULER_VCPU_QUOTA vcpu_quota
/**
+ * VIR_DOMAIN_SCHEDULER_HYPERVISOR_PERIOD:
more of the s/hypervisor/emulator/ renaming
+ *
+ * Macro represents the enforcement period for a quota, in microseconds,
+ * when using the posix scheduler, as a ullong.
Enforcement period for what? I would suggest:
Macro represents the enforcement period for a quota in microseconds,
when using the posix scheduler, for all emulator activity not tied to
vcpus, as a ullong.
+ */
+#define VIR_DOMAIN_SCHEDULER_HYPERVISOR_PERIOD hypervisor_period
+
+/**
+ * VIR_DOMAIN_SCHEDULER_HYPERVISOR_QUOTA:
+ *
+ * Macro represents the maximum bandwidth to be used within a period,
+ * when using the posix scheduler, as an llong.
and similarly. It may mean rewording the existing descriptions of vcpu
quota to be more obvious that they limit only vcpu activity.
@@ -7420,6 +7420,40 @@ cleanup:
}
static int
+qemuSetHypervisorBWLive(virDomainObjPtr vm, virCgroupPtr cgroup,
Probably copying from existing naming, but BW looks awkward to me (I
first read it as black/white, not bandwidth); spelling it out would help.
static int
+qemuGetHypervisorBWLive(virDomainObjPtr vm, virCgroupPtr cgroup,
+unsigned long long *period, long long *quota)
Same comment about spelling out BW as Bandwidth.
+{
+virCgroupPtr cgroup_hypervisor = NULL;
+qemuDomainObjPrivatePtr priv = NULL;
+int rc;
+int ret = -1;
+
+priv = vm-privateData;
+if (priv-nvcpupids == 0 || priv-vcpupids[0] == vm-pid) {
+/* We donot create sub dir for each vcpu */
s/donot/don't/
--
Eric Blake ebl...@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
[libvirt] sending openvswitch port db info during libvirt migration
Someone asked on IRC the other day about sending openvswitch per-port data (normally stored in the switch) to the destination host during a migration. I suggested maybe this could be handled by encoding the information into the interface's virtualport prior to migration, and then writing it back out to openvswitch on the destination when the interface was reconnected there. I think there's a problem with that, though - they don't want to save the port data on the source until the instant that the interface is disconnected (since it is constantly changing), but by then the domain XML has already long ago been formatted and sent to the destination. So is there some other way within the confines of the current migration protocol that this information can be sent from migration source to destination? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5] ESX: Add routines to interface driver
2012/8/6 Ata Bohra ata.hus...@hotmail.com: (Please ignore earlier messages, as for reason the messages got truncated) Thanks Matthias for the explanation, I was working on the project which required libvirt to control/set networking on a given ESX host. As ESXi is different from hosted hypervisors, we need to configure ESX interface (IP, routing, bridge etc), also it is an integral part of the host settings. Okay, now I understand why you choose to make the interface driver list HostVirtualNics. This gives you the possibility to configure this part of an ESX server. But re-thinking the whole design in light of the explanation provided by you, libvirt interfaces are actually mapping physical interfaces to virtual network. Further, hostVirtualSwitch behaves like switch which defines ports where we can plug VM virtual ethernet cards. It seems likely that we still want to have ability of configuring ESXi virtual interfaces (hostVirtualNics) so not sure if listing physical Nics is completly right to get an 100% operational ESX host. This should be possible, but as of the network driver. Laine mentioned that the IP element of a libvirt network config could be used for that purpose. This idea looks promising, but needs to be investigated in detail first. I might be able to spent some time on this next weekend. Regards, Matthias Michal has already reviewed the code and I would do the same if this design is one that maps to libvirt terminology the most. Though my design is not accpeted, but it was an amazing learning experience. Thanks for everyones input that came along the way. Regards, Ata Date: Sun, 5 Aug 2012 22:45:26 +0200 Subject: Re: [libvirt] [PATCH v5] ESX: Add routines to interface driver From: matthias.bo...@googlemail.com To: ata.hus...@hotmail.com CC: libvir-list@redhat.com 2012/8/5 Matthias Bolte matthias.bo...@googlemail.com: 2012/8/2 Ata E Husain Bohra ata.hus...@hotmail.com: Add following routines to esx_interface_driver: esxNumOfInterfaces, esxNumOfDefinedInterfaces, esxListInterfaces, esxListDefinedInterfaces, esxInterfaceLookupByMACString, esxInterfaceGetXMLDesc, esxInterfaceUndefine, esxInterfaceCreate, esxInterfaceDestroy Signed-off-by: Ata E Husain Bohra ata.hus...@hotmail.com Okay, I finally had time to take a detailed look at the interface driver and the related network driver and your proposed implementation for the interface driver. I think listing HostVirtualNics in the interface driver is not the correct mapping between vSphere API and libvirt. Also listing HostVirtualNics as bridges is not correct because a HostVirtualNic is not a bridge. A HostVirtualSwitch can be seen as a kind of bridge, but a HostVirtualSwitch is more like a virtual network in libvirt terms. In terms of libvirt the interface driver is about physical NICs that can be used to connect a libvirt virtual network to the physical network. Therefore, the interface driver should just list the PhysicalNics, but not the HostVirtualNics. I think there is currently no place in the libvirt API to map the HostVirtualNics to. Also I don't think that it is important to make the HostVirtualNics available via libvirt API. I might be wrong here, but mapping them via libvirt interface driver is still wrong in my opinion. Regarding the network driver, I think a libvirt virtual network is best represented by a HostVirtualSwitch and the HostPortGroups are mapped to the portgroups of a libvirt virtual switch. I'm sorry that I let you wait for quite a while now until I came to this understanding that is contrary to your proposed interface driver. I missed to mention that I proposed an implementation for the interface and network drivers according to the described mapping: https://www.redhat.com/archives/libvir-list/2012-August/msg00307.html https://www.redhat.com/archives/libvir-list/2012-August/msg00308.html -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5] ESX: Add routines to interface driver
2012/8/7 Laine Stump la...@laine.org: On 08/06/2012 03:18 PM, Ata Bohra wrote: But re-thinking the whole design in light of the explanation provided by you, libvirt interfaces are actually mapping physical interfaces to virtual network. Further, hostVirtualSwitch behaves like switch which defines ports where we can plug VM virtual ethernet cards. It seems likely that we still want to have ability of configuring ESXi virtual interfaces (hostVirtualNics) so not sure if listing physical Nics is completly right to get an 100% operational ESX host. After reading Matthias' explanation, I was left wondering exactly what is the purpose of a hostVirtualNic. Is it used to give the hypervisor a connection to the hostVirtualSwitch? That its exact purpose. The hypervisor uses a HostVirtualNic to connect through a HostVirtualSwitch and PhysicalNic to the network in order to access network-based storage via NFS or iSCSI. This network connection is also used for migration. If so, that's something that's implied in libvirt's networks when they have an IP address defined - the presence of an IP address for the network is really indicating that there's a connection up to the host's (aka hypervisor) IP stack. (this is a legacy of the design of Linux host bridges - I think of there being an implied port on the bridge that is connected to the host kernel if the bridge has an IP address.) This sounds like a promising idea that might allow to expose a HostVirtualNic as part of a libvirt network. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v6 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On 08/07/2012 02:16 PM, Stefan Hajnoczi wrote:
On Fri, Aug 3, 2012 at 6:28 PM, Corey Bryant cor...@linux.vnet.ibm.com wrote:
diff --git a/monitor.c b/monitor.c
index 49dccfe..9aa9f7e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -140,6 +140,24 @@ struct mon_fd_t {
QLIST_ENTRY(mon_fd_t) next;
};
+/* file descriptor associated with a file descriptor set */
+typedef struct mon_fdset_fd_t mon_fdset_fd_t;
+struct mon_fdset_fd_t {
QEMU coding style is:
typedef struct MonFdsetFd MonFdsetFd;
struct MonFdsetFd {
See ./CODING_STYLE for more info.
Thanks, I'll fix that.
+int fd;
+bool removed;
+QLIST_ENTRY(mon_fdset_fd_t) next;
+};
+
+/* file descriptor set containing fds passed via SCM_RIGHTS */
+typedef struct mon_fdset_t mon_fdset_t;
+struct mon_fdset_t {
+int64_t id;
+int refcount;
+bool in_use;
+QLIST_HEAD(, mon_fdset_fd_t) fds;
+QLIST_ENTRY(mon_fdset_t) next;
+};
At this point in the patch series it's not clear to me whether the
removed and refcount/in_use fields are a clean and correct solution.
Exposing these fields via QMP is also something I'm going to carefully
review because they seem like internals.
Yes, please review the v7 patches and let me know what you think. I
explained the purpose of these fields in the previous email I just sent
you, so I won't go into their details again here. But I will point out
that refcount/in-use came about after concern of fd leakage if libvirt's
monitor connection disconnects. query-fdsets allows the client to
determine the state of the fd sets after reconnecting.
+
typedef struct MonitorControl {
QObject *id;
JSONMessageParser parser;
@@ -176,7 +194,8 @@ struct Monitor {
int print_calls_nr;
#endif
QError *error;
-QLIST_HEAD(,mon_fd_t) fds;
+QLIST_HEAD(, mon_fd_t) fds;
+QLIST_HEAD(, mon_fdset_t) fdsets;
QLIST_ENTRY(Monitor) entry;
};
@@ -2389,6 +2408,157 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
return -1;
}
+static void monitor_fdset_cleanup(mon_fdset_t *mon_fdset)
+{
+mon_fdset_fd_t *mon_fdset_fd;
+mon_fdset_fd_t *mon_fdset_fd_next;
+
+if (mon_fdset-refcount != 0) {
+return;
+}
+
+QLIST_FOREACH_SAFE(mon_fdset_fd, mon_fdset-fds, next, mon_fdset_fd_next)
{
+if (!mon_fdset-in_use || mon_fdset_fd-removed) {
+close(mon_fdset_fd-fd);
+QLIST_REMOVE(mon_fdset_fd, next);
+g_free(mon_fdset_fd);
+}
+}
+
+if (QLIST_EMPTY(mon_fdset-fds)) {
+QLIST_REMOVE(mon_fdset, next);
+g_free(mon_fdset);
+}
+}
+
+AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, Error **errp)
+{
+int fd;
+Monitor *mon = cur_mon;
+mon_fdset_t *mon_fdset;
+mon_fdset_fd_t *mon_fdset_fd;
+AddfdInfo *fdinfo;
+
+fd = qemu_chr_fe_get_msgfd(mon-chr);
+if (fd == -1) {
+qerror_report(QERR_FD_NOT_SUPPLIED);
+return NULL;
+}
+
+if (has_fdset_id) {
+QLIST_FOREACH(mon_fdset, mon-fdsets, next) {
+if (mon_fdset-id == fdset_id) {
+break;
+}
+}
+if (mon_fdset == NULL) {
+qerror_report(QERR_FDSET_NOT_FOUND, fdset_id);
+return NULL;
fd is leaked?
Yes, it looks like it is. I'll fix that.
+}
+} else {
+int64_t fdset_id_prev = -1;
+mon_fdset_t *mon_fdset_cur = QLIST_FIRST(mon-fdsets);
+
+/* Use first available fdset ID */
+QLIST_FOREACH(mon_fdset, mon-fdsets, next) {
+mon_fdset_cur = mon_fdset;
+if (fdset_id_prev == mon_fdset_cur-id - 1) {
+fdset_id_prev = mon_fdset_cur-id;
+continue;
+}
+break;
+}
+
+mon_fdset = g_malloc0(sizeof(*mon_fdset));
+mon_fdset-id = fdset_id_prev + 1;
+mon_fdset-refcount = 0;
+mon_fdset-in_use = true;
+
+/* The fdset list is ordered by fdset ID */
+if (mon_fdset-id == 0) {
+QLIST_INSERT_HEAD(mon-fdsets, mon_fdset, next);
+} else if (mon_fdset-id mon_fdset_cur-id) {
+QLIST_INSERT_BEFORE(mon_fdset_cur, mon_fdset, next);
+} else {
+QLIST_INSERT_AFTER(mon_fdset_cur, mon_fdset, next);
+}
+}
+
+mon_fdset_fd = g_malloc0(sizeof(*mon_fdset_fd));
+mon_fdset_fd-fd = fd;
+mon_fdset_fd-removed = false;
+QLIST_INSERT_HEAD(mon_fdset-fds, mon_fdset_fd, next);
+
+fdinfo = g_malloc0(sizeof(*fdinfo));
+fdinfo-fdset_id = mon_fdset-id;
+fdinfo-fd = mon_fdset_fd-fd;
+
+return fdinfo;
+}
+
+void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
+{
+Monitor *mon = cur_mon;
+mon_fdset_t *mon_fdset;
+mon_fdset_fd_t *mon_fdset_fd;
+char fd_str[20];
+
+QLIST_FOREACH(mon_fdset, mon-fdsets, next) {
+if (mon_fdset-id != fdset_id) {
+continue;
+}
+QLIST_FOREACH(mon_fdset_fd, mon_fdset-fds, next)
Re: [libvirt] [PATCH 17/17] update doc about hypervisor_period/hypervisor_quota
On 08/03/2012 12:36 AM, Hu Tao wrote: --- tools/virsh.pod | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index ffc0777..92cb897 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1202,7 +1202,8 @@ available for each hypervisor are: LXC (posix scheduler) : cpu_shares -QEMU/KVM (posix scheduler): cpu_shares, vcpu_period, vcpu_quota +QEMU/KVM (posix scheduler): cpu_shares, vcpu_period, vcpu_quota, +hypervisor_period, hypervisor_quota Make sure you match the naming in the API, if we go with emulator_quota. I would rebase this series to split 16 and 17 differently - have 16 be the new API (libvirt.h.in and this file) and its first client, but not implementation; then 17 would be the first implementation of that API (qemu_driver.c). -- Eric Blake ebl...@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/3] qemu: add capabilities flags related to scsi controller
On 08/07/2012 11:21 AM, Guannan Ren wrote: QEMU_CAPS_SCSI_LSI set the flag when lsi53c895a, bus PCI, alias lsi in the output of qemu -device ? -device lsi in qemu command line QEMU_CAPS_VIRIO_SCSI_PCI set the flag when name virtio-scsi-pci, bus PCI in the output of qemu devices query. -device virtio-scsi-pci in qemu command line --- src/qemu/qemu_capabilities.c |7 +++ src/qemu/qemu_capabilities.h |2 ++ 2 files changed, 9 insertions(+), 0 deletions(-) ACK. -- Eric Blake ebl...@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 3/3] test: add lsi and virtio-scsi qemu caps in testcases.
On 08/07/2012 11:21 AM, Guannan Ren wrote: --- tests/qemuhelptest.c | 10 +++--- tests/qemuxml2argvtest.c | 16 ++-- 2 files changed, 17 insertions(+), 9 deletions(-) @@ -755,7 +757,9 @@ mymain(void) QEMU_CAPS_NO_USER_CONFIG, QEMU_CAPS_HDA_MICRO, QEMU_CAPS_NEC_USB_XHCI, -QEMU_CAPS_NETDEV_BRIDGE); +QEMU_CAPS_NETDEV_BRIDGE, +QEMU_CAPS_SCSI_LSI, +QEMU_CAPS_VIRIO_SCSI_PCI); I ack'd patch 1 too quickly. Please s/VIRIO/VIRTIO/ throughout your series. ACK with the capability name spelling fixed. -- Eric Blake ebl...@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/3] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
On 08/07/2012 11:21 AM, Guannan Ren wrote:
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel.
When scsi model is given explicitly in XML(model 0) checking if the
underlying QEMU supports it or not first, raise an error on checking
failure.
When the model is not given(mode = 0), return LSI by default, if
the QEMU doesn't support it, raise an error.
---
src/qemu/qemu_command.c | 106 +++---
src/qemu/qemu_command.h |3 +-
src/qemu/qemu_process.c |9 +++-
3 files changed, 88 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a05..d4791c6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -469,19 +469,58 @@ static int
qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk)
}
static int
-qemuDefaultScsiControllerModel(virDomainDefPtr def) {
-if (STREQ(def-os.arch, ppc64)
-STREQ(def-os.machine, pseries)) {
-return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+qemuCheckScsiControllerModel(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
+ int *model)
+{
+if (*model 0) {
+switch (*model) {
This looks a bit odd; why not just:
switch (*model) {
...
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(This QEMU doesn't support
+ lsi scsi controller));
+return -1;
+}
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+if (!qemuCapsGet(qemuCaps, QEMU_CAPS_VIRIO_SCSI_PCI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(This QEMU doesn't support
+ virtio scsi controller));
+return -1;
+}
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
+/*TODO: need checking work here if necessary */
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unsupported controller model: %s),
+ virDomainControllerModelSCSITypeToString(*model));
+return -1;
+}
} else {
...then fold this else branch into:
case 0:
-return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+if (STREQ(def-os.arch, ppc64)
+STREQ(def-os.machine, pseries)) {
+*model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+} else if (qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+*model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to determine model for scsi
controller));
+return -1;
+}
}
+
+return 0;
}
But that's just a formatting recommendation, and not a bug in your code,
so I'm okay if you leave it as written.
+int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps)
{
int i, rc;
int model;
+virBitmapPtr localCaps = NULL;
/* Default values match QEMU. See spapr_(llan|vscsi|vty).c */
+if (!qemuCaps) {
+/* need to get information from real environment */
+if (qemuCapsExtractVersionInfo(def-emulator, def-os.arch,
+ false, NULL,
+ localCaps) 0)
+goto cleanup;
+qemuCaps = localCaps;
+}
Yet more evidence why we need to rewrite the capabilities collection
code to be caching, but that's a project for another day.
ACK.
--
Eric Blake ebl...@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 v7 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On 08/07/2012 11:07 AM, Corey Bryant wrote:
+#
+# Since: 1.2.0
We're not very consistent on '1.2' vs. '1.2.0' in since listings, but
that's probably worth a global cleanup closer to hard freeze.
I'll make a note of it. Or does Luiz usually do a cleanup?
No idea.
+##
+{ 'type': 'FdsetFdInfo', 'data': {'fd': 'int', 'removed': 'bool'} }
Is it worth providing any additional information? For example, knowing
whether the fd is O_RDONLY, O_WRONLY, or O_RDWR might be beneficial to
management apps trying to discover what fds are already present after a
reconnection, in order to decide whether to close them without having to
resort to /proc/$qemupid/fdinfo/nnn lookups. It might even be worth
marking such information optional, present only when 'removed':false.
It makes sense but I'd like to limit the new functionality at this point
so that I can get this support into QEMU 1.2. Can this be added as a
follow up patch?
I'm personally okay with the idea of adding additional output fields in
later releases, but I know that has been questioned in the past, so you
may want to get buy-in from Luiz or Anthony. I guess the other thing is
to see what libvirt would actually find useful, once I complete some
counterpart libvirt patches. If libvirt can get by without any extra
information and without needing to hack things from procfs, then it's
not worth you spending the effort coding something that will be ignored;
conversely, if a piece of info is so important that I end up hacking
procfs anyways, that says we have a hole in QMP. I'm okay waiting for now.
--
Eric Blake ebl...@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/3] qemu: add capabilities flags related to scsi controller
On 08/07/2012 03:46 PM, Eric Blake wrote: On 08/07/2012 11:21 AM, Guannan Ren wrote: QEMU_CAPS_SCSI_LSI set the flag when lsi53c895a, bus PCI, alias lsi in the output of qemu -device ? -device lsi in qemu command line QEMU_CAPS_VIRIO_SCSI_PCI set the flag when name virtio-scsi-pci, bus PCI in the output of qemu devices query. -device virtio-scsi-pci in qemu command line --- src/qemu/qemu_capabilities.c |7 +++ src/qemu/qemu_capabilities.h |2 ++ 2 files changed, 9 insertions(+), 0 deletions(-) ACK. I spoke too soon. In isolation, this patch fails 'make check'. Please take the appropriate hunks out of 3/3 and squash them into this patch so that the testsuite passes (that way, 'git bisect' is easier to run without hitting false failures). -- Eric Blake ebl...@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/3] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
On 08/07/2012 03:55 PM, Eric Blake wrote: On 08/07/2012 11:21 AM, Guannan Ren wrote: Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel. When scsi model is given explicitly in XML(model 0) checking if the underlying QEMU supports it or not first, raise an error on checking failure. When the model is not given(mode = 0), return LSI by default, if the QEMU doesn't support it, raise an error. ACK. This patch introduces even more testsuite breaks than 1/3. Please fold the rest of 3/3 into this patch, and when you push, you should have just two patches, both with testsuite improvements, and where both patches in isolation pass 'make check'. And don't forget the /VIRIO/VIRTIO/ spelling fix. -- Eric Blake ebl...@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
[libvirt] [PATCH 2/5] add qemuAgentCommand()
Add qemuAgentCommand() for general qemu agent command.
include/libvirt/libvirt-qemu.h |5 +
src/qemu/qemu_agent.c | 38 ++
src/qemu/qemu_agent.h |5 +
3 files changed, 48 insertions(+)
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index a37f897..ffc5ae5 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -44,6 +44,11 @@ virDomainPtr virDomainQemuAttach(virConnectPtr domain,
unsigned int pid_value,
unsigned int flags);
+typedef enum {
+VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -1,
+VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
+} virDomainQemuAgentCommandTimeoutFlags;
+
# ifdef __cplusplus
}
# endif
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 7699042..1cfcafc 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1401,3 +1401,41 @@ qemuAgentSuspend(qemuAgentPtr mon,
virJSONValueFree(reply);
return ret;
}
+
+int qemuAgentQemuAgentCommand(qemuAgentPtr mon,
+ const char *cmd_str,
+ char **result,
+ int timeout)
+{
+int ret = -1;
+int seconds;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+cmd = virJSONValueFromString(cmd_str);
+if (!cmd)
+return ret;
+
+if (result == NULL) {
+seconds = 0;
+} else if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) {
+seconds = 5;
+} else {
+seconds = timeout;
+}
+
+repeat:
+ret = qemuAgentCommand(mon, cmd, reply, timeout);
+
+if (ret == 0) {
+ret = qemuAgentCheckError(cmd, reply);
+*result = virJSONValueToString(reply);
+} else {
+if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) goto repeat;
+*result = NULL;
+}
+
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 2fdebb2..fc19c2f 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -77,4 +77,9 @@ int qemuAgentFSThaw(qemuAgentPtr mon);
int qemuAgentSuspend(qemuAgentPtr mon,
unsigned int target);
+
+int qemuAgentQemuAgentCommand(qemuAgentPtr mon,
+ const char *cmd,
+ char **result,
+ int timeout);
#endif /* __QEMU_AGENT_H__ */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] add virDomainQemuAgentCommand() to qemu driver and remote driver
Add virDomainQemuAgentCommand() to qemu driver and remote driver
to support qemuAgentCommand().
daemon/remote.c| 35
include/libvirt/libvirt-qemu.h |3 +
src/driver.h |6 +++
src/libvirt-qemu.c | 58 +
src/libvirt_qemu.syms |5 ++
src/qemu/qemu_driver.c | 71 +
src/qemu_protocol-structs | 10 +
src/remote/qemu_protocol.x | 14 +++-
src/remote/remote_driver.c | 41 +++
9 files changed, 242 insertions(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 832307e..4fd5c9b 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -3948,6 +3948,41 @@ cleanup:
return rv;
}
+static int
+qemuDispatchAgentCommand(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client ATTRIBUTE_UNUSED,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ qemu_agent_command_args *args,
+ qemu_agent_command_ret *ret)
+{
+virDomainPtr dom = NULL;
+int rv = -1;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv-conn) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv-conn, args-dom)))
+goto cleanup;
+
+if (virDomainQemuAgentCommand(dom, args-cmd, ret-result,
+ args-timeout, args-flags) 0) {
+goto cleanup;
+}
+
+rv = 0;
+cleanup:
+if (rv 0)
+virNetMessageSaveError(rerr);
+if (dom)
+virDomainFree(dom);
+return rv;
+}
+
/*- Helpers. -*/
/* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index ffc5ae5..aed1259 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -49,6 +49,9 @@ typedef enum {
VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
} virDomainQemuAgentCommandTimeoutFlags;
+int virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd,
+ char **result, int timeout, unsigned int flags);
+
# ifdef __cplusplus
}
# endif
diff --git a/src/driver.h b/src/driver.h
index aab9766..c368273 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -689,6 +689,11 @@ typedef int
(*virDrvDomainQemuMonitorCommand)(virDomainPtr domain, const char *cmd,
char **result, unsigned int flags);
+typedef int
+(*virDrvDomainQemuAgentCommand)(virDomainPtr domain, const char *cmd,
+char **result, int timeout,
+unsigned int flags);
+
/* Choice of unsigned int rather than pid_t is intentional. */
typedef virDomainPtr
(*virDrvDomainQemuAttach)(virConnectPtr conn,
@@ -1048,6 +1053,7 @@ struct _virDriver {
virDrvDomainGetDiskErrors domainGetDiskErrors;
virDrvDomainSetMetadata domainSetMetadata;
virDrvDomainGetMetadata domainGetMetadata;
+virDrvDomainQemuAgentCommandqemuDomainQemuAgentCommand;
};
typedef int
diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c
index 78480bb..0e12425 100644
--- a/src/libvirt-qemu.c
+++ b/src/libvirt-qemu.c
@@ -185,3 +185,61 @@ error:
virDispatchError(conn);
return NULL;
}
+
+/**
+ * virDomainQemuAgentCommand:
+ * @domain: a domain object
+ * @cmd: the guest agent command string
+ * @result: a string returned by @cmd
+ * @timeout: timeout seconds
+ * @flags: execution flags
+ *
+ * Execution Guest Agent Command
+ *
+ * Issue @cmd to the guest agent running in @domain.
+ * If @result is NULL, then don't wait for a result (and @timeout
+ * must be 0). Otherwise, wait for @timeout seconds for a
+ * successful result to be populated into *@result, with
+ * VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK (-1) meaning to block
+ * forever waiting for a result.
+ *
+ * Returns 0 if succeeded, -1 in failing.
+ */
+int
+virDomainQemuAgentCommand(virDomainPtr domain,
+ const char *cmd,
+ char **result,
+ int timeout,
+ unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DEBUG(domain=%p, cmd=%s, result=%p, timeout=%d, flags=%x,
+ domain, cmd, result, timeout, flags);
+
+if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
+virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+
+conn = domain-conn;
+
+virCheckNonNullArgGoto(result, error);
+
+if (conn-driver-qemuDomainQemuAgentCommand) {
+int ret;
+ret =
[libvirt] [PATCH 5/5] Add qemu-agent-command command to virsh
Add qemu-agent-command command to virsh to support
virDomainQemuAgentCommand().
virsh-host.c | 70 +++
1 file changed, 70 insertions(+)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index d9d09b4..b9180f3 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -633,6 +633,74 @@ cleanup:
}
/*
+ * qemu-agent-command command
+ */
+static const vshCmdInfo info_qemu_agent_command[] = {
+{help, N_(Qemu Guest Agent Command)},
+{desc, N_(Qemu Guest Agent Command)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_qemu_agent_command[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{timeout, VSH_OT_INT, VSH_OFLAG_REQ_OPT, N_(timeout)},
+{cmd, VSH_OT_ARGV, VSH_OFLAG_REQ, N_(command)},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+bool ret = false;
+char *guest_agent_cmd = NULL;
+char *result = NULL;
+int timeout = 0;
+unsigned int flags = 0;
+const vshCmdOpt *opt = NULL;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+bool pad = false;
+
+if (!vshConnectionUsability(ctl, ctl-conn))
+goto cleanup;
+
+dom = vshCommandOptDomain(ctl, cmd, NULL);
+if (dom == NULL)
+goto cleanup;
+
+while ((opt = vshCommandOptArgv(cmd, opt))) {
+if (pad)
+virBufferAddChar(buf, ' ');
+pad = true;
+virBufferAdd(buf, opt-data, -1);
+}
+if (virBufferError(buf)) {
+vshPrint(ctl, %s, _(Failed to collect command));
+goto cleanup;
+}
+guest_agent_cmd = virBufferContentAndReset(buf);
+
+if (vshCommandOptInt(cmd, timeout, timeout) 0) {
+timeout = 0;
+}
+
+if (virDomainQemuAgentCommand(dom, guest_agent_cmd, result,
+ timeout, flags) 0)
+goto cleanup;
+
+printf(%s\n, result);
+
+ret = true;
+
+cleanup:
+VIR_FREE(result);
+VIR_FREE(guest_agent_cmd);
+if (dom)
+virDomainFree(dom);
+
+return ret;
+}
+/*
* sysinfo command
*/
static const vshCmdInfo info_sysinfo[] = {
@@ -832,6 +900,8 @@ static const vshCmdDef hostAndHypervisorCmds[] = {
{qemu-attach, cmdQemuAttach, opts_qemu_attach, info_qemu_attach, 0},
{qemu-monitor-command, cmdQemuMonitorCommand, opts_qemu_monitor_command,
info_qemu_monitor_command, 0},
+{qemu-agent-command, cmdQemuAgentCommand, opts_qemu_agent_command,
+ info_qemu_agent_command, 0},
{sysinfo, cmdSysinfo, NULL, info_sysinfo, 0},
{uri, cmdURI, NULL, info_uri, 0},
{version, cmdVersion, opts_version, info_version, 0},
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] add timeout seconds to qemuAgentSend()
Add @seconds valuable to qemuAgentSend().
It points timeout seconds on @timeout true.
If @seconds is 0 on @timeout true, use default timeout value
(QEMU_AGENT_WAIT_TIME).
If @timeout is false, @seconds is meanless.
qemu_agent.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 15af758..7699042 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -837,6 +837,7 @@ void qemuAgentClose(qemuAgentPtr mon)
* @mon: Monitor
* @msg: Message
* @timeout: use timeout?
+ * @seconds: timeout seconds. if 0 on @timeout true, use default value
*
* Send @msg to agent @mon.
* Wait max QEMU_AGENT_WAIT_TIME for agent
@@ -848,7 +849,8 @@ void qemuAgentClose(qemuAgentPtr mon)
*/
static int qemuAgentSend(qemuAgentPtr mon,
qemuAgentMessagePtr msg,
- bool timeout)
+ bool timeout,
+ int seconds)
{
int ret = -1;
unsigned long long now, then = 0;
@@ -864,7 +866,7 @@ static int qemuAgentSend(qemuAgentPtr mon,
if (timeout) {
if (virTimeMillisNow(now) 0)
return -1;
-then = now + QEMU_AGENT_WAIT_TIME;
+then = now + (seconds == 0 ? QEMU_AGENT_WAIT_TIME : seconds * 1000);
}
mon-msg = msg;
@@ -937,7 +939,7 @@ qemuAgentGuestSync(qemuAgentPtr mon)
VIR_DEBUG(Sending guest-sync command with ID: %llu, id);
-send_ret = qemuAgentSend(mon, sync_msg, true);
+send_ret = qemuAgentSend(mon, sync_msg, true, 0);
VIR_DEBUG(qemuAgentSend returned: %d, send_ret);
@@ -977,7 +979,8 @@ cleanup:
static int
qemuAgentCommand(qemuAgentPtr mon,
virJSONValuePtr cmd,
- virJSONValuePtr *reply)
+ virJSONValuePtr *reply,
+ int timeout)
{
int ret = -1;
qemuAgentMessage msg;
@@ -1005,7 +1008,7 @@ qemuAgentCommand(qemuAgentPtr mon,
VIR_DEBUG(Send command '%s' for write, cmdstr);
-ret = qemuAgentSend(mon, msg, false);
+ret = qemuAgentSend(mon, msg, (timeout 0 ? true : false), timeout);
VIR_DEBUG(Receive command reply ret=%d rxObject=%p,
ret, msg.rxObject);
@@ -1283,7 +1286,7 @@ int qemuAgentShutdown(qemuAgentPtr mon,
return -1;
mon-await_event = QEMU_AGENT_EVENT_SHUTDOWN;
-ret = qemuAgentCommand(mon, cmd, reply);
+ret = qemuAgentCommand(mon, cmd, reply, 0);
if (reply ret == 0)
ret = qemuAgentCheckError(cmd, reply);
@@ -1315,7 +1318,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon)
if (!cmd)
return -1;
-if (qemuAgentCommand(mon, cmd, reply) 0 ||
+if (qemuAgentCommand(mon, cmd, reply, 0) 0 ||
qemuAgentCheckError(cmd, reply) 0)
goto cleanup;
@@ -1352,7 +1355,7 @@ int qemuAgentFSThaw(qemuAgentPtr mon)
if (!cmd)
return -1;
-if (qemuAgentCommand(mon, cmd, reply) 0 ||
+if (qemuAgentCommand(mon, cmd, reply, 0) 0 ||
qemuAgentCheckError(cmd, reply) 0)
goto cleanup;
@@ -1389,7 +1392,7 @@ qemuAgentSuspend(qemuAgentPtr mon,
return -1;
mon-await_event = QEMU_AGENT_EVENT_SUSPEND;
-ret = qemuAgentCommand(mon, cmd, reply);
+ret = qemuAgentCommand(mon, cmd, reply, 0);
if (reply ret == 0)
ret = qemuAgentCheckError(cmd, reply);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] support guest agent general command
Hi, All.
I rewrote the patches as Eric suggested.
virsh # help qemu-agent-command
NAME
qemu-agent-command - Qemu Guest Agent Command
SYNOPSIS
qemu-agent-command domain [--timeout number] {[--cmd] string}...
DESCRIPTION
Qemu Guest Agent Command
OPTIONS
[--domain] string domain name, id or uuid
--timeout number timeout
[--cmd] string command
virsh # qemu-agent-command RHEL58_64 '{execute:guest-info}'
{return:{version:1.1.50,supported_commands:[{enabled:true,name:guest-network-get-interfaces},{enabled:true,name:guest-suspend-hybrid},{enabled:true,name:guest-suspend-ram},{enabled:true,name:guest-suspend-disk},{enabled:true,name:guest-fsfreeze-thaw},{enabled:true,name:guest-fsfreeze-freeze},{enabled:true,name:guest-fsfreeze-status},{enabled:true,name:guest-file-flush},{enabled:true,name:guest-file-seek},{enabled:true,name:guest-file-write},{enabled:true,name:guest-file-read},{enabled:true,name:guest-file-close},{enabled:true,name:guest-file-open},{enabled:true,name:guest-shutdown},{enabled:true,name:guest-info},{enabled:true,name:guest-ping},{enabled:true,name:guest-sync},{enabled:true,name:guest-sync-delimited}]}}
virsh # qemu-agent-command RHEL58_64 '{execute:guest-sync,
arguments:{id:123}}'
{return:123}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] Add virDomainQemuAgentCommand() support function to python module
Add virDomainQemuAgentCommand() support function to python module.
generator.py |1 +
libvirt-qemu-override-api.xml |8
libvirt-qemu-override.c | 31 +++
3 files changed, 40 insertions(+)
diff --git a/python/generator.py b/python/generator.py
index 6559ece..3cec12b 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -431,6 +431,7 @@ skip_impl = (
qemu_skip_impl = (
'virDomainQemuMonitorCommand',
+'virDomainQemuAgentCommand',
)
diff --git a/python/libvirt-qemu-override-api.xml
b/python/libvirt-qemu-override-api.xml
index d69acea..37759d0 100644
--- a/python/libvirt-qemu-override-api.xml
+++ b/python/libvirt-qemu-override-api.xml
@@ -8,5 +8,13 @@
arg name='cmd' type='const char *' info='the command which will be
passed to QEMU monitor'/
arg name='flags' type='unsigned int' info='an ORapos;ed set of
virDomainQemuMonitorCommandFlags'/
/function
+ function name='virDomainQemuAgentCommand' file='python-qemu'
+infoSend a Guest Agent command to domain/info
+return type='str *' info='the command output or None in case of
error'/
+arg name='domain' type='virDomainPtr' info='pointer to the domain'/
+arg name='cmd' type='const char *' info='guest agent command on
domain'/
+arg name='timeout' type='int' info='timeout seconds'/
+arg name='flags' type='unsigned int' info='execution flags'/
+ /function
/symbols
/api
diff --git a/python/libvirt-qemu-override.c b/python/libvirt-qemu-override.c
index e532416..195aadc 100644
--- a/python/libvirt-qemu-override.c
+++ b/python/libvirt-qemu-override.c
@@ -82,6 +82,36 @@ libvirt_qemu_virDomainQemuMonitorCommand(PyObject *self
ATTRIBUTE_UNUSED,
return py_retval;
}
+static PyObject *
+libvirt_qemu_virDomainQemuAgentCommand(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args)
+{
+PyObject *py_retval;
+char *result = NULL;
+virDomainPtr domain;
+PyObject *pyobj_domain;
+int timeout;
+unsigned int flags;
+char *cmd;
+int c_retval;
+
+if (!PyArg_ParseTuple(args, (char *)Ozii:virDomainQemuAgentCommand,
+ pyobj_domain, cmd, timeout, flags))
+return NULL;
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+if (domain == NULL)
+return VIR_PY_NONE;
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_retval = virDomainQemuAgentCommand(domain, cmd, result, timeout, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (c_retval 0)
+return VIR_PY_NONE;
+
+py_retval = PyString_FromString(result);
+return py_retval;
+}
/
* *
* The registration stuff *
@@ -90,6 +120,7 @@ libvirt_qemu_virDomainQemuMonitorCommand(PyObject *self
ATTRIBUTE_UNUSED,
static PyMethodDef libvirtQemuMethods[] = {
#include libvirt-qemu-export.c
{(char *) virDomainQemuMonitorCommand,
libvirt_qemu_virDomainQemuMonitorCommand, METH_VARARGS, NULL},
+{(char *) virDomainQemuAgentCommand,
libvirt_qemu_virDomainQemuAgentCommand, METH_VARARGS, NULL},
{NULL, NULL, 0, NULL}
};
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/17] Introduce the function virCgroupMoveTask
On Mon, Aug 06, 2012 at 03:09:51PM -0600, Eric Blake wrote:
On 08/03/2012 12:36 AM, Hu Tao wrote:
From: Wen Congyang we...@cn.fujitsu.com
Introduce a new API to move tasks of one controller from a cgroup to
another cgroup
Signed-off-by: Wen Congyang we...@cn.fujitsu.com
Signed-off-by: Tang Chen tangc...@cn.fujitsu.com
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
+static int virCgroupAddTaskStrController(virCgroupPtr group,
+const char *pidstr,
+int controller)
+{
+char *str = NULL, *cur = NULL, *next = NULL;
+unsigned long long p = 0;
+int rc = 0;
+
+if (virAsprintf(str, %s, pidstr) 0)
+return -1;
+
+cur = str;
+while ((next = strchr(cur, '\n')) != NULL) {
+*next = '\0';
+rc = virStrToLong_ull(cur, NULL, 10, p);
+if (rc != 0)
+goto cleanup;
+cur = next + 1;
+
+rc = virCgroupAddTaskController(group, p, controller);
+if (rc != 0)
+goto cleanup;
+}
+if (cur != '\0') {
+rc = virStrToLong_ull(cur, NULL, 10, p);
+if (rc != 0)
+goto cleanup;
+rc = virCgroupAddTaskController(group, p, controller);
+if (rc != 0)
+goto cleanup;
+}
Can this last if statement be folded into the while loop for less code
duplication? If the series passes review now, then I'm probably not
worried enough to change it and would probably push as is; but if we
need a v2, then it's worth improving.
OK, I'll improve it.
--
Thanks,
Hu Tao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/17] Enable cpuset cgroup and synchronous vcpupin info to cgroup.
On Mon, Aug 06, 2012 at 03:41:29PM -0600, Eric Blake wrote:
On 08/03/2012 12:36 AM, Hu Tao wrote:
From: Tang Chen tangc...@cn.fujitsu.com
vcpu threads pin are implemented using sched_setaffinity(), but
not controlled by cgroup. This patch does the following things:
1) enable cpuset cgroup
2) reflect all the vcpu threads pin info to cgroup
Signed-off-by: Tang Chen tangc...@cn.fujitsu.com
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
@@ -3667,9 +3669,38 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
if (flags VIR_DOMAIN_AFFECT_LIVE) {
if (priv-vcpupids != NULL) {
+/* Add config to vm-def first, because qemuSetupCgroupVcpuPin
needs it. */
+if (virDomainVcpuPinAdd(vm-def, cpumap, maplen, vcpu) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(failed to update or add vcpupin xml of
+ a running domain));
+goto cleanup;
+}
If this succeeds,
+
+/* Configure the corresponding cpuset cgroup before set
affinity. */
+if (qemuCgroupControllerActive(driver,
VIR_CGROUP_CONTROLLER_CPUSET)) {
+if (virCgroupForDomain(driver-cgroup, vm-def-name,
cgroup_dom, 0) == 0
+virCgroupForVcpu(cgroup_dom, vcpu, cgroup_vcpu, 0) ==
0
+qemuSetupCgroupVcpuPin(cgroup_vcpu, vm-def, vcpu)
0) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _(failed to set cpuset.cpus in cgroup
+ for vcpu %d), vcpu);
+goto cleanup;
but this fails, then where do we roll vm-def back to its pre-attempt state?
+}
+} else {
+/* Here, we should go on because even if cgroup is not
active,
+ * we can still use virProcessInfoSetAffinity.
+ */
+VIR_WARN(cpuset cgroup is not active);
+}
+
if (virProcessInfoSetAffinity(priv-vcpupids[vcpu],
- cpumap, maplen, maxcpu) 0)
+ cpumap, maplen, maxcpu) 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _(failed to set cpu affinity for vcpu %d),
+ vcpu);
goto cleanup;
Same situation - if set_affinity failed, where do we roll back to the
original vm-def state (also, should we roll back to the original cpuset
state)?
If we don't roll back, then a failure can leave the domain in an unknown
state for cpu pinning. Or are we just declaring that if these functions
fail, the domain is in an unknown set of cpu pinning?
Oh yes, this is indeed a problem here, I'll fix it.
Do we need both cpuset and set_affinity? Or is cpuset a superset of
affinity (that is, if I alter cpuset, can I completely skip out on
calling set_affinity and still get the same results)? If cpuset gives
stronger pinning than affinity, then maybe we don't need to try both
methods, which gives us a bit better mechanism for rollbacks; the only
The man page[1] says cpusets are integrated with sched_setaffinity(),
so I think it is better to choose one of them. If cpuset is avaiable, we
use cpuset. Otherwise we use sched_setaffinity().
remaining thing is making sure you avoid altering vm-def except on
success (perhaps by using a temporary structure rather than vm-def at
the time you attempt the pinning).
Yes.
[1] man 7 cpuset
--
Thanks,
Hu Tao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/17] Support hypervisorpin xml parse.
On Mon, Aug 06, 2012 at 04:51:49PM -0600, Eric Blake wrote:
On 08/03/2012 12:36 AM, Hu Tao wrote:
From: Tang Chen tangc...@cn.fujitsu.com
This patch adds a new xml element hypervisorpin cpuset='1',
I would mention that this is a sibling to the existing vcpupin element
under the cputune.
and also the parser functions, docs, and tests.
hypervisorpin means pinning hypervisor threads, and cpuset = '1'
means pinning all hypervisor threads to cpu 1.
Signed-off-by: Tang Chen tangc...@cn.fujitsu.com
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
docs/schemas/domaincommon.rng |7 ++
Missing documentation. I can't apply this as-is unless we also update
the elementsCPUTuning section of docs/formatdomain.html.in. That won't
stop me from reviewing the rest of the patch, though.
src/conf/domain_conf.c | 97
++-
src/conf/domain_conf.h |1 +
tests/qemuxml2argvdata/qemuxml2argv-cputune.xml |1 +
4 files changed, 103 insertions(+), 3 deletions(-)
+++ b/src/conf/domain_conf.c
@@ -7828,6 +7828,51 @@ error:
goto cleanup;
}
+/* Parse the XML definition for hypervisorpin */
+static virDomainVcpuPinDefPtr
+virDomainHypervisorPinDefParseXML(const xmlNodePtr node)
+{
...
+}
This is a lot of code duplication. It might be worth refactoring things
to write a helper function that parses '@cpuset', and which can be
shared by both the existing virDomainVcpuPinDefParseXML and your new
function.
@@ -9280,7 +9353,7 @@ no_memory:
virReportOOMError();
/* fallthrough */
- error:
+error:
VIR_FREE(tmp);
VIR_FREE(nodes);
virBitmapFree(bootMap);
On its own, this whitespace cleanup has no bearing on the patch; it's
generally wise to limit cleanups to portions already affected by the patch.
But in general, the patch looked reasonable.
Thank you, I'll improve this patch in v2.
--
Thanks,
Hu Tao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] storage: support all file permissions
On 08/07/2012 10:40 AM, Dave Allan wrote: On Tue, Aug 07, 2012 at 10:27:15AM +0200, Ján Tomko wrote: On 08/07/12 07:17, Laine Stump wrote: On 07/26/2012 04:52 AM, Ján Tomko wrote: sticky, setuid and setgid are no longer ignored. I'm always automatically wary of any code that allows setting the suid bit, in case it may allow some new security hole. I can't think of anything specific that could be allowed by setting the suid bit of a directory containing a disk image, but then again I haven't thought about it very hard :-) Can anyone convince me one way or the other? SUID on directories is ignored on most systems, but you could be able to create files belonging a group you're not a member of. But this patch enables it on files too, allowing everyone with access to privileged libvirtd to create SUID files. I don't know if it's possible to exploit this, but I don't like it, so NACK NACK NACK. It might help to learn why you want to be able to set these bits. libvirt is generally very specific about explicitly setting the uid of disk images properly as required at runtime... My issue was that vol-dumpxml reported wrong file permissions, as described in https://bugzilla.redhat.com/show_bug.cgi?id=839463 Writing the sticky bit seems harmless to me. Yeah, I think I agree with that. (Heh - I'd never read the history of the sticky bit before (i.e. where it got the name sticky) - retain the text segment http://en.wikipedia.org/wiki/Text_segment of the program in swap space http://en.wikipedia.org/wiki/Virtual_memory after the process http://en.wikipedia.org/wiki/Process_%28computing%29 exited. Remember when loading the text segment of a program took long enough for that to matter?) Would it be safe to just read SGID and SUID without ever setting them? Or is it not worth the risk? IMO we should preserve the existing behavior of not modifying the bits, but we should report them correctly, although I don't feel all that strongly about it. That sounds reasonable to me, as long as the reported difference only shows up during a dumpxml of the live XML (and not during a pool-dumpxml --inactive). I'm wondering if we should generate an error when someone tries to specify those bits in a pool-define (and vol-define), or just ignore them as we currently do. (no opinion, just wondering :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/17] Supports for hypervisor-pin and hypervisor-bandwidth
On Tue, Aug 07, 2012 at 06:35:32PM +0100, Daniel P. Berrange wrote: On Tue, Aug 07, 2012 at 10:13:19AM -0600, Eric Blake wrote: On 08/07/2012 09:47 AM, Daniel P. Berrange wrote: On Fri, Aug 03, 2012 at 02:36:02PM +0800, Hu Tao wrote: This series is a merge of 1) Support hypervisor-threads-pin in vcpupin (https://www.redhat.com/archives/libvir-list/2012-July/msg01361.html) 2) support to set cpu bandwidth for hypervisor threads (https://www.redhat.com/archives/libvir-list/2012-June/msg01161.html) to make life easier because of the two share some patches. This series is really focusing on pinning threads associated with the emulator element, rather than the hypervisor. The hypervisor is a separate entity that is shared. So I'm thinking that this entire patch series could replace 'hypervisor' with 'emulator' everywhere. Any one has agree or disagree ? I definitely agree - when I hear 'hypervisor', I think 'qemu:///system', which is the technology used to run multiple guests, but when I hear 'emulator', I think of a subset of a domain, namely the specific qemu pid_t running a given guest. Also, we're not pinning all of the hypervisor's threads, but just the threads that are associated with emulation but not a specific vcpu. That is, marking up your comment in 1/17: cgroup mount point +--libvirt= setting up a namespace (*) +--qemu= hypervisor level +--domain name = domain level +--vcpu0 = vcpu level ... +--vcpuN +--hypervisor = emulator so a domain really is made up of an 'emulator' and 'vcpu' threads, and a 'hypervisor' contains domains, rather than making up a portion of a domain. Also note that LXC has an emulator /usr/libexec/libvirt_lxc for which all these new APIs apply, but the term 'hypervisor' is meaningless for container based virt. Thank you for your advice on the naming. I'll change hypervisor into emulator all through the series. -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PF interface brought down if guest using a VF in hostdev mode under 802.1Qbh
On 08/07/2012 01:53 PM, Nishank Trivedi (nistrive) wrote: Hi, In 802.1Qbh case where exists a SR-IOV capable network device, if any of the virtual functions of this device is used in hostdev mode for the guest, port-profile disassociate will also cause physical function interface to go down. This appears to be a bug, but wanted to find if this was done intentionally for some reasons. To be more specific, if a network device supports SRIOV and its VF is being used in pci passthrough mode, when the guest is shutdown or destroyed following happens - qemuProcessStop \_ qemuDomainReAttachHostdevDevices(hostdevs) \_ qemuDomainHostdevNetConfigRestore(hostdev) \_ virNetDevPortProfileDisassociate(linkdev) \_ virNetDevSetOnline(linkdev, false) In above, qemuDomainHostdevNetConfigRestore() finds out the PF for provided hostdev (which is VF) and passes it to virNetDevPortProfileDisassociate() as linkdev. Later, linkdev gets passed to virNetDevSetOnline() where the interface is brought down by clearing IFF_UP flag. However, in macvtap emulation mode, virNetDevMacVLanDeleteWithVPortProfile() passes VF as linkdev (and -1 as vf) to virNetDevPortProfileDisassociate(), hence, not affecting PF at all. Bringing down a PF, when only VF is being brought down is not expected behavior (unless, I'm missing something here). A way to get around it would be to check if there exists vf (=0) in virNetDevPortProfileDisassociate(), and if so, it should only pass the interface name of this VF rather than PF itself to virNetDevSetOnline(). If it sounds reasonable, I'll send out a fix. Yes, I believe you are correct that virNetDevSetOnline is being called with the wrong device name (I could be wrong though - is there maybe some odd rule that the PF must be UP before the VFs will be up? In case case, we certainly shouldn't be setting the PF to IFF_DOWN). But in the case of PCI passthrough, does it even need to be called at all? Cc'ing Roopa in case she missed this in all the other traffic. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
