Re: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm
Daniel P. Berrange berra...@redhat.com wrote on 05/05/2010 11:16:37: From: Daniel P. Berrange berra...@redhat.com To: Cole Robinson crobi...@redhat.com Cc: Kenneth Nagin/Haifa/i...@ibmil, list libvirt libvir-list@redhat.com Date: 05/05/2010 11:16 Subject: Re: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm On Mon, May 03, 2010 at 09:52:10AM -0400, Cole Robinson wrote: On 05/03/2010 04:13 AM, Kenneth Nagin wrote: ... This the patch file: (See attached file: libvirt_migration_ns_100503.patch) ACK, patch looks fine now. No objections to the patch that was committed, but FYI there is an improvement that can be done to it. If you run 'virsh domjobinfo $GUEST' while migration is running it tells you the progress of migration. It gives overall progress, and memory progress. The APi was designed to allow us to report storage progress too. To wire this up look at the method qemuDomainWaitForMigrationComplete where it updates the priv-jobInfo variables. You'll also need to add extra params to the qemuMonitorGetMigrationStatus() method to return the disk processed/remaining/total stats virsh should already be able to report this data once the QEMU driver is wired up Regards, Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/:| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org:| |: http://autobuild.org-o- http://search.cpan.org/~danberr/:| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| Thank you. I will look into implementing the improvement when I have time. However, it won't be for a while since I have some pressing deadlines that I have to meet in the next few months. regards, Kenneth Nagin Ph: +972-4-8296227 Cell: 054-6976227 Fx: +972-4- 8296113 http://researcher.watson.ibm.com/researcher/view.php?person=il-NAGIN http://www.reservoir-fp7.eu/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Libvirtd in listen mode.
Hi, If I start the libvirtd service using the command service libvirtd start, it is not starting in listen mode. I am stopping the libvirtd service, and then starting in listen mode externally, using the following command: libvirtd -listen. But, how can I start libvirtd service in listen mode by default (when I issue the command service libvirtd start)? Regards, Srikanth. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK PATCHv3] block devices: allow specification of size for safety
Eric Blake wrote:
I was getting failures of domain/103-blockdev-save-restore.t when
connecting as qemu:///session, since my uid could stat /dev/sdb but
not open it. That test now skips for unprivileged users, as well as
adds a layer of sanity checking against expected size to avoid
trashing the wrong device.
* conf/default.cfg (host_block_devices): Document optional size.
* lib/Sys/Virt/TCK.pm (get_host_block_device): If optional size is
supplied, skip a device that does not match. Also, avoid devices
that can't be opened.
---
Changes from v2:
+ guarantee that the device can be opened by the current user, even if
the .cfg file used the older path-only configuration
ACK.
+# Each block device is either a raw path
# /dev/vdb
+# or a path plus size in 1k blocks, as in /proc/partitions, to avoid
+# trashing the wrong device
+# {
+#path = /dev/disk/by-id/usb-Generic_Flash_Disk_9B46E3C5-0:0
Much improved example.
With by-id/ and the verbose name, there should be no problem.
+#size = 989184
+# }
...
-return $self-config(host_block_devices/[$devindex], undef);
+my $device = $self-config(host_block_devices/[$devindex]/path, undef);
+my $kb_blocks = $self-config(host_block_devices/[$devindex]/size, 0);
+my $match = 1;
+
+$device ||= $self-config(host_block_devices/[$devindex], undef);
+
+# Filter out devices that the current user can't open.
+sysopen(BLK, $device, O_RDONLY) or return undef;
+
+if ($kb_blocks) {
+ $match = sysseek(BLK, 0, SEEK_END) == $kb_blocks * 1024;
+}
+close BLK;
+
+return $match ? $device : undef;
}
That looks fine and correct.
You could move the $kb_blocks definition down to where used.
though there's a good argument for keeping it near the
other config-getting statement. Either way is fine.
However, I find it more readable to group the two $device-setting
statements together:
my $device = $self-config(host_block_devices/[$devindex]/path, undef);
$device ||= $self-config(host_block_devices/[$devindex], undef);
my $kb_blocks = $self-config(host_block_devices/[$devindex]/size, 0);
+my $match = 1;
There is no reason to declare/set $match so early.
Move that down to where it's used.
Rather than an if (...) and a one-line {...} block,
I would write it this way: syntax: less is always (more ;-) better
my $match = 1;
$kb_blocks
and $match = sysseek(BLK, 0, SEEK_END) == $kb_blocks * 1024;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK PATCHv3] block devices: allow specification of size for safety
On Wed, May 05, 2010 at 03:39:16PM -0600, Eric Blake wrote:
I was getting failures of domain/103-blockdev-save-restore.t when
connecting as qemu:///session, since my uid could stat /dev/sdb but
not open it. That test now skips for unprivileged users, as well as
adds a layer of sanity checking against expected size to avoid
trashing the wrong device.
* conf/default.cfg (host_block_devices): Document optional size.
* lib/Sys/Virt/TCK.pm (get_host_block_device): If optional size is
supplied, skip a device that does not match. Also, avoid devices
that can't be opened.
---
Changes from v2:
+ guarantee that the device can be opened by the current user, even if
the .cfg file used the older path-only configuration
conf/default.cfg|8
lib/Sys/Virt/TCK.pm | 17 -
2 files changed, 24 insertions(+), 1 deletions(-)
diff --git a/conf/default.cfg b/conf/default.cfg
index 01f438c..d749f5f 100644
--- a/conf/default.cfg
+++ b/conf/default.cfg
@@ -134,5 +134,13 @@ host_pci_devices = (
# the test suite itself needs to create partitions.
# The disks should be at *least* 512 MB in size
host_block_devices = (
+# Each block device is either a raw path
# /dev/vdb
+# or a path plus size in 1k blocks, as in /proc/partitions, to avoid
+# trashing the wrong device
+# {
+#path = /dev/disk/by-id/usb-Generic_Flash_Disk_9B46E3C5-0:0
+#size = 989184
+# }
+# Can list more than on block device if many are available
)
diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
index 9cdef09..0aba557 100644
--- a/lib/Sys/Virt/TCK.pm
+++ b/lib/Sys/Virt/TCK.pm
@@ -33,6 +33,7 @@ use IO::Uncompress::Gunzip qw(gunzip);
use IO::Uncompress::Bunzip2 qw(bunzip2);
use XML::XPath;
use Carp qw(cluck carp);
+use Fcntl qw(O_RDONLY SEEK_END);
use Test::Builder;
use Sub::Uplevel qw(uplevel);
@@ -833,7 +834,21 @@ sub get_host_block_device {
my $self = shift;
my $devindex = @_ ? shift : 0;
-return $self-config(host_block_devices/[$devindex], undef);
+my $device = $self-config(host_block_devices/[$devindex]/path, undef);
+my $kb_blocks = $self-config(host_block_devices/[$devindex]/size, 0);
+my $match = 1;
+
+$device ||= $self-config(host_block_devices/[$devindex], undef);
+
+# Filter out devices that the current user can't open.
+sysopen(BLK, $device, O_RDONLY) or return undef;
+
+if ($kb_blocks) {
+ $match = sysseek(BLK, 0, SEEK_END) == $kb_blocks * 1024;
+}
+close BLK;
+
+return $match ? $device : undef;
}
NB, this will result in use of a unndefined $device value if no block devices
are listed in the config. You need add
return undef unless $device;
before the sysopen line to avoid it.
Regards,
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK PATCHv3] block devices: allow specification of size for safety
Jim Meyering wrote: However, I find it more readable to group the two $device-setting statements together: my $device = $self-config(host_block_devices/[$devindex]/path, undef); $device ||= $self-config(host_block_devices/[$devindex], undef); Taking Dan's correction into account, you might want to write it like this: my $device = ($self-config(host_block_devices/[$devindex]/path, undef) || $self-config(host_block_devices/[$devindex], undef) || return undef); or perhaps, more technically correct, in case the config value is 0: my $device = ($self-config(host_block_devices/[$devindex]/path, undef) || $self-config(host_block_devices/[$devindex], undef)); defined $device or return undef; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Remove unused nwfilter field from struct remote_error
Change 965466c1 added a new field to struct remote_error, which broke the RPC
protocol. Fortunately the new field is unused, so this change simply removes it
again.
* src/remote/remote_protocol.(c|h|x): Remove remote_nwfilter from struct
remote_error
---
src/remote/remote_protocol.c |2 --
src/remote/remote_protocol.h |1 -
src/remote/remote_protocol.x |1 -
3 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c
index 187281d..972bf52 100644
--- a/src/remote/remote_protocol.c
+++ b/src/remote/remote_protocol.c
@@ -227,8 +227,6 @@ xdr_remote_error (XDR *xdrs, remote_error *objp)
return FALSE;
if (!xdr_remote_network (xdrs, objp-net))
return FALSE;
- if (!xdr_remote_nwfilter (xdrs, objp-nwfilter))
- return FALSE;
return TRUE;
}
diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h
index 6f01da7..a600af6 100644
--- a/src/remote/remote_protocol.h
+++ b/src/remote/remote_protocol.h
@@ -143,7 +143,6 @@ struct remote_error {
int int1;
int int2;
remote_network net;
-remote_nwfilter nwfilter;
};
typedef struct remote_error remote_error;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 8000ee0..1ce488c 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -266,7 +266,6 @@ struct remote_error {
int int1;
int int2;
remote_network net;
-remote_nwfilter nwfilter;
};
/* Authentication types available thus far */
--
1.6.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Remove unused nwfilter field from struct remote_error
On Thu, May 06, 2010 at 11:14:47AM +0100, Matthew Booth wrote:
Change 965466c1 added a new field to struct remote_error, which broke the RPC
protocol. Fortunately the new field is unused, so this change simply removes
it
again.
* src/remote/remote_protocol.(c|h|x): Remove remote_nwfilter from struct
remote_error
---
src/remote/remote_protocol.c |2 --
src/remote/remote_protocol.h |1 -
src/remote/remote_protocol.x |1 -
3 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c
index 187281d..972bf52 100644
--- a/src/remote/remote_protocol.c
+++ b/src/remote/remote_protocol.c
@@ -227,8 +227,6 @@ xdr_remote_error (XDR *xdrs, remote_error *objp)
return FALSE;
if (!xdr_remote_network (xdrs, objp-net))
return FALSE;
- if (!xdr_remote_nwfilter (xdrs, objp-nwfilter))
- return FALSE;
return TRUE;
}
diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h
index 6f01da7..a600af6 100644
--- a/src/remote/remote_protocol.h
+++ b/src/remote/remote_protocol.h
@@ -143,7 +143,6 @@ struct remote_error {
int int1;
int int2;
remote_network net;
-remote_nwfilter nwfilter;
};
typedef struct remote_error remote_error;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 8000ee0..1ce488c 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -266,7 +266,6 @@ struct remote_error {
int int1;
int int2;
remote_network net;
-remote_nwfilter nwfilter;
};
/* Authentication types available thus far */
ACK,
I suggest people packaging libvirt-0.8.0 or 0.8.1 in dustributions to
apply this patch to avoid the protocol break with other versions !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Remove unused nwfilter field from struct remote_error
On Thu, May 06, 2010 at 12:30:27PM +0200, Daniel Veillard wrote:
On Thu, May 06, 2010 at 11:14:47AM +0100, Matthew Booth wrote:
Change 965466c1 added a new field to struct remote_error, which broke the
RPC
protocol. Fortunately the new field is unused, so this change simply
removes it
again.
* src/remote/remote_protocol.(c|h|x): Remove remote_nwfilter from struct
remote_error
---
src/remote/remote_protocol.c |2 --
src/remote/remote_protocol.h |1 -
src/remote/remote_protocol.x |1 -
3 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c
index 187281d..972bf52 100644
--- a/src/remote/remote_protocol.c
+++ b/src/remote/remote_protocol.c
@@ -227,8 +227,6 @@ xdr_remote_error (XDR *xdrs, remote_error *objp)
return FALSE;
if (!xdr_remote_network (xdrs, objp-net))
return FALSE;
- if (!xdr_remote_nwfilter (xdrs, objp-nwfilter))
- return FALSE;
return TRUE;
}
diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h
index 6f01da7..a600af6 100644
--- a/src/remote/remote_protocol.h
+++ b/src/remote/remote_protocol.h
@@ -143,7 +143,6 @@ struct remote_error {
int int1;
int int2;
remote_network net;
-remote_nwfilter nwfilter;
};
typedef struct remote_error remote_error;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 8000ee0..1ce488c 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -266,7 +266,6 @@ struct remote_error {
int int1;
int int2;
remote_network net;
-remote_nwfilter nwfilter;
};
/* Authentication types available thus far */
ACK,
I suggest people packaging libvirt-0.8.0 or 0.8.1 in dustributions to
apply this patch to avoid the protocol break with other versions !
FYI impact is that in the one failure scenario here, yo will be unable
to decode virError objects off the wire, instead of the real error you
wil get an RPC error:
1. 0.7.x client - 0.7.x server : OK
2. 0.7.x client - 0.8.x server : OK (ignores the extra trailing bytes)
3. 0.8.x client - 0.7.x server : FAIL
4. 0.8.x client - 0.8.x server : OK
With matt's patch applied, removing the bogus field, new clients will
work with old servers again.
1. 0.7.x client - 0.7.x server : OK
2. 0.7.x client - 0.8.x server : OK (ignores the extra trailing bytes)
3. 0.7.x client - 0.8.2 server : OK
4. 0.8.x client - 0.7.x server : FAIL (not enough bytes)
5. 0.8.x client - 0.8.x server : OK
6. 0.8.x client - 0.8.2 server : FAIL (not enough bytes)
7. 0.8.2 client - 0.7.x server : OK
8. 0.8.2 client - 0.8.x server : OK (ignores the extra trailing bytes)
9. 0.8.2 client - 0.8.2 server : OK
Anyone with a 0.8.x build should apply this patch too
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirtd in listen mode.
if you're running ubuntu (which I suppose since you use the command service, have a look at /etc/default/libvirtd (or something like that). If not, or if it fails, look at the script in /etc/init.d/libvirtd, and try to guess where it loads its constants! -- frede...@grelot.net Tél : 05 61 27 81 79 / 06 82 23 46 17 Hi, If I start the libvirtd service using the command “service libvirtd start”, it is not starting in listen mode. I am stopping the libvirtd service, and then starting in listen mode externally, using the following command: “libvirtd –listen”. But, how can I start libvirtd service in listen mode by default (when I issue the command “service libvirtd start”)? Regards, Srikanth. -- 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] [PATCH] cgroup: Enable memory.use_hierarchy of cgroup for domain
Through conversation with Kumar L Srikanth-B22348, I found
that the function of getting memory usage (e.g., virsh dominfo)
doesn't work for lxc with ns subsystem of cgroup enabled.
This is because of features of ns and memory subsystems.
Ns creates child cgroup on every process fork and as a result
processes in a container are not assigned in a cgroup for
domain (e.g., libvirt/lxc/test1/). For example, libvirt_lxc
and init (or somewhat specified in XML) are assigned into
libvirt/lxc/test1/8839/ and libvirt/lxc/test1/8839/8849/,
respectively. On the other hand, memory subsystem accounts
memory usage within a group of processes by default, i.e.,
it does not take any child (and descendent) groups into
account. With the two features, virsh dominfo which just
checks memory usage of a cgroup for domain always returns
zero because the cgroup has no process.
Setting memory.use_hierarchy of a group allows to account
(and limit) memory usage of every descendent groups of the group.
By setting it of a cgroup for domain, we can get proper memory
usage of lxc with ns subsystem enabled. (To be exact, the
setting is required only when memory and ns subsystems are
enabled at the same time, e.g., mount -t cgroup none /cgroup.)
---
src/util/cgroup.c | 49 +
1 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index b8b2eb5..93cd6a9 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -443,7 +443,38 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent,
virCgroupPtr group)
return rc;
}
-static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int
create)
+static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group)
+{
+int rc = 0;
+unsigned long long value;
+const char *filename = memory.use_hierarchy;
+
+rc = virCgroupGetValueU64(group,
+ VIR_CGROUP_CONTROLLER_MEMORY,
+ filename, value);
+if (rc != 0) {
+VIR_ERROR(Failed to read %s/%s (%d), group-path, filename, rc);
+return rc;
+}
+
+/* Setting twice causes error, so if already enabled, skip setting */
+if (value == 1)
+return 0;
+
+VIR_DEBUG(Setting up %s/%s, group-path, filename);
+rc = virCgroupSetValueU64(group,
+ VIR_CGROUP_CONTROLLER_MEMORY,
+ filename, 1);
+
+if (rc != 0) {
+VIR_ERROR(Failed to set %s/%s (%d), group-path, filename, rc);
+}
+
+return rc;
+}
+
+static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group,
+ int create, int memory_hierarchy)
{
int i;
int rc = 0;
@@ -477,6 +508,16 @@ static int virCgroupMakeGroup(virCgroupPtr parent,
virCgroupPtr group, int creat
break;
}
}
+if (memory_hierarchy
+group-controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint !=
NULL
+(i == VIR_CGROUP_CONTROLLER_MEMORY ||
+ STREQ(group-controllers[i].mountPoint,
group-controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) {
+rc = virCgroupSetMemoryUseHierarchy(group);
+if (rc != 0) {
+VIR_FREE(path);
+break;
+}
+}
}
VIR_FREE(path);
@@ -553,7 +594,7 @@ static int virCgroupAppRoot(int privileged,
if (rc != 0)
goto cleanup;
-rc = virCgroupMakeGroup(rootgrp, *group, create);
+rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
cleanup:
virCgroupFree(rootgrp);
@@ -653,7 +694,7 @@ int virCgroupForDriver(const char *name,
VIR_FREE(path);
if (rc == 0) {
-rc = virCgroupMakeGroup(rootgrp, *group, create);
+rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
if (rc != 0)
virCgroupFree(group);
}
@@ -703,7 +744,7 @@ int virCgroupForDomain(virCgroupPtr driver,
VIR_FREE(path);
if (rc == 0) {
-rc = virCgroupMakeGroup(driver, *group, create);
+rc = virCgroupMakeGroup(driver, *group, create, 1);
if (rc != 0)
virCgroupFree(group);
}
--
1.6.5.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: fix va_start usage bug
Eric Blake wrote: Detected by clang. POSIX requires that the second argument to va_start be the name of the last variable; and in some implementations, passing *path instead of path would dereference bogus memory instead of pulling arguments off the stack. * src/util/util.c (virBuildPathInternal): Use correct argument to va_start. --- I think this falls under the trivial rule, as it silences a compiler warning and is a one-line fix of a real bug, so I pushed it. In a way it's trivial, but it's also rather subtle. ACK, regardless. src/util/util.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 2d32952..c44d012 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2799,7 +2799,7 @@ int virBuildPathInternal(char **path, ...) va_list ap; int ret = 0; -va_start(ap, *path); +va_start(ap, path); path_component = va_arg(ap, char *); virBufferAdd(buf, path_component, -1); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/5] build: update gnulib
On 05/04/2010 03:16 PM, Eric Blake wrote: This replaces the first cut at this series: https://www.redhat.com/archives/libvir-list/2010-April/msg01341.html Changes since v1: squash the original 1/5 and 2/5 into one patch test every stage of the series on a cross-compile to mingw add new 5/5, and adjust to recent syntax checks available in today's gnulib [PATCHv2 1/5] build: rely on gnulib's pthread module [PATCHv2 2/5] build: use gnulib's uname [PATCHv2 3/5] build: use gnulib's sys/wait.h [PATCHv2 4/5] build: drop more redundant configure checks [PATCHv2 5/5] build: update gnulib Because of the new patch, I'll wait for a re-ACK before pushing; but 1 through 4 are pretty much unchanged from their original ACK. To make it easier, here's the interdiff between my original series and this v2 series: -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org diff --git a/.gnulib b/.gnulib index 7c1b995..e2843e3 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 7c1b995a7041ea366acafeb8632e1080f349f03f +Subproject commit e2843e30e8c2885eb8cbc77e20c4e0f4d562d44d diff --git a/.x-sc_prohibit_always_true_header_tests b/.x-sc_prohibit_always_true_header_tests new file mode 100644 index 000..ff753ce --- /dev/null +++ b/.x-sc_prohibit_always_true_header_tests @@ -0,0 +1,4 @@ +ChangeLog* +docs/news.html.in +python/libvirt-override.c +python/typewrappers.c diff --git a/bootstrap.conf b/bootstrap.conf index baf0bc2..1e93490 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -46,6 +46,7 @@ posix-shell pthread recv random_r +sched send setsockopt socket diff --git a/configure.ac b/configure.ac index c643c56..c187420 100644 --- a/configure.ac +++ b/configure.ac @@ -108,7 +108,7 @@ LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \ - sched.h termios.h sys/poll.h syslog.h mntent.h net/ethernet.h]) + termios.h sys/poll.h syslog.h mntent.h net/ethernet.h]) dnl Where are the XDR functions? dnl If portablexdr is installed, prefer that. @@ -495,33 +495,19 @@ if test $with_libvirtd = no ; then with_lxc=no fi if test $with_lxc = yes || test $with_lxc = check; then -AC_CHECK_HEADER([sched.h], -dnl Header is there, check for unshare() -[ -AC_TRY_LINK([#define _GNU_SOURCE -#include sched.h], [ -unshare (1); - ], [ -with_lxc=yes - ], [ -if test $with_lxc = check; then - with_lxc=no - AC_MSG_NOTICE([Function unshare() not present in sched.h header but required for LXC driver, disabling it]) -else - AC_MSG_ERROR([Function unshare() not present in sched.h header, but required for LXC driver]) -fi - -]) - -dnl Header is not there -],[ +AC_TRY_LINK([#define _GNU_SOURCE +#include sched.h +], [ +unshare (1); +], [ +with_lxc=yes +], [ if test $with_lxc = check; then with_lxc=no -AC_MSG_NOTICE([Header sched.h not found but required for LXC driver, disabling it]) +AC_MSG_NOTICE([Function unshare() not present in sched.h header but required for LXC driver, disabling it]) else -AC_MSG_ERROR([Header sched.h not found but required for LXC driver]) +AC_MSG_ERROR([Function unshare() not present in sched.h header, but required for LXC driver]) fi - ]) fi if test $with_lxc = yes ; then diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 132392b..8432bbc 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -29,7 +29,6 @@ #include limits.h #include string.h #include stdio.h -#include strings.h #include stdarg.h #include stdlib.h #include unistd.h diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 8735cc1..b52f4ac 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -1,6 +1,7 @@ /* * openvz_conf.c: config functions for managing OpenVZ VEs * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * Copyright (C) 2007 Anoop Joe Cyriac @@ -33,7 +34,6 @@ #include fcntl.h #include sys/types.h #include dirent.h -#include strings.h #include time.h #include sys/stat.h #include unistd.h diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 00b8a14..ce159d0 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1,6 +1,7 @@ /* * openvz_driver.c: core driver methods for managing OpenVZ VEs * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * Copyright (C) 2007 Anoop Joe Cyriac @@ -33,7 +34,6 @@ #include limits.h #include string.h #include stdio.h -#include strings.h #include stdarg.h
Re: [libvirt] [PATCH] cgroup: Enable memory.use_hierarchy of cgroup for domain
On Thu, May 6, 2010 at 7:40 PM, Ryota Ozaki ozaki.ry...@gmail.com wrote:
Through conversation with Kumar L Srikanth-B22348, I found
that the function of getting memory usage (e.g., virsh dominfo)
doesn't work for lxc with ns subsystem of cgroup enabled.
This is because of features of ns and memory subsystems.
Ns creates child cgroup on every process fork and as a result
processes in a container are not assigned in a cgroup for
domain (e.g., libvirt/lxc/test1/). For example, libvirt_lxc
and init (or somewhat specified in XML) are assigned into
libvirt/lxc/test1/8839/ and libvirt/lxc/test1/8839/8849/,
respectively. On the other hand, memory subsystem accounts
memory usage within a group of processes by default, i.e.,
it does not take any child (and descendent) groups into
account. With the two features, virsh dominfo which just
checks memory usage of a cgroup for domain always returns
zero because the cgroup has no process.
Setting memory.use_hierarchy of a group allows to account
(and limit) memory usage of every descendent groups of the group.
By setting it of a cgroup for domain, we can get proper memory
usage of lxc with ns subsystem enabled. (To be exact, the
setting is required only when memory and ns subsystems are
enabled at the same time, e.g., mount -t cgroup none /cgroup.)
---
This does sound like a valid use case and the correct fix.
src/util/cgroup.c | 49 +
1 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index b8b2eb5..93cd6a9 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -443,7 +443,38 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent,
virCgroupPtr group)
return rc;
}
-static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int
create)
+static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group)
+{
+ int rc = 0;
+ unsigned long long value;
+ const char *filename = memory.use_hierarchy;
+
+ rc = virCgroupGetValueU64(group,
+ VIR_CGROUP_CONTROLLER_MEMORY,
+ filename, value);
+ if (rc != 0) {
+ VIR_ERROR(Failed to read %s/%s (%d), group-path, filename, rc);
+ return rc;
+ }
+
+ /* Setting twice causes error, so if already enabled, skip setting */
+ if (value == 1)
+ return 0;
+
+ VIR_DEBUG(Setting up %s/%s, group-path, filename);
+ rc = virCgroupSetValueU64(group,
+ VIR_CGROUP_CONTROLLER_MEMORY,
+ filename, 1);
+
+ if (rc != 0) {
+ VIR_ERROR(Failed to set %s/%s (%d), group-path, filename, rc);
+ }
+
+ return rc;
+}
+
+static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group,
+ int create, int memory_hierarchy)
{
int i;
int rc = 0;
@@ -477,6 +508,16 @@ static int virCgroupMakeGroup(virCgroupPtr parent,
virCgroupPtr group, int creat
break;
}
}
Can you please add a comment here stating that memory.use_hierarchy
should always be called prior to creating subcgroups and attaching
tasks
+ if (memory_hierarchy
+ group-controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint
!= NULL
+ (i == VIR_CGROUP_CONTROLLER_MEMORY ||
+ STREQ(group-controllers[i].mountPoint,
group-controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) {
+ rc = virCgroupSetMemoryUseHierarchy(group);
+ if (rc != 0) {
+ VIR_FREE(path);
+ break;
+ }
+ }
}
VIR_FREE(path);
@@ -553,7 +594,7 @@ static int virCgroupAppRoot(int privileged,
if (rc != 0)
goto cleanup;
- rc = virCgroupMakeGroup(rootgrp, *group, create);
+ rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
cleanup:
virCgroupFree(rootgrp);
@@ -653,7 +694,7 @@ int virCgroupForDriver(const char *name,
VIR_FREE(path);
if (rc == 0) {
- rc = virCgroupMakeGroup(rootgrp, *group, create);
+ rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
if (rc != 0)
virCgroupFree(group);
}
@@ -703,7 +744,7 @@ int virCgroupForDomain(virCgroupPtr driver,
VIR_FREE(path);
if (rc == 0) {
- rc = virCgroupMakeGroup(driver, *group, create);
+ rc = virCgroupMakeGroup(driver, *group, create, 1);
if (rc != 0)
virCgroupFree(group);
}
A comment on why Domains get hierarchy support and Drivers don't will
help unless it is very obvious to developers.
Balbir
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/5] build: update gnulib
Eric Blake wrote: On 05/04/2010 03:16 PM, Eric Blake wrote: This replaces the first cut at this series: https://www.redhat.com/archives/libvir-list/2010-April/msg01341.html Changes since v1: squash the original 1/5 and 2/5 into one patch test every stage of the series on a cross-compile to mingw add new 5/5, and adjust to recent syntax checks available in today's gnulib [PATCHv2 1/5] build: rely on gnulib's pthread module [PATCHv2 2/5] build: use gnulib's uname [PATCHv2 3/5] build: use gnulib's sys/wait.h [PATCHv2 4/5] build: drop more redundant configure checks [PATCHv2 5/5] build: update gnulib Because of the new patch, I'll wait for a re-ACK before pushing; but 1 through 4 are pretty much unchanged from their original ACK. To make it easier, here's the interdiff between my original series and this v2 series: Thanks for taking the time. That does indeed help. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cgroup: Enable memory.use_hierarchy of cgroup for domain
On Thu, May 06, 2010 at 08:56:27PM +0530, Balbir Singh wrote:
+ if (memory_hierarchy
+
group-controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint != NULL
+ (i == VIR_CGROUP_CONTROLLER_MEMORY ||
+ STREQ(group-controllers[i].mountPoint,
group-controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) {
+ rc = virCgroupSetMemoryUseHierarchy(group);
+ if (rc != 0) {
+ VIR_FREE(path);
+ break;
+ }
+ }
}
VIR_FREE(path);
@@ -553,7 +594,7 @@ static int virCgroupAppRoot(int privileged,
if (rc != 0)
goto cleanup;
- rc = virCgroupMakeGroup(rootgrp, *group, create);
+ rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
cleanup:
virCgroupFree(rootgrp);
@@ -653,7 +694,7 @@ int virCgroupForDriver(const char *name,
VIR_FREE(path);
if (rc == 0) {
- rc = virCgroupMakeGroup(rootgrp, *group, create);
+ rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
if (rc != 0)
virCgroupFree(group);
}
@@ -703,7 +744,7 @@ int virCgroupForDomain(virCgroupPtr driver,
VIR_FREE(path);
if (rc == 0) {
- rc = virCgroupMakeGroup(driver, *group, create);
+ rc = virCgroupMakeGroup(driver, *group, create, 1);
if (rc != 0)
virCgroupFree(group);
}
A comment on why Domains get hierarchy support and Drivers don't will
help unless it is very obvious to developers.
We never need to query the cummulative usage for the driver as a whole,
only query individual VMs. So it would just be adding overhead to track
it
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cgroup: Enable memory.use_hierarchy of cgroup for domain
On Fri, May 7, 2010 at 12:26 AM, Balbir Singh bal...@linux.vnet.ibm.com wrote:
On Thu, May 6, 2010 at 7:40 PM, Ryota Ozaki ozaki.ry...@gmail.com wrote:
Through conversation with Kumar L Srikanth-B22348, I found
that the function of getting memory usage (e.g., virsh dominfo)
doesn't work for lxc with ns subsystem of cgroup enabled.
This is because of features of ns and memory subsystems.
Ns creates child cgroup on every process fork and as a result
processes in a container are not assigned in a cgroup for
domain (e.g., libvirt/lxc/test1/). For example, libvirt_lxc
and init (or somewhat specified in XML) are assigned into
libvirt/lxc/test1/8839/ and libvirt/lxc/test1/8839/8849/,
respectively. On the other hand, memory subsystem accounts
memory usage within a group of processes by default, i.e.,
it does not take any child (and descendent) groups into
account. With the two features, virsh dominfo which just
checks memory usage of a cgroup for domain always returns
zero because the cgroup has no process.
Setting memory.use_hierarchy of a group allows to account
(and limit) memory usage of every descendent groups of the group.
By setting it of a cgroup for domain, we can get proper memory
usage of lxc with ns subsystem enabled. (To be exact, the
setting is required only when memory and ns subsystems are
enabled at the same time, e.g., mount -t cgroup none /cgroup.)
---
This does sound like a valid use case and the correct fix.
src/util/cgroup.c | 49 +
1 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index b8b2eb5..93cd6a9 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -443,7 +443,38 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent,
virCgroupPtr group)
return rc;
}
-static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int
create)
+static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group)
+{
+ int rc = 0;
+ unsigned long long value;
+ const char *filename = memory.use_hierarchy;
+
+ rc = virCgroupGetValueU64(group,
+ VIR_CGROUP_CONTROLLER_MEMORY,
+ filename, value);
+ if (rc != 0) {
+ VIR_ERROR(Failed to read %s/%s (%d), group-path, filename, rc);
+ return rc;
+ }
+
+ /* Setting twice causes error, so if already enabled, skip setting */
+ if (value == 1)
+ return 0;
+
+ VIR_DEBUG(Setting up %s/%s, group-path, filename);
+ rc = virCgroupSetValueU64(group,
+ VIR_CGROUP_CONTROLLER_MEMORY,
+ filename, 1);
+
+ if (rc != 0) {
+ VIR_ERROR(Failed to set %s/%s (%d), group-path, filename, rc);
+ }
+
+ return rc;
+}
+
+static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group,
+ int create, int memory_hierarchy)
{
int i;
int rc = 0;
@@ -477,6 +508,16 @@ static int virCgroupMakeGroup(virCgroupPtr parent,
virCgroupPtr group, int creat
break;
}
}
Can you please add a comment here stating that memory.use_hierarchy
should always be called prior to creating subcgroups and attaching
tasks
OK, I will.
+ if (memory_hierarchy
+ group-controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint
!= NULL
+ (i == VIR_CGROUP_CONTROLLER_MEMORY ||
+ STREQ(group-controllers[i].mountPoint,
group-controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) {
+ rc = virCgroupSetMemoryUseHierarchy(group);
+ if (rc != 0) {
+ VIR_FREE(path);
+ break;
+ }
+ }
}
VIR_FREE(path);
@@ -553,7 +594,7 @@ static int virCgroupAppRoot(int privileged,
if (rc != 0)
goto cleanup;
- rc = virCgroupMakeGroup(rootgrp, *group, create);
+ rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
cleanup:
virCgroupFree(rootgrp);
@@ -653,7 +694,7 @@ int virCgroupForDriver(const char *name,
VIR_FREE(path);
if (rc == 0) {
- rc = virCgroupMakeGroup(rootgrp, *group, create);
+ rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
if (rc != 0)
virCgroupFree(group);
}
@@ -703,7 +744,7 @@ int virCgroupForDomain(virCgroupPtr driver,
VIR_FREE(path);
if (rc == 0) {
- rc = virCgroupMakeGroup(driver, *group, create);
+ rc = virCgroupMakeGroup(driver, *group, create, 1);
if (rc != 0)
virCgroupFree(group);
}
A comment on why Domains get hierarchy support and Drivers don't will
help unless it is very obvious to developers.
Because of concern of overhead as Daniel said, though I don't figure out
well how much it is. Is it negligible than we expect?
Thanks,
[libvirt] [PATCH] cgroup: Enable memory.use_hierarchy of cgroup for domain
Through conversation with Kumar L Srikanth-B22348, I found
that the function of getting memory usage (e.g., virsh dominfo)
doesn't work for lxc with ns subsystem of cgroup enabled.
This is because of features of ns and memory subsystems.
Ns creates child cgroup on every process fork and as a result
processes in a container are not assigned in a cgroup for
domain (e.g., libvirt/lxc/test1/). For example, libvirt_lxc
and init (or somewhat specified in XML) are assigned into
libvirt/lxc/test1/8839/ and libvirt/lxc/test1/8839/8849/,
respectively. On the other hand, memory subsystem accounts
memory usage within a group of processes by default, i.e.,
it does not take any child (and descendent) groups into
account. With the two features, virsh dominfo which just
checks memory usage of a cgroup for domain always returns
zero because the cgroup has no process.
Setting memory.use_hierarchy of a group allows to account
(and limit) memory usage of every descendent groups of the group.
By setting it of a cgroup for domain, we can get proper memory
usage of lxc with ns subsystem enabled. (To be exact, the
setting is required only when memory and ns subsystems are
enabled at the same time, e.g., mount -t cgroup none /cgroup.)
---
src/util/cgroup.c | 63 +---
1 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index b8b2eb5..f7d6b41 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -443,7 +443,38 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent,
virCgroupPtr group)
return rc;
}
-static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int
create)
+static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group)
+{
+int rc = 0;
+unsigned long long value;
+const char *filename = memory.use_hierarchy;
+
+rc = virCgroupGetValueU64(group,
+ VIR_CGROUP_CONTROLLER_MEMORY,
+ filename, value);
+if (rc != 0) {
+VIR_ERROR(Failed to read %s/%s (%d), group-path, filename, rc);
+return rc;
+}
+
+/* Setting twice causes error, so if already enabled, skip setting */
+if (value == 1)
+return 0;
+
+VIR_DEBUG(Setting up %s/%s, group-path, filename);
+rc = virCgroupSetValueU64(group,
+ VIR_CGROUP_CONTROLLER_MEMORY,
+ filename, 1);
+
+if (rc != 0) {
+VIR_ERROR(Failed to set %s/%s (%d), group-path, filename, rc);
+}
+
+return rc;
+}
+
+static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group,
+ int create, int memory_hierarchy)
{
int i;
int rc = 0;
@@ -477,6 +508,20 @@ static int virCgroupMakeGroup(virCgroupPtr parent,
virCgroupPtr group, int creat
break;
}
}
+/*
+ * Note that virCgroupSetMemoryUseHierarchy should always be
+ * called prior to creating subcgroups and attaching tasks.
+ */
+if (memory_hierarchy
+group-controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint !=
NULL
+(i == VIR_CGROUP_CONTROLLER_MEMORY ||
+ STREQ(group-controllers[i].mountPoint,
group-controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) {
+rc = virCgroupSetMemoryUseHierarchy(group);
+if (rc != 0) {
+VIR_FREE(path);
+break;
+}
+}
}
VIR_FREE(path);
@@ -553,7 +598,7 @@ static int virCgroupAppRoot(int privileged,
if (rc != 0)
goto cleanup;
-rc = virCgroupMakeGroup(rootgrp, *group, create);
+rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
cleanup:
virCgroupFree(rootgrp);
@@ -653,7 +698,7 @@ int virCgroupForDriver(const char *name,
VIR_FREE(path);
if (rc == 0) {
-rc = virCgroupMakeGroup(rootgrp, *group, create);
+rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
if (rc != 0)
virCgroupFree(group);
}
@@ -703,7 +748,17 @@ int virCgroupForDomain(virCgroupPtr driver,
VIR_FREE(path);
if (rc == 0) {
-rc = virCgroupMakeGroup(driver, *group, create);
+/*
+ * Create a cgroup with memory.use_hierarchy enabled to
+ * surely account memory usage of lxc with ns subsystem
+ * enabled. (To be exact, memory and ns subsystems are
+ * enabled at the same time.)
+ *
+ * The reason why doing it here, not a upper group, say
+ * a group for driver, is to avoid overhead to track
+ * cumulative usage that we don't need.
+ */
+rc = virCgroupMakeGroup(driver, *group, create, 1);
if (rc != 0)
virCgroupFree(group);
}
--
1.6.5.2
--
libvir-list mailing list
libvir-list@redhat.com
[libvirt] [PATCH] cygwin: fix abort of virsh on my system
This strange patch fixes aborts of virsh on my system. It seems that the string returned by xmlSaveUri is owned by the xml library?? Signed-off-by: Stefan Berger stef...@us.ibm.com diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 317125f..a916b86 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -542,6 +542,9 @@ doRemoteOpen (virConnectPtr conn, } name = (char *) xmlSaveUri (tmpuri); +#ifdef __CYGWIN__ +name = strdup(name); +#endif #ifdef HAVE_XMLURI_QUERY_RAW VIR_FREE(tmpuri.query_raw); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cygwin: fix abort of virsh on my system
On 05/06/2010 02:11 PM, Stefan Berger wrote:
This strange patch fixes aborts of virsh on my system. It seems that the
string returned by xmlSaveUri is owned by the xml library??
Hm, I don't think so, at least not according to the documentation:
Function: xmlSaveUri
xmlChar * xmlSaveUri (xmlURIPtr uri)
Save the URI as an escaped string
uri:pointer to an xmlURI
Returns:a new string (to be deallocated by caller)
The code in libxml2 also seems to confirm this, although it's not entirely
straightforward:
xmlChar *
xmlSaveUri(xmlURIPtr uri) {
xmlChar *ret = NULL;
xmlChar *temp;
const char *p;
int len;
int max;
if (uri == NULL) return(NULL);
max = 80;
ret = (xmlChar *) xmlMallocAtomic((max + 1) * sizeof(xmlChar));
...
ret[len] = 0;
return(ret);
}
So something else might be going on. What version of libxml2
are you currently building against?
--
Chris Lalancette
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] locking down struct size/layout in remote-protocol.x
This week we noticed that a change to struct remote_error
was causing trouble (new member addition changed the size of
the object being decoded -- bad for the protocol).
In order to ensure that no such changes sneak in again,
I'm planning to do the following.
First, count how many structs must not change size or layout:
$ grep -E '^struct remote_' src/*/remote_protocol.x|wc -l
318
Obviously not appropriate to do it manually.
Running this (pdwtags is part of the dwarves package):
pdwtags src/libvirt_driver_remote_la-remote_protocol.o
prints, among other things, detailed type info for every struct:
/* 89 */
struct remote_nonnull_domain {
remote_nonnull_string name; /* 0 8 */
remote_uuiduuid; /* 816 */
intid; /*24 4 */
/* size: 32, cachelines: 1, members: 3 */
/* last cacheline: 32 bytes */
/* BRAIN FART ALERT! 32 != 28 + 0(holes), diff = 4 */
}; /* size: 32 */
/* 90 */
typedef struct remote_nonnull_domain remote_nonnull_domain; /* size: 32 */
/* 91 */
struct remote_nonnull_network {
remote_nonnull_string name; /* 0 8 */
remote_uuiduuid; /* 816 */
/* size: 24, cachelines: 1, members: 2 */
/* last cacheline: 24 bytes */
}; /* size: 24 */
-
A) For each of the above matching /^struct remote_/, print at the
very least this sort of compile-time check:
verify (sizeof (struct remote_nonnull_domain) == 32);
verify (sizeof (struct remote_nonnull_network) == 24);
B) Even better, verify member offsets, so that we'd detect
reordered members (which often will not change struct size):
verify (sizeof (struct remote_nonnull_domain) == 32);
verify (offsetof (struct remote_nonnull_domain, name) == 0);
verify (offsetof (struct remote_nonnull_domain, uuid) == 8);
verify (offsetof (struct remote_nonnull_domain, id) == 24);
verify (sizeof (struct remote_nonnull_network) == 24);
verify (offsetof (struct remote_nonnull_network, name) == 0);
verify (offsetof (struct remote_nonnull_network, uuid) == 16);
C) Might as well verify member sizes as well.
-
Once we have a small script to generate that (RSN), include the generated,
(and version-controlled) file from remote_protocol.c, and we're done.
For reference, here's code to generate A):
pdwtags src/libvirt_driver_remote_la-remote_protocol.o \
| perl -0777 -n \
-e 'foreach my $p (split m!\n\n/\* \d+ \*/\n!)' \
-e ' { $p =~ m!^struct (remote_\w+).*/\* size: (\d+) \*/!s' \
-e ' and print verify (sizeof (struct $1) == $2);\n }'
And here's a sample of its output:
verify (sizeof (struct remote_nonnull_domain) == 32);
verify (sizeof (struct remote_nonnull_network) == 24);
verify (sizeof (struct remote_nonnull_nwfilter) == 24);
verify (sizeof (struct remote_nonnull_interface) == 16);
verify (sizeof (struct remote_nonnull_storage_pool) == 24);
verify (sizeof (struct remote_nonnull_storage_vol) == 24);
verify (sizeof (struct remote_nonnull_node_device) == 8);
...
verify (sizeof (struct remote_domain_has_current_snapshot_args) == 40);
verify (sizeof (struct remote_domain_has_current_snapshot_ret) == 4);
verify (sizeof (struct remote_domain_snapshot_current_args) == 40);
verify (sizeof (struct remote_domain_snapshot_current_ret) == 40);
verify (sizeof (struct remote_domain_revert_to_snapshot_args) == 48);
verify (sizeof (struct remote_domain_snapshot_delete_args) == 48);
verify (sizeof (struct remote_message_header) == 24);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cygwin: fix abort of virsh on my system
Chris Lalancette clala...@redhat.com wrote on 05/06/2010 02:28:18 PM: On 05/06/2010 02:11 PM, Stefan Berger wrote: This strange patch fixes aborts of virsh on my system. It seems that the string returned by xmlSaveUri is owned by the xml library?? [...] So something else might be going on. What version of libxml2 are you currently building against? Cygwin picks up what I have installed in /cydrive/c/GTK/ and there I have version 2.6.32. xmlversion.h:#define LIBXML_DOTTED_VERSION 2.6.32 The abort occurs upon the VIR_FREE(name). Regards, Stefan -- Chris Lalancette -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cygwin: fix abort of virsh on my system
Stefan Berger/Watson/IBM wrote on 05/06/2010 02:55:31 PM: Chris Lalancette clala...@redhat.com wrote on 05/06/2010 02:28:18 PM: On 05/06/2010 02:11 PM, Stefan Berger wrote: This strange patch fixes aborts of virsh on my system. It seems that the string returned by xmlSaveUri is owned by the xml library?? [...] So something else might be going on. What version of libxml2 are you currently building against? Cygwin picks up what I have installed in /cydrive/c/GTK/ and there I have version 2.6.32. xmlversion.h:#define LIBXML_DOTTED_VERSION 2.6.32 The abort occurs upon the VIR_FREE(name). That was also the problem. Since the includes weren't missing I didn't have cygwin's libvirt-devel package installed and so it took the wrong includes... Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/5] build: update gnulib
On 05/06/2010 09:28 AM, Jim Meyering wrote: Because of the new patch, I'll wait for a re-ACK before pushing; but 1 through 4 are pretty much unchanged from their original ACK. To make it easier, here's the interdiff between my original series and this v2 series: Thanks for taking the time. That does indeed help. ACK. Thanks for the review, and for teaching me some patience while waiting for it :) Now 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 1/2] Refactor disk unplugging
On 05/05/2010 08:52 AM, Wolfgang Mauerer wrote:
We can reuse some of the code for other purposes.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
---
src/qemu/qemu_driver.c | 56 ++-
1 files changed, 36 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 47ae52c..63ca57c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7867,6 +7867,36 @@ cleanup:
}
+static inline int qemudFindDisk(virDomainDefPtr def, char *dst)
dst can be const char *.
+{
+int i;
+
+for (i = 0 ; i def-ndisks ; i++) {
+if (STREQ(def-disks[i]-dst, dst)) {
+return i;
+}
+}
+
+return -1;
+}
+
+static inline void qemudShrinkDisks(virDomainDefPtr def, int i)
And i can be unsigned (better yet, size_t).
+{
+if (def-ndisks 1) {
+memmove(def-disks + i,
+def-disks + i + 1,
+sizeof(*def-disks) *
+(def-ndisks - (i + 1)));
+def-ndisks--;
+if (VIR_REALLOC_N(def-disks, def-ndisks) 0) {
+/* ignore, harmless */
+}
+} else {
+VIR_FREE(def-disks);
+def-ndisks = 0;
+}
+}
But since this is just code motion for future use, it looks fine to me.
ACK, and I went ahead and pushed it with those edits.
--
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] qemu: use better types
* src/qemu/qemu_driver.c (qemudFindDisk): Mark parameter const.
(qemudShrinkDisks): Mark parameter unsigned.
---
+static inline int qemudFindDisk(virDomainDefPtr def, char *dst)
dst can be const char *.
+static inline void qemudShrinkDisks(virDomainDefPtr def, int i)
And i can be unsigned (better yet, size_t).
But since this is just code motion for future use, it looks fine to me.
ACK, and I went ahead and pushed it with those edits.
Silly me; I made the changes in my editor, but did not save them, so I
ended up pushing your patch as written. I'm pushing this followup as
trivial, to match my intentions.
src/qemu/qemu_driver.c | 4 ++-- 1 files changed, 2 insertions(+), 2
deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 90b9089..cbf6602 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7866,7 +7866,7 @@ cleanup:
}
-static inline int qemudFindDisk(virDomainDefPtr def, char *dst)
+static inline int qemudFindDisk(virDomainDefPtr def, const char *dst)
{
int i;
@@ -7879,7 +7879,7 @@ static inline int qemudFindDisk(virDomainDefPtr def, char
*dst)
return -1;
}
-static inline void qemudShrinkDisks(virDomainDefPtr def, int i)
+static inline void qemudShrinkDisks(virDomainDefPtr def, size_t i)
{
if (def-ndisks 1) {
memmove(def-disks + i,
--
1.6.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] Implement SCSI disk unplugging
On 05/05/2010 08:52 AM, Wolfgang Mauerer wrote:
With the introduction of the generic qemu device model, unplugging
SCSI disks works like a charm, so support it in libvirt.
* src/qemu/qemu_driver.c: Add qemudDomainDetachSCSIDiskDevice() to do the
unplugging, extend qemudDomainDetachDeviceAdd().
Good, you addressed Daniel's comments.
@@ -7940,6 +7940,51 @@ static int qemudDomainDetachPciDiskDevice(struct
qemud_driver *driver,
qemudShrinkDisks(vm-def, i);
+if (driver-securityDriver
+driver-securityDriver-domainRestoreSecurityImageLabel
+driver-securityDriver-domainRestoreSecurityImageLabel(vm,
dev-data.disk) 0)
+VIR_WARN(Unable to restore security label on %s,
dev-data.disk-src);
+
+ret = 0;
+
+cleanup:
+return ret;
+}
Sometimes diff algorithms pick the oddest points to start at, don't they :)
+
+static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev,
+ unsigned long long qemuCmdFlags)
Your spacing here wasn't quite right ('SCSI' is longer than 'Pci', so
the '(' is one column further over), so I adjusted that (and this time,
I saved my editor and amended the patch before sending this email). You
also had some trailing blanks, caught by 'make syntax-check'.
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] locking down struct size/layout in remote-protocol.x
On 05/06/2010 12:35 PM, Jim Meyering wrote:
This week we noticed that a change to struct remote_error
was causing trouble (new member addition changed the size of
the object being decoded -- bad for the protocol).
In order to ensure that no such changes sneak in again,
I'm planning to do the following.
pdwtags src/libvirt_driver_remote_la-remote_protocol.o
prints, among other things, detailed type info for every struct:
/* 89 */
struct remote_nonnull_domain {
remote_nonnull_string name; /* 0 8 */
remote_uuiduuid; /* 816 */
intid; /*24 4 */
/* size: 32, cachelines: 1, members: 3 */
/* last cacheline: 32 bytes */
/* BRAIN FART ALERT! 32 != 28 + 0(holes), diff = 4 */
}; /* size: 32 */
Ouch. Architecture sizing plays a role. On a 32-bit machine, the first
struct is:
/* 86 */
struct remote_nonnull_domain {
remote_nonnull_string name; /* 0 4 */
remote_uuiduuid; /* 416 */
intid; /*20 4 */
/* size: 24, cachelines: 1, members: 3 */
/* last cacheline: 24 bytes */
}; /* size: 24 */
Are we sure migration between 32-bit and 64-bit hypervisors works? And
if it does, then these structs don't quite match what is actually sent
over the wire. At any rate,
And here's a sample of its output:
verify (sizeof (struct remote_nonnull_domain) == 32);
we'd have to make this output conditional on sizeof(void*) to be
portable to both 32- and 64- bit machines.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [TCK PATCHv4] block devices: allow specification of size for safety
I was getting failures of domain/103-blockdev-save-restore.t when
connecting as qemu:///session, since my uid could stat /dev/sdb but
not open it. That test now skips for unprivileged users, as well as
adds a layer of sanity checking against expected size to avoid
trashing the wrong device.
* conf/default.cfg (host_block_devices): Document optional size.
* lib/Sys/Virt/TCK.pm (get_host_block_device): If optional size is
supplied, skip a device that does not match. Also, avoid devices
that can't be opened.
---
Thanks again to Jim and Daniel for the helpful feedback.
Here's the version I actually pushed, based on your feedback.
changes from v3:
completely determine $device before moving on to $kb_blocks
early exit if $device is undef after querying cfg file
reduce scope of $match
conf/default.cfg|8
lib/Sys/Virt/TCK.pm | 15 ++-
2 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/conf/default.cfg b/conf/default.cfg
index 01f438c..d749f5f 100644
--- a/conf/default.cfg
+++ b/conf/default.cfg
@@ -134,5 +134,13 @@ host_pci_devices = (
# the test suite itself needs to create partitions.
# The disks should be at *least* 512 MB in size
host_block_devices = (
+# Each block device is either a raw path
# /dev/vdb
+# or a path plus size in 1k blocks, as in /proc/partitions, to avoid
+# trashing the wrong device
+# {
+#path = /dev/disk/by-id/usb-Generic_Flash_Disk_9B46E3C5-0:0
+#size = 989184
+# }
+# Can list more than on block device if many are available
)
diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
index 9cdef09..2485d0f 100644
--- a/lib/Sys/Virt/TCK.pm
+++ b/lib/Sys/Virt/TCK.pm
@@ -33,6 +33,7 @@ use IO::Uncompress::Gunzip qw(gunzip);
use IO::Uncompress::Bunzip2 qw(bunzip2);
use XML::XPath;
use Carp qw(cluck carp);
+use Fcntl qw(O_RDONLY SEEK_END);
use Test::Builder;
use Sub::Uplevel qw(uplevel);
@@ -833,7 +834,19 @@ sub get_host_block_device {
my $self = shift;
my $devindex = @_ ? shift : 0;
-return $self-config(host_block_devices/[$devindex], undef);
+my $device = ($self-config(host_block_devices/[$devindex]/path, undef)
+ || $self-config(host_block_devices/[$devindex], undef));
+return undef unless $device;
+
+my $kb_blocks = $self-config(host_block_devices/[$devindex]/size, 0);
+
+# Filter out devices that the current user can't open.
+sysopen(BLK, $device, O_RDONLY) or return undef;
+my $match = ($kb_blocks ? sysseek(BLK, 0, SEEK_END) == $kb_blocks * 1024
+: 1);
+close BLK;
+
+return $match ? $device : undef;
}
1;
--
1.6.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
