Re: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm
Ph: +972-4-8296227
Cell: 054-6976227
Fx: +972-4- 8296113
Do not look back in anger, or forward in fear, but around in awareness.
--James Thurber
Eric Blake ebl...@redhat.com wrote on 05/05/2010 01:04:23:
From: Eric Blake ebl...@redhat.com
To: Cole Robinson crobi...@redhat.com
Cc: Kenneth Nagin/Haifa/i...@ibmil, list libvirt libvir-list@redhat.com
Date: 05/05/2010 01:04
Subject: Re: [libvirt] (Resend with updates) Live Migration with
non-shared storage for kvm
On 05/03/2010 07:52 AM, Cole Robinson wrote:
On 05/03/2010 04:13 AM, Kenneth Nagin wrote:
...
This the patch file:
(See attached file: libvirt_migration_ns_100503.patch)
ACK, patch looks fine now.
Kenneth,
I added a commit message, broke a few lines to fit in 80 columns, added
a virCheckFlags to doNativeMigrate (which required a type change), and
removed an extra space in the generated line:
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 5823e10..47ae52c 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -4958,7 +4958,9 @@ static int qemudDomainSaveFlag(virDomainPtr dom,
const char *path,
if (header.compressed == QEMUD_SAVE_FORMAT_RAW) {
const char *args[] = { cat, NULL };
qemuDomainObjEnterMonitorWithDriver(driver, vm);
-rc = qemuMonitorMigrateToFile(priv-mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset);
+rc = qemuMonitorMigrateToFile(priv-mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, offset);
qemuDomainObjExitMonitorWithDriver(driver, vm);
} else {
const char *prog =
qemudSaveCompressionTypeToString(header.compressed);
@@ -4968,7 +4970,9 @@ static int qemudDomainSaveFlag(virDomainPtr dom,
const char *path,
NULL
};
qemuDomainObjEnterMonitorWithDriver(driver, vm);
-rc = qemuMonitorMigrateToFile(priv-mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset);
+rc = qemuMonitorMigrateToFile(priv-mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, offset);
qemuDomainObjExitMonitorWithDriver(driver, vm);
}
@@ -5286,7 +5290,9 @@ static int qemudDomainCoreDump(virDomainPtr dom,
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
-ret = qemuMonitorMigrateToFile(priv-mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, 0);
+ret = qemuMonitorMigrateToFile(priv-mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, 0);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret 0)
goto endjob;
@@ -9884,7 +9890,7 @@ cleanup:
static int doNativeMigrate(struct qemud_driver *driver,
virDomainObjPtr vm,
const char *uri,
- unsigned long flags ,
+ unsigned int flags,
const char *dname ATTRIBUTE_UNUSED,
unsigned long resource)
{
@@ -9893,6 +9899,9 @@ static int doNativeMigrate(struct qemud_driver
*driver,
qemuDomainObjPrivatePtr priv = vm-privateData;
unsigned int background_flags = 0;
+virCheckFlags(VIR_MIGRATE_NON_SHARED_DISK |
VIR_MIGRATE_NON_SHARED_INC,
+ -1);
+
/* Issue the migrate command. */
if (STRPREFIX(uri, tcp:) !STRPREFIX(uri, tcp://)) {
/* HACK: source host generates bogus URIs, so fix them up */
@@ -9925,7 +9934,8 @@ static int doNativeMigrate(struct qemud_driver
*driver,
if (flags VIR_MIGRATE_NON_SHARED_INC)
background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
-if (qemuMonitorMigrateToHost(priv-mon, background_flags,
uribits-server, uribits-port) 0) {
+if (qemuMonitorMigrateToHost(priv-mon, background_flags,
uribits-server,
+ uribits-port) 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
goto cleanup;
}
@@ -10011,6 +10021,7 @@ static int doTunnelMigrate(virDomainPtr dom,
int status;
unsigned long long transferred, remaining, total;
unsigned int background_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
+
/*
* The order of operations is important here to avoid touching
* the source VM until we are very sure we can successfully
@@ -10100,7 +10111,8 @@ static int doTunnelMigrate(virDomainPtr dom,
if (flags VIR_MIGRATE_NON_SHARED_INC)
background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
if (qemuCmdFlags QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX){
-internalret = qemuMonitorMigrateToUnix(priv-mon,
background_flags, unixfile);
+internalret = qemuMonitorMigrateToUnix(priv-mon,
background_flags,
+ unixfile);
}
Re: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm
On Mon, May 03, 2010 at 09:52:10AM -0400, Cole Robinson wrote: On 05/03/2010 04:13 AM, Kenneth Nagin wrote: ... This the patch file: (See attached file: libvirt_migration_ns_100503.patch) ACK, patch looks fine now. No objections to the patch that was committed, but FYI there is an improvement that can be done to it. If you run 'virsh domjobinfo $GUEST' while migration is running it tells you the progress of migration. It gives overall progress, and memory progress. The APi was designed to allow us to report storage progress too. To wire this up look at the method qemuDomainWaitForMigrationComplete where it updates the priv-jobInfo variables. You'll also need to add extra params to the qemuMonitorGetMigrationStatus() method to return the disk processed/remaining/total stats virsh should already be able to report this data once the QEMU driver is wired up Regards, Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Alternative to XML Creation and Parsing?
On Mon, May 03, 2010 at 10:23:11PM +0530, Ganesh Pagade wrote: Hello, We plan to develop a fancy GUI which would help creating and managing VMs/Domains for RHEL 5.4 KVM. However looking at the schemas provided in docs/schemas/ (Ex: domain.rng) generating the XML file from the inputs taken from the GUI and pass it on to virDomainDefineXML() turns out to be a huge tasks in itself. I was wondering if there is an better way for me to provide my configurations to the library and also to read data from it (instead of creating, parsing XMLs)? If an alternate approach exist, are there any limitations with it? The only supported APIs at the libvirt level work in terms of the XML since this is the most flexible extensible format. Applications will either operate on XML directly (virt-manager), or have their own data model for representing guest configs (oVirt, RHEV) and have a XML serializer/deserializer in their apps. Also the schema given in docs/schemas/ would be generic for all hyper visor. How can I identify the tags applicable only for a particular hyper visor? We don't currently document what tags are applicable per hypervisor, because it is not in fact that easy to determine. It also varies tremendously based on the version of the hypervisor, and even the options that the hypervisor was compiled with. For example, QEMU 0.9 added SCSI support, but QEMU 0.12 in RHEL turns off the SCSI support. Instead libvirt aims to raise an error at the time you start the guest if you have a config that won't work. I found: virConnectDomainXMLFromNative(virConnectPtr conn, const char * nativeFormat, const char * nativeConfig, unsigned int flags) in API reference, which could solve my first problem but couldn't figure out what nativeFormat would be. The 'nativeFormat' refers to any hypervisor specific configuration style. In the case of QEMU this covers the command line argument syntax. There is an example at http://libvirt.org/drvqemu.html#imex We don't really recommend applications use this API in normal operation. It is really a tool to help migration from a non-libvirt solution into libvirt. The XML it generates will typically need hand-editing afterwards to fix up bits we couldn't convert automatically - in particular networking config. Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rpmbuild: add ebtables ip(6)tables dependency for rpm
Add ebtables,iptables iptables-ipv6 dependency to rpm.
Signed-off-by: Stefan Berger stef...@us.ibm.com
---
libvirt.spec.in |7 +++
1 file changed, 7 insertions(+)
Index: libvirt-acl/libvirt.spec.in
===
--- libvirt-acl.orig/libvirt.spec.in
+++ libvirt-acl/libvirt.spec.in
@@ -61,6 +61,7 @@
%define with_udev 0%{!?_without_udev:0}
%define with_hal 0%{!?_without_hal:0}
%define with_yajl 0%{!?_without_yajl:0}
+%define with_nwfilter 0%{!?_without_nwfilter:0}
%define with_libpcap 0%{!?_without_libpcap:0}
# Non-server/HV driver defaults which are always enabled
@@ -150,6 +151,7 @@
# Enable libpcap library
%if %{with_qemu}
+%define with_nwfilter 0%{!?_without_nwfilter:%{server_drivers}}
%define with_libpcap 0%{!?_without_libpcap:%{server_drivers}}
%endif
@@ -195,6 +197,11 @@ Requires: bridge-utils
Requires: dnsmasq
Requires: iptables
%endif
+%if %{with_nwfilter}
+Requires: ebtables
+Requires: iptables
+Requires: iptables-ipv6
+%endif
# needed for device enumeration
%if %{with_hal}
Requires: hal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvirt with qemu-kvm, not recognizing NIC model
Is this the right list for this question, or should I be elsewhere? I am trying to specify a network card model type of pcnet (to emulate vmware esxi's network card). No matter what I put for model type in my xml config file, it comes up as an Intel e1000. I ran qemu-kvm -net nic,model=? /dev/null and I got back a list of supported models including pcnet,e1000,virtio, and others as expected. No matter which I put in my xml file for /etc/libvirt/qemu/Symantec-bg.xml (a Symantec Brightmail Gateway virtual machine), I just get back that its an Intel card on boot (which doesn't work with Symantec BG). I am running Fedora 12, kernel is 2.6.31.6-166.fc12.x86_64. The version of libvirt is 0.7.1. I am running it on an Intel Core 2 Quad CPU Q8400 @ 2.66 GHz. I have the same problem under Fedora 13 Beta. Thoughts? Jonathan Hoover -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] Implement SCSI disk unplugging
With the introduction of the generic qemu device model, unplugging
SCSI disks works like a charm, so support it in libvirt.
* src/qemu/qemu_driver.c: Add qemudDomainDetachSCSIDiskDevice() to do the
unplugging, extend qemudDomainDetachDeviceAdd().
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
---
src/qemu/qemu_driver.c | 60 +--
1 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 63ca57c..7321960 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7940,6 +7940,51 @@ static int qemudDomainDetachPciDiskDevice(struct
qemud_driver *driver,
qemudShrinkDisks(vm-def, i);
+if (driver-securityDriver
+driver-securityDriver-domainRestoreSecurityImageLabel
+driver-securityDriver-domainRestoreSecurityImageLabel(vm,
dev-data.disk) 0)
+VIR_WARN(Unable to restore security label on %s,
dev-data.disk-src);
+
+ret = 0;
+
+cleanup:
+return ret;
+}
+
+static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev,
+ unsigned long long qemuCmdFlags)
+{
+int i, ret = -1;
+virDomainDiskDefPtr detach = NULL;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+
+i = qemudFindDisk(vm-def, dev-data.disk-dst);
+
+if (i 0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(disk %s not found), dev-data.disk-dst);
+goto cleanup;
+}
+
+if (!(qemuCmdFlags QEMUD_CMD_FLAG_DEVICE)) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _(Underlying qemu does not support SCSI disk removal));
+goto cleanup;
+}
+
+detach = vm-def-disks[i];
+
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+if (qemuMonitorDelDevice(priv-mon, detach-info.alias) 0) {
+qemuDomainObjExitMonitor(vm);
+goto cleanup;
+}
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+qemudShrinkDisks(vm-def, i);
+
virDomainDiskDefFree(detach);
if (driver-securityDriver
@@ -8393,9 +8438,18 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
goto endjob;
if (dev-type == VIR_DOMAIN_DEVICE_DISK
-dev-data.disk-device == VIR_DOMAIN_DISK_DEVICE_DISK
-dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
-ret = qemudDomainDetachPciDiskDevice(driver, vm, dev, qemuCmdFlags);
+dev-data.disk-device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+if (dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
+ret = qemudDomainDetachPciDiskDevice(driver, vm, dev,
qemuCmdFlags);
+}
+else if (dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+ret = qemudDomainDetachSCSIDiskDevice(driver, vm, dev,
+ qemuCmdFlags);
+}
+else {
+qemuReportError(VIR_ERR_NO_SUPPORT,
+%s, _(This type of disk cannot be hot unplugged));
+}
} else if (dev-type == VIR_DOMAIN_DEVICE_NET) {
ret = qemudDomainDetachNetDevice(driver, vm, dev, qemuCmdFlags);
} else if (dev-type == VIR_DOMAIN_DEVICE_CONTROLLER) {
--
1.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix SCSI disk unplugging
Daniel P. Berrange wrote: On Tue, May 04, 2010 at 05:18:24PM +0200, Wolfgang Mauerer wrote: Detaching disk devices is not only possible for VIR_DOMAIN_DISK_BUS_VIRTIO, but also for VIR_DOMAIN_DISK_BUS_SCSI, so take care of this possibility. Additionally, when the new-style device syntax is used, we do not need to check if the PCI address is valid since we don't need it to do the hot-unplugging. And while we're at it, drop the pci part in qemudDomainDetachPciDiskDevice() -- it's misleading since we do not necessarily have to deal with PCI addresses. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com --- src/qemu/qemu_driver.c | 16 +--- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 704f824..f2b8517 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7862,10 +7862,10 @@ cleanup: } -static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - unsigned long long qemuCmdFlags) +static int qemudDomainDetachDiskDevice(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + unsigned long long qemuCmdFlags) I'd rather we introduced a separate method qemudDomainDetachSCSIDiskDevice since logically these are different types of objects/operations, that just happen to share a common monitor command with new QEMU. It also needs to give a more explicit error in the case where QEMUD_CMD_FLAG_DEVICE is not set for SCSI, since we can't support that for SCSI, but can for VirtIO. okay, I've created a separate function for SCSI disk unplug. Since the differences between VirtIO and SCSI disk unplug are tiny, I've tried to extract at least a few commonalities -- see the two follow-up patches. Albeit there's much more duplicated code lurking in qemu_driver.c, it might make sense to look at this some day. Best, Wolfgang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/2] Refactor disk unplugging
We can reuse some of the code for other purposes.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
---
src/qemu/qemu_driver.c | 56 ++-
1 files changed, 36 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 47ae52c..63ca57c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7867,6 +7867,36 @@ cleanup:
}
+static inline int qemudFindDisk(virDomainDefPtr def, char *dst)
+{
+int i;
+
+for (i = 0 ; i def-ndisks ; i++) {
+if (STREQ(def-disks[i]-dst, dst)) {
+return i;
+}
+}
+
+return -1;
+}
+
+static inline void qemudShrinkDisks(virDomainDefPtr def, int i)
+{
+if (def-ndisks 1) {
+memmove(def-disks + i,
+def-disks + i + 1,
+sizeof(*def-disks) *
+(def-ndisks - (i + 1)));
+def-ndisks--;
+if (VIR_REALLOC_N(def-disks, def-ndisks) 0) {
+/* ignore, harmless */
+}
+} else {
+VIR_FREE(def-disks);
+def-ndisks = 0;
+}
+}
+
static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev,
@@ -7876,19 +7906,16 @@ static int qemudDomainDetachPciDiskDevice(struct
qemud_driver *driver,
virDomainDiskDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm-privateData;
-for (i = 0 ; i vm-def-ndisks ; i++) {
-if (STREQ(vm-def-disks[i]-dst, dev-data.disk-dst)) {
-detach = vm-def-disks[i];
-break;
-}
-}
+i = qemudFindDisk(vm-def, dev-data.disk-dst);
-if (!detach) {
+if (i 0) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
_(disk %s not found), dev-data.disk-dst);
goto cleanup;
}
+detach = vm-def-disks[i];
+
if (!virDomainDeviceAddressIsValid(detach-info,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
@@ -7911,19 +7938,8 @@ static int qemudDomainDetachPciDiskDevice(struct
qemud_driver *driver,
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
-if (vm-def-ndisks 1) {
-memmove(vm-def-disks + i,
-vm-def-disks + i + 1,
-sizeof(*vm-def-disks) *
-(vm-def-ndisks - (i + 1)));
-vm-def-ndisks--;
-if (VIR_REALLOC_N(vm-def-disks, vm-def-ndisks) 0) {
-/* ignore, harmless */
-}
-} else {
-VIR_FREE(vm-def-disks);
-vm-def-ndisks = 0;
-}
+qemudShrinkDisks(vm-def, i);
+
virDomainDiskDefFree(detach);
if (driver-securityDriver
--
1.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/5] build: simplify checks for sched.h
* configure.ac: Remove redundant checks. --- Noticed this simplification after sending my series. configure.ac | 34 ++ 1 files changed, 10 insertions(+), 24 deletions(-) diff --git a/configure.ac b/configure.ac index c643c56..c187420 100644 --- a/configure.ac +++ b/configure.ac @@ -108,7 +108,7 @@ LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \ - sched.h termios.h sys/poll.h syslog.h mntent.h net/ethernet.h]) + termios.h sys/poll.h syslog.h mntent.h net/ethernet.h]) dnl Where are the XDR functions? dnl If portablexdr is installed, prefer that. @@ -495,33 +495,19 @@ if test $with_libvirtd = no ; then with_lxc=no fi if test $with_lxc = yes || test $with_lxc = check; then -AC_CHECK_HEADER([sched.h], -dnl Header is there, check for unshare() -[ -AC_TRY_LINK([#define _GNU_SOURCE -#include sched.h], [ -unshare (1); - ], [ -with_lxc=yes - ], [ -if test $with_lxc = check; then - with_lxc=no - AC_MSG_NOTICE([Function unshare() not present in sched.h header but required for LXC driver, disabling it]) -else - AC_MSG_ERROR([Function unshare() not present in sched.h header, but required for LXC driver]) -fi - -]) - -dnl Header is not there -],[ +AC_TRY_LINK([#define _GNU_SOURCE +#include sched.h +], [ +unshare (1); +], [ +with_lxc=yes +], [ if test $with_lxc = check; then with_lxc=no -AC_MSG_NOTICE([Header sched.h not found but required for LXC driver, disabling it]) +AC_MSG_NOTICE([Function unshare() not present in sched.h header but required for LXC driver, disabling it]) else -AC_MSG_ERROR([Header sched.h not found but required for LXC driver]) +AC_MSG_ERROR([Function unshare() not present in sched.h header, but required for LXC driver]) fi - ]) fi if test $with_lxc = yes ; then -- 1.6.6.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm
On 05/05/2010 12:26 AM, Kenneth Nagin wrote: Eric Blake ebl...@redhat.com wrote on 05/05/2010 01:04:23: [feel free to trim content unrelated to your reply, it makes it easier to get to your comments] Then I verified that it passes 'make check' and 'make syntax-check', and finally pushed this in your name. Thanks again for the contribution, and for the reminders to review it. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org [attachment signature.asc deleted by Kenneth Nagin/Haifa/IBM] Thanks both of you for your assistance and thorough reviews. Since this is my first contribution, what happens next? How do I know whether it was accepted into an official release? It's already been applied in the upstream git repository. Which means that the next libvirt release (probably 0.8.2, and hopefully at the end of May per our typical release schedule) will include your changes. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] rpmbuild: add ebtables ip(6)tables dependency for rpm
On 05/05/2010 05:24 AM, Stefan Berger wrote: Add ebtables,iptables iptables-ipv6 dependency to rpm. Signed-off-by: Stefan Berger stef...@us.ibm.com ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] rpmbuild: add ebtables ip(6)tables dependency for rpm
On Wed, May 05, 2010 at 07:24:29AM -0400, Stefan Berger wrote:
Add ebtables,iptables iptables-ipv6 dependency to rpm.
Signed-off-by: Stefan Berger stef...@us.ibm.com
---
libvirt.spec.in |7 +++
1 file changed, 7 insertions(+)
Index: libvirt-acl/libvirt.spec.in
===
--- libvirt-acl.orig/libvirt.spec.in
+++ libvirt-acl/libvirt.spec.in
@@ -61,6 +61,7 @@
%define with_udev 0%{!?_without_udev:0}
%define with_hal 0%{!?_without_hal:0}
%define with_yajl 0%{!?_without_yajl:0}
+%define with_nwfilter 0%{!?_without_nwfilter:0}
%define with_libpcap 0%{!?_without_libpcap:0}
# Non-server/HV driver defaults which are always enabled
@@ -150,6 +151,7 @@
# Enable libpcap library
%if %{with_qemu}
+%define with_nwfilter 0%{!?_without_nwfilter:%{server_drivers}}
%define with_libpcap 0%{!?_without_libpcap:%{server_drivers}}
%endif
@@ -195,6 +197,11 @@ Requires: bridge-utils
Requires: dnsmasq
Requires: iptables
%endif
+%if %{with_nwfilter}
+Requires: ebtables
+Requires: iptables
+Requires: iptables-ipv6
+%endif
# needed for device enumeration
%if %{with_hal}
Requires: hal
Also need to wire up the %{with_nwfilter} arg to the %configure call
eg,
%if ! %{with_nwfilter}
%define _without_nwfilter --without-nwfilter
%endif
%configure \
...snip...
%{?_without_nwfilter} \
Regards,
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt with qemu-kvm, not recognizing NIC model
On 05/04/2010 12:46 PM, Jonathan Hoover wrote: Is this the right list for this question, or should I be elsewhere? I am trying to specify a network card “model type” of “pcnet” (to emulate vmware esxi’s network card). No matter what I put for model type in my xml config file, it comes up as an Intel e1000. I ran “qemu-kvm –net nic,model=? /dev/null” and I got back a list of supported models including pcnet,e1000,virtio, and others as expected. No matter which I put in my xml file for /etc/libvirt/qemu/Symantec-bg.xml (a Symantec Brightmail Gateway virtual machine), I just get back that its an Intel card on boot (which doesn’t work with Symantec BG). Hm, this works just fine for me; I'm able to choose any of the above when I create a guest. However, you don't want to edit the /etc/libvirt file directly. Libvirt only reads that at startup and reload; during runtime it isn't re-read. To make the edit, you'll want to either use virt-manager or virsh edit guest. If that doesn't work, post your entire XML file. -- Chris Lalancette -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt with qemu-kvm, not recognizing NIC model
Sorry, it turned out to be the operating system of the guest OS having the issue. It was not an issue with libvirt. My bad, Jon -Original Message- From: Chris Lalancette [mailto:clala...@redhat.com] Sent: Wednesday, May 05, 2010 10:34 AM To: Jonathan Hoover Cc: libvirt-l...@redhat.com Subject: Re: [libvirt] libvirt with qemu-kvm, not recognizing NIC model On 05/04/2010 12:46 PM, Jonathan Hoover wrote: Is this the right list for this question, or should I be elsewhere? I am trying to specify a network card model type of pcnet (to emulate vmware esxi's network card). No matter what I put for model type in my xml config file, it comes up as an Intel e1000. I ran qemu-kvm -net nic,model=? /dev/null and I got back a list of supported models including pcnet,e1000,virtio, and others as expected. No matter which I put in my xml file for /etc/libvirt/qemu/Symantec-bg.xml (a Symantec Brightmail Gateway virtual machine), I just get back that its an Intel card on boot (which doesn't work with Symantec BG). Hm, this works just fine for me; I'm able to choose any of the above when I create a guest. However, you don't want to edit the /etc/libvirt file directly. Libvirt only reads that at startup and reload; during runtime it isn't re-read. To make the edit, you'll want to either use virt-manager or virsh edit guest. If that doesn't work, post your entire XML file. -- Chris Lalancette -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpmbuild: add ebtables ip(6)tables dependency for rpm
Daniel P. Berrange berra...@redhat.com wrote on 05/05/2010 11:33:15
AM:
Also need to wire up the %{with_nwfilter} arg to the %configure call
eg,
%if ! %{with_nwfilter}
%define _without_nwfilter --without-nwfilter
%endif
%configure \
...snip...
%{?_without_nwfilter} \
It's not an option for the user to configure so that parameter cannot
currently be passed and understood by configure. I did this similar to how
'secrets' is handled.
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] rpmbuild: add ebtables ip(6)tables dependency for rpm
Add ebtables,iptables iptables-ipv6 dependency to rpm.
Changes from V1 to V2:
-passing --without-libpcap to configure script, if libpcap is not to be used
Signed-off-by: Stefan Berger stef...@us.ibm.com
---
libvirt.spec.in |7 +++
1 file changed, 7 insertions(+)
Index: libvirt-acl/libvirt.spec.in
===
--- libvirt-acl.orig/libvirt.spec.in
+++ libvirt-acl/libvirt.spec.in
@@ -61,6 +61,7 @@
%define with_udev 0%{!?_without_udev:0}
%define with_hal 0%{!?_without_hal:0}
%define with_yajl 0%{!?_without_yajl:0}
+%define with_nwfilter 0%{!?_without_nwfilter:0}
%define with_libpcap 0%{!?_without_libpcap:0}
# Non-server/HV driver defaults which are always enabled
@@ -150,6 +151,7 @@
# Enable libpcap library
%if %{with_qemu}
+%define with_nwfilter 0%{!?_without_nwfilter:%{server_drivers}}
%define with_libpcap 0%{!?_without_libpcap:%{server_drivers}}
%endif
@@ -195,6 +197,11 @@ Requires: bridge-utils
Requires: dnsmasq
Requires: iptables
%endif
+%if %{with_nwfilter}
+Requires: ebtables
+Requires: iptables
+Requires: iptables-ipv6
+%endif
# needed for device enumeration
%if %{with_hal}
Requires: hal
@@ -517,6 +524,10 @@ of recent versions of Linux (and other O
%define _without_yajl --without-yajl
%endif
+%if ! %{with_libpcap}
+%define _without_libpcap --without-libpcap
+%endif
+
%configure %{?_without_xen} \
%{?_without_qemu} \
%{?_without_openvz} \
@@ -545,6 +556,7 @@ of recent versions of Linux (and other O
%{?_without_hal} \
%{?_without_udev} \
%{?_without_yajl} \
+ %{?_without_libpcap} \
--with-qemu-user=%{qemu_user} \
--with-qemu-group=%{qemu_group} \
--with-init-script=redhat \
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] rpmbuild: add ebtables ip(6)tables dependency for rpm
Daniel P. Berrange berra...@redhat.com wrote on 05/05/2010 12:03:15
PM:
[...]
%configure %{?_without_xen} \
%{?_without_qemu} \
%{?_without_openvz} \
@@ -545,6 +556,7 @@ of recent versions of Linux (and other O
%{?_without_hal} \
%{?_without_udev} \
%{?_without_yajl} \
+ %{?_without_libpcap} \
--with-qemu-user=%{qemu_user} \
--with-qemu-group=%{qemu_group} \
--with-init-script=redhat \
ACK, looks fine
Pushed.
Stefan
Daniel
--
|: Red Hat, Engineering, London-o-
http://people.redhat.com/berrange/:|
|: http://libvirt.org -o- http://virt-manager.org -o-
http://deltacloud.org:|
|: http://autobuild.org-o-
http://search.cpan.org/~danberr/:|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B
9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: silence a clang false positive
* src/qemu/qemu_monitor.c (qemuMonitorIOWriteWithFD): Work around recent clang shortcoming in analysis. --- False positive encountered with the latest F-13 clang :( src/qemu/qemu_monitor.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 443d216..abf1338 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1,7 +1,7 @@ /* * qemu_monitor.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -374,6 +374,9 @@ qemuMonitorIOWriteWithFD(qemuMonitorPtr mon, msg.msg_controllen = sizeof(control); cmsg = CMSG_FIRSTHDR(msg); +/* Some static analyzers, like clang 2.6-0.6.pre2, fail to see + that our use of CMSG_FIRSTHDR will not return NULL. */ +sa_assert(cmsg); cmsg-cmsg_len = CMSG_LEN(sizeof(int)); cmsg-cmsg_level = SOL_SOCKET; cmsg-cmsg_type = SCM_RIGHTS; -- 1.6.6.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/5] build: simplify checks for sched.h
Eric Blake wrote: * configure.ac: Remove redundant checks. Noticed this simplification after sending my series. configure.ac | 34 ++ 1 files changed, 10 insertions(+), 24 deletions(-) Nice. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [TCK PATCH] block devices: allow specification of size for safety
I was getting failures of domain/103-blockdev-save-restore.t when
connecting as qemu:///session, since my uid could stat /dev/sdb
but not open it. That test now skips for unprivileged users, as well
as adds a layer of sanity checking against expected size to avoid
trashing the wrong device.
* conf/default.cfg (host_block_devices): Document optional size.
* lib/Sys/Virt/TCK.pm (get_host_block_device): If optional size is
supplied, skip a device that does not match. Also, avoid devices
that can't be opened.
---
Go easy on me - I'm not that fluent in perl (yet); if there's
a better way to do the sanity check, I'm all ears.
conf/default.cfg|8
lib/Sys/Virt/TCK.pm | 14 +-
2 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/conf/default.cfg b/conf/default.cfg
index 01f438c..12c05b7 100644
--- a/conf/default.cfg
+++ b/conf/default.cfg
@@ -134,5 +134,13 @@ host_pci_devices = (
# the test suite itself needs to create partitions.
# The disks should be at *least* 512 MB in size
host_block_devices = (
+# Each block device is either a raw path
# /dev/vdb
+# or a path plus size in 1k blocks, as in /proc/partitions, to avoid
+# trashing the wrong device
+# {
+#path = /dev/sdb
+#size = 989184
+# }
+# Can list more than on block device if many are available
)
diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
index 9cdef09..fc325a3 100644
--- a/lib/Sys/Virt/TCK.pm
+++ b/lib/Sys/Virt/TCK.pm
@@ -33,6 +33,7 @@ use IO::Uncompress::Gunzip qw(gunzip);
use IO::Uncompress::Bunzip2 qw(bunzip2);
use XML::XPath;
use Carp qw(cluck carp);
+use Fcntl qw(O_RDONLY SEEK_END);
use Test::Builder;
use Sub::Uplevel qw(uplevel);
@@ -833,7 +834,18 @@ sub get_host_block_device {
my $self = shift;
my $devindex = @_ ? shift : 0;
-return $self-config(host_block_devices/[$devindex], undef);
+my $device = $self-config(host_block_devices/[$devindex]/path, undef);
+my $size = $self-config(host_block_devices/[$devindex]/size, 0);
+
+if (!defined $device) {
+ $device = $self-config(host_block_devices/[$devindex], undef);
+}
+if ($size) {
+ sysopen(BLK, $device, O_RDONLY) or return undef;
+ return undef unless sysseek(BLK, 0, SEEK_END) == $size * 1024;
+ close(BLK);
+}
+return $device;
}
1;
--
1.6.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK PATCH] block devices: allow specification of size for safety
On Wed, May 05, 2010 at 08:54:35PM +0200, Jim Meyering wrote:
Eric Blake wrote:
I was getting failures of domain/103-blockdev-save-restore.t when
connecting as qemu:///session, since my uid could stat /dev/sdb
but not open it. That test now skips for unprivileged users, as well
as adds a layer of sanity checking against expected size to avoid
trashing the wrong device.
Can we provide the option to specify the device serial number so that
it's really impossible to trash the wrong device?
* conf/default.cfg (host_block_devices): Document optional size.
* lib/Sys/Virt/TCK.pm (get_host_block_device): If optional size is
supplied, skip a device that does not match. Also, avoid devices
that can't be opened.
---
Go easy on me - I'm not that fluent in perl (yet); if there's
a better way to do the sanity check, I'm all ears.
The overall approach sounds fine, from my limited perspective.
diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
...
-return $self-config(host_block_devices/[$devindex], undef);
+my $device = $self-config(host_block_devices/[$devindex]/path,
undef);
+my $size = $self-config(host_block_devices/[$devindex]/size, 0);
+
+if (!defined $device) {
+ $device = $self-config(host_block_devices/[$devindex], undef);
+}
This can be equivalently (idiomatically) written as:
$device ||= $self-config(host_block_devices/[$devindex], undef);
I prefer that, since it eliminates one use of $device.
+if ($size) {
+ sysopen(BLK, $device, O_RDONLY) or return undef;
+ return undef unless sysseek(BLK, 0, SEEK_END) == $size * 1024;
+ close(BLK);
+}
(Note no need for double quotes around $device;
Leaving parens off of some built-ins like close is a personal
preference (less syntax is better), but obviously keeping in sync
with the prevailing style is more important)
This is similar, but doesn't leave BLK open upon failure or size mismatch:
if ($size) {
my $match = (sysopen (BLK, $device, O_RDONLY)
sysseek (BLK, 0, SEEK_END) == $size * 1024);
close BLK;
$match or return undef;
}
--
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] [TCK PATCH] block devices: allow specification of size for safety
Dave Allan wrote: On Wed, May 05, 2010 at 08:54:35PM +0200, Jim Meyering wrote: Eric Blake wrote: I was getting failures of domain/103-blockdev-save-restore.t when connecting as qemu:///session, since my uid could stat /dev/sdb but not open it. That test now skips for unprivileged users, as well as adds a layer of sanity checking against expected size to avoid trashing the wrong device. Can we provide the option to specify the device serial number so that it's really impossible to trash the wrong device? Good point. That would be even better. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK PATCH] block devices: allow specification of size for safety
Dave Allan wrote:
On Wed, May 05, 2010 at 08:54:35PM +0200, Jim Meyering wrote:
Eric Blake wrote:
I was getting failures of domain/103-blockdev-save-restore.t when
connecting as qemu:///session, since my uid could stat /dev/sdb
but not open it. That test now skips for unprivileged users, as well
as adds a layer of sanity checking against expected size to avoid
trashing the wrong device.
Can we provide the option to specify the device serial number so that
it's really impossible to trash the wrong device?
Given that this is a good idea, next question is obviously
how to get the serial number. One way seems to be via hdparm,
e.g., hdparm -i /dev/sda
/dev/sda:
Model=ST3320620AS, FwRev=3.AAK, SerialNo=9QF6ET0H
Config={ HardSect NotMFM HdSw15uSec Fixed DTR10Mbs RotSpdTol.5% }
RawCHS=16383/16/63, TrkSize=0, SectSize=0, ECCbytes=4
BuffType=unknown, BuffSize=16384kB, MaxMultSect=16, MultSect=off
CurCHS=16383/16/63, CurSects=16514064, LBA=yes, LBAsects=625142448
IORDY=on/off, tPIO={min:120,w/IORDY:120}, tDMA={min:120,rec:120}
PIO modes: pio0 pio1 pio2 pio3 pio4
DMA modes: mdma0 mdma1 mdma2
UDMA modes: udma0 udma1 udma2 udma3 udma4 udma5 *udma6
AdvancedPM=no WriteCache=enabled
Drive conforms to: Unspecified: ATA/ATAPI-1,2,3,4,5,6,7
* signifies the current active mode
which reminds me that some devices have a serial number of all zeros,
so maybe to be truly bullet-proof you'd want all three from that
first line: Model, FwRev, SerialNo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] cgroup: Change virCgroupRemove to remove all child groups at first
As same as normal directories, a cgroup cannot be removed if it
contains sub groups. This patch changes virCgroupRemove to remove
all child groups (subdirectories) of a target group before removing
the target group.
The handling is required when we run lxc with ns subsystem of cgroup.
Ns subsystem automatically creates child cgroups on every process
forks, but unfortunately the groups are not removed on process exits,
so we have to remove them by ourselves.
With this patch, such child groups are surely removed at lxc
shutdown, i.e., lxcVmCleanup which calls virCgroupRemove.
---
src/util/cgroup.c | 48
1 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index b8b2eb5..531c131 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -23,6 +23,7 @@
#include sys/stat.h
#include sys/types.h
#include libgen.h
+#include dirent.h
#include internal.h
#include util.h
@@ -561,11 +562,52 @@ cleanup:
}
#endif
+static int virCgroupRemoveRecursively(char *grppath)
+{
+DIR *grpdir;
+struct dirent *ent;
+int rc = 0;
+
+grpdir = opendir(grppath);
+if (grpdir == NULL) {
+VIR_ERROR(Unable to open %s (%d), grppath, errno);
+rc = -errno;
+return rc;
+}
+
+while ((ent = readdir(grpdir)) != NULL) {
+char path[PATH_MAX];
+int ret;
+
+if (ent-d_name[0] == '.') continue;
+if (ent-d_type != DT_DIR) continue;
+
+ret = snprintf(path, sizeof(path), %s/%s, grppath, ent-d_name);
+if (ret 0)
+break;
+rc = virCgroupRemoveRecursively(path);
+if (rc != 0)
+break;
+}
+DEBUG(Removing cgroup %s, grppath);
+if (rmdir(grppath) != 0 errno != ENOENT) {
+rc = -errno;
+VIR_ERROR(Unable to remove %s (%d), grppath, errno);
+}
+
+return rc;
+}
+
/**
* virCgroupRemove:
*
* @group: The group to be removed
*
+ * It first removes all child groups recursively
+ * in depth first order and then removes @group
+ * because the presence of the child groups
+ * prevents removing @group.
+ *
* Returns: 0 on success
*/
int virCgroupRemove(virCgroupPtr group)
@@ -585,10 +627,8 @@ int virCgroupRemove(virCgroupPtr group)
grppath) != 0)
continue;
-DEBUG(Removing cgroup %s, grppath);
-if (rmdir(grppath) != 0 errno != ENOENT) {
-rc = -errno;
-}
+DEBUG(Removing cgroup %s and all child cgroups, grppath);
+rc = virCgroupRemoveRecursively(grppath);
VIR_FREE(grppath);
}
--
1.6.5.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK PATCH] block devices: allow specification of size for safety
On 05/05/2010 01:31 PM, Jim Meyering wrote: Can we provide the option to specify the device serial number so that it's really impossible to trash the wrong device? Given that this is a good idea, next question is obviously how to get the serial number. One way seems to be via hdparm, e.g., hdparm -i /dev/sda /dev/sda: Model=ST3320620AS, FwRev=3.AAK, SerialNo=9QF6ET0H Great for SCSI, not so great for USB sticks: # hdparm -i /dev/sdb /dev/sdb: HDIO_DRIVE_CMD(identify) failed: Invalid exchange HDIO_GET_IDENTITY failed: Invalid argument # echo $? 22 -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] [TCK PATCH] block devices: allow specification of size for safety
On 05/05/2010 12:54 PM, Jim Meyering wrote:
+if (!defined $device) {
+$device = $self-config(host_block_devices/[$devindex], undef);
+}
This can be equivalently (idiomatically) written as:
$device ||= $self-config(host_block_devices/[$devindex], undef);
Thanks for the tip.
+if ($size) {
+sysopen(BLK, $device, O_RDONLY) or return undef;
+return undef unless sysseek(BLK, 0, SEEK_END) == $size * 1024;
+close(BLK);
+}
(Note no need for double quotes around $device;
Leaving parens off of some built-ins like close is a personal
preference (less syntax is better), but obviously keeping in sync
with the prevailing style is more important)
This is similar, but doesn't leave BLK open upon failure or size mismatch:
Aargh. I noticed that too, and even have it in my editor, but I hadn't
hit save before I did 'git commit'. But your alternative
if ($size) {
my $match = (sysopen (BLK, $device, O_RDONLY)
sysseek (BLK, 0, SEEK_END) == $size * 1024);
close BLK;
$match or return undef;
}
is even shorter that what 'git diff' says was still in my editor:
diff --git i/lib/Sys/Virt/TCK.pm w/lib/Sys/Virt/TCK.pm
index fc325a3..1fcfdf0 100644
--- i/lib/Sys/Virt/TCK.pm
+++ w/lib/Sys/Virt/TCK.pm
@@ -835,15 +835,16 @@ sub get_host_block_device {
my $devindex = @_ ? shift : 0;
my $device = $self-config(host_block_devices/[$devindex]/path,
undef);
-my $size = $self-config(host_block_devices/[$devindex]/size, 0);
+my $blocks = $self-config(host_block_devices/[$devindex]/size, 0);
if (!defined $device) {
$device = $self-config(host_block_devices/[$devindex], undef);
}
-if ($size) {
+if ($blocks) {
sysopen(BLK, $device, O_RDONLY) or return undef;
- return undef unless sysseek(BLK, 0, SEEK_END) == $size * 1024;
+ my $size = sysseek(BLK, 0, SEEK_END);
close(BLK);
+ return undef unless $size == $blocks * 1024;
}
return $device;
}
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: silence a clang false positive
Eric Blake wrote: * src/qemu/qemu_monitor.c (qemuMonitorIOWriteWithFD): Work around recent clang shortcoming in analysis. ... diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c ... cmsg = CMSG_FIRSTHDR(msg); +/* Some static analyzers, like clang 2.6-0.6.pre2, fail to see + that our use of CMSG_FIRSTHDR will not return NULL. */ +sa_assert(cmsg); cmsg-cmsg_len = CMSG_LEN(sizeof(int)); cmsg-cmsg_level = SOL_SOCKET; cmsg-cmsg_type = SCM_RIGHTS; ACK. Note that with clang-2.7, which is in rawhide, this sa_assert is not needed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK PATCH] block devices: allow specification of size for safety
Eric Blake wrote:
On 05/05/2010 12:54 PM, Jim Meyering wrote:
+if (!defined $device) {
+ $device = $self-config(host_block_devices/[$devindex], undef);
+}
This can be equivalently (idiomatically) written as:
$device ||= $self-config(host_block_devices/[$devindex], undef);
Thanks for the tip.
+if ($size) {
+ sysopen(BLK, $device, O_RDONLY) or return undef;
+ return undef unless sysseek(BLK, 0, SEEK_END) == $size * 1024;
+ close(BLK);
+}
(Note no need for double quotes around $device;
Leaving parens off of some built-ins like close is a personal
preference (less syntax is better), but obviously keeping in sync
with the prevailing style is more important)
This is similar, but doesn't leave BLK open upon failure or size mismatch:
Aargh. I noticed that too, and even have it in my editor, but I hadn't
hit save before I did 'git commit'. But your alternative
if ($size) {
my $match = (sysopen (BLK, $device, O_RDONLY)
sysseek (BLK, 0, SEEK_END) == $size * 1024);
close BLK;
$match or return undef;
}
is even shorter that what 'git diff' says was still in my editor:
Your $blocks is better than $size, but
how about $kb_blocks instead blocks, to distinguish from
the relatively common connotation of 512-byte blocks.
diff --git i/lib/Sys/Virt/TCK.pm w/lib/Sys/Virt/TCK.pm
index fc325a3..1fcfdf0 100644
--- i/lib/Sys/Virt/TCK.pm
+++ w/lib/Sys/Virt/TCK.pm
@@ -835,15 +835,16 @@ sub get_host_block_device {
my $devindex = @_ ? shift : 0;
my $device = $self-config(host_block_devices/[$devindex]/path,
undef);
-my $size = $self-config(host_block_devices/[$devindex]/size, 0);
+my $blocks = $self-config(host_block_devices/[$devindex]/size, 0);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK PATCH] block devices: allow specification of size for safety
Eric Blake wrote: On 05/05/2010 01:31 PM, Jim Meyering wrote: Can we provide the option to specify the device serial number so that it's really impossible to trash the wrong device? Given that this is a good idea, next question is obviously how to get the serial number. One way seems to be via hdparm, e.g., hdparm -i /dev/sda /dev/sda: Model=ST3320620AS, FwRev=3.AAK, SerialNo=9QF6ET0H Great for SCSI, not so great for USB sticks: # hdparm -i /dev/sdb /dev/sdb: HDIO_DRIVE_CMD(identify) failed: Invalid exchange HDIO_GET_IDENTITY failed: Invalid argument # echo $? 22 Using a device path in /dev/disk/by-id/ would make more sense than specifying /dev/sdX if you're concerned about hitting the wrong disk. -jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK PATCH] block devices: allow specification of size for safety
On 05/05/2010 02:21 PM, Jim Paris wrote: Great for SCSI, not so great for USB sticks: # hdparm -i /dev/sdb /dev/sdb: HDIO_DRIVE_CMD(identify) failed: Invalid exchange HDIO_GET_IDENTITY failed: Invalid argument # echo $? 22 Using a device path in /dev/disk/by-id/ would make more sense than specifying /dev/sdX if you're concerned about hitting the wrong disk. At this point, I'm happy making default.cfg show an example using /dev/disk/by-id/xxx + size. Look for v2 of the patch coming up soon. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] [TCK PATCHv2] block devices: allow specification of size for safety
I was getting failures of domain/103-blockdev-save-restore.t when
connecting as qemu:///session, since my uid could stat /dev/sdb but
not open it. That test now skips for unprivileged users, as well as
adds a layer of sanity checking against expected size to avoid
trashing the wrong device.
* conf/default.cfg (host_block_devices): Document optional size.
* lib/Sys/Virt/TCK.pm (get_host_block_device): If optional size is
supplied, skip a device that does not match. Also, avoid devices
that can't be opened.
---
Changes from v1:
+ perl is more idiomatic
+ guarantee that BLK is closed, even if sysseek failed
+ mention /dev/disk/by-id/ trick in default.cfg
conf/default.cfg|8
lib/Sys/Virt/TCK.pm | 14 +-
2 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/conf/default.cfg b/conf/default.cfg
index 01f438c..d749f5f 100644
--- a/conf/default.cfg
+++ b/conf/default.cfg
@@ -134,5 +134,13 @@ host_pci_devices = (
# the test suite itself needs to create partitions.
# The disks should be at *least* 512 MB in size
host_block_devices = (
+# Each block device is either a raw path
# /dev/vdb
+# or a path plus size in 1k blocks, as in /proc/partitions, to avoid
+# trashing the wrong device
+# {
+#path = /dev/disk/by-id/usb-Generic_Flash_Disk_9B46E3C5-0:0
+#size = 989184
+# }
+# Can list more than on block device if many are available
)
diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
index 9cdef09..7d97314 100644
--- a/lib/Sys/Virt/TCK.pm
+++ b/lib/Sys/Virt/TCK.pm
@@ -33,6 +33,7 @@ use IO::Uncompress::Gunzip qw(gunzip);
use IO::Uncompress::Bunzip2 qw(bunzip2);
use XML::XPath;
use Carp qw(cluck carp);
+use Fcntl qw(O_RDONLY SEEK_END);
use Test::Builder;
use Sub::Uplevel qw(uplevel);
@@ -833,7 +834,18 @@ sub get_host_block_device {
my $self = shift;
my $devindex = @_ ? shift : 0;
-return $self-config(host_block_devices/[$devindex], undef);
+my $device = $self-config(host_block_devices/[$devindex]/path, undef);
+my $kb_blocks = $self-config(host_block_devices/[$devindex]/size, 0);
+
+$device ||= $self-config(host_block_devices/[$devindex], undef);
+
+if ($kb_blocks $device) {
+ my $match = (sysopen(BLK, $device, O_RDONLY)
+ sysseek(BLK, 0, SEEK_END) == $kb_blocks * 1024);
+ close BLK;
+ $match or return undef;
+}
+return $device;
}
1;
--
1.6.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: silence a clang false positive
On 05/05/2010 01:59 PM, Jim Meyering wrote: Eric Blake wrote: * src/qemu/qemu_monitor.c (qemuMonitorIOWriteWithFD): Work around recent clang shortcoming in analysis. ... diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c ... cmsg = CMSG_FIRSTHDR(msg); +/* Some static analyzers, like clang 2.6-0.6.pre2, fail to see + that our use of CMSG_FIRSTHDR will not return NULL. */ +sa_assert(cmsg); cmsg-cmsg_len = CMSG_LEN(sizeof(int)); cmsg-cmsg_level = SOL_SOCKET; cmsg-cmsg_type = SCM_RIGHTS; ACK. Thanks; pushed. Note that with clang-2.7, which is in rawhide, this sa_assert is not needed. Fair enough, but it will be a few months before that version propagates into common usage :) -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] [TCK PATCH] block devices: allow specification of size for safety
On Wed, May 05, 2010 at 01:56:20PM -0600, Eric Blake wrote: On 05/05/2010 01:31 PM, Jim Meyering wrote: Can we provide the option to specify the device serial number so that it's really impossible to trash the wrong device? Given that this is a good idea, next question is obviously how to get the serial number. One way seems to be via hdparm, e.g., hdparm -i /dev/sda /dev/sda: Model=ST3320620AS, FwRev=3.AAK, SerialNo=9QF6ET0H Great for SCSI, not so great for USB sticks: # hdparm -i /dev/sdb What does the following give you? scsi_id --whitelisted --replace-whitespace --device=/dev/sdb /dev/sdb: HDIO_DRIVE_CMD(identify) failed: Invalid exchange HDIO_GET_IDENTITY failed: Invalid argument # echo $? 22 -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK PATCH] block devices: allow specification of size for safety
On 05/05/2010 02:48 PM, Dave Allan wrote: Great for SCSI, not so great for USB sticks: # hdparm -i /dev/sdb What does the following give you? scsi_id --whitelisted --replace-whitespace --device=/dev/sdb # scsi_id --whitelisted --replace-whitespace --device=/dev/sdb # echo $? 1 Not a peep on stderr (for shame). -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] [TCK PATCH] block devices: allow specification of size for safety
On Wed, May 05, 2010 at 02:49:44PM -0600, Eric Blake wrote: On 05/05/2010 02:48 PM, Dave Allan wrote: Great for SCSI, not so great for USB sticks: # hdparm -i /dev/sdb What does the following give you? scsi_id --whitelisted --replace-whitespace --device=/dev/sdb # scsi_id --whitelisted --replace-whitespace --device=/dev/sdb # echo $? 1 Not a peep on stderr (for shame). Ok, looks like that device doesn't return a serial number. For devices that do have serials, especially when there are lots of nearly identical disks on a system, it'd be really helpful. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [TCK PATCHv3] block devices: allow specification of size for safety
I was getting failures of domain/103-blockdev-save-restore.t when
connecting as qemu:///session, since my uid could stat /dev/sdb but
not open it. That test now skips for unprivileged users, as well as
adds a layer of sanity checking against expected size to avoid
trashing the wrong device.
* conf/default.cfg (host_block_devices): Document optional size.
* lib/Sys/Virt/TCK.pm (get_host_block_device): If optional size is
supplied, skip a device that does not match. Also, avoid devices
that can't be opened.
---
Changes from v2:
+ guarantee that the device can be opened by the current user, even if
the .cfg file used the older path-only configuration
conf/default.cfg|8
lib/Sys/Virt/TCK.pm | 17 -
2 files changed, 24 insertions(+), 1 deletions(-)
diff --git a/conf/default.cfg b/conf/default.cfg
index 01f438c..d749f5f 100644
--- a/conf/default.cfg
+++ b/conf/default.cfg
@@ -134,5 +134,13 @@ host_pci_devices = (
# the test suite itself needs to create partitions.
# The disks should be at *least* 512 MB in size
host_block_devices = (
+# Each block device is either a raw path
# /dev/vdb
+# or a path plus size in 1k blocks, as in /proc/partitions, to avoid
+# trashing the wrong device
+# {
+#path = /dev/disk/by-id/usb-Generic_Flash_Disk_9B46E3C5-0:0
+#size = 989184
+# }
+# Can list more than on block device if many are available
)
diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
index 9cdef09..0aba557 100644
--- a/lib/Sys/Virt/TCK.pm
+++ b/lib/Sys/Virt/TCK.pm
@@ -33,6 +33,7 @@ use IO::Uncompress::Gunzip qw(gunzip);
use IO::Uncompress::Bunzip2 qw(bunzip2);
use XML::XPath;
use Carp qw(cluck carp);
+use Fcntl qw(O_RDONLY SEEK_END);
use Test::Builder;
use Sub::Uplevel qw(uplevel);
@@ -833,7 +834,21 @@ sub get_host_block_device {
my $self = shift;
my $devindex = @_ ? shift : 0;
-return $self-config(host_block_devices/[$devindex], undef);
+my $device = $self-config(host_block_devices/[$devindex]/path, undef);
+my $kb_blocks = $self-config(host_block_devices/[$devindex]/size, 0);
+my $match = 1;
+
+$device ||= $self-config(host_block_devices/[$devindex], undef);
+
+# Filter out devices that the current user can't open.
+sysopen(BLK, $device, O_RDONLY) or return undef;
+
+if ($kb_blocks) {
+ $match = sysseek(BLK, 0, SEEK_END) == $kb_blocks * 1024;
+}
+close BLK;
+
+return $match ? $device : undef;
}
1;
--
1.6.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
