Re: [libvirt] Release of libvirt-0.8.8
On Thu, Feb 17, 2011 at 05:39, Daniel Veillard veill...@redhat.com wrote: As scheduled the release is out, and available from the site: ftp://libvirt.org/libvirt/ Thanks, it's working great so far! One minor issue: [root@ev004 ~]# service libvirt-guests status /etc/init.d/libvirt-guests: line 37: /usr/bin/gettext.sh: No such file or directory Looks like the rpm needs a Requires: gettext? Regards, Ruben -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Remove redundant error reporting codes
As virDomainDefParseString already reported the error if it
fails, and the redundant error reports codes will override
error reported by virDomainDefParseString with some unclear
messages, removed them.
* src/qemu/qemu_driver.c
---
src/qemu/qemu_driver.c | 33 +
1 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ab664a0..fd8e401 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3600,8 +3600,10 @@ static virDomainPtr qemudDomainCreate(virConnectPtr
conn, const char *xml,
virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL);
qemuDriverLock(driver);
-if (!(def = virDomainDefParseString(driver-caps, xml,
-VIR_DOMAIN_XML_INACTIVE)))
+
+def = virDomainDefParseString(driver-caps, xml, VIR_DOMAIN_XML_INACTIVE);
+if (!def)
+/* virDomainDefParseString reports the error. */
goto cleanup;
if (virSecurityManagerVerify(driver-securityManager, def) 0)
@@ -5746,12 +5748,9 @@ qemudDomainSaveImageOpen(struct qemud_driver *driver,
}
/* Create a domain from this XML */
-if (!(def = virDomainDefParseString(driver-caps, xml,
-VIR_DOMAIN_XML_INACTIVE))) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(failed to parse XML));
+def = virDomainDefParseString(driver-caps, xml, VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto error;
-}
VIR_FREE(xml);
@@ -6412,8 +6411,9 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn,
const char *xml) {
int dupVM;
qemuDriverLock(driver);
-if (!(def = virDomainDefParseString(driver-caps, xml,
-VIR_DOMAIN_XML_INACTIVE)))
+
+def = virDomainDefParseString(driver-caps, xml, VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto cleanup;
if (virSecurityManagerVerify(driver-securityManager, def) 0)
@@ -8046,13 +8046,9 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn,
}
/* Parse the domain XML. */
-if (!(def = virDomainDefParseString(driver-caps, dom_xml,
-VIR_DOMAIN_XML_INACTIVE))) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(failed to parse XML, libvirt version may be
-different between source and destination
host));
+def = virDomainDefParseString(driver-caps, dom_xml,
VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto cleanup;
-}
if (!qemuDomainIsMigratable(def))
goto cleanup;
@@ -8320,12 +8316,9 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
VIR_DEBUG(Generated uri_out=%s, *uri_out);
/* Parse the domain XML. */
-if (!(def = virDomainDefParseString(driver-caps, dom_xml,
-VIR_DOMAIN_XML_INACTIVE))) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(failed to parse XML));
+def = virDomainDefParseString(driver-caps, dom_xml,
VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto cleanup;
-}
if (!qemuDomainIsMigratable(def))
goto cleanup;
--
1.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add support for multiple serial ports into the Xen driver
Eric, thanks for reply but I'm having some issues now...
I've pulled libvirt sources from upstream and I'm unable to compile it
since it keeps saying following:
CCLD libvirt_parthelper
./.libs/libvirt_util.a(libvirt_util_la-util.o): In function
`virFileOperationNoFork':
/home/mig/Work/virt-related/libvirt/src/util/util.c:1363: undefined
reference to `rpl_open'
./.libs/libvirt_util.a(libvirt_util_la-util.o): In function
`virFileReadAll':
/home/mig/Work/virt-related/libvirt/src/util/util.c:1159: undefined
reference to `rpl_open'
./.libs/libvirt_util.a(libvirt_util_la-util.o): In function
`virFileWriteStr':
/home/mig/Work/virt-related/libvirt/src/util/util.c:1184: undefined
reference to `rpl_open'
/home/mig/Work/virt-related/libvirt/src/util/util.c:1186: undefined
reference to `rpl_open'
./.libs/libvirt_util.a(libvirt_util_la-util.o): In function
`virFileOperation':
/home/mig/Work/virt-related/libvirt/src/util/util.c:1519: undefined
reference to `rpl_open'
./.libs/libvirt_util.a(libvirt_util_la-util.o):/home/mig/Work/virt-related/libvirt/src/util/util.c:1795:
more undefined references to `rpl_open' follow
collect2: ld returned 1 exit status
Unfortunately I was unable to find definition of 'rpl_open' function
using git grep so I don't really know what to do there. Some bug or do I
miss something?
+++ b/src/xen/xend_internal.c
@@ -1218,6 +1218,9 @@ xenDaemonParseSxprChar(const char *value,
if (value[0] == '/') {
def-source.type = VIR_DOMAIN_CHR_TYPE_DEV;
+def-data.file.path = strdup(value);
+if (!def-data.file.path)
+goto error;
This should be goto no_memory.
To my knowledge this is not the only one issue there since this has been
changed from def-data.file.path to def-source.data.file.path to
compile successfully but that's fine. It seems like codebase changed so
it took some time to rewrite the code for the current upstream codebase.
@@ -5958,10 +6011,22 @@ xenDaemonFormatSxpr(virConnectPtr conn,
virBufferAddLit(buf, (parallel none));
}
if (def-serials) {
-virBufferAddLit(buf, (serial );
-if (xenDaemonFormatSxprChr(def-serials[0],buf) 0)
-goto error;
-virBufferAddLit(buf, ));
+if (def-nserials 1) {
+virBufferAddLit(buf, (serial ();
+for (i = 0; i def-nserials; i++) {
+if
(xenDaemonFormatSxprChr(def-serials[i],buf) 0)
+goto error;
+if (i def-nserials - 1)
+virBufferAddLit(buf, );
+}
+virBufferAddLit(buf, )));
+}
You skipped none as a placeholder when parsing; but I'm not sure if
this generates none on output to match.
Basically I don't know what you mean by this. Also, when I was defining
XMLs I saw that the console node copies the first serial node however
how to define it if first serial node is just none?
I mean the XML is like:
serial type='dev'
source path='/dev/ttyS0'/
target port='0'/
/serial
serial type='dev'
source path='/dev/ttyS1'/
target port='1'/
/serial
console type='dev'
source path='/dev/ttyS0'/
target port='0'/
/console
But what should the XML part be for case of first serial device to be none ?
Michal
--
Michal Novotnyminov...@redhat.com, RHCE
Virtualization Team (xen userspace), Red Hat
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Reconfiguring doesn't trigger regeneration of libvirt.syms
Consider this sequence of commands $ ./configure ... $ make ... GENlibvirt.syms ... $ ./configure --with-driver-modules ... $ make ... libvirt.syms doesn't get regenerated but it should as it should contain virDriverLoadModule now. make clean after a configure run doesn't help, as make clean dosen't remove libvirt.syms. So either the libvirt.syms rule needs to depend on something that triggers a regeneration of libvirt.syms after a reconfiguration or make clean needs to remove libvirt.syms. I'm not sure about the correct solution here. Any suggestions? Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Requires gettext for client package
libvirt-guests invokes functions in gettext.sh, so we need to
require gettext package in spec file.
Demo with the fix:
package gettext is not installed
error: Failed dependencies:
gettext is needed by libvirt-client-0.8.8-1.fc14.x86_64
* libvirt.spec.in
---
libvirt.spec.in |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index d4208e8..c4b4e7f 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -415,6 +415,7 @@ Requires: ncurses
# So remote clients can access libvirt over SSH tunnel
# (client invokes 'nc' against the UNIX socket on the server)
Requires: nc
+Requires: gettext
%if %{with_sasl}
Requires: cyrus-sasl
# Not technically required, but makes 'out-of-box' config
--
1.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt-0.8.8
于 2011年02月17日 17:31, Ruben Kerkhof 写道: On Thu, Feb 17, 2011 at 05:39, Daniel Veillardveill...@redhat.com wrote: As scheduled the release is out, and available from the site: ftp://libvirt.org/libvirt/ Thanks, it's working great so far! One minor issue: [root@ev004 ~]# service libvirt-guests status /etc/init.d/libvirt-guests: line 37: /usr/bin/gettext.sh: No such file or directory Looks like the rpm needs a Requires: gettext? Regards, Made a patch to fix it. Regards Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Requires gettext for client package
libvirt-guests invokes functions in gettext.sh, so we need to
require gettext package in spec file.
Demo with the fix:
% rpm -q gettext
package gettext is not installed
% rpm -ivh libvirt-client-0.8.8-1.fc14.x86_64.rpm
error: Failed dependencies:
gettext is needed by libvirt-client-0.8.8-1.fc14.x86_64
* libvirt.spec.in
---
libvirt.spec.in |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index d4208e8..c4b4e7f 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -415,6 +415,7 @@ Requires: ncurses
# So remote clients can access libvirt over SSH tunnel
# (client invokes 'nc' against the UNIX socket on the server)
Requires: nc
+Requires: gettext
%if %{with_sasl}
Requires: cyrus-sasl
# Not technically required, but makes 'out-of-box' config
--
1.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Requires gettext for client package
于 2011年02月17日 18:02, Osier Yang 写道:
libvirt-guests invokes functions in gettext.sh, so we need to
require gettext package in spec file.
Demo with the fix:
package gettext is not installed
error: Failed dependencies:
gettext is needed by libvirt-client-0.8.8-1.fc14.x86_64
Urgh, please skip this patch, # is eaten, v2 comes following.
* libvirt.spec.in
---
libvirt.spec.in |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index d4208e8..c4b4e7f 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -415,6 +415,7 @@ Requires: ncurses
# So remote clients can access libvirt over SSH tunnel
# (client invokes 'nc' against the UNIX socket on the server)
Requires: nc
+Requires: gettext
%if %{with_sasl}
Requires: cyrus-sasl
# Not technically required, but makes 'out-of-box' config
--
1.7.4
--
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
[libvirt] [BUG] storage_backend_fs: failung virStorageBackendProbeTarget() disabled whole pool
Hello,
I have a problem with the following code fragment, which refreshes a directory
based storage pool:
storage_backend_fs.c#virStorageBackendFileSystemRefresh(...)
...
while ((ent = readdir(dir)) != NULL) {
...
if ((ret = virStorageBackendProbeTarget(...) 0) {
if (ret == -1)
goto cleanup;
...
}
...
}
closedir(dir);
...
return 0;
cleanup:
...
return -1;
}
This disables the whole pool, if it contains an Qcow2 volume while, whose
backfile is (currently) unavailable (because, for example, the central NFS
store is currently unavailable or the permissions don't allow reading). This
is very annoying, since it's hard to find the Qcow2 volume, which breaks the
pool. I use the following command to find broken volumes:
find $dir -maxdepth 1 \( -type f -o -type l \) \( -exec kvm-img info {}
\; -o -print \) 2/dev/null
Even worse, you can't delete the broken or re-create the missing volume with
virsh allone.
To reproduce:
dir=$(mktemp -d)
virsh pool-create-as tmp dir '' '' '' '' $dir
virsh vol-create-as --format qcow2 tmp back 1G
virsh vol-create-as --format qcow2 --backing-vol-format qcow2 --backing-vol
back tmp cow 1G
virsh vol-delete --pool tmp back
virsh pool-refresh tmp
After the last step, the pool will be gone (because it was not persistent). As
long as the now broken image stays in the directory, you will not be able to
re-create or re-start the pool.
The easiest 'fix' would be to ignore all errors regarding the detection of the
backing files file format. This would at least allow users to still create
new volumes and list existing volumes.
I appreciate any comments.
BYtE
Philipp
--
Philipp Hahn Open Source Software Engineer h...@univention.de
Univention GmbHLinux for Your Businessfon: +49 421 22 232- 0
Mary-Somerville-Str.1 28359 Bremen fax: +49 421 22 232-99
http://www.univention.de/
** Besuchen Sie uns auf der CeBIT in Hannover **
** Auf dem Univention Stand D36 in Halle 2**
** Vom 01. bis 05. März 2011 **
signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Still in freeze, third rc tarball available
On Wed, Feb 16, 2011 at 05:10:10PM -0700, Jim Fehlig wrote:
Jim Fehlig wrote:
Daniel Veillard wrote:
Thanks everybody for the testing feedback and fixes, in retrospect
I should really had done this in previous releases ! So the third
rc tarball is out, it's likely to be the last one before the release
(within 48 hours):
ftp://libvirt.org/libvirt/libvirt-0.8.8-rc3.tar.gz
give it a try !
Building and passing minimal tests on all supported openSUSE/SLE
distros, which in itself is a good exercise of the various configure
options.
However, I'm trying to run libvirt-tck on openSUSE11.4rc1, qemu0.14rc1,
and libvirt 0.8.8rc3 (lots of rc's :)) and getting quite a few
failures. The failing qemu invocations look like:
2011-02-15 12:15:05.863: starting up
LC_ALL=C
PATH=/sbin:/usr/sbin:/usr/local/sbin:/root/bin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/X11R6/bin:/usr/games:/usr/lib64/jvm/jre/bin
HOME=/root USER=root LOGNAME=root /usr/bin/qemu-kvm -S -M pc-0.14
-enable-kvm -m 64 -smp 1,sockets=1,cores=1,threads=1 -name tck -uuid
7767756e-9160-41d3-f7a5-0af2aed17d1b -nographic -nodefconfig -nodefaults
-chardev
socket,id=charmonitor,path=/var/lib/libvirt/qemu/tck.monitor,server,nowait
-mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -boot c
-kernel /var/cache/libvirt-tck/os-i686-hvm/vmlinuz -initrd
/var/cache/libvirt-tck/os-i686-hvm/initrd -drive
file=/var/cache/libvirt-tck/os-i686-hvm/disk.img,if=none,id=drive-virtio-disk0,boot=on,format=raw
-device
virtio-blk-pci,bus=pci.0,addr=0x2,drive=drive-virtio-disk0,id=virtio-disk0
-chardev pty,id=charserial0 -device
isa-serial,chardev=charserial0,id=serial0 -usb -device
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
char device redirected to /dev/pts/1
Two devices with same boot index 0
I think qemu0.14 is less tolerable of specifying 2 boot devices, in this
case implicitly with -kernel and boot=on for the disk.
I'll try to look more later but for now need some sleep.
I see Bruce brought this up on qemu ml [1]. Seems removing boot=on when
using linuxboot.bin is preferred solution. I'm attaching a hack that
allows me to pass the libvirt-tck domain tests but would really like
comment from someone familiar with code, particularly the bootindex
stuff in qemu-kvm.
Thanks!
Jim
[1] http://lists.nongnu.org/archive/html/qemu-devel/2011-02/msg01892.html
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 05f427c..9186707 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3112,7 +3112,7 @@ qemuBuildCommandLine(virConnectPtr conn,
}
/* If QEMU supports -drive param instead of old -hda, -hdb, -cdrom .. */
-if (qemuCmdFlags QEMUD_CMD_FLAG_DRIVE) {
+if (qemuCmdFlags QEMUD_CMD_FLAG_DRIVE !def-os.kernel) {
int bootCD = 0, bootFloppy = 0, bootDisk = 0;
/* If QEMU supports boot=on for -drive param... */
This patch prevents libvirt actually creating any disks at all. Your
idea sounds good though. I think if you put the ' !def-os.kernel'
bit in the next conditional it will work, eg try
/* If QEMU supports boot=on for -drive param... */
if ((qemuCmdFlags QEMUD_CMD_FLAG_DRIVE_BOOT)
!def-os.kernel) {
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
[libvirt] [libvirt-snmp] Status
Hi, i'd like to let you know about current status of this project. There is a wiki page http://wiki.libvirt.org/page/Libvirt-snmp which is currently a documentation. I am currently waiting for ACK for patch I've sent (snmp trap support) - so if anyone's interested and have a lot of free time ... :) I'd like to also ask for a hint. Those of you who tried libvirt-snmp out might already have noticed. snmpwalk displays a ugly domain UUID: http://wiki.libvirt.org/page/Libvirt-snmp#Examples_of_use Does anybody know how to get rid of it? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] logging: make VIR_ERROR and friends preserve errno
Followup to commit 17e19add, and would have prevented the bug
independently fixed in commit 76c57a7c.
* src/util/logging.c (virLogMessage): Preserve errno, since
logging should be as unintrusive as possible.
...
-if (fmt == NULL)
- return;
+if (fmt == NULL) {
+errno = saved_errno;
+return;
+}
...
virLogUnlock();
VIR_FREE(msg);
+errno = saved_errno;
}
I would have implemented this as
if (...)
-return;
+goto out;
...
VIR_FREE(msg);
+
+out:
+errno = saved_errno;
}
to avoid having to set errno in several places but I can live with your
solution too :-)
ACK regardless on which one of the two versions you decide to push.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-snmp] Status
On 17/02/2011, at 9:24 PM, Michal Prívozník wrote: Hi, i'd like to let you know about current status of this project. There is a wiki page http://wiki.libvirt.org/page/Libvirt-snmp which is currently a documentation. This is very good work. Nice picture!, clearly written, and explains things well. As a note, the example command lines didn't have $ or # at the start of them, to make it 100% clear they're commands people type in. I've added those in where I think it's correct, plus added in sudo before super user commands (ie yum), where it seems to be correct. Probably a good idea for you to check things are still all correct, just to be safe. :) Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Check return value of qemuMonitorDriveDel.
On Thu, Feb 17, 2011 at 02:32:49PM +0800, Hu Tao wrote:
If we don't check it, virsh users will get Disk detached successfully
even when qemuMonitorDriveDel fails.
---
src/qemu/qemu_hotplug.c | 10 --
src/qemu/qemu_monitor_json.c |2 +-
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index fb9db5a..70e9d8e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1187,7 +1187,10 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver
*driver,
}
/* disconnect guest from host device */
-qemuMonitorDriveDel(priv-mon, drivestr);
+if (qemuMonitorDriveDel(priv-mon, drivestr) != 0) {
+qemuDomainObjExitMonitor(vm);
+goto cleanup;
+}
qemuDomainObjExitMonitorWithDriver(driver, vm);
@@ -1269,7 +1272,10 @@ int qemuDomainDetachSCSIDiskDevice(struct qemud_driver
*driver,
}
/* disconnect guest from host device */
-qemuMonitorDriveDel(priv-mon, drivestr);
+if (qemuMonitorDriveDel(priv-mon, drivestr) != 0) {
+qemuDomainObjExitMonitor(vm);
+goto cleanup;
+}
This needs to be ' 0'. 0 indicates success, and '1' indicates
that the command was not available in qemu which we need to treat
as success too otherwise we cause a regression.
qemuDomainObjExitMonitorWithDriver(driver, vm);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index d5e8d37..b088405 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2338,7 +2338,7 @@ int qemuMonitorJSONDriveDel(qemuMonitorPtr mon,
if (ret == 0) {
/* See if drive_del isn't supported */
if (qemuMonitorJSONHasError(reply, CommandNotFound)) {
-VIR_ERROR0(_(deleting disk is not supported.
+qemuReportError(VIR_ERR_NO_SUPPORT, _(deleting disk is not
supported.
This may leak data if disk is reassigned));
NACK to this chunk. We deliberately do not report an error when the
command is not found, because the 'drive_del' command does not exist
in any released QEMU yet and we don't want to cause a regression on
hotunplug for existing libvirt users.
ret = 1;
goto cleanup;
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: Remove redundant error reporting codes
On Thu, Feb 17, 2011 at 05:30:23PM +0800, Osier Yang wrote:
As virDomainDefParseString already reported the error if it
fails, and the redundant error reports codes will override
error reported by virDomainDefParseString with some unclear
messages, removed them.
* src/qemu/qemu_driver.c
---
src/qemu/qemu_driver.c | 33 +
1 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ab664a0..fd8e401 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3600,8 +3600,10 @@ static virDomainPtr qemudDomainCreate(virConnectPtr
conn, const char *xml,
virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL);
qemuDriverLock(driver);
-if (!(def = virDomainDefParseString(driver-caps, xml,
-VIR_DOMAIN_XML_INACTIVE)))
+
+def = virDomainDefParseString(driver-caps, xml,
VIR_DOMAIN_XML_INACTIVE);
+if (!def)
+/* virDomainDefParseString reports the error. */
goto cleanup;
This is a needless change that increases line length.
if (virSecurityManagerVerify(driver-securityManager, def) 0)
@@ -5746,12 +5748,9 @@ qemudDomainSaveImageOpen(struct qemud_driver *driver,
}
/* Create a domain from this XML */
-if (!(def = virDomainDefParseString(driver-caps, xml,
-VIR_DOMAIN_XML_INACTIVE))) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(failed to parse XML));
+def = virDomainDefParseString(driver-caps, xml,
VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto error;
-}
VIR_FREE(xml);
@@ -6412,8 +6411,9 @@ static virDomainPtr qemudDomainDefine(virConnectPtr
conn, const char *xml) {
int dupVM;
qemuDriverLock(driver);
-if (!(def = virDomainDefParseString(driver-caps, xml,
-VIR_DOMAIN_XML_INACTIVE)))
+
+def = virDomainDefParseString(driver-caps, xml,
VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto cleanup;
So is this chunk.
if (virSecurityManagerVerify(driver-securityManager, def) 0)
@@ -8046,13 +8046,9 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn,
}
/* Parse the domain XML. */
-if (!(def = virDomainDefParseString(driver-caps, dom_xml,
-VIR_DOMAIN_XML_INACTIVE))) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(failed to parse XML, libvirt version may be
-different between source and destination
host));
+def = virDomainDefParseString(driver-caps, dom_xml,
VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto cleanup;
-}
if (!qemuDomainIsMigratable(def))
goto cleanup;
@@ -8320,12 +8316,9 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
VIR_DEBUG(Generated uri_out=%s, *uri_out);
/* Parse the domain XML. */
-if (!(def = virDomainDefParseString(driver-caps, dom_xml,
-VIR_DOMAIN_XML_INACTIVE))) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(failed to parse XML));
+def = virDomainDefParseString(driver-caps, dom_xml,
VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto cleanup;
-}
These other chunks are fine, but it'd be preferrable to keep the existing
line formattingif (!(def = ))
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] [BUG] storage_backend_fs: failung virStorageBackendProbeTarget() disabled whole pool
On Thu, Feb 17, 2011 at 11:21:26AM +0100, Philipp Hahn wrote:
Hello,
I have a problem with the following code fragment, which refreshes a
directory
based storage pool:
storage_backend_fs.c#virStorageBackendFileSystemRefresh(...)
...
while ((ent = readdir(dir)) != NULL) {
...
if ((ret = virStorageBackendProbeTarget(...) 0) {
if (ret == -1)
goto cleanup;
...
}
...
}
closedir(dir);
...
return 0;
cleanup:
...
return -1;
}
This disables the whole pool, if it contains an Qcow2 volume while, whose
backfile is (currently) unavailable (because, for example, the central NFS
store is currently unavailable or the permissions don't allow reading). This
is very annoying, since it's hard to find the Qcow2 volume, which breaks the
pool. I use the following command to find broken volumes:
find $dir -maxdepth 1 \( -type f -o -type l \) \( -exec kvm-img info {}
\; -o -print \) 2/dev/null
Even worse, you can't delete the broken or re-create the missing volume with
virsh allone.
To reproduce:
dir=$(mktemp -d)
virsh pool-create-as tmp dir '' '' '' '' $dir
virsh vol-create-as --format qcow2 tmp back 1G
virsh vol-create-as --format qcow2 --backing-vol-format qcow2 --backing-vol
back tmp cow 1G
virsh vol-delete --pool tmp back
virsh pool-refresh tmp
After the last step, the pool will be gone (because it was not persistent).
As
long as the now broken image stays in the directory, you will not be able to
re-create or re-start the pool.
The easiest 'fix' would be to ignore all errors regarding the detection of
the
backing files file format. This would at least allow users to still create
new volumes and list existing volumes.
I agree, we should be more robust in that respect treat failure to probe
the backing store as non-fatal.
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] Perl bindings 0.2.6 release for libvirt 0.8.7
A new release of the Perl bindings, Sys::Virt, is now available from: http://search.cpan.org/~danberr/Sys-Virt-0.2.6/ Direct download link: http://search.cpan.org/CPAN/authors/id/D/DA/DANBERR/Sys-Virt-0.2.6.tar.gz This supports all the APIs upto and including libvirt 0.8.7, with the exception of the stream APIs 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
[libvirt] [PATCH 2/2] Added to authors to please make syntax-check
--- AUTHORS |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/AUTHORS b/AUTHORS index 6ff7c14..fb42662 100644 --- a/AUTHORS +++ b/AUTHORS @@ -155,6 +155,7 @@ Patches have also been contributed by: Gui Jianfeng guijianf...@cn.fujitsu.com Michal Novotny minov...@redhat.com Christophe Fergeau t...@gnome.org + Markus Groà gr...@univention.de [send patches to get your name here] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Refactored XM and SEXPR parsing
On Thu, Feb 17, 2011 at 01:45:42PM +0100, Markus Groß wrote: Hi, I refactored the XM and SEXPR parsing routines from the xen-unified driver into a seperate directory (xenxs; x for xm and s for sexpr). This way different xen-drivers besides xen-unified are able to use the parsing functionality. For example the upcoming XenLight (libxl) driver. To use the XM parsing functions one includes xen_xm.h and for SEXPR parsing xen_sxpr.h. The patch is rather big, but most of it are code movements. FYI in case people are wondering where the patch is, it was blocked at mailman level because it was seriously huge! After discussion with Markus on IRC, we have figured a way to split it up into smaller pieces for reposting 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 1/2] virsh.c: new xPathULong() to parse XML returning ulong value of selected node.
Parse XML from given buffer, select appropiate XPath node and return its
value as unsigned long.
---
tools/virsh.c | 49 +
1 files changed, 49 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 50d5e33..8804fc3 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2270,6 +2270,55 @@ static const vshCmdInfo info_freecell[] = {
{NULL, NULL}
};
+static int
+xPathULong(char *xml, char *xpath, unsigned long *value) {
+
+xmlDocPtr doc = NULL;
+xmlXPathContextPtr ctxt = NULL;
+xmlXPathObjectPtr obj = NULL;
+int ret = -1;
+
+doc = xmlReadDoc((const xmlChar *) xml, domain.xml, NULL,
+ XML_PARSE_NOENT | XML_PARSE_NONET | XML_PARSE_NOWARNING);
+if (!doc)
+goto cleanup;
+
+ctxt = xmlXPathNewContext(doc);
+if (!ctxt)
+goto cleanup;
+
+obj = xmlXPathEval(BAD_CAST xpath, ctxt);
+if (!obj)
+goto cleanup;
+
+if (!value)
+goto cleanup;
+
+*value = 0;
+if ((obj-type == XPATH_STRING)
+(obj-stringval != NULL) (obj-stringval[0] != 0)) {
+char *conv = NULL;
+unsigned long val;
+
+val = strtoul((const char *) obj-stringval, conv, 10);
+if (*conv =='\0') {
+*value = val;
+ret = 0;
+}
+} else if ((obj != NULL) (obj-type == XPATH_NUMBER)) {
+*value = (unsigned long) obj-floatval;
+if (*value == obj-floatval) {
+ret = 0;
+}
+}
+
+cleanup:
+xmlXPathFreeObject(obj);
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(doc);
+return ret;
+}
+
static const vshCmdOptDef opts_freecell[] = {
{cellno, VSH_OT_INT, 0, N_(NUMA cell number)},
{all, VSH_OT_BOOL, 0, N_(show free memory for all NUMA cells)},
--
1.7.3.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] virsh: freecell --all getting wrong NUMA nodes count
This is fix for virsh freecell command. I was using virNodeGetInfo() instead of virConnectGetCapabilities() which returns XML with the right number. Therefore we need XML/XPath function to pick node values from XML kept in buffer. Moreover, this function might be used to replace bad practise in cmdVcpucount(). Michal Privoznik (2): virsh.c: new xPathULong() to parse XML returning ulong value of selected node. virsh: fix wrong NUMA nodes count getting tools/virsh.c | 76 1 files changed, 65 insertions(+), 11 deletions(-) -- 1.7.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] virsh: fix wrong NUMA nodes count getting
as written in docs, one must parse capabilities XML to get the right
count of NUMA nodes.
---
tools/virsh.c | 27 ---
1 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8804fc3..1086555 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2333,8 +2333,9 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
int cell, cell_given;
unsigned long long memory;
unsigned long long *nodes = NULL;
+unsigned long nodes_cnt;
int all_given;
-virNodeInfo info;
+char *node_xml;
if (!vshConnectionUsability(ctl, ctl-conn))
@@ -2350,30 +2351,33 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
}
if (all_given) {
-if (virNodeGetInfo(ctl-conn, info) 0) {
-vshError(ctl, %s, _(failed to get NUMA nodes count));
+node_xml = virConnectGetCapabilities(ctl-conn);
+if (!node_xml) {
+vshError(ctl, %s, _(unable to get node capabilities));
goto cleanup;
}
-if (!info.nodes) {
-vshError(ctl, %s, _(no NUMA nodes present));
-goto cleanup;
-}
+ret = xPathULong(node_xml,
+
(char*)string(/capabilities/host/topology/cells/@num),
+ nodes_cnt);
+
+if (ret)
+nodes_cnt = 1;
-if (VIR_ALLOC_N(nodes, info.nodes) 0) {
+if (VIR_ALLOC_N(nodes, nodes_cnt) 0) {
vshError(ctl, %s, _(could not allocate memory));
goto cleanup;
}
-ret = virNodeGetCellsFreeMemory(ctl-conn, nodes, 0, info.nodes);
-if (ret != info.nodes) {
+ret = virNodeGetCellsFreeMemory(ctl-conn, nodes, 0, nodes_cnt);
+if (ret != nodes_cnt) {
vshError(ctl, %s, _(could not get information about
all NUMA nodes));
goto cleanup;
}
memory = 0;
-for (cell = 0; cell info.nodes; cell++) {
+for (cell = 0; cell nodes_cnt; cell++) {
vshPrint(ctl, %5d: %10llu kB\n, cell, (nodes[cell]/1024));
memory += nodes[cell];
}
@@ -2401,6 +2405,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
cleanup:
VIR_FREE(nodes);
+VIR_FREE(node_xml);
return func_ret;
}
--
1.7.3.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6 v3] Add blkio cgroup support
Hi not that I tested this patch or the patch that followed (adding blkiotune command) ... but the way I understand, the cgroup blkio controller does only work for sync'd IO/queues. quote blkio-controller.txt from 2.6.37: Currently only sync IO queues are support. All the buffered writes are still system wide and not per group. When I last tested this (using manual cgroups commands), this seemed true. So may I ask how you use this and how this helps in which setup? Regards Dominik On 02/08/2011 07:38 AM, Gui Jianfeng wrote: Hi This patchset adds blkio cgroup support for qemu and lxc. [PATCH 1/6] cgroup: Enable cgroup hierarchy for blkio cgroup [PATCH 2/6 v3] cgroup: Implement blkio.weight tuning API. [PATCH 3/6] Update XML Schema for new entries. [PATCH 4/6 v3] qemu: Implement blkio tunable XML configuration and parsing. [PATCH 5/6 v3] LXC: LXC Blkio weight configuration support. [PATCH 6/6] Add documentation for blkiotune elements. Will post a patchset to implement virsh command blkiotune to tune blkio cgroup parameter later on. v2 - v3 Changes: o Remove an unused local variable o Rename virCgroup(Set/Get)Weight to virCgroup(Set/Get)BlkioWeight o Add documentation in docs/formatdomain.html.in o Update XML Schema for new entries. docs/formatdomain.html.in | 10 ++ docs/schemas/domain.rng | 20 src/conf/domain_conf.c| 13 + src/conf/domain_conf.h|4 src/libvirt_private.syms |2 ++ src/lxc/lxc_controller.c | 10 ++ src/qemu/qemu_cgroup.c| 16 +++- src/qemu/qemu_conf.c |3 ++- src/util/cgroup.c | 41 - src/util/cgroup.h |4 10 files changed, 120 insertions(+), 3 deletions(-) Thanks Gui -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- IN-telegence GmbH Oskar-Jäger-Str. 125 50825 Köln Registergericht AG Köln - HRB 34038 USt-ID DE210882245 Geschäftsführende Gesellschafter: Christian Plätke und Holger Jansen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] virsh.c: new xPathULong() to parse XML returning ulong value of selected node.
2011/2/17 Michal Privoznik mpriv...@redhat.com:
Parse XML from given buffer, select appropiate XPath node and return its
value as unsigned long.
---
tools/virsh.c | 49 +
1 files changed, 49 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 50d5e33..8804fc3 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2270,6 +2270,55 @@ static const vshCmdInfo info_freecell[] = {
{NULL, NULL}
};
+static int
+xPathULong(char *xml, char *xpath, unsigned long *value) {
+
+ xmlDocPtr doc = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlXPathObjectPtr obj = NULL;
+ int ret = -1;
+
+ doc = xmlReadDoc((const xmlChar *) xml, domain.xml, NULL,
+ XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOWARNING);
+ if (!doc)
+ goto cleanup;
+
+ ctxt = xmlXPathNewContext(doc);
+ if (!ctxt)
+ goto cleanup;
+
+ obj = xmlXPathEval(BAD_CAST xpath, ctxt);
+ if (!obj)
+ goto cleanup;
+
+ if (!value)
+ goto cleanup;
+
+ *value = 0;
+ if ((obj-type == XPATH_STRING)
+ (obj-stringval != NULL) (obj-stringval[0] != 0)) {
+ char *conv = NULL;
+ unsigned long val;
+
+ val = strtoul((const char *) obj-stringval, conv, 10);
+ if (*conv =='\0') {
+ *value = val;
+ ret = 0;
+ }
+ } else if ((obj != NULL) (obj-type == XPATH_NUMBER)) {
+ *value = (unsigned long) obj-floatval;
+ if (*value == obj-floatval) {
+ ret = 0;
+ }
+ }
+
+cleanup:
+ xmlXPathFreeObject(obj);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(doc);
+ return ret;
+}
+
static const vshCmdOptDef opts_freecell[] = {
{cellno, VSH_OT_INT, 0, N_(NUMA cell number)},
{all, VSH_OT_BOOL, 0, N_(show free memory for all NUMA cells)},
--
1.7.3.5
NACK, there already is a virXPathULong function. Look at how other
virXPath* functions are used in virsh.
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: Fix VPATH build
On Wed, Feb 16, 2011 at 09:50:57 -0700, Eric Blake wrote: On 02/16/2011 09:41 AM, Eric Blake wrote: On 02/16/2011 09:01 AM, Jiri Denemark wrote: Even VPATH make dist succeeds now ACK. I think this is okay for 0.8.8, since it doesn't invalidate any .c code, and since it makes the tarball more useful. I still maintain the ACK, and would like to see this go in as-is. I pushed the patch. Thanks. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: Replace tabs in apibuild.py with spaces
On Wed, Feb 16, 2011 at 09:15:43 -0700, Eric Blake wrote: On 02/16/2011 09:01 AM, Jiri Denemark wrote: --- docs/apibuild.py | 3194 +++--- 1 files changed, 1597 insertions(+), 1597 deletions(-) ACK; confirmed by 'git diff -b' that there is no substance change. However, should we be tweaking cfg.mk's sc_TAB_in_indentation regex to automatically enforce this for all *.py files in the future? That makes sense. Since it revealed quite a lot of tabs in some other *.py files, I squashed this patch in so that there's just one commit which expands tabs in *.py files. The updated patch will follow shortly. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Refactored XM and SEXPR parsing
Markus Groß wrote: Hi, I refactored the XM and SEXPR parsing routines from the xen-unified driver into a seperate directory (xenxs; x for xm and s for sexpr). This way different xen-drivers besides xen-unified are able to use the parsing functionality. For example the upcoming XenLight (libxl) driver. To use the XM parsing functions one includes xen_xm.h and for SEXPR parsing xen_sxpr.h. libxl does not support sexpr at all so I don't think there is a need to move that code, only the xm config parsing functions. Regards, Jim The patch is rather big, but most of it are code movements. Some parsing functions required a driver object to fetch the tty path and vncport. I removed all references to a specific driver and added additional parameters as a replacement. The tests sexpr2xml, xmconfig and xml2sexpr are adapted and show no error. Thanks in advance for your comments about this. Cheers, Markus Markus Groß (2): Moved Xen SEXPR and XM parsing functionality to seperate directory. Added to authors to please make syntax-check AUTHORS |1 + configure.ac|4 + include/libvirt/virterror.h |2 +- src/Makefile.am | 15 +- src/util/virterror.c|4 +- src/xen/sexpr.c | 568 src/xen/sexpr.h | 55 - src/xen/xen_driver.c| 19 +- src/xen/xend_internal.c | 6673 ++- src/xen/xend_internal.h | 30 - src/xen/xm_internal.c | 1706 +--- src/xen/xm_internal.h |3 - src/xenxs/sexpr.c | 647 + src/xenxs/sexpr.h | 63 + src/xenxs/xen_sxpr.c| 2149 ++ src/xenxs/xen_sxpr.h| 62 + src/xenxs/xen_xm.c | 1716 +++ src/xenxs/xen_xm.h | 41 + src/xenxs/xenxs_private.h | 63 + tests/sexpr2xmltest.c | 12 +- tests/xmconfigtest.c|5 +- tests/xml2sexprtest.c |4 +- 22 files changed, 7012 insertions(+), 6830 deletions(-) delete mode 100644 src/xen/sexpr.c delete mode 100644 src/xen/sexpr.h create mode 100644 src/xenxs/sexpr.c create mode 100644 src/xenxs/sexpr.h create mode 100644 src/xenxs/xen_sxpr.c create mode 100644 src/xenxs/xen_sxpr.h create mode 100644 src/xenxs/xen_xm.c create mode 100644 src/xenxs/xen_xm.h create mode 100644 src/xenxs/xenxs_private.h -- 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 0/2] Refactored XM and SEXPR parsing
On Thu, Feb 17, 2011 at 07:46:59AM -0700, Jim Fehlig wrote: Markus Groß wrote: Hi, I refactored the XM and SEXPR parsing routines from the xen-unified driver into a seperate directory (xenxs; x for xm and s for sexpr). This way different xen-drivers besides xen-unified are able to use the parsing functionality. For example the upcoming XenLight (libxl) driver. To use the XM parsing functions one includes xen_xm.h and for SEXPR parsing xen_sxpr.h. libxl does not support sexpr at all so I don't think there is a need to move that code, only the xm config parsing functions. The reason was that the 'XM' code reuses several functions from the 'SEXPR' code... 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: Remove redundant error reporting codes
于 2011年02月17日 19:15, Daniel P. Berrange 写道:
On Thu, Feb 17, 2011 at 05:30:23PM +0800, Osier Yang wrote:
As virDomainDefParseString already reported the error if it
fails, and the redundant error reports codes will override
error reported by virDomainDefParseString with some unclear
messages, removed them.
* src/qemu/qemu_driver.c
---
src/qemu/qemu_driver.c | 33 +
1 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ab664a0..fd8e401 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3600,8 +3600,10 @@ static virDomainPtr qemudDomainCreate(virConnectPtr
conn, const char *xml,
virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL);
qemuDriverLock(driver);
-if (!(def = virDomainDefParseString(driver-caps, xml,
-VIR_DOMAIN_XML_INACTIVE)))
+
+def = virDomainDefParseString(driver-caps, xml, VIR_DOMAIN_XML_INACTIVE);
+if (!def)
+/* virDomainDefParseString reports the error. */
goto cleanup;
This is a needless change that increases line length.
if (virSecurityManagerVerify(driver-securityManager, def) 0)
@@ -5746,12 +5748,9 @@ qemudDomainSaveImageOpen(struct qemud_driver *driver,
}
/* Create a domain from this XML */
-if (!(def = virDomainDefParseString(driver-caps, xml,
-VIR_DOMAIN_XML_INACTIVE))) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(failed to parse XML));
+def = virDomainDefParseString(driver-caps, xml, VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto error;
-}
VIR_FREE(xml);
@@ -6412,8 +6411,9 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn,
const char *xml) {
int dupVM;
qemuDriverLock(driver);
-if (!(def = virDomainDefParseString(driver-caps, xml,
-VIR_DOMAIN_XML_INACTIVE)))
+
+def = virDomainDefParseString(driver-caps, xml, VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto cleanup;
So is this chunk.
if (virSecurityManagerVerify(driver-securityManager, def) 0)
@@ -8046,13 +8046,9 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn,
}
/* Parse the domain XML. */
-if (!(def = virDomainDefParseString(driver-caps, dom_xml,
-VIR_DOMAIN_XML_INACTIVE))) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(failed to parse XML, libvirt version may be
-different between source and destination
host));
+def = virDomainDefParseString(driver-caps, dom_xml,
VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto cleanup;
-}
if (!qemuDomainIsMigratable(def))
goto cleanup;
@@ -8320,12 +8316,9 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
VIR_DEBUG(Generated uri_out=%s, *uri_out);
/* Parse the domain XML. */
-if (!(def = virDomainDefParseString(driver-caps, dom_xml,
-VIR_DOMAIN_XML_INACTIVE))) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(failed to parse XML));
+def = virDomainDefParseString(driver-caps, dom_xml,
VIR_DOMAIN_XML_INACTIVE);
+if (!def)
goto cleanup;
-}
These other chunks are fine, but it'd be preferrable to keep the existing
line formattingif (!(def = ))
Okay, will push with these updates, thanks for the reviewing.
Regards,
Osier
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Restructure domain struct interface driver data for easier expansion
On 02/07/2011 11:55 AM, Eric Blake wrote: On 02/04/2011 02:00 PM, Laine Stump wrote: When thedriver element (and its name attribute) was added to the domain XML's interface element, a backend enum was simply added to the toplevel of the virDomainNetDef struct. This patch changes virDomainNetDef in two ways: s/two/three/, given: Sigh. I just noticed this after I pushed. So the comment is out of whack :-/ 1) Rename the item in the struct from backend to name, so that it's the same in the XML and in the struct, hopefully avoiding confusion for someone unfamiliar with the function of the attribute. 2) Create a driver union within virDomainNetDef, and a virtio struct in that struct, which contains the name enum value. 3) Move around the virDomainNetParse and virDomainNetFormat functions to allow for simple plugin of new attributes without disturbing existing code. (you'll note that this results in a seemingly redundant if() in the format function, but that will no longer be the case as soon as a 2nd attribute is added). ...all nice changes. ACK to this half of the series. Pushed, now that 0.8.8 is tagged and out. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 2/2] Add txmode attribute to interface XML for virtio backend
On 02/10/2011 03:35 PM, Eric Blake wrote: On 02/10/2011 02:53 AM, Laine Stump wrote: == V3 changes: 0) The actual code remains untouched from V2. 1) Add txmode attribute to the domain schema, and to an xml2xml and xml2argv test (note that the particular file used for the xml2xml test was previously only being used for xml2argv, and it needed some cleaning up before it would work properly. 2) Document the attribute on the website, along with a bold warning to leave it alone! Looks good. +++ b/tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61-device @@ -72,3 +72,28 @@ virtio-blk-pci.ioeventfd=on/off virtio-blk-pci.vectors=uint32 virtio-blk-pci.indirect_desc=on/off virtio-blk-pci.scsi=on/off +virtio-net-pci.vectors=uint32 +virtio-net-pci.indirect_desc=on/off +virtio-net-pci.csum=on/off Nice. Should we also be updating qemu-kvm-0.13.0-device with similar content (using the Fedora 14 qemu-kvm, for example), which has virtio-net-pci but lacks the tx parameter, to prove that the flag does not get set in that test case? ACK with that nit addressed (I trust you can amend that file without having to see a v4). However, I'm not sure if this is okay to push now (since you proposed the series before feature freeze) or not (since it missed the freeze announcement and therefore is missing out on release candidate testing); DV, can you give an answer? Now pushed, with amendment as noted above. Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] virsh: freecell --all getting wrong NUMA nodes count
Virsh freecell --all was not olny getting wrong NUMA nodes count, but
even the NUMA nodes ids. They doesn't have to be continuous, as I've
found out during testing this. Therefore a modification of
nodeGetCellsFreeMemory() error message.
---
src/nodeinfo.c |3 +-
tools/virsh.c | 70
2 files changed, 57 insertions(+), 16 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 22d53e5..ba2f793 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -468,7 +468,8 @@ nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
long long mem;
if (numa_node_size64(n, mem) 0) {
nodeReportError(VIR_ERR_INTERNAL_ERROR,
-%s, _(Failed to query NUMA free memory));
+%s: %d, _(Failed to query NUMA free memory
+for node), n);
goto cleanup;
}
freeMems[numCells++] = mem;
diff --git a/tools/virsh.c b/tools/virsh.c
index 50d5e33..165a791 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2283,9 +2283,15 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
int ret;
int cell, cell_given;
unsigned long long memory;
-unsigned long long *nodes = NULL;
+xmlNodePtr *nodes = NULL;
+unsigned long nodes_cnt;
+unsigned long *nodes_id = NULL;
+unsigned long long *nodes_free = NULL;
int all_given;
-virNodeInfo info;
+int i;
+char *cap_xml = NULL;
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
if (!vshConnectionUsability(ctl, ctl-conn))
@@ -2301,32 +2307,60 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
}
if (all_given) {
-if (virNodeGetInfo(ctl-conn, info) 0) {
-vshError(ctl, %s, _(failed to get NUMA nodes count));
+cap_xml = virConnectGetCapabilities(ctl-conn);
+if (!cap_xml) {
+vshError(ctl, %s, _(unable to get node capabilities));
goto cleanup;
}
-if (!info.nodes) {
-vshError(ctl, %s, _(no NUMA nodes present));
+xml = xmlReadDoc((const xmlChar *) cap_xml, node.xml, NULL,
+ XML_PARSE_NOENT | XML_PARSE_NONET |
+ XML_PARSE_NOWARNING);
+
+if (!xml) {
+vshError(ctl, %s, _(unable to get node capabilities));
goto cleanup;
}
-if (VIR_ALLOC_N(nodes, info.nodes) 0) {
-vshError(ctl, %s, _(could not allocate memory));
+ctxt = xmlXPathNewContext(xml);
+nodes_cnt = virXPathNodeSet(/capabilities/host/topology/cells/cell,
+ctxt, nodes);
+
+if (nodes_cnt == -1) {
+vshError(ctl, %s, _(could not get information about
+ NUMA topology));
goto cleanup;
}
-ret = virNodeGetCellsFreeMemory(ctl-conn, nodes, 0, info.nodes);
-if (ret != info.nodes) {
-vshError(ctl, %s, _(could not get information about
- all NUMA nodes));
+if ((VIR_ALLOC_N(nodes_free, nodes_cnt) 0) ||
+(VIR_ALLOC_N(nodes_id, nodes_cnt) 0)){
+vshError(ctl, %s, _(could not allocate memory));
goto cleanup;
}
+for (i = 0; i nodes_cnt; i++) {
+unsigned long id;
+char *val = virXMLPropString(nodes[i], id);
+char *conv = NULL;
+id = strtoul(val, conv, 10);
+ if (*conv !='\0') {
+ vshError(ctl, %s, _(conversion from string failed));
+ goto cleanup;
+ }
+nodes_id[i]=id;
+ret = virNodeGetCellsFreeMemory(ctl-conn, (nodes_free[i]), id,
1);
+if (ret != 1) {
+vshError(ctl, %s %lu, _(failed to get free memory for
+ NUMA node nuber:), id);
+goto cleanup;
+}
+}
+
memory = 0;
-for (cell = 0; cell info.nodes; cell++) {
-vshPrint(ctl, %5d: %10llu kB\n, cell, (nodes[cell]/1024));
-memory += nodes[cell];
+for (cell = 0; cell nodes_cnt; cell++) {
+vshPrint(ctl, %5lu: %10llu kB\n, nodes_id[cell],
+(nodes_free[cell]/1024));
+memory += nodes_free[cell];
}
vshPrintExtra(ctl, \n);
@@ -2351,7 +2385,13 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
func_ret = TRUE;
cleanup:
+xmlXPathFreeContext(ctxt);
+if (xml)
+xmlFreeDoc(xml);
VIR_FREE(nodes);
+VIR_FREE(nodes_free);
+VIR_FREE(nodes_id);
+VIR_FREE(cap_xml);
return func_ret;
}
--
1.7.3.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Release of libvirt-0.8.8
On 17/02/2011, at 3:39 PM, Daniel Veillard wrote: As scheduled the release is out, and available from the site: ftp://libvirt.org/libvirt/ Thanks everybody for the earlier testing on the release candidate series that was I think quite useful ! This release is now available through MacOS X Homebrew. It's also in queue for MacOS X MacPorts: https://trac.macports.org/ticket/28161 Hoping for this weekend for that, though it's not under my control. Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add support for multiple serial ports into the Xen driver
On 02/17/2011 02:37 AM, Michal Novotny wrote: Eric, thanks for reply but I'm having some issues now... I've pulled libvirt sources from upstream and I'm unable to compile it since it keeps saying following: CCLD libvirt_parthelper ./.libs/libvirt_util.a(libvirt_util_la-util.o): In function `virFileOperationNoFork': /home/mig/Work/virt-related/libvirt/src/util/util.c:1363: undefined reference to `rpl_open' Huh. rpl_open is a gnulib replacement for open(); I'm not sure why gnulib thinks open() needs replacing on Linux (unless it is trying to provide O_CLOEXEC support for older kernels). Was this an incremental build, where you had already built before updating to the latest libvirt.git, and just typed 'make'? Can you try 'rm gnulib/lib/fcntl.h', then 'make', to see if it clears up the issue? If it doesn't, then maybe you'll have to resort to a full-blown clean checkout. Unfortunately I was unable to find definition of 'rpl_open' function using git grep so I don't really know what to do there. Some bug or do I miss something? It's defined by gnulib, which is copied into place from the gnulib.git submodule during ./autogen.sh (and not under libvirt.git's control). Even then, it only exists in the replacement fcntl.h header provided by gnulib when gnulib thinks that open() isn't up to snuff with POSIX requirements. Basically I don't know what you mean by this. Also, when I was defining XMLs I saw that the console node copies the first serial node however how to define it if first serial node is just none? I mean the XML is like: serial type='dev' source path='/dev/ttyS0'/ target port='0'/ /serial serial type='dev' source path='/dev/ttyS1'/ target port='1'/ /serial console type='dev' source path='/dev/ttyS0'/ target port='0'/ /console But what should the XML part be for case of first serial device to be none ? I think the key is the target port='1'/. You don't need any placeholder in the XML for a missing port 0, but you do need it in the xm format where port is implied by position. That is: devices serial type='dev' source path='/dev/ttyS1'/ target port='1'/ /serial /devices should be sufficient XML to mark that you want to use multi-serial xen support, and that you are explicitly leaving virtual port 0 unoccupied and plugging in your one serial device into virtual port 1; mapping to: serial = [ none, /dev/ttyS1 ] in the xm file. The sexpr rationale should be the same. -- 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 v2] Requires gettext for client package
On 02/17/2011 03:05 AM, Osier Yang wrote: libvirt-guests invokes functions in gettext.sh, so we need to require gettext package in spec file. Demo with the fix: % rpm -q gettext package gettext is not installed % rpm -ivh libvirt-client-0.8.8-1.fc14.x86_64.rpm error: Failed dependencies: gettext is needed by libvirt-client-0.8.8-1.fc14.x86_64 +++ b/libvirt.spec.in @@ -415,6 +415,7 @@ Requires: ncurses # So remote clients can access libvirt over SSH tunnel # (client invokes 'nc' against the UNIX socket on the server) Requires: nc [note to self - it might be possible to drop the nc dependency in the future, once I finish my conversion from -migration exec: to -migration fd:, since the spec file only targets distros with new-enough qemu 0.12.0 or newer; nc would still be needed for qemu 0.10.0, though] +Requires: gettext Maybe worth adding a comment before this line, justifying the requires (just like we did before the nc requirement): # Needed by libvirt-guests init script ACK with that nit addressed. -- 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] [PATCH] build: recompute symbols after changing configure options
$ ./configure
...
$ make
...
GENlibvirt.syms
...
$ ./configure --with-driver-modules
...
$ make
...
libvirt.syms doesn't get regenerated but it should as it should
contain virDriverLoadModule now.
* src/Makefile.am (libvirt.syms): Depend on configure changes.
Reported by Matthias Bolte.
---
This is not the first time we've had a file that needs to be
rebuilt after a configure change (see {daemon/tools}/Makefile.am
for precedence).
src/Makefile.am |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 02d5250..30c6cd3 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1074,7 +1074,8 @@ EXTRA_DIST += \
BUILT_SOURCES += libvirt.syms libvirt.def libvirt_qemu.def
-libvirt.syms: libvirt_public.syms $(USED_SYM_FILES)
+libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) \
+ $(top_builddir)/config.status
$(AM_V_GEN)rm -f $@-tmp $@ ; \
printf '# WARNING: generated from the following:\n# $^\n\n' $@-tmp \
cat $(srcdir)/libvirt_public.syms $@-tmp \
--
1.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Requires gettext for client package
On 18/02/2011, at 7:12 AM, Eric Blake wrote: snip [note to self - it might be possible to drop the nc dependency in the future, once I finish my conversion from -migration exec: to -migration fd:, since the spec file only targets distros with new-enough qemu 0.12.0 or newer; nc would still be needed for qemu 0.10.0, though] Does your conversion affect how libvirt uses ssh for general communication? Pretty sure I've seen some part of libvirt (other than migration) use nc + ssh for general communication to remote qemu (ie with qemu+ssh://). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: Expand tabs in python code
On 02/17/2011 07:34 AM, Jiri Denemark wrote: Also cfg.mk is tweaked to force this for all future changes to *.py files. --- cfg.mk |4 +- docs/apibuild.py | 3194 docs/index.py | 834 +++--- python/generator.py| 578 +- python/tests/create.py |8 +- 5 files changed, 2309 insertions(+), 2309 deletions(-) diff --git a/cfg.mk b/cfg.mk index 4bbeb96..f870723 100644 --- a/cfg.mk +++ b/cfg.mk @@ -327,8 +327,8 @@ sc_prohibit_ctype_h: # files in gnulib, since they're imported. sc_TAB_in_indentation: @prohibit='^ * ' \ - in_vc_files='(\.(rng|[ch](\.in)?|html.in)|(daemon|tools)/.*\.in)$$' \ - halt='use leading spaces, not TAB, in C, sh, html, and RNG schemas' \ + in_vc_files='(\.(rng|[ch](\.in)?|html.in|py)|(daemon|tools)/.*\.in)$$' \ + halt='use leading spaces, not TAB, in C, sh, html, py, and RNG schemas' \ $(_sc_search_regexp) ACK. And confirmed that the 'git diff -b' is just the above hunk. -- 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] maint: avoid 'make syntax-check' from tarball
On 02/16/2011 08:14 PM, Daniel Veillard wrote: On Wed, Feb 16, 2011 at 02:39:30PM -0700, Eric Blake wrote: * .gnulib: update to latest gnulib for maint.mk fixes --- In testing the latest release candidate, I ran 'make syntax-check' out of habit, and it died miserably (even hanging waiting for input on stdin). So I fixed maint.mk upstream: http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/25340 However, we're so close to the release that I don't know if we want this now, or if we just document that 0.8.8 is the last release where 'make syntax-check' on the tarball is expected to fail. Let's keep as is for 0.8.8 considering the existing amount of testing of rc3 Assuming this was an ACK for post-release, I've now pushed the patch. -- 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] logging: make VIR_ERROR and friends preserve errno
On 02/17/2011 04:01 AM, Jiri Denemark wrote: Followup to commit 17e19add, and would have prevented the bug independently fixed in commit 76c57a7c. I would have implemented this as if (...) -return; +goto out; ... VIR_FREE(msg); + +out: +errno = saved_errno; } Yeah, that does look a bit nicer. to avoid having to set errno in several places but I can live with your solution too :-) ACK regardless on which one of the two versions you decide to push. Pushed with the modification to use goto. -- 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] check more error info about whether drive_add failed
On 02/16/2011 08:32 PM, Wen Congyang wrote:
When we attach a disk, but we specify a wrong format of disk image,
qemu monitor command drive_add will fail, but libvirt does not detect
this error.
Signed-off-by: Wen Congyang we...@cn.fujitsu.com
---
src/qemu/qemu_monitor_text.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 6d0ba4c..0fd7546 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -2453,6 +2453,12 @@ int qemuMonitorTextAddDrive(qemuMonitorPtr mon,
goto cleanup;
}
+if (strstr(reply, could not open disk image)) {
+qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
+_(open disk image file failed));
+goto cleanup;
+}
ACK and pushed.
--
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] [PATCH 1/5] use virReportErrorHelper instead of xmlGenericError
--- src/util/hash.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/util/hash.c b/src/util/hash.c index c7a52c9..ba156b1 100644 --- a/src/util/hash.c +++ b/src/util/hash.c @@ -190,9 +190,10 @@ virHashGrow(virHashTablePtr table, int size) VIR_FREE(oldtable); #ifdef DEBUG_GROW -xmlGenericError(xmlGenericErrorContext, -virHashGrow : from %d to %d, %d elems\n, oldsize, -size, nbElem); +virReportErrorHelper(NULL, VIR_FROM_NONE, VIR_ERR_OK, __FILE__, + __FUNCTION__, __LINE__, + virHashGrow : from %d to %d, %d elems, + oldsize, size, nbElem); #endif return (0); -- 1.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virHash patches (resend)
Hi, This is a resend of my virHash patches. I rebased my git branch on top of origin/master after the release, since 2 of them needed rediffing, here they are again. Christophe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] add hash table rebalancing in virHashUpdateEntry
The code in virHashUpdateEntry and virHashAddEntry is really
similar. However, the latter rebalances the hash table when
one of its buckets contains too many elements while the former
does not. Fix this discrepancy.
---
src/util/hash.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/util/hash.c b/src/util/hash.c
index ba156b1..595f447 100644
--- a/src/util/hash.c
+++ b/src/util/hash.c
@@ -331,7 +331,7 @@ int
virHashUpdateEntry(virHashTablePtr table, const char *name,
void *userdata, virHashDeallocator f)
{
-unsigned long key;
+unsigned long key, len = 0;
virHashEntryPtr entry;
virHashEntryPtr insert;
char *new_name;
@@ -354,6 +354,7 @@ virHashUpdateEntry(virHashTablePtr table, const char *name,
insert-payload = userdata;
return (0);
}
+len++;
}
if (STREQ(insert-name, name)) {
if (f)
@@ -386,6 +387,10 @@ virHashUpdateEntry(virHashTablePtr table, const char *name,
if (insert != NULL) {
insert-next = entry;
}
+
+if (len MAX_HASH_LEN)
+virHashGrow(table, MAX_HASH_LEN * table-size);
+
return (0);
}
--
1.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] factor common code in virHashAddEntry and virHashUpdateEntry
The only difference between these 2 functions is that one errors
out when the entry is already present while the other modifies
the existing entry. Add an helper function with a boolean argument
indicating whether existing entries should be updated or not, and
use this helper in both functions.
---
src/util/hash.c | 116 +++
1 files changed, 40 insertions(+), 76 deletions(-)
diff --git a/src/util/hash.c b/src/util/hash.c
index 595f447..f19d249 100644
--- a/src/util/hash.c
+++ b/src/util/hash.c
@@ -21,6 +21,7 @@
#include config.h
#include string.h
+#include stdbool.h
#include stdlib.h
#include virterror_internal.h
@@ -243,24 +244,16 @@ virHashFree(virHashTablePtr table, virHashDeallocator f)
VIR_FREE(table);
}
-/**
- * virHashAddEntry:
- * @table: the hash table
- * @name: the name of the userdata
- * @userdata: a pointer to the userdata
- *
- * Add the @userdata to the hash @table. This can later be retrieved
- * by using @name. Duplicate entries generate errors.
- *
- * Returns 0 the addition succeeded and -1 in case of error.
- */
-int
-virHashAddEntry(virHashTablePtr table, const char *name, void *userdata)
+static int
+virHashAddOrUpdateEntry(virHashTablePtr table, const char *name,
+void *userdata, virHashDeallocator f,
+bool is_update)
{
unsigned long key, len = 0;
virHashEntryPtr entry;
virHashEntryPtr insert;
char *new_name;
+bool found;
if ((table == NULL) || (name == NULL))
return (-1);
@@ -268,18 +261,32 @@ virHashAddEntry(virHashTablePtr table, const char *name,
void *userdata)
/*
* Check for duplicate and insertion location.
*/
+found = false;
key = virHashComputeKey(table, name);
if (table-table[key].valid == 0) {
insert = NULL;
} else {
for (insert = (table-table[key]); insert-next != NULL;
insert = insert-next) {
-if (STREQ(insert-name, name))
-return (-1);
+if (STREQ(insert-name, name)) {
+found = true;
+break;
+}
len++;
}
if (STREQ(insert-name, name))
+found = true;
+}
+
+if (found) {
+if (is_update) {
+if (f)
+f(insert-payload, insert-name);
+insert-payload = userdata;
+return (0);
+} else {
return (-1);
+}
}
if (insert == NULL) {
@@ -315,6 +322,23 @@ virHashAddEntry(virHashTablePtr table, const char *name,
void *userdata)
}
/**
+ * virHashAddEntry:
+ * @table: the hash table
+ * @name: the name of the userdata
+ * @userdata: a pointer to the userdata
+ *
+ * Add the @userdata to the hash @table. This can later be retrieved
+ * by using @name. Duplicate entries generate errors.
+ *
+ * Returns 0 the addition succeeded and -1 in case of error.
+ */
+int
+virHashAddEntry(virHashTablePtr table, const char *name, void *userdata)
+{
+return virHashAddOrUpdateEntry(table, name, userdata, NULL, false);
+}
+
+/**
* virHashUpdateEntry:
* @table: the hash table
* @name: the name of the userdata
@@ -331,67 +355,7 @@ int
virHashUpdateEntry(virHashTablePtr table, const char *name,
void *userdata, virHashDeallocator f)
{
-unsigned long key, len = 0;
-virHashEntryPtr entry;
-virHashEntryPtr insert;
-char *new_name;
-
-if ((table == NULL) || name == NULL)
-return (-1);
-
-/*
- * Check for duplicate and insertion location.
- */
-key = virHashComputeKey(table, name);
-if (table-table[key].valid == 0) {
-insert = NULL;
-} else {
-for (insert = (table-table[key]); insert-next != NULL;
- insert = insert-next) {
-if (STREQ(insert-name, name)) {
-if (f)
-f(insert-payload, insert-name);
-insert-payload = userdata;
-return (0);
-}
-len++;
-}
-if (STREQ(insert-name, name)) {
-if (f)
-f(insert-payload, insert-name);
-insert-payload = userdata;
-return (0);
-}
-}
-
-if (insert == NULL) {
-entry = (table-table[key]);
-} else {
-if (VIR_ALLOC(entry) 0)
-return (-1);
-}
-
-new_name= strdup(name);
-if (new_name == NULL) {
-if (insert != NULL)
-VIR_FREE(entry);
-return (-1);
-}
-entry-name = new_name;
-entry-payload = userdata;
-entry-next = NULL;
-entry-valid = 1;
-table-nbElems++;
-
-
-if (insert != NULL) {
-insert-next = entry;
-}
-
-if (len MAX_HASH_LEN)
-virHashGrow(table, MAX_HASH_LEN * table-size);
-
-return (0);
+return virHashAddOrUpdateEntry(table, name, userdata, f, true);
}
/**
--
1.7.4
--
libvir-list mailing list
[libvirt] [PATCH 5/5] don't check for NULL before calling virHashFree
virHashFree follows the convention described in HACKING that
XXXFree() functions can be called with a NULL argument.
---
src/conf/domain_conf.c |6 +---
src/datatypes.c | 51 +++---
src/qemu/qemu_process.c |4 +--
3 files changed, 20 insertions(+), 41 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 351daf7..e7c3409 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -410,8 +410,7 @@ static void virDomainObjListDeallocator(void *payload,
const char *name ATTRIBUT
void virDomainObjListDeinit(virDomainObjListPtr doms)
{
-if (doms-objs)
-virHashFree(doms-objs, virDomainObjListDeallocator);
+virHashFree(doms-objs, virDomainObjListDeallocator);
}
@@ -8770,8 +8769,7 @@ static void virDomainSnapshotObjListDeallocator(void
*payload,
static void virDomainSnapshotObjListDeinit(virDomainSnapshotObjListPtr
snapshots)
{
-if (snapshots-objs)
-virHashFree(snapshots-objs, virDomainSnapshotObjListDeallocator);
+virHashFree(snapshots-objs, virDomainSnapshotObjListDeallocator);
}
struct virDomainSnapshotNameData {
diff --git a/src/datatypes.c b/src/datatypes.c
index d870e5d..7cc37c1 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -209,22 +209,14 @@ virGetConnect(void) {
failed:
if (ret != NULL) {
-if (ret-domains != NULL)
-virHashFree(ret-domains, (virHashDeallocator) virDomainFreeName);
-if (ret-networks != NULL)
-virHashFree(ret-networks, (virHashDeallocator)
virNetworkFreeName);
-if (ret-interfaces != NULL)
- virHashFree(ret-interfaces, (virHashDeallocator)
virInterfaceFreeName);
-if (ret-storagePools != NULL)
-virHashFree(ret-storagePools, (virHashDeallocator)
virStoragePoolFreeName);
-if (ret-storageVols != NULL)
-virHashFree(ret-storageVols, (virHashDeallocator)
virStorageVolFreeName);
-if (ret-nodeDevices != NULL)
-virHashFree(ret-nodeDevices, (virHashDeallocator)
virNodeDeviceFree);
-if (ret-secrets != NULL)
-virHashFree(ret-secrets, (virHashDeallocator) virSecretFreeName);
-if (ret-nwfilters != NULL)
-virHashFree(ret-nwfilters, (virHashDeallocator)
virNWFilterFreeName);
+virHashFree(ret-domains, (virHashDeallocator) virDomainFreeName);
+virHashFree(ret-networks, (virHashDeallocator) virNetworkFreeName);
+virHashFree(ret-interfaces, (virHashDeallocator)
virInterfaceFreeName);
+virHashFree(ret-storagePools, (virHashDeallocator)
virStoragePoolFreeName);
+virHashFree(ret-storageVols, (virHashDeallocator)
virStorageVolFreeName);
+virHashFree(ret-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
+virHashFree(ret-secrets, (virHashDeallocator) virSecretFreeName);
+virHashFree(ret-nwfilters, (virHashDeallocator) virNWFilterFreeName);
virMutexDestroy(ret-lock);
VIR_FREE(ret);
@@ -267,22 +259,14 @@ virReleaseConnect(virConnectPtr conn) {
virMutexLock(conn-lock);
-if (conn-domains != NULL)
-virHashFree(conn-domains, (virHashDeallocator) virDomainFreeName);
-if (conn-networks != NULL)
-virHashFree(conn-networks, (virHashDeallocator) virNetworkFreeName);
-if (conn-interfaces != NULL)
-virHashFree(conn-interfaces, (virHashDeallocator)
virInterfaceFreeName);
-if (conn-storagePools != NULL)
-virHashFree(conn-storagePools, (virHashDeallocator)
virStoragePoolFreeName);
-if (conn-storageVols != NULL)
-virHashFree(conn-storageVols, (virHashDeallocator)
virStorageVolFreeName);
-if (conn-nodeDevices != NULL)
-virHashFree(conn-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
-if (conn-secrets != NULL)
-virHashFree(conn-secrets, (virHashDeallocator) virSecretFreeName);
-if (conn-nwfilters != NULL)
-virHashFree(conn-nwfilters, (virHashDeallocator) virNWFilterFreeName);
+virHashFree(conn-domains, (virHashDeallocator) virDomainFreeName);
+virHashFree(conn-networks, (virHashDeallocator) virNetworkFreeName);
+virHashFree(conn-interfaces, (virHashDeallocator) virInterfaceFreeName);
+virHashFree(conn-storagePools, (virHashDeallocator)
virStoragePoolFreeName);
+virHashFree(conn-storageVols, (virHashDeallocator) virStorageVolFreeName);
+virHashFree(conn-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
+virHashFree(conn-secrets, (virHashDeallocator) virSecretFreeName);
+virHashFree(conn-nwfilters, (virHashDeallocator) virNWFilterFreeName);
virResetError(conn-err);
@@ -435,8 +419,7 @@ virReleaseDomain(virDomainPtr domain) {
domain-magic = -1;
domain-id = -1;
VIR_FREE(domain-name);
-if (domain-snapshots != NULL)
-virHashFree(domain-snapshots, (virHashDeallocator)
virDomainSnapshotFreeName);
+virHashFree(domain-snapshots, (virHashDeallocator)
[libvirt] [PATCH 4/5] remove no longer needed calls to virReportOOMError
Now that the virHash handling functions call virReportOOMError by
themselves when needed, users of the virHash API no longer need to
do it by themselves. Since users of the virHash API were not
consitently calling virReportOOMError after memory failures from
the virHash code, this has the added benefit of making OOM
reporting from this code more consistent and reliable.
---
src/conf/domain_conf.c | 18 --
src/conf/nwfilter_conf.c |4 +---
src/conf/nwfilter_params.c |1 -
src/nwfilter/nwfilter_gentech_driver.c |2 --
src/nwfilter/nwfilter_learnipaddr.c|3 ---
src/openvz/openvz_conf.c |2 +-
src/qemu/qemu_command.c|2 +-
7 files changed, 7 insertions(+), 25 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 01a5b2c..351daf7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -394,10 +394,8 @@ VIR_ENUM_IMPL(virDomainTimerMode,
VIR_DOMAIN_TIMER_MODE_LAST,
int virDomainObjListInit(virDomainObjListPtr doms)
{
doms-objs = virHashCreate(50);
-if (!doms-objs) {
-virReportOOMError();
+if (!doms-objs)
return -1;
-}
return 0;
}
@@ -1061,7 +1059,6 @@ virDomainObjPtr virDomainAssignDef(virCapsPtr caps,
virUUIDFormat(def-uuid, uuidstr);
if (virHashAddEntry(doms-objs, uuidstr, domain) 0) {
VIR_FREE(domain);
-virReportOOMError();
return NULL;
}
@@ -8189,10 +8186,8 @@ static virDomainObjPtr virDomainLoadStatus(virCapsPtr
caps,
goto error;
}
-if (virHashAddEntry(doms-objs, uuidstr, obj) 0) {
-virReportOOMError();
+if (virHashAddEntry(doms-objs, uuidstr, obj) 0)
goto error;
-}
if (notify)
(*notify)(obj, 1, opaque);
@@ -8750,7 +8745,6 @@ virDomainSnapshotObjPtr
virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr s
if (virHashAddEntry(snapshots-objs, snap-def-name, snap) 0) {
VIR_FREE(snap);
-virReportOOMError();
return NULL;
}
@@ -8761,10 +8755,8 @@ virDomainSnapshotObjPtr
virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr s
int virDomainSnapshotObjListInit(virDomainSnapshotObjListPtr snapshots)
{
snapshots-objs = virHashCreate(50);
-if (!snapshots-objs) {
-virReportOOMError();
+if (!snapshots-objs)
return -1;
-}
return 0;
}
@@ -9053,10 +9045,8 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
_(could not close file %s),
path);
-if (virHashAddEntry(paths, path, (void*)0x1) 0) {
-virReportOOMError();
+if (virHashAddEntry(paths, path, (void*)0x1) 0)
goto cleanup;
-}
depth++;
nextpath = meta.backingStore;
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index a5703cb..c6a4d6f 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2300,10 +2300,8 @@ virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
.skipInterfaces = virHashCreate(0),
};
-if (!cb.skipInterfaces) {
-virReportOOMError();
+if (!cb.skipInterfaces)
return 1;
-}
for (i = 0; i nCallbackDriver; i++) {
callbackDrvArray[i]-vmFilterRebuild(conn,
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index 7a59387..23423fa 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -121,7 +121,6 @@ virNWFilterHashTableCreate(int n) {
}
ret-hashTable = virHashCreate(n);
if (!ret-hashTable) {
-virReportOOMError();
VIR_FREE(ret);
return NULL;
}
diff --git a/src/nwfilter/nwfilter_gentech_driver.c
b/src/nwfilter/nwfilter_gentech_driver.c
index 9ef3692..d81aac8 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -1037,8 +1037,6 @@ virNWFilterDomainFWUpdateCB(void *payload,
cb-err = virHashAddEntry(cb-skipInterfaces,
net-ifname,
(void *)~0);
-if (cb-err)
-virReportOOMError();
}
break;
diff --git a/src/nwfilter/nwfilter_learnipaddr.c
b/src/nwfilter/nwfilter_learnipaddr.c
index c593481..02af918 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -165,7 +165,6 @@ virNWFilterLockIface(const char *ifname) {
}
while (virHashAddEntry(ifaceLockMap, ifname, ifaceLock)) {
-virReportOOMError();
VIR_FREE(ifaceLock);
goto err_exit;
}
@@ -825,7 +824,6 @@ virNWFilterLearnInit(void) {
pendingLearnReq = virHashCreate(0);
if (!pendingLearnReq) {
-virReportOOMError();
[libvirt] cleanup patches
Hi, Here are 2 cleanup patches for 2 smallish issues I found when looking at how virHash was used in libvirt. Christophe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] remove space between function name and (
There were several occurrences of an extra space inserted between
a function name and the ( opening the argument list in
datatypes.c. This is not consistent with the coding style used in
the rest of this file so removing this extra space makes the
code slightly more readable.
---
src/datatypes.c | 14 +++---
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/datatypes.c b/src/datatypes.c
index 7cc37c1..1b68f6a 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -243,19 +243,19 @@ virReleaseConnect(virConnectPtr conn) {
virMutexUnlock(conn-lock);
if (conn-networkDriver)
-conn-networkDriver-close (conn);
+conn-networkDriver-close(conn);
if (conn-interfaceDriver)
-conn-interfaceDriver-close (conn);
+conn-interfaceDriver-close(conn);
if (conn-storageDriver)
-conn-storageDriver-close (conn);
+conn-storageDriver-close(conn);
if (conn-deviceMonitor)
-conn-deviceMonitor-close (conn);
+conn-deviceMonitor-close(conn);
if (conn-secretDriver)
-conn-secretDriver-close (conn);
+conn-secretDriver-close(conn);
if (conn-nwfilterDriver)
-conn-nwfilterDriver-close (conn);
+conn-nwfilterDriver-close(conn);
if (conn-driver)
-conn-driver-close (conn);
+conn-driver-close(conn);
virMutexLock(conn-lock);
--
1.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] remove duplicated call to reportOOMError
---
src/qemu/qemu_command.c |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c9feb9b..371a7ed 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4243,7 +4243,6 @@ qemuBuildCommandLine(virConnectPtr conn,
if (configfd = 0) {
if (virAsprintf(configfd_name, %d, configfd) 0) {
VIR_FORCE_CLOSE(configfd);
-virReportOOMError();
goto no_memory;
}
--
1.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Do not add drive 'boot=on' param when a kernel is specified
libvirt-tck was failing several domain tests [1] with qemu 0.14, which
is now less tolerable of specifying 2 bootroms with the same boot index [2].
Drop the 'boot=on' param if kernel has been specfied.
[1] https://www.redhat.com/archives/libvir-list/2011-February/msg00559.html
[2] http://lists.nongnu.org/archive/html/qemu-devel/2011-02/msg01892.html
---
src/qemu/qemu_command.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 618d3a9..a9cc23b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3173,7 +3173,7 @@ qemuBuildCommandLine(virConnectPtr conn,
int bootCD = 0, bootFloppy = 0, bootDisk = 0;
/* If QEMU supports boot=on for -drive param... */
-if (qemuCmdFlags QEMUD_CMD_FLAG_DRIVE_BOOT) {
+if (qemuCmdFlags QEMUD_CMD_FLAG_DRIVE_BOOT !def-os.kernel) {
for (i = 0 ; i def-os.nBootDevs ; i++) {
switch (def-os.bootDevs[i]) {
case VIR_DOMAIN_BOOT_CDROM:
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Still in freeze, third rc tarball available
Daniel P. Berrange wrote:
On Wed, Feb 16, 2011 at 05:10:10PM -0700, Jim Fehlig wrote:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 05f427c..9186707 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3112,7 +3112,7 @@ qemuBuildCommandLine(virConnectPtr conn,
}
/* If QEMU supports -drive param instead of old -hda, -hdb, -cdrom .. */
-if (qemuCmdFlags QEMUD_CMD_FLAG_DRIVE) {
+if (qemuCmdFlags QEMUD_CMD_FLAG_DRIVE !def-os.kernel) {
int bootCD = 0, bootFloppy = 0, bootDisk = 0;
/* If QEMU supports boot=on for -drive param... */
This patch prevents libvirt actually creating any disks at all. Your
idea sounds good though. I think if you put the ' !def-os.kernel'
bit in the next conditional it will work, eg try
/* If QEMU supports boot=on for -drive param... */
if ((qemuCmdFlags QEMUD_CMD_FLAG_DRIVE_BOOT)
!def-os.kernel) {
Err, right. That's the patch I actually tested. I doubt many of the
tck tests would have passed with the version I posted here :-).
The correct patch has been sent ...
Thanks,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not add drive 'boot=on' param when a kernel is specified
On 02/17/2011 02:38 PM, Jim Fehlig wrote:
libvirt-tck was failing several domain tests [1] with qemu 0.14, which
is now less tolerable of specifying 2 bootroms with the same boot index [2].
Drop the 'boot=on' param if kernel has been specfied.
[1] https://www.redhat.com/archives/libvir-list/2011-February/msg00559.html
[2] http://lists.nongnu.org/archive/html/qemu-devel/2011-02/msg01892.html
---
src/qemu/qemu_command.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 618d3a9..a9cc23b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3173,7 +3173,7 @@ qemuBuildCommandLine(virConnectPtr conn,
int bootCD = 0, bootFloppy = 0, bootDisk = 0;
/* If QEMU supports boot=on for -drive param... */
-if (qemuCmdFlags QEMUD_CMD_FLAG_DRIVE_BOOT) {
+if (qemuCmdFlags QEMUD_CMD_FLAG_DRIVE_BOOT !def-os.kernel) {
ACK; that looks better than v1.
--
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] 0.8.8 and virtio console
Hi all, This used to work on libvirt-0.8.7: console type='pty' target type='virtio'/ /console Using the same configuration with libvirt-0.8.8: [root@ev004 ~]# virsh create /data/4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355.xml error: Failed to create domain from /data/4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355.xml error: internal error no assigned pty for device console0 I'm using qemu-kvm-0.14.0-0.1.201102107aa8c46. Here's the log from starting a guest with 0.8.7: LC_ALL=C PATH=/sbin:/usr/sbin:/bin:/usr/bin QEMU_AUDIO_DRV=none /usr/bin/qemu-kvm -S -M pc-0.14 -cpu core2duo,+lahf_lm,+popcnt,+sse4.2,+sse4.1,+cx16,-monitor,-vme -enable-kvm -m 512 -smp 2,sockets=2,cores=1,threads=1 -name 4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355 -uuid 4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355 -nodefconfig -nodefaults -chardev socket,id=monitor,path=/var/lib/libvirt/qemu/4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355.monitor,server,nowait -mon chardev=monitor,mode=control -rtc base=utc -boot c -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive file=/dev/vgdata/4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355-root,if=none,id=drive-virtio-disk0,boot=on,format=raw,cache=none -device virtio-blk-pci,bus=pci.0,addr=0x6,drive=drive-virtio-disk0,id=virtio-disk0 -drive file=/dev/vgdata/4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355-swap,if=none,id=drive-virtio-disk1,format=raw,cache=none -device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk1,id=virtio-disk1 -netdev tap,fd=64,id=hostnet0,vhost=on,vhostfd=65 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:16:3e:50:a4:55,bus=pci.0,addr=0x3 -netdev tap,fd=66,id=hostnet1,vhost=on,vhostfd=67 -device virtio-net-pci,netdev=hostnet1,id=net1,mac=00:16:3e:71:c1:49,bus=pci.0,addr=0x4 -chardev pty,id=console0 -device virtconsole,chardev=console0 -usb -device usb-tablet,id=input0 -vnc 0.0.0.0:1,password -vga std char device redirected to /dev/pts/14 And the same guest with 0.8.8: LC_ALL=C PATH=/sbin:/usr/sbin:/bin:/usr/bin QEMU_AUDIO_DRV=none /usr/bin/qemu-kvm -S -M pc-0.14 -cpu core2duo,+lahf_lm,+popcnt,+sse4.2,+sse4.1,+cx16,-monitor,-vme -enable-kvm -m 512 -smp 2,sockets=2,cores=1,threads=1 -name 4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355 -uuid 4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -boot c -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive file=/dev/vgdata/4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355-root,if=none,id=drive-virtio-disk0,boot=on,format=raw,cache=none -device virtio-blk-pci,bus=pci.0,addr=0x6,drive=drive-virtio-disk0,id=virtio-disk0 -drive file=/dev/vgdata/4d5d8a24-bb70-4eff-b1b5-3d8e5bd5c355-swap,if=none,id=drive-virtio-disk1,format=raw,cache=none -device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk1,id=virtio-disk1 -netdev tap,fd=33,id=hostnet0,vhost=on,vhostfd=34 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:16:3e:50:a4:55,bus=pci.0,addr=0x3 -netdev tap,fd=35,id=hostnet1,vhost=on,vhostfd=36 -device virtio-net-pci,netdev=hostnet1,id=net1,mac=00:16:3e:71:c1:49,bus=pci.0,addr=0x4 -chardev pty,id=charconsole0 -device virtconsole,chardev=charconsole0,id=console0 -usb -device usb-tablet,id=input0 -vnc 0.0.0.0:1,password -vga std char device redirected to /dev/pts/14 Kind regards, Ruben Kerkhof -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] virsh: fix wrong NUMA nodes count getting
On 02/17/2011 06:39 AM, Michal Privoznik wrote:
as written in docs, one must parse capabilities XML to get the right
count of NUMA nodes.
---
tools/virsh.c | 27 ---
1 files changed, 16 insertions(+), 11 deletions(-)
-if (!info.nodes) {
-vshError(ctl, %s, _(no NUMA nodes present));
-goto cleanup;
-}
+ret = xPathULong(node_xml,
+
(char*)string(/capabilities/host/topology/cells/@num),
+ nodes_cnt);
+
+if (ret)
+nodes_cnt = 1;
As Matthias pointed out, we already have a parse function. If you ditch
patch 1/2, then squash this into 2/2, does it all work for you (only
compile-tested here)?
diff --git i/tools/virsh.c w/tools/virsh.c
index dd844ea..a2bff60 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -2287,7 +2287,8 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
unsigned long nodes_cnt;
int all_given;
char *node_xml;
-
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
if (!vshConnectionUsability(ctl, ctl-conn))
return FALSE;
@@ -2308,12 +2309,24 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
-ret = xPathULong(node_xml,
-
(char*)string(/capabilities/host/topology/cells/@num),
- nodes_cnt);
+xml = xmlReadDoc((const xmlChar *) node_xml,
capabilities.xml, NULL,
+ XML_PARSE_NOENT | XML_PARSE_NONET |
+ XML_PARSE_NOWARNING);
+if (!xml)
+goto cleanup;
+
+ctxt = xmlXPathNewContext(xml);
+if (!ctxt)
+goto cleanup;
-if (ret)
+ret =
virXPathULong(string(/capabilities/host/topology/cells/@num),
+ctxt, nodes_cnt);
+if (ret == -1) {
nodes_cnt = 1;
+} else if (ret 0) {
+vshError(ctl, %s, _(unable to determine number of nodes));
+goto cleanup;
+}
if (VIR_ALLOC_N(nodes, nodes_cnt) 0) {
vshError(ctl, %s, _(could not allocate memory));
@@ -2357,6 +2370,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
cleanup:
VIR_FREE(nodes);
VIR_FREE(node_xml);
+xmlXPathFreeContext(ctxt);
return func_ret;
}
--
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 0/2] Refactored XM and SEXPR parsing
On 02/17/2011 06:14 AM, Daniel P. Berrange wrote: On Thu, Feb 17, 2011 at 01:45:42PM +0100, Markus Groß wrote: Hi, I refactored the XM and SEXPR parsing routines from the xen-unified driver into a seperate directory (xenxs; x for xm and s for sexpr). This way different xen-drivers besides xen-unified are able to use the parsing functionality. For example the upcoming XenLight (libxl) driver. To use the XM parsing functions one includes xen_xm.h and for SEXPR parsing xen_sxpr.h. The patch is rather big, but most of it are code movements. FYI in case people are wondering where the patch is, it was blocked at mailman level because it was seriously huge! After discussion with Markus on IRC, we have figured a way to split it up into smaller pieces for reposting Also, if it involves file movement, then be sure you are using git's diff.rename = true configuration option, as it compresses the file motion to only show the few tweaks needed across the rename, rather than showing the entire old file as deleted and new file as created. -- 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: improve 'make install' for VPATH builds
On 02/17/2011 08:06 AM, Jiri Denemark wrote:
On Wed, Feb 16, 2011 at 16:19:38 -0700, Eric Blake wrote:
Anything distributed in the tarball should be built into srcdir.
* docs/Makefile.am (%.html): Build into srcdir.
---
This gets a VPATH build further, but things still aren't complete for
doing 'make install' from a brand-new clone (or from autobuild.sh). I
don't know how to make xsltproc output the files
{html,devhelp}/libvirt-{libvirt,virterror}.html into $(srcdir).
I haven't tried that but xsltproc should support -o directory/
My recollection was that it didn't quite work:
https://www.redhat.com/archives/libvir-list/2010-June/msg00675.html
But I got stumped at that point.
docs/Makefile.am |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
ACK
At any rate, even if this patch is incomplete as-is, it's better than
nothing, so I've pushed it.
--
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 1/5] use virReportErrorHelper instead of xmlGenericError
On 02/17/2011 02:14 PM, Christophe Fergeau wrote: --- src/util/hash.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/util/hash.c b/src/util/hash.c index c7a52c9..ba156b1 100644 --- a/src/util/hash.c +++ b/src/util/hash.c @@ -190,9 +190,10 @@ virHashGrow(virHashTablePtr table, int size) VIR_FREE(oldtable); #ifdef DEBUG_GROW -xmlGenericError(xmlGenericErrorContext, -virHashGrow : from %d to %d, %d elems\n, oldsize, -size, nbElem); +virReportErrorHelper(NULL, VIR_FROM_NONE, VIR_ERR_OK, __FILE__, + __FUNCTION__, __LINE__, + virHashGrow : from %d to %d, %d elems, + oldsize, size, nbElem); I'd rather use VIR_DEBUG than virReportErrorHelper here. I pushed this instead: diff --git i/src/util/hash.c w/src/util/hash.c index c7a52c9..41df7ae 100644 --- i/src/util/hash.c +++ w/src/util/hash.c @@ -3,6 +3,7 @@ * * Reference: Your favorite introductory book on algorithms * + * Copyright (C) 2011 Red Hat, Inc. * Copyright (C) 2000 Bjorn Reese and Daniel Veillard. * * Permission to use, copy, modify, and distribute this software for any @@ -26,6 +27,7 @@ #include virterror_internal.h #include hash.h #include memory.h +#include logging.h #define VIR_FROM_THIS VIR_FROM_NONE @@ -190,9 +192,8 @@ virHashGrow(virHashTablePtr table, int size) VIR_FREE(oldtable); #ifdef DEBUG_GROW -xmlGenericError(xmlGenericErrorContext, -virHashGrow : from %d to %d, %d elems\n, oldsize, -size, nbElem); +VIR_DEBUG(virHashGrow : from %d to %d, %ld elems\n, oldsize, + size, nbElem); #endif return (0); -- 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 3/5] factor common code in virHashAddEntry and virHashUpdateEntry
On 02/17/2011 02:14 PM, Christophe Fergeau wrote: The only difference between these 2 functions is that one errors out when the entry is already present while the other modifies the existing entry. Add an helper function with a boolean argument indicating whether existing entries should be updated or not, and use this helper in both functions. --- src/util/hash.c | 116 +++ 1 files changed, 40 insertions(+), 76 deletions(-) ACK, and the diffstat shows how nice it is. -- 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 2/5] add hash table rebalancing in virHashUpdateEntry
On 02/17/2011 02:14 PM, Christophe Fergeau wrote: The code in virHashUpdateEntry and virHashAddEntry is really similar. However, the latter rebalances the hash table when one of its buckets contains too many elements while the former does not. Fix this discrepancy. --- src/util/hash.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) ACK and pushed. -- 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 4/5] remove no longer needed calls to virReportOOMError
On 02/17/2011 02:14 PM, Christophe Fergeau wrote:
Now that the virHash handling functions call virReportOOMError by
themselves when needed, users of the virHash API no longer need to
do it by themselves. Since users of the virHash API were not
consitently calling virReportOOMError after memory failures from
s/consitently/consistently/
diff --git a/src/nwfilter/nwfilter_learnipaddr.c
b/src/nwfilter/nwfilter_learnipaddr.c
index c593481..02af918 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -165,7 +165,6 @@ virNWFilterLockIface(const char *ifname) {
}
while (virHashAddEntry(ifaceLockMap, ifname, ifaceLock)) {
-virReportOOMError();
VIR_FREE(ifaceLock);
goto err_exit;
}
What an odd-looking control flow, when an 'if' would have been more
idiomatic. But independent of this patch.
You missed one in qemu_process:
diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c
index 46da600..5237e2b 100644
--- i/src/qemu/qemu_process.c
+++ w/src/qemu/qemu_process.c
@@ -995,10 +995,8 @@ qemuProcessWaitForMonitor(struct qemud_driver* driver,
* Note that the monitor itself can be on a pty, so we still need
to try the
* log output method. */
paths = virHashCreate(0);
-if (paths == NULL) {
-virReportOOMError();
+if (paths == NULL)
goto cleanup;
-}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
qemuDomainObjPrivatePtr priv = vm-privateData;
ACK with those corrections, so pushed.
--
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 5/5] don't check for NULL before calling virHashFree
[adding Jim to cc, as author of useless-if-before-free]
On 02/17/2011 02:14 PM, Christophe Fergeau wrote:
virHashFree follows the convention described in HACKING that
XXXFree() functions can be called with a NULL argument.
---
src/conf/domain_conf.c |6 +---
src/datatypes.c | 51 +++---
src/qemu/qemu_process.c |4 +--
3 files changed, 20 insertions(+), 41 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 351daf7..e7c3409 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -410,8 +410,7 @@ static void virDomainObjListDeallocator(void *payload,
const char *name ATTRIBUT
void virDomainObjListDeinit(virDomainObjListPtr doms)
{
-if (doms-objs)
-virHashFree(doms-objs, virDomainObjListDeallocator);
+virHashFree(doms-objs, virDomainObjListDeallocator);
I tried adding --name=virHashFree to the useless_free_options variable
in cfg.mk, to see if that would prevent regressions. However, it
appears that this two-argument free-like function is not picked up by
the heuristics in the useless-if-before-free script (it only works on
one-argument functions).
ACK to your patch as-is, so I pushed it. But I can't help but wonder if
we could make this easier to enforce at 'make syntax-check' time by
tweaking the perl script somehow.
--
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 1/2] remove space between function name and (
On 02/17/2011 02:18 PM, Christophe Fergeau wrote:
There were several occurrences of an extra space inserted between
a function name and the ( opening the argument list in
datatypes.c. This is not consistent with the coding style used in
the rest of this file so removing this extra space makes the
code slightly more readable.
---
src/datatypes.c | 14 +++---
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/datatypes.c b/src/datatypes.c
index 7cc37c1..1b68f6a 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -243,19 +243,19 @@ virReleaseConnect(virConnectPtr conn) {
virMutexUnlock(conn-lock);
if (conn-networkDriver)
-conn-networkDriver-close (conn);
+conn-networkDriver-close(conn);
ACK and pushed.
In the past, there has been talk about running the entire code base
through a formatting tool, but we haven't yet found one that quite
matches existing coding style, so it has been a series of manually
touching up inconsistencies while in the area.
--
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 2/2] remove duplicated call to reportOOMError
On 02/17/2011 02:18 PM, Christophe Fergeau wrote:
---
src/qemu/qemu_command.c |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c9feb9b..371a7ed 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4243,7 +4243,6 @@ qemuBuildCommandLine(virConnectPtr conn,
if (configfd = 0) {
if (virAsprintf(configfd_name, %d, configfd) 0)
{
VIR_FORCE_CLOSE(configfd);
-virReportOOMError();
goto no_memory;
ACK and pushed.
--
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 v2] Requires gettext for client package
On 02/17/2011 01:19 PM, Justin Clift wrote:
On 18/02/2011, at 7:12 AM, Eric Blake wrote:
snip
[note to self - it might be possible to drop the nc dependency in the
future, once I finish my conversion from -migration exec: to -migration
fd:, since the spec file only targets distros with new-enough qemu
0.12.0 or newer; nc would still be needed for qemu 0.10.0, though]
Does your conversion affect how libvirt uses ssh for general communication?
Pretty sure I've seen some part of libvirt (other than migration) use
nc + ssh for general communication to remote qemu (ie with qemu+ssh://).
git grep 'nc' has two hits:
src/qemu/qemu_migration.c:const char *args[] = { nc, -U,
unixfile, NULL };
Will still need to exist for qemu 0.10.x and 0.11.x (so we'd need it for
RHEL 5.6, but not for Fedora or RHEL 6; we've already got other
RHEL-version-related checks in the specfile to make that a conditional
dependency).
src/remote/remote_driver.c:cmd_argv[j++] = strdup (netcat ?
netcat : nc);
Oh wow - we need to convert remote_driver.c over to virCommand.
However, that's the only other use of nc, and it involves running nc
on the remote host, not on the local host, so you _don't_ need to
'Requires: nc' just because you installed libvirt-client; rather, it
seems like it should be libvirt (the server package) that should be
requiring nc.
In other words, I think we have a missing dependency if you install
libvirt but not libvirt-client; the reality is that both packages should
require nc (and libvirt-client can make the dependency conditional if I
improve migration on newer qemu).
--
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 0/6 v3] Add blkio cgroup support
Dominik Klein wrote: Hi not that I tested this patch or the patch that followed (adding blkiotune command) ... but the way I understand, the cgroup blkio controller does only work for sync'd IO/queues. quote blkio-controller.txt from 2.6.37: Currently only sync IO queues are support. All the buffered writes are still system wide and not per group. When I last tested this (using manual cgroups commands), this seemed true. So may I ask how you use this and how this helps in which setup? Hi, Dominik Actually, this two series doesn't care how blkio cgroup works. They just provide the ability to put Guest into given cgroup and tune the tunables. Currently, You can only control the blkio.weight tunable. For examle, If you'd like to give more I/O bandwidth to a given Guest, you can assign a bigger value to blkio.weight by running blkiotune command. Thanks, Gui Regards Dominik On 02/08/2011 07:38 AM, Gui Jianfeng wrote: Hi This patchset adds blkio cgroup support for qemu and lxc. [PATCH 1/6] cgroup: Enable cgroup hierarchy for blkio cgroup [PATCH 2/6 v3] cgroup: Implement blkio.weight tuning API. [PATCH 3/6] Update XML Schema for new entries. [PATCH 4/6 v3] qemu: Implement blkio tunable XML configuration and parsing. [PATCH 5/6 v3] LXC: LXC Blkio weight configuration support. [PATCH 6/6] Add documentation for blkiotune elements. Will post a patchset to implement virsh command blkiotune to tune blkio cgroup parameter later on. v2 - v3 Changes: o Remove an unused local variable o Rename virCgroup(Set/Get)Weight to virCgroup(Set/Get)BlkioWeight o Add documentation in docs/formatdomain.html.in o Update XML Schema for new entries. docs/formatdomain.html.in | 10 ++ docs/schemas/domain.rng | 20 src/conf/domain_conf.c| 13 + src/conf/domain_conf.h|4 src/libvirt_private.syms |2 ++ src/lxc/lxc_controller.c | 10 ++ src/qemu/qemu_cgroup.c| 16 +++- src/qemu/qemu_conf.c |3 ++- src/util/cgroup.c | 41 - src/util/cgroup.h |4 10 files changed, 120 insertions(+), 3 deletions(-) Thanks Gui -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Regards Gui Jianfeng -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6 v2] qemu: Implement domainSetBlkioParamters and domainGetBlkioParamters for QEmu
Implement domainSetBlkioParamters and domainGetBlkioParamters for QEmu
v1- v2 Changes:
- Improve error message output suggested by Osier.
Signed-off-by: Gui Jianfeng guijianf...@cn.fujitsu.com
---
src/qemu/qemu_driver.c | 162 +++-
1 files changed, 160 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 882f323..abaefca 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -93,6 +93,7 @@
#define QEMU_NB_MEM_PARAM 3
+#define QEMU_NB_BLKIO_PARAM 1
#define timeval_to_ms(tv) (((tv).tv_sec * 1000ull) + ((tv).tv_usec /
1000))
@@ -7009,6 +7010,163 @@ cleanup:
return ret;
}
+static int qemuDomainSetBlkioParameters(virDomainPtr dom,
+ virBlkioParameterPtr params,
+ int nparams,
+ unsigned int flags ATTRIBUTE_UNUSED)
+{
+struct qemud_driver *driver = dom-conn-privateData;
+int i;
+virCgroupPtr group = NULL;
+virDomainObjPtr vm = NULL;
+int ret = -1;
+
+qemuDriverLock(driver);
+if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
+qemuReportError(VIR_ERR_NO_SUPPORT, _(blkio cgroup isn't mounted));
+goto cleanup;
+}
+
+vm = virDomainFindByUUID(driver-domains, dom-uuid);
+
+if (vm == NULL) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(No such domain %s), dom-uuid);
+goto cleanup;
+}
+
+if (virCgroupForDomain(driver-cgroup, vm-def-name, group, 0) != 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(cannot find cgroup for domain %s), vm-def-name);
+goto cleanup;
+}
+
+ret = 0;
+for (i = 0; i nparams; i++) {
+virBlkioParameterPtr param = params[i];
+
+if (STREQ(param-field, VIR_DOMAIN_BLKIO_WEIGHT)) {
+int rc;
+if (param-type != VIR_DOMAIN_BLKIO_PARAM_UINT) {
+qemuReportError(VIR_ERR_INVALID_ARG, %s,
+_(invalid type for blkio weight tunable,
expected a 'unsigned int'));
+ret = -1;
+continue;
+}
+
+if (params[i].value.ui 1000 || params[i].value.ui 100) {
+qemuReportError(VIR_ERR_INVALID_ARG, %s,
+_(out of blkio weight range.));
+ret = -1;
+continue;
+}
+
+rc = virCgroupSetBlkioWeight(group, params[i].value.ui);
+if (rc != 0) {
+virReportSystemError(-rc, %s,
+ _(unable to set blkio weight tunable));
+ret = -1;
+}
+} else {
+qemuReportError(VIR_ERR_INVALID_ARG,
+_(Parameter `%s' not supported), param-field);
+ret = -1;
+}
+}
+
+cleanup:
+virCgroupFree(group);
+if (vm)
+virDomainObjUnlock(vm);
+qemuDriverUnlock(driver);
+return ret;
+}
+
+static int qemuDomainGetBlkioParameters(virDomainPtr dom,
+ virBlkioParameterPtr params,
+ int *nparams,
+ unsigned int flags ATTRIBUTE_UNUSED)
+{
+struct qemud_driver *driver = dom-conn-privateData;
+int i;
+virCgroupPtr group = NULL;
+virDomainObjPtr vm = NULL;
+unsigned int val;
+int ret = -1;
+int rc;
+
+qemuDriverLock(driver);
+
+if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
+qemuReportError(VIR_ERR_NO_SUPPORT, _(blkio cgroup isn't mounted));
+goto cleanup;
+}
+
+vm = virDomainFindByUUID(driver-domains, dom-uuid);
+
+if (vm == NULL) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(No such domain %s), dom-uuid);
+goto cleanup;
+}
+
+if ((*nparams) == 0) {
+/* Current number of blkio parameters supported by cgroups */
+*nparams = QEMU_NB_BLKIO_PARAM;
+ret = 0;
+goto cleanup;
+}
+
+if ((*nparams) != QEMU_NB_BLKIO_PARAM) {
+qemuReportError(VIR_ERR_INVALID_ARG,
+%s, _(Invalid parameter count));
+goto cleanup;
+}
+
+if (virCgroupForDomain(driver-cgroup, vm-def-name, group, 0) != 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(cannot find cgroup for domain %s), vm-def-name);
+goto cleanup;
+}
+
+for (i = 0; i *nparams; i++) {
+virBlkioParameterPtr param = params[i];
+val = 0;
+param-value.ui = 0;
+param-type = VIR_DOMAIN_BLKIO_PARAM_UINT;
+
+switch(i) {
+case 0: /* fill blkio weight here */
+rc = virCgroupGetBlkioWeight(group, val);
+if (rc != 0) {
+
[libvirt] [PATCH 4/6 v2] virsh: Adding blkiotune command to virsh tool
Adding blkiotune command to virsh tool
v1 - v2 Changes:
- Fix some typo.
Signed-off-by: Gui Jianfeng guijianf...@cn.fujitsu.com
---
tools/virsh.c | 127 +
1 files changed, 127 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 50d5e33..864900d 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2973,6 +2973,132 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
}
/*
+ * blkiotune command
+ */
+static const vshCmdInfo info_blkiotune[] = {
+{help, N_(Get or set blkio parameters)},
+{desc, N_(Get or set the current blkio parameters for a guest \
+ domain.\n \
+To get the blkio parameters use following command: \n\n \
+virsh # blkiotune domain)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_blkiotune[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{weight, VSH_OT_INT, VSH_OFLAG_NONE,
+ N_(IO Weight in range [100, 1000])},
+{NULL, 0, 0, NULL}
+};
+
+static int
+cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
+{
+virDomainPtr dom;
+int weight;
+int nparams = 0;
+unsigned int i = 0;
+virBlkioParameterPtr params = NULL, temp = NULL;
+int ret = FALSE;
+int found;
+
+if (!vshConnectionUsability(ctl, ctl-conn))
+return FALSE;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return FALSE;
+
+weight = vshCommandOptInt(cmd, weight, found);
+if (found) {
+nparams++;
+if (weight 0) {
+virDomainFree(dom);
+vshError(ctl, _(Invalid value of %d for I/O weight), weight);
+return FALSE;
+}
+}
+
+if (nparams == 0) {
+/* get the number of blkio parameters */
+if (virDomainGetBlkioParameters(dom, NULL, nparams, 0) != 0) {
+vshError(ctl, %s,
+ _(Unable to get number of blkio parameters));
+goto cleanup;
+}
+
+if (nparams == 0) {
+/* nothing to output */
+ret = TRUE;
+goto cleanup;
+}
+
+/* now go get all the blkio parameters */
+params = vshCalloc(ctl, nparams, sizeof(*params));
+if (virDomainGetBlkioParameters(dom, params, nparams, 0) != 0) {
+vshError(ctl, %s, _(Unable to get blkio parameters));
+goto cleanup;
+}
+
+for (i = 0; i nparams; i++) {
+switch (params[i].type) {
+case VIR_DOMAIN_BLKIO_PARAM_INT:
+vshPrint(ctl, %-15s: %d\n, params[i].field,
+ params[i].value.i);
+break;
+case VIR_DOMAIN_BLKIO_PARAM_UINT:
+vshPrint(ctl, %-15s: %u\n, params[i].field,
+ params[i].value.ui);
+break;
+case VIR_DOMAIN_BLKIO_PARAM_LLONG:
+vshPrint(ctl, %-15s: %lld\n, params[i].field,
+ params[i].value.l);
+break;
+case VIR_DOMAIN_BLKIO_PARAM_ULLONG:
+vshPrint(ctl, %-15s: %llu\n, params[i].field,
+ params[i].value.ul);
+break;
+case VIR_DOMAIN_BLKIO_PARAM_DOUBLE:
+vshPrint(ctl, %-15s: %f\n, params[i].field,
+ params[i].value.d);
+break;
+case VIR_DOMAIN_BLKIO_PARAM_BOOLEAN:
+vshPrint(ctl, %-15s: %d\n, params[i].field,
+ params[i].value.b);
+break;
+default:
+vshPrint(ctl, unimplemented blkio parameter type\n);
+}
+}
+
+ret = TRUE;
+} else {
+/* set the blkio parameters */
+params = vshCalloc(ctl, nparams, sizeof(*params));
+
+for (i = 0; i nparams; i++) {
+temp = params[i];
+temp-type = VIR_DOMAIN_BLKIO_PARAM_UINT;
+
+if (weight) {
+temp-value.ui = (unsigned int)weight;
+strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
+sizeof(temp-field));
+weight = 0;
+}
+}
+if (virDomainSetBlkioParameters(dom, params, nparams, 0) != 0)
+vshError(ctl, %s, _(Unable to change blkio parameters));
+else
+ret = TRUE;
+}
+
+ cleanup:
+VIR_FREE(params);
+virDomainFree(dom);
+return ret;
+}
+
+/*
* memtune command
*/
static const vshCmdInfo info_memtune[] = {
@@ -10287,6 +10413,7 @@ static const vshCmdDef domManagementCmds[] = {
{managedsave, cmdManagedSave, opts_managedsave, info_managedsave},
{managedsave-remove, cmdManagedSaveRemove, opts_managedsaveremove,
info_managedsaveremove},
{maxvcpus, cmdMaxvcpus, opts_maxvcpus, info_maxvcpus},
+
[libvirt] [PATCH 6/6 v2] virsh: Documents for blkiotune
Documents for blkiotune. Signed-off-by: Gui Jianfeng guijianf...@cn.fujitsu.com --- tools/virsh.pod |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index a2ca384..014d953 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -610,6 +610,10 @@ of 4MB, while 266240 (260MB) is valid without rounding. Note, to change the maximum memory allocation for a QEMU/KVM guest domain, use the virsh Bedit command instead to update its XML memory element. +=item Bblkiotune Idomain-id optional I--weight Bweight + +Display or set the blkio cgroup parameters. QEMU/KVM supports I--weight. + =item Bmemtune Idomain-id optional I--hard-limit Bkilobytes optional I--soft-limit Bkilobytes optional I--swap-hard-limit Bkilobytes -I--min-guarantee Bkilobytes -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6 v2] virsh: Documents for blkiotune
Eric Blake wrote: On 02/17/2011 07:05 PM, Gui Jianfeng wrote: Documents for blkiotune. Signed-off-by: Gui Jianfeng guijianf...@cn.fujitsu.com --- tools/virsh.pod |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index a2ca384..014d953 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -610,6 +610,10 @@ of 4MB, while 266240 (260MB) is valid without rounding. Note, to change the maximum memory allocation for a QEMU/KVM guest domain, use the virsh Bedit command instead to update its XML memory element. +=item Bblkiotune Idomain-id optional I--weight Bweight + +Display or set the blkio cgroup parameters. QEMU/KVM supports I--weight. + Is it worth listing the range 100-1000? (Am I right on that range?) Yes, range is [100, 1000] now. listing the range here makes more sense i think. Thanks, Gui -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6 v2] virsh: Documents for blkiotune
Eric Blake wrote: On 02/17/2011 07:05 PM, Gui Jianfeng wrote: Documents for blkiotune. Signed-off-by: Gui Jianfeng guijianf...@cn.fujitsu.com --- tools/virsh.pod |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index a2ca384..014d953 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -610,6 +610,10 @@ of 4MB, while 266240 (260MB) is valid without rounding. Note, to change the maximum memory allocation for a QEMU/KVM guest domain, use the virsh Bedit command instead to update its XML memory element. +=item Bblkiotune Idomain-id optional I--weight Bweight + +Display or set the blkio cgroup parameters. QEMU/KVM supports I--weight. + Is it worth listing the range 100-1000? (Am I right on that range?) Update it. From: Gui Jianfeng guijianf...@cn.fujitsu.com Documents for blkiotune Signed-off-by: Gui Jianfeng guijianf...@cn.fujitsu.com --- tools/virsh.pod |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index a2ca384..83a86af 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -610,6 +610,11 @@ of 4MB, while 266240 (260MB) is valid without rounding. Note, to change the maximum memory allocation for a QEMU/KVM guest domain, use the virsh Bedit command instead to update its XML memory element. +=item Bblkiotune Idomain-id optional I--weight Bweight + +Display or set the blkio cgroup parameters. QEMU/KVM supports I--weight. +I--weight is in range [100, 1000]. + =item Bmemtune Idomain-id optional I--hard-limit Bkilobytes optional I--soft-limit Bkilobytes optional I--swap-hard-limit Bkilobytes -I--min-guarantee Bkilobytes -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] check device-mapper when building without storage mpath
When I build libvirt without libvirtd, I receive the following errors: GENvirsh.1 CCLD virsh ../src/.libs/libvirt.so: undefined reference to `dm_is_dm_major' collect2: ld returned 1 exit status make[3]: *** [virsh] Error 1 My build step: # ./autogen.sh --without-libvirtd # make dist # rpmbuild --nodeps --define _sourcedir `pwd` --define _without_libvirtd 1 -ba libvirt.spec This bug was caused by commit df1011ca. Signed-off-by: Wen Congyang we...@cn.fujitsu.com --- configure.ac | 35 --- 1 files changed, 16 insertions(+), 19 deletions(-) diff --git a/configure.ac b/configure.ac index 2bb6918..0f81484 100644 --- a/configure.ac +++ b/configure.ac @@ -1697,25 +1697,22 @@ if test $with_storage_mpath = check test $with_linux = yes; then fi AM_CONDITIONAL([WITH_STORAGE_MPATH], [test $with_storage_mpath = yes]) -if test $with_storage_mpath = yes; then - DEVMAPPER_CFLAGS= - DEVMAPPER_LIBS= - PKG_CHECK_MODULES([DEVMAPPER], [devmapper = $DEVMAPPER_REQUIRED], [], [DEVMAPPER_FOUND=no]) - if test $DEVMAPPER_FOUND = no; then - # devmapper is missing pkg-config files in ubuntu, suse, etc - save_LIBS=$LIBS - save_CFLAGS=$CFLAGS - DEVMAPPER_FOUND=yes - AC_CHECK_HEADER([libdevmapper.h],,[DEVMAPPER_FOUND=no]) - AC_CHECK_LIB([devmapper], [dm_task_run],,[DEVMAPPER_FOUND=no]) - DEVMAPPER_LIBS=-ldevmapper - LIBS=$save_LIBS - CFLAGS=$save_CFLAGS - fi - if test $DEVMAPPER_FOUND = no ; then - AC_MSG_ERROR([You must install device-mapper-devel/libdevmapper = $DEVMAPPER_REQUIRED to compile libvirt]) - fi - +DEVMAPPER_CFLAGS= +DEVMAPPER_LIBS= +PKG_CHECK_MODULES([DEVMAPPER], [devmapper = $DEVMAPPER_REQUIRED], [], [DEVMAPPER_FOUND=no]) +if test $DEVMAPPER_FOUND = no; then + # devmapper is missing pkg-config files in ubuntu, suse, etc + save_LIBS=$LIBS + save_CFLAGS=$CFLAGS + DEVMAPPER_FOUND=yes + AC_CHECK_HEADER([libdevmapper.h],,[DEVMAPPER_FOUND=no]) + AC_CHECK_LIB([devmapper], [dm_task_run],,[DEVMAPPER_FOUND=no]) + DEVMAPPER_LIBS=-ldevmapper + LIBS=$save_LIBS + CFLAGS=$save_CFLAGS +fi +if test $DEVMAPPER_FOUND = no ; then + AC_MSG_ERROR([You must install device-mapper-devel/libdevmapper = $DEVMAPPER_REQUIRED to compile libvirt]) fi AC_SUBST([DEVMAPPER_CFLAGS]) AC_SUBST([DEVMAPPER_LIBS]) -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not add drive 'boot=on' param when a kernel is specified
Eric Blake wrote:
On 02/17/2011 02:38 PM, Jim Fehlig wrote:
libvirt-tck was failing several domain tests [1] with qemu 0.14, which
is now less tolerable of specifying 2 bootroms with the same boot index [2].
Drop the 'boot=on' param if kernel has been specfied.
[1] https://www.redhat.com/archives/libvir-list/2011-February/msg00559.html
[2] http://lists.nongnu.org/archive/html/qemu-devel/2011-02/msg01892.html
---
src/qemu/qemu_command.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 618d3a9..a9cc23b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3173,7 +3173,7 @@ qemuBuildCommandLine(virConnectPtr conn,
int bootCD = 0, bootFloppy = 0, bootDisk = 0;
/* If QEMU supports boot=on for -drive param... */
-if (qemuCmdFlags QEMUD_CMD_FLAG_DRIVE_BOOT) {
+if (qemuCmdFlags QEMUD_CMD_FLAG_DRIVE_BOOT !def-os.kernel) {
ACK; that looks better than v1.
Thanks; pushed.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] update domain status forcibly even if attach a device failed
At 02/18/2011 05:09 AM, Eric Blake Write: On 02/16/2011 08:32 PM, Wen Congyang wrote: Steps to reproduce this bug: 1. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver qcow2 error: Failed to attach disk error: operation failed: adding scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 device failed: Property 'scsi-disk.drive' can't find value 'drive-scsi0-0-1' 2. service libvirtd restart Stopping libvirtd daemon: [ OK ] Starting libvirtd daemon: [ OK ] 3. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver raw error: Failed to attach disk error: operation failed: adding lsi,id=scsi0,bus=pci.0,addr=0x6 device failed: Duplicate ID 'scsi0' for device The reason is that we create a new scsi controller but we do not update /var/run/libvirt/qemu/domain.xml. I agree that this is a bug, and that this patch is one way to solve things; however, I'm not convinced it's necessarily the right way. Should we be instead rolling back and undoing the new controller creation if adding the device fails? We use qemuDomainFindOrCreateSCSIDiskController() to create scsi disk controller, so we do not know whether we create a new controller. Should the successful addition of a controller save domain state at that point, even though that means two saves instead of one if this succeeds here? Yes, it may be another way to solve this bug. But, it will cause too many saves(For example, if we attach a scsi which uses scsi disk controller 100, and we will create scsi disk controller 1 to 100) But, if others agree with this approach, then ACK to the code. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Requires gettext for client package
于 2011年02月18日 04:12, Eric Blake 写道: On 02/17/2011 03:05 AM, Osier Yang wrote: libvirt-guests invokes functions in gettext.sh, so we need to require gettext package in spec file. Demo with the fix: % rpm -q gettext package gettext is not installed % rpm -ivh libvirt-client-0.8.8-1.fc14.x86_64.rpm error: Failed dependencies: gettext is needed by libvirt-client-0.8.8-1.fc14.x86_64 +++ b/libvirt.spec.in @@ -415,6 +415,7 @@ Requires: ncurses # So remote clients can access libvirt over SSH tunnel # (client invokes 'nc' against the UNIX socket on the server) Requires: nc [note to self - it might be possible to drop the nc dependency in the future, once I finish my conversion from -migration exec: to -migration fd:, since the spec file only targets distros with new-enough qemu 0.12.0 or newer; nc would still be needed for qemu 0.10.0, though] +Requires: gettext Maybe worth adding a comment before this line, justifying the requires (just like we did before the nc requirement): # Needed by libvirt-guests init script ACK with that nit addressed. Pushed with the comments added Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] virsh: fix wrong NUMA nodes count getting
On 02/17/2011 11:11 PM, Eric Blake wrote: On 02/17/2011 06:39 AM, Michal Privoznik wrote: ... As Matthias pointed out, we already have a parse function. If you ditch patch 1/2, then squash this into 2/2, does it all work for you (only compile-tested here)? I've sent the second version, which uses the libvirt xml parsing functions. it was tested on some exotic architeture where NUMA nodes IDs are not continuous (hp-magnycours-02.rhts.eng.bos.redhat.com to be specific), but are something like 0,2,4,7. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Requires gettext for client package
于 2011年02月18日 08:30, Eric Blake 写道:
On 02/17/2011 01:19 PM, Justin Clift wrote:
On 18/02/2011, at 7:12 AM, Eric Blake wrote:
snip
[note to self - it might be possible to drop the nc dependency in the
future, once I finish my conversion from -migration exec: to -migration
fd:, since the spec file only targets distros with new-enough qemu
0.12.0 or newer; nc would still be needed for qemu 0.10.0, though]
Does your conversion affect how libvirt uses ssh for general communication?
Pretty sure I've seen some part of libvirt (other than migration) use
nc + ssh for general communication to remote qemu (ie with qemu+ssh://).
git grep 'nc' has two hits:
src/qemu/qemu_migration.c:const char *args[] = { nc, -U,
unixfile, NULL };
Will still need to exist for qemu 0.10.x and 0.11.x (so we'd need it for
RHEL 5.6, but not for Fedora or RHEL 6; we've already got other
RHEL-version-related checks in the specfile to make that a conditional
dependency).
src/remote/remote_driver.c:cmd_argv[j++] = strdup (netcat ?
netcat : nc);
Oh wow - we need to convert remote_driver.c over to virCommand.
However, that's the only other use of nc, and it involves running nc
on the remote host, not on the local host, so you _don't_ need to
'Requires: nc' just because you installed libvirt-client; rather, it
seems like it should be libvirt (the server package) that should be
requiring nc.
snip
rather, it seems like it should be libvirt (the server package) that
should berequiring nc.
/snip
Would like to confirm it and make patch if it is needed indeed.
In other words, I think we have a missing dependency if you install
libvirt but not libvirt-client; the reality is that both packages should
require nc (and libvirt-client can make the dependency conditional if I
improve migration on newer qemu).
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Remove redundant error reporting codes
于 2011年02月17日 23:36, Osier Yang 写道:
于 2011年02月17日 19:15, Daniel P. Berrange 写道:
On Thu, Feb 17, 2011 at 05:30:23PM +0800, Osier Yang wrote:
As virDomainDefParseString already reported the error if it
fails, and the redundant error reports codes will override
error reported by virDomainDefParseString with some unclear
messages, removed them.
* src/qemu/qemu_driver.c
---
src/qemu/qemu_driver.c | 33 +
1 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ab664a0..fd8e401 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3600,8 +3600,10 @@ static virDomainPtr
qemudDomainCreate(virConnectPtr conn, const char *xml,
virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL);
qemuDriverLock(driver);
- if (!(def = virDomainDefParseString(driver-caps, xml,
- VIR_DOMAIN_XML_INACTIVE)))
+
+ def = virDomainDefParseString(driver-caps, xml,
VIR_DOMAIN_XML_INACTIVE);
+ if (!def)
+ /* virDomainDefParseString reports the error. */
goto cleanup;
This is a needless change that increases line length.
if (virSecurityManagerVerify(driver-securityManager, def) 0)
@@ -5746,12 +5748,9 @@ qemudDomainSaveImageOpen(struct qemud_driver
*driver,
}
/* Create a domain from this XML */
- if (!(def = virDomainDefParseString(driver-caps, xml,
- VIR_DOMAIN_XML_INACTIVE))) {
- qemuReportError(VIR_ERR_OPERATION_FAILED,
- %s, _(failed to parse XML));
+ def = virDomainDefParseString(driver-caps, xml,
VIR_DOMAIN_XML_INACTIVE);
+ if (!def)
goto error;
- }
VIR_FREE(xml);
@@ -6412,8 +6411,9 @@ static virDomainPtr
qemudDomainDefine(virConnectPtr conn, const char *xml) {
int dupVM;
qemuDriverLock(driver);
- if (!(def = virDomainDefParseString(driver-caps, xml,
- VIR_DOMAIN_XML_INACTIVE)))
+
+ def = virDomainDefParseString(driver-caps, xml,
VIR_DOMAIN_XML_INACTIVE);
+ if (!def)
goto cleanup;
So is this chunk.
if (virSecurityManagerVerify(driver-securityManager, def) 0)
@@ -8046,13 +8046,9 @@ qemudDomainMigratePrepareTunnel(virConnectPtr
dconn,
}
/* Parse the domain XML. */
- if (!(def = virDomainDefParseString(driver-caps, dom_xml,
- VIR_DOMAIN_XML_INACTIVE))) {
- qemuReportError(VIR_ERR_OPERATION_FAILED,
- %s, _(failed to parse XML, libvirt version may be
- different between source and destination host));
+ def = virDomainDefParseString(driver-caps, dom_xml,
VIR_DOMAIN_XML_INACTIVE);
+ if (!def)
goto cleanup;
- }
if (!qemuDomainIsMigratable(def))
goto cleanup;
@@ -8320,12 +8316,9 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
VIR_DEBUG(Generated uri_out=%s, *uri_out);
/* Parse the domain XML. */
- if (!(def = virDomainDefParseString(driver-caps, dom_xml,
- VIR_DOMAIN_XML_INACTIVE))) {
- qemuReportError(VIR_ERR_OPERATION_FAILED,
- %s, _(failed to parse XML));
+ def = virDomainDefParseString(driver-caps, dom_xml,
VIR_DOMAIN_XML_INACTIVE);
+ if (!def)
goto cleanup;
- }
These other chunks are fine, but it'd be preferrable to keep the existing
line formatting if (!(def = ))
Okay, will push with these updates, thanks for the reviewing.
It's already fixed as part of 766de43, so no pushing.
Regards
Osier
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-php 0/2] Another update for building system
Hi all: This patch updeted the building system. * Added necessary checkings in configure.ac * Cleared up libvirt-php.spec and src/Makefile.am BTW, I've changed my email to business address. Lyre (2): spec file cleanup Added checkings for tools and libraries aclocal.m4 | 189 ++ configure.ac | 12 libvirt-php.spec |8 +- src/Makefile.am | 30 +++-- 4 files changed, 216 insertions(+), 23 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-php 2/2] Added checkings for tools and libraries
* configure.ac: Added checkings for compiler, sed, install, and so on;
Added AM_PATH_XML2 to check libxml2.
* src/Makefile.am: dropped needless macros since it is done by autoconf;
Use $INSTALL to install files rather than cp.
---
aclocal.m4 | 189 +++
configure.ac| 12
src/Makefile.am | 30 +++--
3 files changed, 212 insertions(+), 19 deletions(-)
diff --git a/aclocal.m4 b/aclocal.m4
index a1fa316..982143e 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -19,6 +19,195 @@ You have another version of autoconf. It may work, but is
not guaranteed to.
If you have problems, you may need to regenerate the build system entirely.
To do so, use the procedure documented by the package, typically
`autoreconf'.])])
+# Configure paths for LIBXML2
+# Mike Hommey 2004-06-19
+# use CPPFLAGS instead of CFLAGS
+# Toshio Kuratomi 2001-04-21
+# Adapted from:
+# Configure paths for GLIB
+# Owen Taylor 97-11-3
+
+dnl AM_PATH_XML2([MINIMUM-VERSION, [ACTION-IF-FOUND [, ACTION-IF-NOT-FOUND]]])
+dnl Test for XML, and define XML_CPPFLAGS and XML_LIBS
+dnl
+AC_DEFUN([AM_PATH_XML2],[
+AC_ARG_WITH(xml-prefix,
+[ --with-xml-prefix=PFX Prefix where libxml is installed
(optional)],
+xml_config_prefix=$withval, xml_config_prefix=)
+AC_ARG_WITH(xml-exec-prefix,
+[ --with-xml-exec-prefix=PFX Exec prefix where libxml is
installed (optional)],
+xml_config_exec_prefix=$withval, xml_config_exec_prefix=)
+AC_ARG_ENABLE(xmltest,
+ [ --disable-xmltest Do not try to compile and run a test
LIBXML program],,
+ enable_xmltest=yes)
+
+ if test x$xml_config_exec_prefix != x ; then
+ xml_config_args=$xml_config_args
+ if test x${XML2_CONFIG+set} != xset ; then
+XML2_CONFIG=$xml_config_exec_prefix/bin/xml2-config
+ fi
+ fi
+ if test x$xml_config_prefix != x ; then
+ xml_config_args=$xml_config_args --prefix=$xml_config_prefix
+ if test x${XML2_CONFIG+set} != xset ; then
+XML2_CONFIG=$xml_config_prefix/bin/xml2-config
+ fi
+ fi
+
+ AC_PATH_PROG(XML2_CONFIG, xml2-config, no)
+ min_xml_version=ifelse([$1], ,2.0.0,[$1])
+ AC_MSG_CHECKING(for libxml - version = $min_xml_version)
+ no_xml=
+ if test $XML2_CONFIG = no ; then
+no_xml=yes
+ else
+XML_CPPFLAGS=`$XML2_CONFIG $xml_config_args --cflags`
+XML_LIBS=`$XML2_CONFIG $xml_config_args --libs`
+xml_config_major_version=`$XML2_CONFIG $xml_config_args --version | \
+ sed 's/\([[0-9]]*\).\([[0-9]]*\).\([[0-9]]*\)/\1/'`
+xml_config_minor_version=`$XML2_CONFIG $xml_config_args --version | \
+ sed 's/\([[0-9]]*\).\([[0-9]]*\).\([[0-9]]*\)/\2/'`
+xml_config_micro_version=`$XML2_CONFIG $xml_config_args --version | \
+ sed 's/\([[0-9]]*\).\([[0-9]]*\).\([[0-9]]*\)/\3/'`
+if test x$enable_xmltest = xyes ; then
+ ac_save_CPPFLAGS=$CPPFLAGS
+ ac_save_LIBS=$LIBS
+ CPPFLAGS=$CPPFLAGS $XML_CPPFLAGS
+ LIBS=$XML_LIBS $LIBS
+dnl
+dnl Now check if the installed libxml is sufficiently new.
+dnl (Also sanity checks the results of xml2-config to some extent)
+dnl
+ rm -f conf.xmltest
+ AC_TRY_RUN([
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include libxml/xmlversion.h
+
+int
+main()
+{
+ int xml_major_version, xml_minor_version, xml_micro_version;
+ int major, minor, micro;
+ char *tmp_version;
+
+ system(touch conf.xmltest);
+
+ /* Capture xml2-config output via autoconf/configure variables */
+ /* HP/UX 9 (%@#!) writes to sscanf strings */
+ tmp_version = (char *)strdup($min_xml_version);
+ if (sscanf(tmp_version, %d.%d.%d, major, minor, micro) != 3) {
+ printf(%s, bad version string from xml2-config\n, $min_xml_version);
+ exit(1);
+ }
+ free(tmp_version);
+
+ /* Capture the version information from the header files */
+ tmp_version = (char *)strdup(LIBXML_DOTTED_VERSION);
+ if (sscanf(tmp_version, %d.%d.%d, xml_major_version, xml_minor_version,
xml_micro_version) != 3) {
+ printf(%s, bad version string from libxml includes\n,
LIBXML_DOTTED_VERSION);
+ exit(1);
+ }
+ free(tmp_version);
+
+ /* Compare xml2-config output to the libxml headers */
+ if ((xml_major_version != $xml_config_major_version) ||
+ (xml_minor_version != $xml_config_minor_version) ||
+ (xml_micro_version != $xml_config_micro_version))
+{
+ printf(*** libxml header files (version %d.%d.%d) do not match\n,
+ xml_major_version, xml_minor_version, xml_micro_version);
+ printf(*** xml2-config (version %d.%d.%d)\n,
+ $xml_config_major_version, $xml_config_minor_version,
$xml_config_micro_version);
+ return 1;
+}
+/* Compare the headers to the library to make sure we match */
+ /* Less than ideal -- doesn't provide us with return value feedback,
+ * only exits if there's a serious mismatch between header and library.
+ */
+
