Re: [libvirt] Java binding blockStats bug (libvirt-java-0.2.1)
On Sun, May 10, 2009 at 02:06:54AM +0300, Zvi Dubitzky wrote: running blcokStats() with Java binding (libvirt-java-0.2.1 ) I get an exception Inspecting the src/jni/org_libvirt_Domain.c is see the following lines of code : [...] The bold line should be replaced by : jclass stats_cls=(*env)-FindClass(env, org/libvirt/DomainBlockStats) as we deal here with blockStats and not network interface Stats ! Ah, right, good catch ! Applied and commited, thanks ! 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] [RFC][PATCH] lxc: fix for ns cgroups subsystem
Hi, On Mon, May 11, 2009 at 10:26 AM, Ryota Ozaki ozaki.ry...@gmail.com wrote: Hi, On Sun, May 10, 2009 at 8:49 AM, Ryota Ozaki ozaki.ry...@gmail.com wrote: Hi Serge and Daniel, On Sat, May 9, 2009 at 4:03 AM, Serge E. Hallyn se...@us.ibm.com wrote: Quoting Daniel P. Berrange (berra...@redhat.com): On Fri, May 08, 2009 at 08:34:12AM -0500, Serge E. Hallyn wrote: Quoting Ryota Ozaki (ozaki.ry...@gmail.com): Hi Serge, On Fri, May 8, 2009 at 11:48 AM, Serge E. Hallyn se...@us.ibm.com wrote: IIUC, the real problem is that src/cgroup.c assumes that the cgroup name should be $CGROUP_MOUNTPOINT/groupname. But of course if the ns cgroup is enabled, then the unshare(CLONE_NEWNS) to create a new namespace in which to mount the new devpts locks the driver under $CGROUP_MOUNTPOINT/pid_of_driver/ or somesuch. If this fixes the problem I have no objections, but it seems more fragile than perhaps trying to teach src/cgroup.c to consider it's current cgroup as a starting point. hmm, I don't know why the assumption is bad and how the approach you are suggesting helps the ns problem. To be clear, the asssumption is that the driver starts in the root cgroup, i.e. it's pid is listed in $CGROUP_MOUNTPOINT/tasks. And that it can create $CGROUP_MOUNTPOINT/groupname and move itself into $CGROUP_MOUNTPOINT/groupname/tasks. So, the assumption is bad because when the driver does a unshare(CLONE_NEWNS), it gets moved into $CGROUP_MOUNTPOINT/X, and after that can only move itself into $CGROUP_MOUNTPOINT/X/groupname. Even with your patch, it's possible for the lxc driver to have been started under say $CGROUP_MOUNTPOINT/libvir or $CGROUP_MOUNTPOINT/username through libcgroup/PAM for instance, in which case your patch would be insufficient. Indeed, we can't assume we're in the root group at any time. Our general plan is that libvirt itself uses 3 levels of cgroup starting at the cgroup that libvirtd was placed in by the admin of the host, then a per-driver group, then per-guest groups $LIBVIRTD_ROOT_CGROUP | +- lxc | | | +- LXC-GUEST-1 | +- LXC-GUEST-2 | +- LXC-GUEST-3 | +- ... | +- qemu | +- QEMU-GUEST-1 +- QEMU-GUEST-2 +- QEMU-GUEST-3 +- ... $LIBVIRTD_ROOT_CGROUP, may be the actaul root mount point for the cgroup controller in question, or it may be a sub-directory that the admin chose to put it in. Also, if running libvirtd as a normal user, the admin may have created per-user account cgroups and so libvirtd would end up in there. So we have to be prepared for anything wrt initial libvirtd cgroup root. NB The code for putting QEMU in a cgroup isn't merged yet. That sounds good. I just don't think the code currently does it. To do the right thing, IIUC, virCgroupPathOfGroup() should parse the /proc/pid/cgroup contents of the 'controller' process, and insert that between the virCgroupGetMount(controller) result and the group name. Or something... (right?) thanks, -serge OK I probably understood the problem and wrote a patch for that according to the Serge's suggestion. I expect this patch fixes the problem, however unfortunately I've not tried it yet under the situation you are worrying and just done regression tests. Because I don't know easy way to produce the situation. If you know that, please let me know... I found a way to go. lxc-unshare helps me and unveils my second path is broken... ozaki-r Thanks, ozaki-r I've updated the patch. The change includes support for multiple mount points of cgroups that I didn't cope with in the previous patch. Through the work, I found a bit messy problem. Current lxc controller writes pid in a 'tasks' file multiple times if one mount point has multiple subsystems. It is bad because the first write changes the cgroups path of a controller, and then the second write points a missing file like $CGROUPS_MOUNTPOINT/path_to_domain/path_to_domain/tasks where the correct file is $CGROUPS_MOUNTPOINT/path_to_domain/tasks. I did workaround this problem with a tricky way by truncating the duplicated path. We probably need a more feasible solution. Note that this patch intends a proof of concept to be clear the correct way to go and thus it remains a couple of awkward codes. Of course the codes should be removed if we find the correct way. Thanks, ozaki-r From 473183a77fbdb002d55acfed0d8f9bd485f548cd Mon Sep 17 00:00:00 2001 From: Ryota Ozaki ozaki.ry...@gmail.com Date: Mon, 11 May 2009 15:18:47 +0900 Subject: [PATCH] [v2] cgroups: address libvirtd's bad assumption for cgroups hierarchy --- src/cgroup.c | 87 - 1 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/cgroup.c b/src/cgroup.c index 50517e2..58ba588 100644 --- a/src/cgroup.c +++
[libvirt] Status and plans for next release
Seems I didn't send any reminder about the next release plans on Friday, so here is a recap: - feature freeze on the 22nd so we have 2 more week to work on reviewing and adding OpenNebula/Power drivers and try to get the NPIV, netcf and secure migration patches in. It's likely not everything will make the release cut but we can try ! - target for next release is Friday 29th So far we have mostly a lot of bug fixes and VirtualBox driver updates commited since 0.6.3, 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 1/3] support for Internal network in libvirt
On Wed, May 06, 2009 at 06:11:42PM +0200, Pritesh Kothari wrote:
Hi All,
As discussed on the list resending the networking patch's. the patch's are as
below:
[PATCH 1/3]: contains support for Internal network in libvirt
[...]
+case VIR_DOMAIN_NET_TYPE_INTERNAL:
+if (internal == NULL) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+No source 'name' attribute specified with interface
type='internal'/);
Okay, looks fine, I just had to mark that diagnostic string for
localization as pointed out by make syntax-check.
The patch being standalone, applied and commited,
thanks !
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 2/3] support for Host only and Internal networks in VirtualBox driver
On Wed, May 06, 2009 at 06:11:50PM +0200, Pritesh Kothari wrote: [PATCH 2/3]: contains support for Host only and Internal networks in VirtualBox driver Okay, standalone, looks fine, so applied and commited too, thanks ! 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 3/3] networking API for hostonly networks in VirtualBox driver in libvirt
On Wed, May 06, 2009 at 06:12:21PM +0200, Pritesh Kothari wrote:
+
+static virNetworkPtr vboxNetworkCreateXML(virConnectPtr conn, const char
*xml) {
+vboxGlobalData *data = conn-privateData;
+virNetworkDefPtr def = NULL;
+virNetworkPtr ret = NULL;
+nsID *iid = NULL;
+char *networkNameUtf8 = NULL;
+int i = 0;
+
+if ((def = virNetworkDefParseString(conn, xml)) == NULL)
+goto cleanup;
+
+if (VIR_ALLOC(iid) 0) {
+virReportOOMError(conn);
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(networkNameUtf8, sizeof(HostInterfaceNetworking-) +
+ sizeof(def-name) + 1) 0) {
Fairly sure that should be 'strlen()' there.
+virReportOOMError(conn);
+goto cleanup;
+}
+
+strcpy (networkNameUtf8, HostInterfaceNetworking-);
+strcat (networkNameUtf8, def-name);
That said, how about just using virAsprintf(nmetworkNameUtf8..
instead of alloc+strcpy
+
+nsIDFromChar(iid, def-uuid);
+
+DEBUG(Network Name: %s, def-name);
+DEBUG(Network UUID:
+ {%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x},
+ (unsigned)iid-m0,(unsigned)iid-m1,
+ (unsigned)iid-m2,(unsigned)iid-m3[0],
+ (unsigned)iid-m3[1], (unsigned)iid-m3[2],
+ (unsigned)iid-m3[3], (unsigned)iid-m3[4],
+ (unsigned)iid-m3[5], (unsigned)iid-m3[6],
+ (unsigned)iid-m3[7]);
+DEBUG(bridge : %s, def-bridge);
+DEBUG(domain : %s, def-domain);
+DEBUG(forwardType : %d, def-forwardType);
+DEBUG(forwardDev : %s, def-forwardDev);
+DEBUG(ipAddress : %s, def-ipAddress);
+DEBUG(netmask : %s, def-netmask);
+DEBUG(network : %s, def-network);
+for (i = 0; i def-nranges; i++) {
+DEBUG(DHCP(%d) start: %s, i, def-ranges[i].start);
+DEBUG(DHCP(%d) end : %s, i, def-ranges[i].end);
+}
+for (i = 0; i def-nhosts; i++) {
+DEBUG(DHCP Host(%d) mac : %s, i, def-hosts[i].mac);
+DEBUG(DHCP Host(%d) name: %s, i, def-hosts[i].name);
+DEBUG(DHCP Host(%d) ip : %s, i, def-hosts[i].ip);
+}
This is rather overkill, since we already log the entire XML document
passed into the public API which has all this info
+
+static virNetworkPtr vboxNetworkDefineXML(virConnectPtr conn, const char
*xml) {
+vboxGlobalData *data = conn-privateData;
+virNetworkDefPtr def = NULL;
+virNetworkPtr ret = NULL;
+nsID *iid = NULL;
+char *networkNameUtf8 = NULL;
+int i = 0;
+
+/* vboxNetworkDefineXML() is not exactly network definition
+ * as the network is up and running, only the DHCP server is off,
+ * so you can always assign static IP and get the network running.
+ */
+if ((def = virNetworkDefParseString(conn, xml)) == NULL)
+goto cleanup;
+
+if (VIR_ALLOC(iid) 0) {
+virReportOOMError(conn);
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(networkNameUtf8, sizeof(HostInterfaceNetworking-) +
+ sizeof(def-name) + 1) 0) {
+virReportOOMError(conn);
+goto cleanup;
+}
+
+strcpy (networkNameUtf8, HostInterfaceNetworking-);
+strcat (networkNameUtf8, def-name);
Same note here, as before, and is most other methods that follow
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 3/3] networking API for hostonly networks in VirtualBox driver in libvirt
On Wed, May 06, 2009 at 06:12:21PM +0200, Pritesh Kothari wrote:
Hi All,
As discussed on the list resending the networking patch's. the patch's are as
below:
[PATCH 3/3]: contains networking API for hostonly networks in VirtualBox
driver in libvirt (it contains all the fix's proposed on list along with two
extra *DefinedNetworks functions)
Hum, I have a doubt here:
+/**
+ * The Network Functions here on
+ */
+static virDrvOpenStatus vboxNetworkOpen(virConnectPtr conn,
+virConnectAuthPtr auth
ATTRIBUTE_UNUSED,
+int flags ATTRIBUTE_UNUSED) {
+vboxGlobalData *data = conn-privateData;
[...]
+DEBUG0(network intialized);
+/* conn-networkPrivateData = some network specific data */
+return VIR_DRV_OPEN_SUCCESS;
[...]
+static int vboxNetworkClose(virConnectPtr conn) {
+DEBUG0(network unintialized);
+conn-networkPrivateData = NULL;
+return 0;
+}
You really don't need to keep any data about the networking driver
in libvirt(d) itself ?
[...]
+/* Currently support only one dhcp server per network
+ * with contigious address space from start to end
+ */
+if ((def-nranges == 1)
+(def-ranges[0].start)
+(def-ranges[0].end)) {
Hum, what about at least allowing the first range if multiple were
defined instead of disabling DHCP completely. I would suggest to
start with if ((def-nranges = 1) instead.
[...]
+/* Currently support only one dhcp server per network
+ * with contigious address space from start to end
+ */
+if ((def-nranges == 1)
+(def-ranges[0].start)
+(def-ranges[0].end)) {
Same here.
The patch is large but reuses the common routines for conversion
to/from XML, a lot of the code is vbox internal structures peek/poke
which are a bit hard to follow but this looks regular, the patch looks
fine to me but feedback on previous points would be appreciated :-)
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 1/3] Support for vrdp/sdl/gui while defining a machine
On Thu, May 07, 2009 at 03:49:28PM +0200, Pritesh Kothari wrote:
Hi All,
I have added support for vrdp/sdl/gui modes for VirtualBox driver in libvirt.
Tha patch's are as below:
[PATCH 1/3]: contains support for vrdp/sdl/gui while defining a machine.
[PATCH 2/3]: contains support for vrdp/sdl/gui while dumping xml
[PATCH 3/3]: contains support for vrdp/sdl/gui while starting the machine
Regards,
Pritesh
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index b25e93b..87db6ab 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3103,9 +3093,86 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr
conn, const char *xml) {
VRDPServer-vtbl-nsisupports.Release((nsISupports
*)VRDPServer);
}
}
+
+if ((def-graphics[i]-type ==
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP) (guiPresent == 0)) {
+guiPresent = 1;
+guiDisplay =
strdup(def-graphics[i]-data.desktop.display);
+}
+
+if ((def-graphics[i]-type == VIR_DOMAIN_GRAPHICS_TYPE_SDL)
(sdlPresent == 0)) {
+sdlPresent = 1;
+sdlDisplay = strdup(def-graphics[i]-data.sdl.display);
+}
+}
Need to check for OOM failure here.
+
+if ((vrdpPresent == 1) (guiPresent == 0) (sdlPresent ==
0)) {
+/* store extradata key that frontend is set to vrdp */
+PRUnichar *keyTypeUtf16 = NULL;
+PRUnichar *valueTypeUtf16 = NULL;
+
+data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Type, keyTypeUtf16);
+data-pFuncs-pfnUtf8ToUtf16(vrdp, valueTypeUtf16);
+
+machine-vtbl-SetExtraData(machine, keyTypeUtf16,
valueTypeUtf16);
+
+data-pFuncs-pfnUtf16Free(keyTypeUtf16);
+data-pFuncs-pfnUtf16Free(valueTypeUtf16);
+
+} else if ((guiPresent == 0) (sdlPresent == 1)) {
+/* store extradata key that frontend is set to sdl */
+PRUnichar *keyTypeUtf16 = NULL;
+PRUnichar *valueTypeUtf16= NULL;
+PRUnichar *keyDislpayUtf16 = NULL;
+PRUnichar *valueDisplayUtf16 = NULL;
+
+data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Type, keyTypeUtf16);
+data-pFuncs-pfnUtf8ToUtf16(sdl, valueTypeUtf16);
+
+machine-vtbl-SetExtraData(machine, keyTypeUtf16,
valueTypeUtf16);
+
+data-pFuncs-pfnUtf16Free(keyTypeUtf16);
+data-pFuncs-pfnUtf16Free(valueTypeUtf16);
+
+if (sdlDisplay) {
+data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Display,
keyDislpayUtf16);
+data-pFuncs-pfnUtf8ToUtf16(sdlDisplay,
valueDisplayUtf16);
+
+machine-vtbl-SetExtraData(machine, keyDislpayUtf16,
valueDisplayUtf16);
+
+data-pFuncs-pfnUtf16Free(keyDislpayUtf16);
+data-pFuncs-pfnUtf16Free(valueDisplayUtf16);
+}
+
+} else {
+/* if all are set then default is gui, with vrdp turned on */
+PRUnichar *keyTypeUtf16 = NULL;
+PRUnichar *valueTypeUtf16= NULL;
+PRUnichar *keyDislpayUtf16 = NULL;
+PRUnichar *valueDisplayUtf16 = NULL;
+
+data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Type, keyTypeUtf16);
+data-pFuncs-pfnUtf8ToUtf16(gui, valueTypeUtf16);
+
+machine-vtbl-SetExtraData(machine, keyTypeUtf16,
valueTypeUtf16);
+
+data-pFuncs-pfnUtf16Free(keyTypeUtf16);
+data-pFuncs-pfnUtf16Free(valueTypeUtf16);
+
+if (guiDisplay) {
+data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Display,
keyDislpayUtf16);
+data-pFuncs-pfnUtf8ToUtf16(guiDisplay,
valueDisplayUtf16);
+
+machine-vtbl-SetExtraData(machine, keyDislpayUtf16,
valueDisplayUtf16);
+
+data-pFuncs-pfnUtf16Free(keyDislpayUtf16);
+data-pFuncs-pfnUtf16Free(valueDisplayUtf16);
+}
}
+
+VIR_FREE(guiDisplay);
+VIR_FREE(sdlDisplay);
+
} /* Finished:Block to attach the Remote Display to VM */
-#endif
{ /* Started:Block to attach USB Devices to VM */
if (def-nhostdevs 0) {
Generally, looks fine apart from the OOM check
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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
Re: [libvirt] [PATCH 2/3] Support for vrdp/sdl/gui while dumping xml
On Thu, May 07, 2009 at 03:49:38PM +0200, Pritesh Kothari wrote:
Hi All,
I have added support for vrdp/sdl/gui modes for VirtualBox driver in libvirt.
Tha patch's are as below:
[PATCH 1/3]: contains support for vrdp/sdl/gui while defining a machine.
[PATCH 2/3]: contains support for vrdp/sdl/gui while dumping xml
[PATCH 3/3]: contains support for vrdp/sdl/gui while starting the machine
Regards,
Pritesh
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 87db6ab..223009a 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
+
+if (valueDisplayUtf16) {
+
data-pFuncs-pfnUtf16ToUtf8(valueDisplayUtf16, valueDisplayUtf8);
+
data-pFuncs-pfnUtf16Free(valueDisplayUtf16);
+
+if (strlen(valueDisplayUtf8) = 0) {
+
data-pFuncs-pfnUtf8Free(valueDisplayUtf8);
+valueDisplayUtf8 = getenv(DISPLAY);
+valueDisplayFree = 0;
+}
} else {
-def-graphics-data.rdp.autoport = 1;
+valueDisplayUtf8 = getenv(DISPLAY);
+valueDisplayFree = 0;
}
Using getenv() here is not right, since that's running in the context of the app
using libvirt. If there's no display available via virtualbox's API, then just
leave this attribute blank when generating the XML.
-VRDPServer-vtbl-GetNetAddress(VRDPServer,
netAddressUtf16);
-if (netAddressUtf16) {
-
data-pFuncs-pfnUtf16ToUtf8(netAddressUtf16, netAddressUtf8);
-if (STRNEQ(netAddressUtf8, ))
-def-graphics-data.rdp.listenAddr =
strdup(netAddressUtf8);
-data-pFuncs-pfnUtf16Free(netAddressUtf16);
-data-pFuncs-pfnUtf8Free(netAddressUtf8);
+if (STREQ(valueTypeUtf8, sdl)) {
+sdlPresent = 1;
+sdlDisplay = strdup(valueDisplayUtf8);
+totalPresent++;
}
-VRDPServer-vtbl-GetAuthType(VRDPServer,
authType);
-if (authType == VRDPAuthType_External) {
-PRUint32 authTimeout = 0;
+if (STREQ(valueTypeUtf8, gui)) {
+guiPresent = 1;
+guiDisplay = strdup(valueDisplayUtf8);
+totalPresent++;
+}
+if (valueDisplayFree)
+data-pFuncs-pfnUtf8Free(valueDisplayUtf8);
+}
-VRDPServer-vtbl-GetAuthTimeout(VRDPServer,
authTimeout);
+if (STREQ(valueTypeUtf8, vrdp))
+vrdpPresent = 1;
-def-graphics-data.rdp.auth =
strdup(external);
-def-graphics-data.rdp.authtimeout =
authTimeout;
-} else if (authType == VRDPAuthType_Guest) {
-PRUint32 authTimeout = 0;
+data-pFuncs-pfnUtf8Free(valueTypeUtf8);
+}
-VRDPServer-vtbl-GetAuthTimeout(VRDPServer,
authTimeout);
+if ((totalPresent 0) (VIR_ALLOC_N(def-graphics,
totalPresent) = 0)) {
+if ((guiPresent)
(VIR_ALLOC(def-graphics[def-ngraphics]) = 0)) {
+def-graphics[def-ngraphics]-type =
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
+
def-graphics[def-ngraphics]-data.desktop.display = guiDisplay;
+def-ngraphics++;
+}
-def-graphics-data.rdp.auth =
strdup(guest);
-def-graphics-data.rdp.authtimeout =
authTimeout;
-}
+if ((sdlPresent)
(VIR_ALLOC(def-graphics[def-ngraphics]) = 0)) {
+def-graphics[def-ngraphics]-type =
VIR_DOMAIN_GRAPHICS_TYPE_SDL;
+def-graphics[def-ngraphics]-data.sdl.display
= sdlDisplay;
+def-ngraphics++;
+}
+} else if ((vrdpPresent != 1) (totalPresent == 0)
(VIR_ALLOC_N(def-graphics, 1) = 0)) {
+if (VIR_ALLOC(def-graphics[def-ngraphics]) = 0) {
+
Re: [libvirt] [PATCH 3/3] Support for vrdp/sdl/gui while starting the machine
On Thu, May 07, 2009 at 03:50:13PM +0200, Pritesh Kothari wrote:
Hi All,
I have added support for vrdp/sdl/gui modes for VirtualBox driver in libvirt.
Tha patch's are as below:
[PATCH 1/3]: contains support for vrdp/sdl/gui while defining a machine.
[PATCH 2/3]: contains support for vrdp/sdl/gui while dumping xml
[PATCH 3/3]: contains support for vrdp/sdl/gui while starting the machine
Regards,
Pritesh
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 223009a..97a686f 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2322,20 +2322,12 @@ static int vboxDomainCreate(virDomainPtr dom) {
IProgress *progress= NULL;
PRUint32 machineCnt= 0;
PRUnichar *env = NULL;
-const char *display= getenv(DISPLAY);
PRUnichar *sessionType = NULL;
char displayutf8[32] = {0};
unsigned char iidl[VIR_UUID_BUFLEN] = {0};
int i, ret = -1;
-if (display) {
-sprintf(displayutf8, DISPLAY=%s, display);
-data-pFuncs-pfnUtf8ToUtf16(displayutf8, env);
-}
-
-data-pFuncs-pfnUtf8ToUtf16(gui, sessionType);
-
if (!dom-name) {
vboxError(dom-conn, VIR_ERR_INTERNAL_ERROR,%s,
Error while reading the domain name);
@@ -2373,6 +2365,92 @@ static int vboxDomainCreate(virDomainPtr dom) {
if ( (state == MachineState_PoweredOff) ||
(state == MachineState_Saved) ||
(state == MachineState_Aborted) ) {
+int vrdpPresent = 0;
+int sdlPresent = 0;
+int guiPresent = 0;
+char *guiDisplay = NULL;
+char *sdlDisplay = NULL;
+PRUnichar *keyTypeUtf16 = NULL;
+PRUnichar *valueTypeUtf16= NULL;
+char *valueTypeUtf8 = NULL;
+PRUnichar *keyDislpayUtf16 = NULL;
+PRUnichar *valueDisplayUtf16 = NULL;
+char *valueDisplayUtf8 = NULL;
+int valueDisplayFree = 1;
+
+data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Type,
keyTypeUtf16);
+machine-vtbl-GetExtraData(machine, keyTypeUtf16,
valueTypeUtf16);
+data-pFuncs-pfnUtf16Free(keyTypeUtf16);
+
+if (valueTypeUtf16) {
+data-pFuncs-pfnUtf16ToUtf8(valueTypeUtf16,
valueTypeUtf8);
+data-pFuncs-pfnUtf16Free(valueTypeUtf16);
+
+if ( STREQ(valueTypeUtf8, sdl) ||
STREQ(valueTypeUtf8, gui) ) {
+
+
data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Display, keyDislpayUtf16);
+machine-vtbl-GetExtraData(machine,
keyDislpayUtf16, valueDisplayUtf16);
+data-pFuncs-pfnUtf16Free(keyDislpayUtf16);
+
+if (valueDisplayUtf16) {
+
data-pFuncs-pfnUtf16ToUtf8(valueDisplayUtf16, valueDisplayUtf8);
+
data-pFuncs-pfnUtf16Free(valueDisplayUtf16);
+
+if (strlen(valueDisplayUtf8) = 0) {
+
data-pFuncs-pfnUtf8Free(valueDisplayUtf8);
+valueDisplayUtf8 = getenv(DISPLAY);
+valueDisplayFree = 0;
+}
+} else {
+valueDisplayUtf8 = getenv(DISPLAY);
+valueDisplayFree = 0;
+}
+
+if (STREQ(valueTypeUtf8, sdl)) {
+sdlPresent = 1;
+sdlDisplay = strdup(valueDisplayUtf8);
+}
+
+if (STREQ(valueTypeUtf8, gui)) {
+guiPresent = 1;
+guiDisplay = strdup(valueDisplayUtf8);
+}
+}
+
+if (STREQ(valueTypeUtf8, vrdp)) {
+vrdpPresent = 1;
+}
+
+data-pFuncs-pfnUtf8Free(valueTypeUtf8);
+
+} else {
+guiPresent = 1;
+guiDisplay = strdup(getenv(DISPLAY));
+}
+if (valueDisplayFree)
+data-pFuncs-pfnUtf8Free(valueDisplayUtf8);
+
+
Re: [libvirt] [PATCH 1/5] Public API for new virInterface* functions, which facilitate
On Fri, May 08, 2009 at 01:52:23PM -0400, Laine Stump wrote: diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 2f7076f..cee3d94 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -854,6 +854,90 @@ int virNetworkGetAutostart (virNetworkPtr network, int virNetworkSetAutostart (virNetworkPtr network, int autostart); +/* + * Physical host interface configuration API + */ + +/** + * virInterface: + * + * a virInterface is a private structure representing a virtual interface. + */ +typedef struct _virInterface virInterface; + +/** + * virInterfacePtr: + * + * a virInterfacePtr is pointer to a virInterface private structure, this is the + * type used to reference a virtual interface in the API. + */ +typedef virInterface *virInterfacePtr; + +/* + * Get connection from interface. + */ +virConnectPtr virInterfaceGetConnect(virInterfacePtr interface); + +/* + * List defined interfaces + */ +int virConnectNumOfInterfaces (virConnectPtr conn); +int virConnectListInterfaces (virConnectPtr conn, + char **const names, + int maxnames); + +/* + * Lookup interface by name or MAC address + */ +virInterfacePtr virInterfaceLookupByName (virConnectPtr conn, + const char *name); +virInterfacePtr virInterfaceLookupByMAC (virConnectPtr conn, + const unsigned char *mac); +virInterfacePtr virInterfaceLookupByMACString (virConnectPtr conn, + const char *mac); + +/* + * Interface information + */ +const char* virInterfaceGetName (virInterfacePtr interface); +int virInterfaceGetMAC(virInterfacePtr interface, + unsigned char *mac); +int virInterfaceGetMACString (virInterfacePtr interface, + char *mac); + +char * virInterfaceGetXMLDesc(virInterfacePtr interface, + int flags); +/* + * Define interface (or modify existing interface configuration) + */ +virInterfacePtr virInterfaceDefineXML (virConnectPtr conn, + const char *xmlDesc, + int flags); + +/* + * Delete interface + */ +int virInterfaceUndefine (virInterfacePtr interface); + +/* + * Activate interface (ie call ifup) + */ +int virInterfaceCreate(virInterfacePtr interface, + int flags); + +/* + * De-activate interface (call ifdown) + */ +int virInterfaceDestroy (virInterfacePtr interface, + int flags); + +/* + * interface object memory management - you must call + * virInterfaceFree() once for each call to virInterfaceRef, or any + * API function that returns a virInterfacePtr. + */ +int virInterfaceRef (virInterfacePtr interface); +int virInterfaceFree (virInterfacePtr interface); /** * virStoragePool: ACK, looks OK to me. BTW, there's no need for the comments in the header, as the src/libvirt.c file contains formal docs for each API that are then put in website in generated API docs Regards Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 2/5] First level of plumbing for virInterface*.
On Fri, May 08, 2009 at 01:52:24PM -0400, Laine Stump wrote: From: Laine Stump la...@redhat.com --- include/libvirt/libvirt.h| 18 ++ include/libvirt/libvirt.h.in | 18 ++ include/libvirt/virterror.h |4 + src/datatypes.h | 25 ++ src/driver.h | 60 src/libvirt.c| 695 ++ src/util.h |2 - src/virterror.c | 21 ++ 8 files changed, 841 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index 91af6fd..b0d93a2 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -433,6 +433,24 @@ extern virConnectAuthPtr virConnectAuthPtrDefault; #define VIR_UUID_STRING_BUFLEN (36+1) +/** + * VIR_MAC_BUFLEN: + * + * This macro provides the length of the buffer required + * for an interface MAC address + */ + +#define VIR_MAC_BUFLEN (6) + +/** + * VIR_MAC_STRING_BUFLEN: + * + * This macro provides the length of the buffer required + * for virInterfaceGetMACString() + */ + +#define VIR_MAC_STRING_BUFLEN (VIR_MAC_BUFLEN * 3) + Since this is now in our public API, I'm wondering whether we shouldn't make the buflen longer. 6 bytes is fine with wired ethernet, but I'm not sure its sufficient for all network device types in general. eg, my wmaster0 device has a rather longer hardware address wmaster0 Link encap:UNSPEC HWaddr 00-13-02-B9-F9-D3-F4-1F-00-00-00-00-00-00-00-00 UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 RX packets:0 errors:0 dropped:0 overruns:0 frame:0 TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:0 (0.0 b) TX bytes:0 (0.0 b) Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 5/5] Use pkg_config in configure.in to detect presence of libnetcf and
On Fri, May 08, 2009 at 01:52:27PM -0400, Laine Stump wrote: From: Laine Stump la...@redhat.com --- configure.in| 35 --- src/Makefile.am |5 + 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/configure.in b/configure.in index 1cdb64c..4ce4342 100644 --- a/configure.in +++ b/configure.in @@ -28,6 +28,7 @@ GNUTLS_REQUIRED=1.0.25 AVAHI_REQUIRED=0.6.0 POLKIT_REQUIRED=0.6 PARTED_REQUIRED=1.8.0 +NETCF_REQUIRED=0.0.1 dnl Checks for C compiler. AC_PROG_CC @@ -789,9 +790,31 @@ if test $with_qemu:$with_lxc:$with_network != no:no:no; then fi AM_CONDITIONAL([WITH_BRIDGE], [test $with_bridge = yes]) -dnl -dnl Storage driver checks -dnl +dnl netcf library +AC_ARG_WITH([netcf], +[ --with-netcf libnetcf support to configure physical host network interfaces], +[], [with_netcf=check]) + +NETCF_CFLAGS= +NETCF_LIBS= +if test $with_netcf = yes -o $with_netcf = check; then + PKG_CHECK_MODULES(NETCF, netcf = $NETCF_REQUIRED, +[with_netcf=yes], [ +if test $with_netcf = check ; then + with_netcf=no +else + AC_MSG_ERROR( + [You must install libnetcf = $NETCF_REQUIRED to compile libvirt]) +fi + ]) + if test $with_netcf = yes ; then +AC_DEFINE_UNQUOTED([WITH_NETCF], 1, + [whether libnetcf is available to configure physical host network interfaces]) + fi +fi +AM_CONDITIONAL([WITH_NETCF], [test $with_netcf = yes]) +AC_SUBST([NETCF_CFLAGS]) +AC_SUBST([NETCF_LIBS]) AC_ARG_WITH([storage-fs], [ --with-storage-fs with FileSystem backend for the storage driver (on)],[],[with_storage_fs=check]) @@ -1376,6 +1399,7 @@ AC_MSG_NOTICE([Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) AC_MSG_NOTICE([ Network: $with_network]) AC_MSG_NOTICE([Libvirtd: $with_libvirtd]) +AC_MSG_NOTICE([ netcf: $with_netcf]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Storage Drivers]) AC_MSG_NOTICE([]) @@ -1443,6 +1467,11 @@ AC_MSG_NOTICE([ devkit: $DEVKIT_CFLAGS $DEVKIT_LIBS]) else AC_MSG_NOTICE([ devkit: no]) fi +if test $with_netcf = yes ; then +AC_MSG_NOTICE([ netcf: $NETCF_CFLAGS $NETCF_LIBS]) +else +AC_MSG_NOTICE([ netcf: no]) +fi AC_MSG_NOTICE([]) AC_MSG_NOTICE([Test suite]) AC_MSG_NOTICE([]) diff --git a/src/Makefile.am b/src/Makefile.am index fd692b4..7d7ef74 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -351,6 +351,11 @@ endif libvirt_driver_network_la_SOURCES = $(NETWORK_DRIVER_SOURCES) endif +if WITH_NETCF +libvirt_driver_interface_la_CFLAGS = $(NETCF_CFLAGS) +libvirt_driver_interface_la_LDFLAGS = $(NETCF_LIBS) +endif + # Needed to keep automake quiet about conditionals libvirt_driver_storage_la_SOURCES = if WITH_STORAGE_DIR ACK, looks good to me. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 3/3] networking API for hostonly networks in VirtualBox driver in libvirt
On Mon, May 11, 2009 at 12:17:17PM +0200, Daniel Veillard wrote:
On Wed, May 06, 2009 at 06:12:21PM +0200, Pritesh Kothari wrote:
+/**
+ * The Network Functions here on
+ */
+static virDrvOpenStatus vboxNetworkOpen(virConnectPtr conn,
+virConnectAuthPtr auth
ATTRIBUTE_UNUSED,
+int flags ATTRIBUTE_UNUSED) {
+vboxGlobalData *data = conn-privateData;
[...]
+DEBUG0(network intialized);
+/* conn-networkPrivateData = some network specific data */
+return VIR_DRV_OPEN_SUCCESS;
[...]
+static int vboxNetworkClose(virConnectPtr conn) {
+DEBUG0(network unintialized);
+conn-networkPrivateData = NULL;
+return 0;
+}
You really don't need to keep any data about the networking driver
in libvirt(d) itself ?
Nah, this is fine in the virtualbox case. The driver is essentially
a stateless driver, the only info being the RPC function tables
which are stored in conn-privateData.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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] how to configure keymap keyboard layout with virt-manager
On Sat, May 09, 2009 at 11:07:46PM +0200, libv...@pivert.org wrote: Hi, I'm fighting to configure the keyboard on libvirt (kvm) (0.6.1-0ubuntu5). I'm using a belgian keyboard, so, my question is : - Which keyboard layout should I place in the xml file (or through the vm display adapter configuration). - Which keyboard layout should I use in the vm ? PS: When I use 'be' as keymap on the vnc line, the libvirtd just segfault at startup. libvirtd or QEMU ? I don't see why libvirtd would SEGV for a 'be' keyboard, but QEMU certainly might if it were not supported. QEMU only supports a handful of keymaps (see /usr/share/qemu/keymaps) and I dont think 'be' is one of them. Our recommendation is to never set the '-k' /keymap option at all these days. Recent QEMU / KVM and GTK-VNC releases support a VNC extension for sending raw keycodes, instead of localized keysyms. Thus is best to leave off the keymap in the config, and just configure it inside the guest OS. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 3/5] Implement RPC part of interface config API.
On Fri, May 08, 2009 at 01:52:25PM -0400, Laine Stump wrote: --- qemud/remote.c | 235 qemud/remote_dispatch_args.h |8 + qemud/remote_dispatch_prototypes.h | 63 +++ qemud/remote_dispatch_ret.h|6 + qemud/remote_dispatch_table.h | 50 + qemud/remote_protocol.c| 156 qemud/remote_protocol.h| 127 + qemud/remote_protocol.x| 90 +- src/datatypes.c| 154 src/datatypes.h|6 + src/libvirt.c | 10 - src/remote_internal.c | 351 12 files changed, 1245 insertions(+), 11 deletions(-) ACK, this all looks fine. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 4/5] Publish the new Interface config API beginning with 0.6.4.
On Fri, May 08, 2009 at 01:52:26PM -0400, Laine Stump wrote:
---
src/libvirt_public.syms | 20
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index b8f9128..cd369cb 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -264,4 +264,24 @@ LIBVIRT_0.6.3 {
virNodeDeviceDestroy;
} LIBVIRT_0.6.1;
+LIBVIRT_0.6.4 {
+global:
+ virInterfaceGetConnect;
+ virConnectNumOfInterfaces;
+ virConnectListInterfaces;
+ virInterfaceLookupByName;
+ virInterfaceLookupByMAC;
+ virInterfaceLookupByMACString;
+ virInterfaceGetName;
+ virInterfaceGetMAC;
+ virInterfaceGetMACString;
+ virInterfaceGetXMLDesc;
+ virInterfaceRef;
+ virInterfaceFree;
+ virInterfaceDefineXML;
+ virInterfaceUndefine;
+ virInterfaceCreate;
+ virInterfaceDestroy;
+
+} LIBVIRT_0.6.3;
# define new API here using predicted next version number
--
ACK, standard stuff.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: 0/4: Fix the event loop (again) no really, this time its fixed.
Back in March I attempted to fix the event loop handling of deleted file handles http://www.redhat.com/archives/libvir-list/2009-March/msg00246.html but have merely succeeded in screwing it up in a different fun exciting way. That patch fixed a case where there was a deleted file handle in the list at the time virEventRunOnce() starts. Which was nice. Not nice, though was that it then breaks the case where a file handle is deleted during the course of dispatching events afer poll(). Investigating this also identified that when we were marking a file handle as deleted, we were forgetting to run virEventInterrupt(), which meant that in some cases the main thread would not be woken up until the next event triggers. Not very critical, but at the same time, not good because it delays cleanup release of resources. This series of patches fixes the problems, and more importantly, adds a unit test which cover these nasty edge cases. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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: 1/4: Remove qemudSetNonlock/SetCloseExec
The qemudSetNonBlock() and qemudSetCloseExec() should not exist anymore
since we have impls virSetNonBlock virSetCloseExec in util.h. Removing
these makes it possible to link directly to qemud/event.c for a later
test case.
Daniel
diff -r d5dc15adcbea qemud/event.c
--- a/qemud/event.c Fri May 08 11:58:19 2009 +0100
+++ b/qemud/event.c Fri May 08 15:52:29 2009 +0100
@@ -30,7 +30,8 @@
#include errno.h
#include unistd.h
-#include qemud.h
+#include threads.h
+#include logging.h
#include event.h
#include memory.h
#include util.h
@@ -597,10 +598,10 @@ int virEventInit(void)
return -1;
if (pipe(eventLoop.wakeupfd) 0 ||
-qemudSetNonBlock(eventLoop.wakeupfd[0]) 0 ||
-qemudSetNonBlock(eventLoop.wakeupfd[1]) 0 ||
-qemudSetCloseExec(eventLoop.wakeupfd[0]) 0 ||
-qemudSetCloseExec(eventLoop.wakeupfd[1]) 0)
+virSetNonBlock(eventLoop.wakeupfd[0]) 0 ||
+virSetNonBlock(eventLoop.wakeupfd[1]) 0 ||
+virSetCloseExec(eventLoop.wakeupfd[0]) 0 ||
+virSetCloseExec(eventLoop.wakeupfd[1]) 0)
return -1;
if (virEventAddHandleImpl(eventLoop.wakeupfd[0],
diff -r d5dc15adcbea qemud/qemud.c
--- a/qemud/qemud.c Fri May 08 11:58:19 2009 +0100
+++ b/qemud/qemud.c Fri May 08 15:52:29 2009 +0100
@@ -371,32 +371,6 @@ qemudDispatchSignalEvent(int watch ATTRI
virMutexUnlock(server-lock);
}
-int qemudSetCloseExec(int fd) {
-int flags;
-if ((flags = fcntl(fd, F_GETFD)) 0)
-goto error;
-flags |= FD_CLOEXEC;
-if ((fcntl(fd, F_SETFD, flags)) 0)
-goto error;
-return 0;
- error:
-VIR_ERROR0(_(Failed to set close-on-exec file descriptor flag));
-return -1;
-}
-
-
-int qemudSetNonBlock(int fd) {
-int flags;
-if ((flags = fcntl(fd, F_GETFL)) 0)
-goto error;
-flags |= O_NONBLOCK;
-if ((fcntl(fd, F_SETFL, flags)) 0)
-goto error;
-return 0;
- error:
-VIR_ERROR0(_(Failed to set non-blocking file descriptor flag));
-return -1;
-}
static int qemudGoDaemon(void) {
int pid = fork();
@@ -525,8 +499,8 @@ static int qemudListenUnix(struct qemud_
goto cleanup;
}
-if (qemudSetCloseExec(sock-fd) 0 ||
-qemudSetNonBlock(sock-fd) 0)
+if (virSetCloseExec(sock-fd) 0 ||
+virSetNonBlock(sock-fd) 0)
goto cleanup;
memset(addr, 0, sizeof(addr));
@@ -687,8 +661,8 @@ remoteListenTCP (struct qemud_server *se
else
sock-port = -1;
-if (qemudSetCloseExec(sock-fd) 0 ||
-qemudSetNonBlock(sock-fd) 0)
+if (virSetCloseExec(sock-fd) 0 ||
+virSetNonBlock(sock-fd) 0)
goto cleanup;
if (listen (sock-fd, 30) 0) {
@@ -1273,8 +1247,8 @@ static int qemudDispatchServer(struct qe
setsockopt (fd, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start,
sizeof no_slow_start);
-if (qemudSetCloseExec(fd) 0 ||
-qemudSetNonBlock(fd) 0) {
+if (virSetCloseExec(fd) 0 ||
+virSetNonBlock(fd) 0) {
close(fd);
return -1;
}
@@ -2872,10 +2846,10 @@ int main(int argc, char **argv) {
goto error1;
if (pipe(sigpipe) 0 ||
-qemudSetNonBlock(sigpipe[0]) 0 ||
-qemudSetNonBlock(sigpipe[1]) 0 ||
-qemudSetCloseExec(sigpipe[0]) 0 ||
-qemudSetCloseExec(sigpipe[1]) 0) {
+virSetNonBlock(sigpipe[0]) 0 ||
+virSetNonBlock(sigpipe[1]) 0 ||
+virSetCloseExec(sigpipe[0]) 0 ||
+virSetCloseExec(sigpipe[1]) 0) {
char ebuf[1024];
VIR_ERROR(_(Failed to create pipe: %s),
virStrerror(errno, ebuf, sizeof ebuf));
diff -r d5dc15adcbea qemud/qemud.h
--- a/qemud/qemud.h Fri May 08 11:58:19 2009 +0100
+++ b/qemud/qemud.h Fri May 08 15:52:29 2009 +0100
@@ -198,9 +198,6 @@ void qemudLog(int priority, const char *
ATTRIBUTE_FORMAT(printf,2,3);
-int qemudSetCloseExec(int fd);
-int qemudSetNonBlock(int fd);
-
int
remoteDispatchClientRequest (struct qemud_server *server,
struct qemud_client *client,
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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: 2/4: Interrupt main thread if in poll()
Several methods forgot to call virEventInterruptLocked() in critical
times, or called it at the wrong point. This fixes that problem.
It also changes watch and timer numbers to start from 1, instead of 0.
This is because we've had a number of bugs where code has passed an
uninitialized timer/watch ID to an update/delete method, and this has
resulted in deleting a real important timer/watch. Changing to start
from 1, and logging invalid watches will protect us from uninitialized
watches/timers.
Daniel
diff -r df82b18a3425 qemud/event.c
--- a/qemud/event.c Fri May 08 15:52:29 2009 +0100
+++ b/qemud/event.c Fri May 08 16:06:40 2009 +0100
@@ -84,10 +84,10 @@ struct virEventLoop {
static struct virEventLoop eventLoop;
/* Unique ID for the next FD watch to be registered */
-static int nextWatch = 0;
+static int nextWatch = 1;
/* Unique ID for the next timer to be registered */
-static int nextTimer = 0;
+static int nextTimer = 1;
static void virEventLock(void)
{
@@ -143,15 +143,22 @@ int virEventAddHandleImpl(int fd, int ev
void virEventUpdateHandleImpl(int watch, int events) {
int i;
+EVENT_DEBUG(Update handle w=%d e=%d, watch, events);
+
+if (watch = 0) {
+VIR_WARN(Ignoring invalid update watch %d, watch);
+return;
+}
+
virEventLock();
for (i = 0 ; i eventLoop.handlesCount ; i++) {
if (eventLoop.handles[i].watch == watch) {
eventLoop.handles[i].events =
virEventHandleTypeToPollEvent(events);
+virEventInterruptLocked();
break;
}
}
-virEventInterruptLocked();
virEventUnlock();
}
@@ -164,6 +171,12 @@ void virEventUpdateHandleImpl(int watch,
int virEventRemoveHandleImpl(int watch) {
int i;
EVENT_DEBUG(Remove handle %d, watch);
+
+if (watch = 0) {
+VIR_WARN(Ignoring invalid remove watch %d, watch);
+return -1;
+}
+
virEventLock();
for (i = 0 ; i eventLoop.handlesCount ; i++) {
if (eventLoop.handles[i].deleted)
@@ -172,11 +185,11 @@ int virEventRemoveHandleImpl(int watch)
if (eventLoop.handles[i].watch == watch) {
EVENT_DEBUG(mark delete %d %d, i, eventLoop.handles[i].fd);
eventLoop.handles[i].deleted = 1;
+virEventInterruptLocked();
virEventUnlock();
return 0;
}
}
-virEventInterruptLocked();
virEventUnlock();
return -1;
}
@@ -232,6 +245,12 @@ void virEventUpdateTimeoutImpl(int timer
struct timeval tv;
int i;
EVENT_DEBUG(Updating timer %d timeout with %d ms freq, timer, frequency);
+
+if (timer = 0) {
+VIR_WARN(Ignoring invalid update timer %d, timer);
+return;
+}
+
if (gettimeofday(tv, NULL) 0) {
return;
}
@@ -244,10 +263,10 @@ void virEventUpdateTimeoutImpl(int timer
frequency = 0 ? frequency +
(((unsigned long long)tv.tv_sec)*1000) +
(((unsigned long long)tv.tv_usec)/1000) : 0;
+virEventInterruptLocked();
break;
}
}
-virEventInterruptLocked();
virEventUnlock();
}
@@ -260,6 +279,12 @@ void virEventUpdateTimeoutImpl(int timer
int virEventRemoveTimeoutImpl(int timer) {
int i;
EVENT_DEBUG(Remove timer %d, timer);
+
+if (timer = 0) {
+VIR_WARN(Ignoring invalid remove timer %d, timer);
+return -1;
+}
+
virEventLock();
for (i = 0 ; i eventLoop.timeoutsCount ; i++) {
if (eventLoop.timeouts[i].deleted)
@@ -267,11 +292,11 @@ int virEventRemoveTimeoutImpl(int timer)
if (eventLoop.timeouts[i].timer == timer) {
eventLoop.timeouts[i].deleted = 1;
+virEventInterruptLocked();
virEventUnlock();
return 0;
}
}
-virEventInterruptLocked();
virEventUnlock();
return -1;
}
@@ -617,9 +642,12 @@ static int virEventInterruptLocked(void)
char c = '\0';
if (!eventLoop.running ||
-pthread_self() == eventLoop.leader)
+pthread_self() == eventLoop.leader) {
+VIR_DEBUG(Skip interrupt, %d %d, eventLoop.running,
(int)eventLoop.leader);
return 0;
+}
+VIR_DEBUG0(Interrupting);
if (safewrite(eventLoop.wakeupfd[1], c, sizeof(c)) != sizeof(c))
return -1;
return 0;
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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: 3/4: Fix handling of deleted file handles
This patch fixes handling of deleted file handles. First of all it reverts
the patch http://www.redhat.com/archives/libvir-list/2009-March/msg00246.html
As an alternative solution to that original problem, the first thing that
virEventRunOnce() does, is to purge all timers/watches which were marked as
deleted. This deals with deletes that happen between invocations of the
virEventRunOnce() method. It also ensures that when going into poll(), all
the registered timers/watches are live. This guarentees that array indexes
match up between the poll FD array and our list of watches. Then during
the dispatch of FDs, we have a simpler check to skip invocation of file
handles / timers marked as deleted.
Daniel
diff -r 63d6824fe2a3 qemud/event.c
--- a/qemud/event.c Fri May 08 16:06:40 2009 +0100
+++ b/qemud/event.c Fri May 08 16:07:04 2009 +0100
@@ -313,7 +313,7 @@ static int virEventCalculateTimeout(int
EVENT_DEBUG(Calculate expiry of %d timers, eventLoop.timeoutsCount);
/* Figure out if we need a timeout */
for (i = 0 ; i eventLoop.timeoutsCount ; i++) {
-if (eventLoop.timeouts[i].deleted || eventLoop.timeouts[i].frequency
0)
+if (eventLoop.timeouts[i].frequency 0)
continue;
EVENT_DEBUG(Got a timeout scheduled for %llu,
eventLoop.timeouts[i].expiresAt);
@@ -350,32 +350,26 @@ static int virEventCalculateTimeout(int
* file handles. The caller must free the returned data struct
* returns: the pollfd array, or NULL on error
*/
-static int virEventMakePollFDs(struct pollfd **retfds) {
+static struct pollfd *virEventMakePollFDs(void) {
struct pollfd *fds;
-int i, nfds = 0;
+int i;
+
+/* Setup the poll file handle data structs */
+if (VIR_ALLOC_N(fds, eventLoop.handlesCount) 0)
+return NULL;
for (i = 0 ; i eventLoop.handlesCount ; i++) {
-if (eventLoop.handles[i].deleted)
-continue;
-nfds++;
-}
-*retfds = NULL;
-/* Setup the poll file handle data structs */
-if (VIR_ALLOC_N(fds, nfds) 0)
-return -1;
-
-for (i = 0, nfds = 0 ; i eventLoop.handlesCount ; i++) {
-if (eventLoop.handles[i].deleted)
-continue;
-fds[nfds].fd = eventLoop.handles[i].fd;
-fds[nfds].events = eventLoop.handles[i].events;
-fds[nfds].revents = 0;
+EVENT_DEBUG(Prepare n=%d w=%d, f=%d e=%d, i,
+eventLoop.handles[i].watch,
+eventLoop.handles[i].fd,
+eventLoop.handles[i].events);
+fds[i].fd = eventLoop.handles[i].fd;
+fds[i].events = eventLoop.handles[i].events;
+fds[i].revents = 0;
//EVENT_DEBUG(Wait for %d %d, eventLoop.handles[i].fd,
eventLoop.handles[i].events);
-nfds++;
}
-*retfds = fds;
-return nfds;
+return fds;
}
@@ -435,26 +429,30 @@ static int virEventDispatchTimeouts(void
* Returns 0 upon success, -1 if an error occurred
*/
static int virEventDispatchHandles(int nfds, struct pollfd *fds) {
-int i, n;
+int i;
-for (i = 0, n = 0 ; i eventLoop.handlesCount n nfds ; i++) {
+/* NB, use nfds not eventLoop.handlesCount, because new
+ * fds might be added on end of list, and they're not
+ * in the fds array we've got */
+for (i = 0 ; i nfds ; i++) {
if (eventLoop.handles[i].deleted) {
-EVENT_DEBUG(Skip deleted %d, eventLoop.handles[i].fd);
+EVENT_DEBUG(Skip deleted n=%d w=%d f=%d, i,
+eventLoop.handles[i].watch, eventLoop.handles[i].fd);
continue;
}
-if (fds[n].revents) {
+if (fds[i].revents) {
virEventHandleCallback cb = eventLoop.handles[i].cb;
void *opaque = eventLoop.handles[i].opaque;
-int hEvents = virPollEventToEventHandleType(fds[n].revents);
-EVENT_DEBUG(Dispatch %d %d %p, fds[n].fd,
-fds[n].revents, eventLoop.handles[i].opaque);
+int hEvents = virPollEventToEventHandleType(fds[i].revents);
+EVENT_DEBUG(Dispatch n=%d f=%d w=%d e=%d %p, i,
+fds[i].fd, eventLoop.handles[i].watch,
+fds[i].revents, eventLoop.handles[i].opaque);
virEventUnlock();
(cb)(eventLoop.handles[i].watch,
- fds[n].fd, hEvents, opaque);
+ fds[i].fd, hEvents, opaque);
virEventLock();
}
-n++;
}
return 0;
@@ -545,22 +543,21 @@ static int virEventCleanupHandles(void)
* at least one file handle has an event, or a timer expires
*/
int virEventRunOnce(void) {
-struct pollfd *fds;
+struct pollfd *fds = NULL;
int ret, timeout, nfds;
virEventLock();
eventLoop.running = 1;
eventLoop.leader = pthread_self();
-if ((nfds = virEventMakePollFDs(fds)) 0) {
-virEventUnlock();
-return -1;
-}
-if
Re: [libvirt] PATCH: 4/4: Test the event loop impl
This patch adds a new test case to validate the various wierd scenarios that
can occur when adding/removing/updating watches timers in the event loop.
In particular it checks
- Handling of watches marked as deleted before virEventRunOnce
- Handling of watches marked as deleted during dispatch of poll() results
- Correct interrupt of main thread if watches change during poll() sleep
These are the 3 problems we've been hitting with all these issues. I've
tested they all fail prior to this patch
http://www.redhat.com/archives/libvir-list/2009-March/msg00246.html
and now all 3 pass
Daniel
diff -r d5dc15adcbea tests/.cvsignore
--- a/tests/.cvsignore Fri May 08 11:58:19 2009 +0100
+++ b/tests/.cvsignore Fri May 08 15:51:11 2009 +0100
@@ -16,6 +16,7 @@ nodeinfotest
statstest
qparamtest
seclabeltest
+eventtest
*.gcda
*.gcno
*.exe
diff -r d5dc15adcbea tests/Makefile.am
--- a/tests/Makefile.am Fri May 08 11:58:19 2009 +0100
+++ b/tests/Makefile.am Fri May 08 15:51:11 2009 +0100
@@ -125,6 +125,11 @@ if WITH_SECDRIVER_SELINUX
TESTS += seclabeltest
endif
+if WITH_LIBVIRTD
+noinst_PROGRAMS += eventtest
+TESTS += eventtest
+endif
+
TESTS += nodedevxml2xmltest
path_add = $$abs_top_builddir/src$(PATH_SEPARATOR)$$abs_top_builddir/qemud
@@ -226,4 +231,10 @@ qparamtest_SOURCES = \
qparamtest.c testutils.h testutils.c
qparamtest_LDADD = $(LDADDS)
+if WITH_LIBVIRTD
+eventtest_SOURCES = \
+ eventtest.c testutils.h testutils.c ../qemud/event.c
+eventtest_LDADD = -lrt $(LDADDS)
+endif
+
CLEANFILES = *.cov *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda
diff -r d5dc15adcbea tests/eventtest.c
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/tests/eventtest.c Fri May 08 15:51:11 2009 +0100
@@ -0,0 +1,444 @@
+/*
+ * eventtest.c: Test the libvirtd event loop impl
+ *
+ * Copyright (C) 2009 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: Daniel P. Berrange berra...@redhat.com
+ */
+
+#include config.h
+
+#include stdlib.h
+#include signal.h
+#include time.h
+
+#include testutils.h
+#include internal.h
+#include threads.h
+#include logging.h
+#include ../qemud/event.h
+
+#define NUM_FDS 5
+#define NUM_TIME 5
+
+static struct handleInfo {
+int pipeFD[2];
+int fired;
+int watch;
+int error;
+int delete;
+} handles[NUM_FDS];
+
+static struct timerInfo {
+int timeout;
+int timer;
+int fired;
+int error;
+int delete;
+} timers[NUM_TIME];
+
+enum {
+EV_ERROR_NONE,
+EV_ERROR_WATCH,
+EV_ERROR_FD,
+EV_ERROR_EVENT,
+EV_ERROR_DATA,
+};
+
+static void
+testPipeReader(int watch, int fd, int events, void *data)
+{
+struct handleInfo *info = data;
+char one;
+
+info-fired = 1;
+
+if (watch != info-watch) {
+info-error = EV_ERROR_WATCH;
+return;
+}
+
+if (fd != info-pipeFD[0]) {
+info-error = EV_ERROR_FD;
+return;
+}
+
+if (!(events VIR_EVENT_HANDLE_READABLE)) {
+info-error = EV_ERROR_EVENT;
+return;
+}
+if (read(fd, one, 1) != 1) {
+info-error = EV_ERROR_DATA;
+return;
+}
+info-error = EV_ERROR_NONE;
+
+if (info-delete != -1)
+virEventRemoveHandleImpl(info-delete);
+}
+
+
+static void
+testTimer(int timer, void *data)
+{
+struct timerInfo *info = data;
+
+info-fired = 1;
+
+if (timer != info-timer) {
+info-error = EV_ERROR_WATCH;
+return;
+}
+
+info-error = EV_ERROR_NONE;
+
+if (info-delete != -1)
+virEventRemoveTimeoutImpl(info-delete);
+}
+
+static pthread_mutex_t eventThreadMutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t eventThreadRunCond = PTHREAD_COND_INITIALIZER;
+static int eventThreadRunOnce = 0;
+static pthread_cond_t eventThreadJobCond = PTHREAD_COND_INITIALIZER;
+static int eventThreadJobDone = 0;
+
+
+static void *eventThreadLoop(void *data ATTRIBUTE_UNUSED) {
+while (1) {
+pthread_mutex_lock(eventThreadMutex);
+while (!eventThreadRunOnce) {
+pthread_cond_wait(eventThreadRunCond, eventThreadMutex);
+}
+eventThreadRunOnce = 0;
+pthread_mutex_unlock(eventThreadMutex);
+
+virEventRunOnce();
+
+pthread_mutex_lock(eventThreadMutex);
+eventThreadJobDone = 1;
Re: [libvirt] [PATCH 3/3] networking API for hostonly networks in VirtualBox driver in libvirt
+
+strcpy (networkNameUtf8, HostInterfaceNetworking-);
+strcat (networkNameUtf8, def-name);
That said, how about just using virAsprintf(nmetworkNameUtf8..
instead of alloc+strcpy
Done.
+DEBUG(Network Name: %s, def-name);
+DEBUG(Network UUID:
+ {%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x},
+ (unsigned)iid-m0,(unsigned)iid-m1,
+ (unsigned)iid-m2,(unsigned)iid-m3[0],
+ (unsigned)iid-m3[1], (unsigned)iid-m3[2],
+ (unsigned)iid-m3[3], (unsigned)iid-m3[4],
+ (unsigned)iid-m3[5], (unsigned)iid-m3[6],
+ (unsigned)iid-m3[7]);
This is rather overkill, since we already log the entire XML document
passed into the public API which has all this info
just keeping the above two because atleast the debug logs should show the UUID
user wants to set and the one which we have, the reason being the default
hostonly adaptor in VirtualBox 2.2.* is created while install and can't be
changed.
+
+strcpy (networkNameUtf8, HostInterfaceNetworking-);
+strcat (networkNameUtf8, def-name);
Same note here, as before, and is most other methods that follow
Done.
Will post a patch soon with other changes suggested on the list.
Regards,
Pritesh
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 0/4: Fix the event loop (again) no really, this time its fixed.
On Mon, 2009-05-11 at 12:11 +0100, Daniel P. Berrange wrote: Back in March I attempted to fix the event loop handling of deleted file handles http://www.redhat.com/archives/libvir-list/2009-March/msg00246.html but have merely succeeded in screwing it up in a different fun exciting way. That patch fixed a case where there was a deleted file handle in the list at the time virEventRunOnce() starts. Which was nice. Not nice, though was that it then breaks the case where a file handle is deleted during the course of dispatching events afer poll(). Investigating this also identified that when we were marking a file handle as deleted, we were forgetting to run virEventInterrupt(), which meant that in some cases the main thread would not be woken up until the next event triggers. Not very critical, but at the same time, not good because it delays cleanup release of resources. This series of patches fixes the problems, and more importantly, adds a unit test which cover these nasty edge cases. It's not obvious to me from your description, so for others, one of the problems these patches fix is where destroying one running guest can cause another to also be destroyed: https://bugzilla.redhat.com/499698 Cheers, Mark. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 0/4: Fix the event loop (again) no really, this time its fixed.
On Mon, May 11, 2009 at 12:47:38PM +0100, Mark McLoughlin wrote: On Mon, 2009-05-11 at 12:11 +0100, Daniel P. Berrange wrote: Back in March I attempted to fix the event loop handling of deleted file handles http://www.redhat.com/archives/libvir-list/2009-March/msg00246.html but have merely succeeded in screwing it up in a different fun exciting way. That patch fixed a case where there was a deleted file handle in the list at the time virEventRunOnce() starts. Which was nice. Not nice, though was that it then breaks the case where a file handle is deleted during the course of dispatching events afer poll(). Investigating this also identified that when we were marking a file handle as deleted, we were forgetting to run virEventInterrupt(), which meant that in some cases the main thread would not be woken up until the next event triggers. Not very critical, but at the same time, not good because it delays cleanup release of resources. This series of patches fixes the problems, and more importantly, adds a unit test which cover these nasty edge cases. It's not obvious to me from your description, so for others, one of the problems these patches fix is where destroying one running guest can cause another to also be destroyed: https://bugzilla.redhat.com/499698 NB That's just one symptom. The core problem is that you end up with an off-by-1 error when dispatching the callbacks. ie, instead of dispatching the nth callback, we dispatch the nth+1 callbck. There are lots of different things registering callbacks so effects are pretty unpredictable. Normally the callbacks for running guests don't align, but if you restart libvirtd while a set of guests are running, you end up with all the guest callbacks registered in series, so are quite likely to see this double destroy. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 3/3] networking API for hostonly networks in VirtualBox driver in libvirt
You really don't need to keep any data about the networking driver
in libvirt(d) itself ?
as danpb explained it :)
+/* Currently support only one dhcp server per
network + * with contigious address space from
start to end + */
+if ((def-nranges == 1)
+(def-ranges[0].start)
+(def-ranges[0].end)) {
Fixed this to def-nranges = 1 instead of above.
The patch is large but reuses the common routines for conversion
to/from XML, a lot of the code is vbox internal structures peek/poke
which are a bit hard to follow but this looks regular, the patch looks
fine to me but feedback on previous points would be appreciated :-)
Reposting the patch as below with all the fix's mentioned on the list.
Regards,
Pritesh.
commit ec631aa294a08bca13543289368449886d5398b0
Author: pk221555 pk221...@krishna.(none)
Date: Mon May 11 14:15:31 2009 +0200
libvirt: networking API for hostonly networks
diff --git a/src/vbox/vbox_driver.c b/src/vbox/vbox_driver.c
index 7db437c..6b782b1 100644
--- a/src/vbox/vbox_driver.c
+++ b/src/vbox/vbox_driver.c
@@ -39,19 +39,23 @@
extern virDriver vbox22Driver;
+extern virNetworkDriver vbox22NetworkDriver;
#if 0
extern virDriver vbox25Driver;
+extern virNetworkDriver vbox25NetworkDriver;
#endif
int vboxRegister(void) {
virDriverPtrdriver;
+virNetworkDriverPtr networkDriver;
uint32_tuVersion;
/* vboxRegister() shouldn't fail as that will render libvirt unless.
* So, we use the v2.2 driver as a fallback/dummy.
*/
driver= vbox22Driver;
+networkDriver = vbox22NetworkDriver;
/* Init the glue and get the API version. */
if (VBoxCGlueInit() == 0) {
@@ -69,10 +73,12 @@ int vboxRegister(void) {
if (uVersion = 2001052 uVersion 2002051) {
DEBUG0(VirtualBox API version: 2.2);
driver= vbox22Driver;
+networkDriver = vbox22NetworkDriver;
#if 0
} else if (uVersion = 2002051 uVersion 2005051) {
DEBUG0(VirtualBox API version: 2.5);
driver= vbox25Driver;
+networkDriver = vbox25NetworkDriver;
#endif
} else {
DEBUG0(Unsupport VirtualBox API version);
@@ -84,6 +90,8 @@ int vboxRegister(void) {
if (virRegisterDriver(driver) 0)
return -1;
+if (virRegisterNetworkDriver(networkDriver) 0)
+return -1;
return 0;
}
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index a27eada..8e42958 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3633,6 +3633,1001 @@ cleanup:
return ret;
}
+/**
+ * The Network Functions here on
+ */
+static virDrvOpenStatus vboxNetworkOpen(virConnectPtr conn,
+virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+int flags ATTRIBUTE_UNUSED) {
+vboxGlobalData *data = conn-privateData;
+
+if (STRNEQ(conn-driver-name, VBOX))
+goto cleanup;
+
+if ((data-pFuncs == NULL) ||
+(data-vboxObj == NULL) ||
+(data-vboxSession == NULL))
+goto cleanup;
+
+DEBUG0(network intialized);
+/* conn-networkPrivateData = some network specific data */
+return VIR_DRV_OPEN_SUCCESS;
+
+cleanup:
+return VIR_DRV_OPEN_DECLINED;
+}
+
+static int vboxNetworkClose(virConnectPtr conn) {
+DEBUG0(network unintialized);
+conn-networkPrivateData = NULL;
+return 0;
+}
+
+static int vboxNumOfNetworks(virConnectPtr conn) {
+vboxGlobalData *data = conn-privateData;
+int numActive = 0;
+
+if (data-vboxObj) {
+IHost *host = NULL;
+
+data-vboxObj-vtbl-GetHost(data-vboxObj, host);
+if (host) {
+int i = 0;
+PRUint32 networkInterfacesSize = 0;
+IHostNetworkInterface **networkInterfaces = NULL;
+
+host-vtbl-GetNetworkInterfaces(host, networkInterfacesSize, networkInterfaces);
+
+for (i = 0; i networkInterfacesSize; i++) {
+if (networkInterfaces[i]) {
+PRUint32 interfaceType = 0;
+
+networkInterfaces[i]-vtbl-GetInterfaceType(networkInterfaces[i], interfaceType);
+if (interfaceType == HostNetworkInterfaceType_HostOnly) {
+PRUint32 status = HostNetworkInterfaceStatus_Unknown;
+
+networkInterfaces[i]-vtbl-GetStatus(networkInterfaces[i], status);
+
+if (status == HostNetworkInterfaceStatus_Up) {
+numActive++;
+}
+}
+
+networkInterfaces[i]-vtbl-nsisupports.Release((nsISupports *) networkInterfaces[i]);
+}
+}
+
+host-vtbl-nsisupports.Release((nsISupports *) host);
+
Re: [libvirt] [PATCH 1/7] Add public API stubs for virStorageVolCreateXMLFrom
On Mon, May 04, 2009 at 01:42:56PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson crobi...@redhat.com
---
include/libvirt/libvirt.h|4 +++
include/libvirt/libvirt.h.in |4 +++
src/driver.h |6
src/libvirt.c| 61 -
src/libvirt_public.syms |5 +++
5 files changed, 78 insertions(+), 2 deletions(-)
ACK, this looks good to me.
Daniel
diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index 30f559d..b1e45e4 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -1047,6 +1047,10 @@ const char* virStorageVolGetKey
(virStorageVolPtr vol);
virStorageVolPtrvirStorageVolCreateXML (virStoragePoolPtr
pool,
const char *xmldesc,
unsigned int flags);
+virStorageVolPtrvirStorageVolCreateXMLFrom (virStoragePoolPtr
pool,
+ const char *xmldesc,
+ unsigned int flags,
+ virStorageVolPtr
clonevol);
int virStorageVolDelete (virStorageVolPtr
vol,
unsigned int flags);
int virStorageVolRef(virStorageVolPtr
vol);
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 2f7076f..f5cadb4 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1047,6 +1047,10 @@ const char* virStorageVolGetKey
(virStorageVolPtr vol);
virStorageVolPtrvirStorageVolCreateXML (virStoragePoolPtr
pool,
const char *xmldesc,
unsigned int flags);
+virStorageVolPtrvirStorageVolCreateXMLFrom (virStoragePoolPtr
pool,
+ const char *xmldesc,
+ unsigned int flags,
+ virStorageVolPtr
clonevol);
int virStorageVolDelete (virStorageVolPtr
vol,
unsigned int flags);
int virStorageVolRef(virStorageVolPtr
vol);
diff --git a/src/driver.h b/src/driver.h
index c357b76..ff12ada 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -586,6 +586,11 @@ typedef char *
typedef char *
(*virDrvStorageVolGetPath) (virStorageVolPtr vol);
+typedef virStorageVolPtr
+(*virDrvStorageVolCreateXMLFrom) (virStoragePoolPtr pool,
+ const char *xmldesc,
+ unsigned int flags,
+ virStorageVolPtr clone);
typedef struct _virStorageDriver virStorageDriver;
@@ -633,6 +638,7 @@ struct _virStorageDriver {
virDrvStorageVolLookupByKey volLookupByKey;
virDrvStorageVolLookupByPath volLookupByPath;
virDrvStorageVolCreateXML volCreateXML;
+virDrvStorageVolCreateXMLFrom volCreateXMLFrom;
virDrvStorageVolDelete volDelete;
virDrvStorageVolGetInfo volGetInfo;
virDrvStorageVolGetXMLDesc volGetXMLDesc;
diff --git a/src/libvirt.c b/src/libvirt.c
index ded18a7..5a51c45 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -6767,6 +6767,65 @@ error:
/**
+ * virStorageVolCreateXMLFrom:
+ * @pool: pointer to parent pool for the new volume
+ * @xmldesc: description of volume to create
+ * @flags: flags for creation (unused, pass 0)
+ * @clonevol: storage volume to use as input
+ *
+ * Create a storage volume in the parent pool, using the
+ * 'clonevol' volume as input. Information for the new
+ * volume (name, perms) are passed via a typical volume
+ * XML description.
+ *
+ * return the storage volume, or NULL on error
+ */
+virStorageVolPtr
+virStorageVolCreateXMLFrom(virStoragePoolPtr pool,
+ const char *xmldesc,
+ unsigned int flags,
+ virStorageVolPtr clonevol)
+{
+DEBUG(pool=%p, flags=%u, clonevol=%p, pool, flags, clonevol);
+
+virResetLastError();
+
+if (!VIR_IS_STORAGE_POOL(pool)) {
+virLibConnError(NULL, VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__);
+return (NULL);
+}
+
+if (!VIR_IS_STORAGE_VOL(clonevol)) {
+virLibConnError(NULL, VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__);
+return (NULL);
+}
+
+if (pool-conn-flags VIR_CONNECT_RO ||
+
Re: [libvirt] [PATCH 1/5] Public API for new virInterface* functions, which facilitate
On Fri, May 08, 2009 at 01:52:23PM -0400, Laine Stump wrote: From: Laine Stump la...@redhat.com --- include/libvirt/libvirt.h| 84 ++ include/libvirt/libvirt.h.in | 84 ++ 2 files changed, 168 insertions(+), 0 deletions(-) + +char * virInterfaceGetXMLDesc(virInterfacePtr interface, + int flags); +virInterfacePtr virInterfaceDefineXML (virConnectPtr conn, + const char *xmlDesc, + int flags); +int virInterfaceCreate(virInterfacePtr interface, + int flags); +int virInterfaceDestroy (virInterfacePtr interface, + int flags); Oh, one very minor point - can you change these to 'unsigned int', since that's what we now prefer to use for flags (yes, some existing APIs are bad in this respect) Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 2/7] Remote driver plumbing for virStorageVolCreateXMLFrom
On Mon, May 04, 2009 at 01:42:57PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson crobi...@redhat.com
---
qemud/remote.c | 34 ++
qemud/remote_dispatch_args.h |1 +
qemud/remote_dispatch_prototypes.h |7 +++
qemud/remote_dispatch_ret.h|1 +
qemud/remote_dispatch_table.h |5 +
qemud/remote_protocol.c| 24
qemud/remote_protocol.h| 18 ++
qemud/remote_protocol.x| 15 ++-
src/remote_internal.c | 33 +
9 files changed, 137 insertions(+), 1 deletions(-)
ACK, all fine, boring stuff.
Daniel
diff --git a/qemud/remote.c b/qemud/remote.c
index 8d24a3a..5ba9430 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -3831,6 +3831,40 @@ remoteDispatchStorageVolCreateXml (struct qemud_server
*server ATTRIBUTE_UNUSED,
return 0;
}
+static int
+remoteDispatchStorageVolCreateXmlFrom (struct qemud_server *server
ATTRIBUTE_UNUSED,
+ struct qemud_client *client
ATTRIBUTE_UNUSED,
+ virConnectPtr conn,
+ remote_error *rerr,
+
remote_storage_vol_create_xml_from_args *args,
+
remote_storage_vol_create_xml_from_ret *ret)
+{
+virStoragePoolPtr pool;
+virStorageVolPtr clonevol, newvol;
+
+pool = get_nonnull_storage_pool (conn, args-pool);
+if (pool == NULL) {
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
+
+clonevol = get_nonnull_storage_vol (conn, args-clonevol);
+if (clonevol == NULL) {
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
+
+newvol = virStorageVolCreateXMLFrom (pool, args-xml, args-flags,
+ clonevol);
+if (newvol == NULL) {
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
+
+make_nonnull_storage_vol (ret-vol, newvol);
+virStorageVolFree(newvol);
+return 0;
+}
static int
remoteDispatchStorageVolDelete (struct qemud_server *server ATTRIBUTE_UNUSED,
diff --git a/qemud/remote_dispatch_args.h b/qemud/remote_dispatch_args.h
index 58eccf5..8f8b05b 100644
--- a/qemud/remote_dispatch_args.h
+++ b/qemud/remote_dispatch_args.h
@@ -105,3 +105,4 @@
remote_domain_get_security_label_args
val_remote_domain_get_security_label_args;
remote_node_device_create_xml_args
val_remote_node_device_create_xml_args;
remote_node_device_destroy_args val_remote_node_device_destroy_args;
+remote_storage_vol_create_xml_from_args
val_remote_storage_vol_create_xml_from_args;
diff --git a/qemud/remote_dispatch_prototypes.h
b/qemud/remote_dispatch_prototypes.h
index 96dcb2a..1a2d98b 100644
--- a/qemud/remote_dispatch_prototypes.h
+++ b/qemud/remote_dispatch_prototypes.h
@@ -814,6 +814,13 @@ static int remoteDispatchStorageVolCreateXml(
remote_error *err,
remote_storage_vol_create_xml_args *args,
remote_storage_vol_create_xml_ret *ret);
+static int remoteDispatchStorageVolCreateXmlFrom(
+struct qemud_server *server,
+struct qemud_client *client,
+virConnectPtr conn,
+remote_error *err,
+remote_storage_vol_create_xml_from_args *args,
+remote_storage_vol_create_xml_from_ret *ret);
static int remoteDispatchStorageVolDelete(
struct qemud_server *server,
struct qemud_client *client,
diff --git a/qemud/remote_dispatch_ret.h b/qemud/remote_dispatch_ret.h
index 3325c8b..75e2ca6 100644
--- a/qemud/remote_dispatch_ret.h
+++ b/qemud/remote_dispatch_ret.h
@@ -89,3 +89,4 @@
remote_domain_get_security_label_ret
val_remote_domain_get_security_label_ret;
remote_node_get_security_model_ret
val_remote_node_get_security_model_ret;
remote_node_device_create_xml_ret val_remote_node_device_create_xml_ret;
+remote_storage_vol_create_xml_from_ret
val_remote_storage_vol_create_xml_from_ret;
diff --git a/qemud/remote_dispatch_table.h b/qemud/remote_dispatch_table.h
index ac7f9b9..e601a6c 100644
--- a/qemud/remote_dispatch_table.h
+++ b/qemud/remote_dispatch_table.h
@@ -627,3 +627,8 @@
.args_filter = (xdrproc_t) xdr_remote_node_device_destroy_args,
.ret_filter = (xdrproc_t) xdr_void,
},
+{ /* StorageVolCreateXmlFrom = 125 */
+.fn = (dispatch_fn) remoteDispatchStorageVolCreateXmlFrom,
+.args_filter = (xdrproc_t) xdr_remote_storage_vol_create_xml_from_args,
+.ret_filter = (xdrproc_t) xdr_remote_storage_vol_create_xml_from_ret,
+},
diff --git a/qemud/remote_protocol.c b/qemud/remote_protocol.c
index af3c792..738f95c 100644
--- a/qemud/remote_protocol.c
+++ b/qemud/remote_protocol.c
@@ -1992,6 +1992,30 @@
Re: [libvirt] [PATCH 3/7] Test driver implementation of virStorageVolCreateXMLFrom
On Mon, May 04, 2009 at 01:42:58PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson crobi...@redhat.com
---
src/test.c | 95
1 files changed, 95 insertions(+), 0 deletions(-)
+
+if (VIR_ALLOC_N(privvol-target.path,
+strlen(privpool-def-target.path) +
+1 + strlen(privvol-name) + 1) 0) {
+virReportOOMError(pool-conn);
+goto cleanup;
+}
+
+strcpy(privvol-target.path, privpool-def-target.path);
+strcat(privvol-target.path, /);
+strcat(privvol-target.path, privvol-name);
+privvol-key = strdup(privvol-target.path);
+if (privvol-key == NULL) {
+virReportOOMError(pool-conn);
+goto cleanup;
+}
Be a little shorter to just call virAsprintf() for target.path
ACK aside from that
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 5/7] Storage driver implementation for CreateXMLFrom
On Mon, May 04, 2009 at 01:43:00PM -0400, Cole Robinson wrote: There is some funkiness here, since we are either dealing with 2 different pools (which means validation x 2) or the same pool, where we have to be careful not to deadlock. Ahhh, tricky stuff. + +newvol-building = 1; +buildret = backend-buildVolFrom(obj-conn, newvol, origvol, flags); +newvol-building = 0; +newvol = NULL; I'm wondering if we should also take care to mark origvol-building as non-zero, to incidate that it is in use too. Regardless though, we need to ensure buildVolFrom is safe against someone deleting the file from the filesystem behind our back Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 6/7] Break out FS volume build routines to their own functions.
On Mon, May 04, 2009 at 01:43:01PM -0400, Cole Robinson wrote:
Improves readability, particularly wrt the pending CreateFromXML changes.
This will also help implementing File-Block volume creation in the future,
since some of this code will need to be moved to a backend agnostic file.
This is a nice idea - it was getting rather unwieldly as it grew.
ACK.
Daniel
Signed-off-by: Cole Robinson crobi...@redhat.com
---
src/storage_backend_fs.c | 327
+-
1 files changed, 179 insertions(+), 148 deletions(-)
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
index 7a1bba8..241fb29 100644
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -62,6 +62,8 @@ static int qcowXGetBackingStore(virConnectPtr, char **,
static int vmdk4GetBackingStore(virConnectPtr, char **,
const unsigned char *, size_t);
+typedef int (*createFile)(virConnectPtr conn, virStorageVolDefPtr vol);
+
static int track_allocation_progress = 0;
/* Either 'magic' or 'extension' *must* be provided */
@@ -1011,183 +1013,202 @@ virStorageBackendFileSystemVolCreate(virConnectPtr
conn,
return 0;
}
-/**
- * Allocate a new file as a volume. This is either done directly
- * for raw/sparse files, or by calling qemu-img/qcow-create for
- * special kinds of files
- */
-static int
-virStorageBackendFileSystemVolBuild(virConnectPtr conn,
-virStorageVolDefPtr vol)
-{
+// XXX: Have these functions all use the same format?
+static int createRaw(virConnectPtr conn,
+ virStorageVolDefPtr vol) {
int fd;
-if (vol-target.format == VIR_STORAGE_VOL_FILE_RAW) {
-if ((fd = open(vol-target.path, O_RDWR | O_CREAT | O_EXCL,
- vol-target.perms.mode)) 0) {
-virReportSystemError(conn, errno,
- _(cannot create path '%s'),
- vol-target.path);
-return -1;
-}
+if ((fd = open(vol-target.path, O_RDWR | O_CREAT | O_EXCL,
+ vol-target.perms.mode)) 0) {
+virReportSystemError(conn, errno,
+ _(cannot create path '%s'),
+ vol-target.path);
+return -1;
+}
-/* Seek to the final size, so the capacity is available upfront
- * for progress reporting */
-if (ftruncate(fd, vol-capacity) 0) {
-virReportSystemError(conn, errno,
- _(cannot extend file '%s'),
- vol-target.path);
-close(fd);
-return -1;
-}
+/* Seek to the final size, so the capacity is available upfront
+ * for progress reporting */
+if (ftruncate(fd, vol-capacity) 0) {
+virReportSystemError(conn, errno,
+ _(cannot extend file '%s'),
+ vol-target.path);
+close(fd);
+return -1;
+}
-/* Pre-allocate any data if requested */
-/* XXX slow on non-extents-based file systems */
-/* FIXME: Add in progress bars bg thread if progress bar requested
*/
-if (vol-allocation) {
-if (track_allocation_progress) {
-unsigned long long remain = vol-allocation;
-
-while (remain) {
-/* Allocate in chunks of 512MiB: big-enough chunk
- * size and takes approx. 9s on ext3. A progress
- * update every 9s is a fair-enough trade-off
- */
-unsigned long long bytes = 512 * 1024 * 1024;
-int r;
-
-if (bytes remain)
-bytes = remain;
-if ((r = safezero(fd, 0, vol-allocation - remain,
- bytes)) != 0) {
-virReportSystemError(conn, r,
- _(cannot fill file '%s'),
- vol-target.path);
-close(fd);
-return -1;
-}
-remain -= bytes;
-}
-} else { /* No progress bars to be shown */
+/* Pre-allocate any data if requested */
+/* XXX slow on non-extents-based file systems */
+if (vol-allocation) {
+if (track_allocation_progress) {
+unsigned long long remain = vol-allocation;
+
+while (remain) {
+/* Allocate in chunks of 512MiB: big-enough chunk
+ * size and takes approx. 9s on ext3. A progress
+ * update every 9s is a fair-enough trade-off
+ */
+unsigned
Re: [libvirt] PATCH: Another attempt to fix vbox driver open
On Fri, May 08, 2009 at 05:42:07PM +0100, Daniel P. Berrange wrote: The patches we just applied for the VirtualBox open method still were not quite right. It would return VIR_DRV_OPEN_DECLINED when uri==NULL, but before doing so it would have set conn-uri to vbox:///session. So even though it declined the connection, all the later drivers would now ignore it. Also, it now returns DECLINED for some real errors that should be reported to the user. Here's an alternative idea I've had for trying to address this. Some goals: - If the user gives a URI with a vbox:/// prefix, we should always handle it, unless a 'server' is set when we leave it to the remote driver - If an invalid path is given we must give back a real error code - If after deciding the URI is for us, any initialization fails we must raise an error. - If the vbox glue layer is missing, we should still raise errors for requested URIs, so user knows their URI is correct. Looks very sensible to me and much nicer than my original suggestion to move VBoxCGlueInit() early into vboxOpen. [..snip..] + _(no VirtualBox drviver path specified (try vbox:///session))); s/drviver/driver/ Cheers, -- Guido -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/7] VolumeCreateXMLFrom FS storage backend implementation.
On Mon, May 04, 2009 at 01:43:02PM -0400, Cole Robinson wrote:
Add an extra 'inputvol' parameter to the file volume building routines. This
allows us to easily reuse the duplicate functionality.
@@ -1032,15 +1046,68 @@ static int createRaw(virConnectPtr conn,
virReportSystemError(conn, errno,
_(cannot extend file '%s'),
vol-target.path);
-close(fd);
-return -1;
+goto cleanup;
+}
+
+remain = vol-capacity;
+
+if (inputfd != -1) {
+size_t bytes = 1024 * 1024;
+int sparse_interval = 512;
+int amtread = -1;
+
+while (amtread != 0) {
+char buf[bytes];
Preferrable not to allocate 1 MB on the stack, since not all OS
default to such large stacks as Linux does for threads.
+char *tmp;
+int newamt;
+
+if (remain bytes)
+bytes = remain;
+
+if ((amtread = saferead(inputfd, buf, bytes)) 0) {
+virReportSystemError(conn, errno,
+ _(failed reading from file '%s'),
+ inputvol-target.path);
+goto cleanup;
+}
+remain -= amtread;
+
+/* Loop over amt read in 512 byte increments, looking for sparse
+ * blocks */
+tmp = buf;
+newamt = amtread;
+do {
+int interval = ((sparse_interval newamt) ? newamt
+ : sparse_interval
);
+int sparse_counter = interval;
+
+while (sparse_counter-- *(tmp+sparse_counter) == '\0')
+continue;
+
+if (sparse_counter 0) {
+if (lseek(fd, interval, SEEK_CUR) 0) {
+virReportSystemError(conn, errno,
+ _(cannot extend file '%s'),
+ vol-target.path);
+goto cleanup;
+}
+} else if (safewrite(fd, tmp, interval) 0) {
+virReportSystemError(conn, errno,
+ _(failed writing to file '%s'),
+ vol-target.path);
+goto cleanup;
+
+}
+
+tmp += interval;
+} while ((newamt -= sparse_interval) 0);
+}
I'm finding this bit of code a little hard to follow. Since you're scanning
in fixed 512 byte chunks, how about just doing
char buf[512];
memset(buf, 0, sizeof buf);
and then just doing a simple loop using memcmp(tmp, buf) over the 1 MB
data. Sure, the current code allows for detection of chunks of zeros
512 in size, but since data is allocated in underlying FS in at least
that size chunks, its not worth trying to detect zeros 512 in length
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 3/3] networking API for hostonly networks in VirtualBox driver in libvirt
On Mon, May 11, 2009 at 02:19:06PM +0200, Pritesh Kothari wrote:
You really don't need to keep any data about the networking driver
in libvirt(d) itself ?
+static int vboxListNetworks(virConnectPtr conn, char **const names, int
nnames) {
+vboxGlobalData *data = conn-privateData;
+int numActive = 0;
+
+if (data-vboxObj) {
+IHost *host = NULL;
+
+data-vboxObj-vtbl-GetHost(data-vboxObj, host);
+if (host) {
+int i = 0;
+PRUint32 networkInterfacesSize = 0;
+IHostNetworkInterface **networkInterfaces = NULL;
+
+host-vtbl-GetNetworkInterfaces(host, networkInterfacesSize,
networkInterfaces);
+
+for (i = 0; (numActive nnames) (i networkInterfacesSize);
i++) {
+if (networkInterfaces[i]) {
+PRUint32 interfaceType = 0;
+
+
networkInterfaces[i]-vtbl-GetInterfaceType(networkInterfaces[i],
interfaceType);
+
+if (interfaceType == HostNetworkInterfaceType_HostOnly) {
+PRUint32 status = HostNetworkInterfaceStatus_Unknown;
+
+
networkInterfaces[i]-vtbl-GetStatus(networkInterfaces[i], status);
+
+if (status == HostNetworkInterfaceStatus_Up) {
+char *nameUtf8 = NULL;
+PRUnichar *nameUtf16 = NULL;
+
+
networkInterfaces[i]-vtbl-GetName(networkInterfaces[i], nameUtf16);
+data-pFuncs-pfnUtf16ToUtf8(nameUtf16,
nameUtf8);
+
+DEBUG(nnames[%d]: %s, numActive, nameUtf8);
+names[numActive++] = strdup(nameUtf8);
+
+data-pFuncs-pfnUtf8Free(nameUtf8);
+data-pFuncs-pfnUtf16Free(nameUtf16);
+}
+}
+}
+}
+
+for (i = 0; i networkInterfacesSize; i++) {
+if (networkInterfaces[i]) {
+
networkInterfaces[i]-vtbl-nsisupports.Release((nsISupports *)
networkInterfaces[i]);
+}
+}
+
+host-vtbl-nsisupports.Release((nsISupports *) host);
+}
+}
+
+return numActive;
+}
There's a strdup() call here that's not checked for failure, and quite a
few more in other functions in the patch.
This reminds me that we really need to make a wrapper for strdup, as we did
for malloc() to validate failure checking at compile time.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 1/3] Support for vrdp/sdl/gui while defining a machine
[PATCH 1/3]: contains support for vrdp/sdl/gui while defining a machine.
+guiDisplay =
strdup(def-graphics[i]-data.desktop.display); +}
+
+if ((def-graphics[i]-type ==
VIR_DOMAIN_GRAPHICS_TYPE_SDL) (sdlPresent == 0)) { +
sdlPresent = 1;
+sdlDisplay =
strdup(def-graphics[i]-data.sdl.display); +}
+}
Need to check for OOM failure here.
Done and reposting the patch
Regards,
Pritesh
commit 7f23240a48f1f46e0aa4aaaf7f4fde63723e6b98
Author: pk221555 pk221...@krishna.(none)
Date: Mon May 11 14:45:09 2009 +0200
libvirt: added support for vrdp/sdl/gui while defining a machine
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 8e42958..4ef1488 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3041,24 +3041,29 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
}
} /* Finished:Block to attach the Parallel Port to the VM */
-#if 0
{ /* Started:Block to attach the Remote Display to VM */
-if (def-graphics) {
+int vrdpPresent = 0;
+int sdlPresent = 0;
+int guiPresent = 0;
+char *guiDisplay = NULL;
+char *sdlDisplay = NULL;
+int i = 0;
+
+for (i = 0; i def-ngraphics; i++) {
IVRDPServer *VRDPServer = NULL;
-/* TODO: include the support for headless stuff
- */
+if ((def-graphics[i]-type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) (vrdpPresent == 0)) {
-if (def-graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) {
+vrdpPresent = 1;
machine-vtbl-GetVRDPServer(machine, VRDPServer);
if (VRDPServer) {
VRDPServer-vtbl-SetEnabled(VRDPServer, PR_TRUE);
DEBUG0(VRDP Support turned ON on port: 3389);
-if (def-graphics-data.rdp.port) {
-VRDPServer-vtbl-SetPort(VRDPServer, def-graphics-data.rdp.port);
-DEBUG(VRDP Port changed to: %d, def-graphics-data.rdp.port);
-} else if (def-graphics-data.rdp.autoport) {
+if (def-graphics[i]-data.rdp.port) {
+VRDPServer-vtbl-SetPort(VRDPServer, def-graphics[i]-data.rdp.port);
+DEBUG(VRDP Port changed to: %d, def-graphics[i]-data.rdp.port);
+} else if (def-graphics[i]-data.rdp.autoport) {
/* Setting the port to 0 will reset its value to
* the default one which is 3389 currently
*/
@@ -3066,37 +3071,22 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
DEBUG0(VRDP Port changed to default, which is 3389 currently);
}
-if (def-graphics-data.rdp.reuseconnection) {
+if (def-graphics[i]-data.rdp.replaceUser) {
VRDPServer-vtbl-SetReuseSingleConnection(VRDPServer, PR_TRUE);
DEBUG0(VRDP set to reuse single connection);
}
-if (def-graphics-data.rdp.multiconnections) {
+if (def-graphics[i]-data.rdp.multiUser) {
VRDPServer-vtbl-SetAllowMultiConnection(VRDPServer, PR_TRUE);
DEBUG0(VRDP set to allow multiple connection);
}
-if (def-graphics-data.rdp.auth) {
-if (STREQ(def-graphics-data.rdp.auth, guest)) {
-VRDPServer-vtbl-SetAuthType(VRDPServer, VRDPAuthType_Guest);
-DEBUG0(VRDP authentication method set to Guest);
-} else if (STREQ(def-graphics-data.rdp.auth, external)) {
-VRDPServer-vtbl-SetAuthType(VRDPServer, VRDPAuthType_External);
-DEBUG0(VRDP authentication method set to External);
-}
-
-if (def-graphics-data.rdp.authtimeout) {
-VRDPServer-vtbl-SetAuthTimeout(VRDPServer, def-graphics-data.rdp.authtimeout);
-DEBUG(VRDP authentication timeout is set to %llu, def-graphics-data.rdp.authtimeout);
-}
-}
-
-if (def-graphics-data.rdp.listenAddr) {
+if (def-graphics[i]-data.rdp.listenAddr) {
PRUnichar *netAddressUtf16 = NULL;
-
Re: [libvirt] [PATCH 2/3] Support for vrdp/sdl/gui while dumping xml
-def-graphics-data.rdp.autoport = 1;
+valueDisplayUtf8 = getenv(DISPLAY);
+valueDisplayFree = 0;
}
Using getenv() here is not right, since that's running in the context of
the app using libvirt. If there's no display available via virtualbox's
API, then just leave this attribute blank when generating the XML.
There's generally quite a few uses of strdup() here without OOM
checking
Fixed these two and reposting the patch.
Regards,
Pritesh
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 4ef1488..85a336a 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -1366,9 +1366,7 @@ static char *vboxDomainDumpXML(virDomainPtr dom, int flags) {
PRBool PAEEnabled = PR_FALSE;
PRBool ACPIEnabled = PR_FALSE;
PRBool IOAPICEnabled= PR_FALSE;
-#if 0
PRBool VRDPEnabled = PR_FALSE;
-#endif
PRInt32 hddNum = 0;
PRUint32 CPUCount = 0;
PRUint32 memorySize = 0;
@@ -1387,9 +1385,7 @@ static char *vboxDomainDumpXML(virDomainPtr dom, int flags) {
IHardDisk *hardDiskPS = NULL;
IHardDisk *hardDiskSS = NULL;
PRUnichar *hddBusUtf16 = NULL;
-#if 0
IVRDPServer *VRDPServer = NULL;
-#endif
IFloppyDrive *floppyDrive = NULL;
IAudioAdapter *audioAdapter = NULL;
IUSBController *USBController = NULL;
@@ -1482,83 +1478,162 @@ static char *vboxDomainDumpXML(virDomainPtr dom, int flags) {
* so locatime is always true here */
def-localtime = 1;
-#if 0
-machine-vtbl-GetVRDPServer(machine, VRDPServer);
-if (VRDPServer) {
-VRDPServer-vtbl-GetEnabled(VRDPServer, VRDPEnabled);
-if (VRDPEnabled) {
-if (VIR_ALLOC(def-graphics) = 0) {
-PRUint32 VRDPport= 0;
-PRUnichar *netAddressUtf16 = NULL;
-PRUnichar *sessionTypeUtf16 = NULL;
-char *sessionTypeUtf8= NULL;
-char *netAddressUtf8 = NULL;
-PRUint32 authType= VRDPAuthType_Null;
-PRBool allowMultiConnection = PR_FALSE;
-PRBool reuseSingleConnection = PR_FALSE;
-
-def-graphics-type = VIR_DOMAIN_GRAPHICS_TYPE_RDP;
-
-VRDPServer-vtbl-GetPort(VRDPServer, VRDPport);
-if (VRDPport) {
-def-graphics-data.rdp.port = VRDPport;
-} else {
-def-graphics-data.rdp.autoport = 1;
+/* dump display options vrdp/gui/sdl */
+{
+int vrdpPresent = 0;
+int sdlPresent= 0;
+int guiPresent= 0;
+int totalPresent = 0;
+char *guiDisplay = NULL;
+char *sdlDisplay = NULL;
+PRUnichar *keyTypeUtf16 = NULL;
+PRUnichar *valueTypeUtf16 = NULL;
+char *valueTypeUtf8 = NULL;
+
+def-ngraphics = 0;
+
+data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Type, keyTypeUtf16);
+machine-vtbl-GetExtraData(machine, keyTypeUtf16, valueTypeUtf16);
+data-pFuncs-pfnUtf16Free(keyTypeUtf16);
+
+if (valueTypeUtf16) {
+data-pFuncs-pfnUtf16ToUtf8(valueTypeUtf16, valueTypeUtf8);
+data-pFuncs-pfnUtf16Free(valueTypeUtf16);
+
+if ( STREQ(valueTypeUtf8, sdl) || STREQ(valueTypeUtf8, gui) ) {
+PRUnichar *keyDislpayUtf16 = NULL;
+PRUnichar *valueDisplayUtf16 = NULL;
+char *valueDisplayUtf8 = NULL;
+
+data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Display, keyDislpayUtf16);
+machine-vtbl-GetExtraData(machine, keyDislpayUtf16, valueDisplayUtf16);
+data-pFuncs-pfnUtf16Free(keyDislpayUtf16);
+
+if (valueDisplayUtf16) {
+data-pFuncs-pfnUtf16ToUtf8(valueDisplayUtf16, valueDisplayUtf8);
+data-pFuncs-pfnUtf16Free(valueDisplayUtf16);
+
+
Re: [libvirt] [PATCH 3/3] Support for vrdp/sdl/gui while starting the machine
Same issue about use of 'getenv' strdup OOM handling here.
Fixed these two and reposting the patch.
Regards,
Pritesh
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 85a336a..274afd8 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2343,20 +2343,12 @@ static int vboxDomainCreate(virDomainPtr dom) {
IProgress *progress= NULL;
PRUint32 machineCnt= 0;
PRUnichar *env = NULL;
-const char *display= getenv(DISPLAY);
PRUnichar *sessionType = NULL;
char displayutf8[32] = {0};
unsigned char iidl[VIR_UUID_BUFLEN] = {0};
int i, ret = -1;
-if (display) {
-sprintf(displayutf8, DISPLAY=%s, display);
-data-pFuncs-pfnUtf8ToUtf16(displayutf8, env);
-}
-
-data-pFuncs-pfnUtf8ToUtf16(gui, sessionType);
-
if (!dom-name) {
vboxError(dom-conn, VIR_ERR_INTERNAL_ERROR,%s,
Error while reading the domain name);
@@ -2394,6 +2386,106 @@ static int vboxDomainCreate(virDomainPtr dom) {
if ( (state == MachineState_PoweredOff) ||
(state == MachineState_Saved) ||
(state == MachineState_Aborted) ) {
+int vrdpPresent = 0;
+int sdlPresent = 0;
+int guiPresent = 0;
+char *guiDisplay = NULL;
+char *sdlDisplay = NULL;
+PRUnichar *keyTypeUtf16 = NULL;
+PRUnichar *valueTypeUtf16= NULL;
+char *valueTypeUtf8 = NULL;
+PRUnichar *keyDislpayUtf16 = NULL;
+PRUnichar *valueDisplayUtf16 = NULL;
+char *valueDisplayUtf8 = NULL;
+
+data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Type, keyTypeUtf16);
+machine-vtbl-GetExtraData(machine, keyTypeUtf16, valueTypeUtf16);
+data-pFuncs-pfnUtf16Free(keyTypeUtf16);
+
+if (valueTypeUtf16) {
+data-pFuncs-pfnUtf16ToUtf8(valueTypeUtf16, valueTypeUtf8);
+data-pFuncs-pfnUtf16Free(valueTypeUtf16);
+
+if ( STREQ(valueTypeUtf8, sdl) || STREQ(valueTypeUtf8, gui) ) {
+
+data-pFuncs-pfnUtf8ToUtf16(FRONTEND/Display, keyDislpayUtf16);
+machine-vtbl-GetExtraData(machine, keyDislpayUtf16, valueDisplayUtf16);
+data-pFuncs-pfnUtf16Free(keyDislpayUtf16);
+
+if (valueDisplayUtf16) {
+data-pFuncs-pfnUtf16ToUtf8(valueDisplayUtf16, valueDisplayUtf8);
+data-pFuncs-pfnUtf16Free(valueDisplayUtf16);
+
+if (strlen(valueDisplayUtf8) = 0) {
+data-pFuncs-pfnUtf8Free(valueDisplayUtf8);
+valueDisplayUtf8 = NULL;
+}
+}
+
+if (STREQ(valueTypeUtf8, sdl)) {
+sdlPresent = 1;
+if (valueDisplayUtf8) {
+sdlDisplay = strdup(valueDisplayUtf8);
+if (sdlDisplay == NULL) {
+vboxError(dom-conn, VIR_ERR_SYSTEM_ERROR, %s, strdup failed);
+/* just don't go to cleanup yet as it is ok to have
+ * sdlDisplay as NULL and we check it below if it
+ * exist and then only use it there
+ */
+}
+}
+}
+
+if (STREQ(valueTypeUtf8, gui)) {
+guiPresent = 1;
+if (valueDisplayUtf8) {
+guiDisplay = strdup(valueDisplayUtf8);
+if (guiDisplay == NULL) {
+vboxError(dom-conn, VIR_ERR_SYSTEM_ERROR, %s, strdup failed);
+/* just don't go to cleanup yet as it is ok to have
+ * guiDisplay as NULL and we check it below if it
+ * exist and then only use it there
+ */
+}
+}
+
[libvirt] [PATCH] Fix error squashing when refreshing file volumes
When refreshing a file based pool, errors hit when determining a
volume's format were being squashed, reporting OOM instead. The attached
patch fixes the error reporting here.
Reported by Jason Guiditta.
Thanks,
Cole
diff --git a/src/storage_backend.c b/src/storage_backend.c
index b154140..acdb288 100644
--- a/src/storage_backend.c
+++ b/src/storage_backend.c
@@ -156,6 +156,10 @@ virStorageBackendUpdateVolInfo(virConnectPtr conn,
return 0;
}
+/*
+ * Return -1 on a legitimate error condition
+ * Return -2 if passed FD isn't a regular, char, or block file.
+ */
int
virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn,
virStorageVolTargetPtr target,
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
index 92ff3cb..d64b64d 100644
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -843,7 +843,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn,
vol-allocation,
vol-capacity) 0)) {
if (ret == -1)
-goto no_memory;
+goto cleanup;
else {
/* Silently ignore non-regular files,
* eg '.' '..', 'lost+found' */
@@ -883,7 +883,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn,
vol-backingStore,
NULL, NULL, NULL)) 0) {
if (ret == -1)
-goto no_memory;
+goto cleanup;
else {
/* Silently ignore non-regular files,
* eg '.' '..', 'lost+found' */
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix error squashing when refreshing file volumes
On Mon, May 11, 2009 at 10:03:24AM -0400, Cole Robinson wrote: When refreshing a file based pool, errors hit when determining a volume's format were being squashed, reporting OOM instead. The attached patch fixes the error reporting here. Reported by Jason Guiditta. ACK. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to prevent rebooting from inside containers
On Fri, May 08, 2009 at 09:04:35AM +0900, Ryota Ozaki wrote:
Hi,
Current lxc driver unexpectedly allows users inside containers to reboot
host physical machine. This patch prevents this by dropping CAP_SYS_BOOT
capability in the bounding set of the init processes in every containers.
Note that the patch intends to make it easy to add further capabilities
to drop if needed, although I'm not sure which capabilities should be
dropped. (We might need to drop CAP_SETFCAP as well to be strict...)
Thanks,
ozaki-r
Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com
From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001
From: Ryota Ozaki ozaki.ry...@gmail.com
Date: Fri, 8 May 2009 04:29:24 +0900
Subject: [PATCH] lxc: drop CAP_SYS_BOOT capability to prevent
rebooting from inside containers
Current lxc driver unexpectedly allows users inside containers to reboot
host physical machine. This patch prevents this by dropping CAP_SYS_BOOT
capability in the bounding set of the init processes in every containers.
---
src/lxc_container.c | 30 ++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/src/lxc_container.c b/src/lxc_container.c
index 3946b84..37ab216 100644
--- a/src/lxc_container.c
+++ b/src/lxc_container.c
@@ -32,6 +32,8 @@
#include sys/ioctl.h
#include sys/mount.h
#include sys/wait.h
+#include sys/prctl.h
+#include sys/capability.h
#include unistd.h
#include mntent.h
I had to move those 2 includes after #include linux/fs.h
otherwise MS_MOVE which is defined in the later would not be found
anymore. Weird but true !
@@ -639,6 +641,30 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef,
return lxcContainerSetupExtraMounts(vmDef);
}
+
+static int lxcContainerDropCapabilities( virDomainDefPtr vmDef )
+{
+int i;
+const struct {
+int id;
+const char *name;
+} caps[] = {
+#define ID_STRING(name) name, #name
+{ ID_STRING(CAP_SYS_BOOT) },
+};
+
+for (i = 0 ; i ARRAY_CARDINALITY(caps) ; i++) {
+if (prctl(PR_CAPBSET_DROP, caps[i].id, 0, 0, 0)) {
+lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ %s, _(failed to drop %s), caps[i].name);
Here the compiler complained about the args it really should be
lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_(failed to drop %s), caps[i].name);
+return -1;
+}
+}
+
+return 0;
+}
+
That said with the two fixes this looks like a good patch,
so applied and commited, thanks !
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: Fix QEMU arg detection for kvm = 85
On Fri, May 08, 2009 at 11:39:28AM +0100, Daniel P. Berrange wrote: The -help output from KVM = 85 is now large than we expected. This patch increases the buffer from 8k to 64k when reading help. It also provides more detailed error messages should something go wrong in the future. Two examples of improved error reporting # virsh -c qemu:///system start VirtTest error: Failed to start domain VirtTest error: Unable to read QEMU help output: Value too large for defined data type # virsh -c qemu:///system start VirtTest error: Failed to start domain VirtTest error: internal error cannot parse QEMU version number in 'QEMU PC emulator version 0.9.1 (kvm-79), Copyright (c) 2003-2008 Fabrice Bellard' ACK, a rather annoying bug ! 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 3/7] Test driver implementation of virStorageVolCreateXMLFrom
On Mon, May 11, 2009 at 08:54:23AM -0400, Cole Robinson wrote:
Daniel P. Berrange wrote:
On Mon, May 04, 2009 at 01:42:58PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson crobi...@redhat.com
---
src/test.c | 95
1 files changed, 95 insertions(+), 0 deletions(-)
+
+if (VIR_ALLOC_N(privvol-target.path,
+strlen(privpool-def-target.path) +
+1 + strlen(privvol-name) + 1) 0) {
+virReportOOMError(pool-conn);
+goto cleanup;
+}
+
+strcpy(privvol-target.path, privpool-def-target.path);
+strcat(privvol-target.path, /);
+strcat(privvol-target.path, privvol-name);
+privvol-key = strdup(privvol-target.path);
+if (privvol-key == NULL) {
+virReportOOMError(pool-conn);
+goto cleanup;
+}
Be a little shorter to just call virAsprintf() for target.path
ACK aside from that
Daniel
That's largely just duplication of code that was already in the storage
driver, so there are other culprits as well. I'll send a cleanup patch
after this series is applied (or before a v2) that fixes all the cases.
Sure, that works for me
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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: Another attempt to fix vbox driver open
On Fri, May 08, 2009 at 05:42:07PM +0100, Daniel P. Berrange wrote:
The patches we just applied for the VirtualBox open method still were
not quite right. It would return VIR_DRV_OPEN_DECLINED when uri==NULL,
but before doing so it would have set conn-uri to vbox:///session. So
even though it declined the connection, all the later drivers would now
ignore it. Also, it now returns DECLINED for some real errors that
should be reported to the user.
Here's an alternative idea I've had for trying to address this. Some
goals:
- If the user gives a URI with a vbox:/// prefix, we should always
handle it, unless a 'server' is set when we leave it to the remote
driver
- If an invalid path is given we must give back a real error code
- If after deciding the URI is for us, any initialization fails
we must raise an error.
- If the vbox glue layer is missing, we should still raise errors
for requested URIs, so user knows their URI is correct.
All sounds good !
To do this, I've taken the approach of registering a dummy vbox driver
if the glue layer is missing. This just parses the URI and always returns
an error for any vbox:// URIs that would otherwise work
Interesting ...
[...]
-/* vboxRegister() shouldn't fail as that will render libvirt unless.
- * So, we use the v2.2 driver as a fallback/dummy.
+/*
+ * If the glue layer won' initialize, we register a driver
won't or does not :-)
+ * with a dummy open method, so we can report nicer errors
+ * if the user requests a vbox:// URI which we know won't
+ * ever work
will never ?
[...]
+if (conn-uri == NULL ||
+conn-uri-scheme == NULL ||
+STRNEQ (conn-uri-scheme, vbox) ||
+conn-uri-server != NULL)
+return VIR_DRV_OPEN_DECLINED;
Hum, we accept NULL to indicate Xen or KVM/QEmu by default.
Maybe if none of them is available, we should allow NULL to start
the VirtualBox driver. It could be useful say on MacOS, or
just if VirtualBox is installed on Linux while we know KVM is not.
+if (conn-uri-path == NULL || STREQ(conn-uri-path, )) {
+vboxError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(no VirtualBox drviver path specified (try
vbox:///session)));
+return VIR_DRV_OPEN_ERROR;
+}
[...]
+static virDriver vboxDriverDummy = {
+VIR_DRV_VBOX,
+VBOX,
+.open = vboxOpenDummy,
+};
In that case the new style initilization makes sense.
@@ -291,13 +281,13 @@ static int vboxExtractVersion(virConnect
}
static void vboxUninitialize(vboxGlobalData *data) {
+if (!data)
+return;
+
if (data-pFuncs)
data-pFuncs-pfnComUninitialize();
VBoxCGlueTerm();
-if (!data)
-return;
-
virDomainObjListFree(data-domains);
virCapabilitiesFree(data-caps);
VIR_FREE(data);
That's a bug fix which should be applied in any case.
@@ -306,52 +296,62 @@ static void vboxUninitialize(vboxGlobalD
static virDrvOpenStatus vboxOpen(virConnectPtr conn,
virConnectAuthPtr auth ATTRIBUTE_UNUSED,
int flags ATTRIBUTE_UNUSED) {
-vboxGlobalData *data;
+vboxGlobalData *data = NULL;
uid_t uid = getuid();
if (conn-uri == NULL) {
conn-uri = xmlParseURI(uid ? vbox:///session : vbox:///system);
Ah, okay, so NULL is still accepted, this is getting complex
If Pritesh can review and test the patch, I'm fine with it.
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] Fix error squashing when refreshing file volumes
On Mon, May 11, 2009 at 10:03:24AM -0400, Cole Robinson wrote: When refreshing a file based pool, errors hit when determining a volume's format were being squashed, reporting OOM instead. The attached patch fixes the error reporting here. Looks fine, though if you start to add function comments it would be better to have them match the format used in libvirt.c (i.e. one Return sentence and description of args and functionality. ACK, 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: Another attempt to fix vbox driver open
On Mon, May 11, 2009 at 04:27:36PM +0200, Daniel Veillard wrote: On Fri, May 08, 2009 at 05:42:07PM +0100, Daniel P. Berrange wrote: [...] +if (conn-uri == NULL || +conn-uri-scheme == NULL || +STRNEQ (conn-uri-scheme, vbox) || +conn-uri-server != NULL) +return VIR_DRV_OPEN_DECLINED; Hum, we accept NULL to indicate Xen or KVM/QEmu by default. Maybe if none of them is available, we should allow NULL to start the VirtualBox driver. It could be useful say on MacOS, or just if VirtualBox is installed on Linux while we know KVM is not. This block of code is in the vboxOpenDummy() function. In this scenario we have already determined that the VirtualBox RPC libraries are not available. Hence there is no need to perform a probe when uri == NULL here. The other real vboxOpen() method you noticed later in the patch do still handle NULL Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 3/3] networking API for hostonly networks in VirtualBox driver in libvirt
There's a strdup() call here that's not checked for failure, and quite a
few more in other functions in the patch.
Hopefully covered all the strdup's and fixed it
Reposting the patch.
This reminds me that we really need to make a wrapper for strdup, as we did
for malloc() to validate failure checking at compile time.
that would be very helpful :)
Regards,
Pritesh
commit 9541fea6230439d04688843fff010ea3250a5ed4
Author: pk221555 pk221...@krishna.(none)
Date: Mon May 11 16:45:05 2009 +0200
libvirt: networking API for hostonly networks
diff --git a/src/vbox/vbox_driver.c b/src/vbox/vbox_driver.c
index 7db437c..6b782b1 100644
--- a/src/vbox/vbox_driver.c
+++ b/src/vbox/vbox_driver.c
@@ -39,19 +39,23 @@
extern virDriver vbox22Driver;
+extern virNetworkDriver vbox22NetworkDriver;
#if 0
extern virDriver vbox25Driver;
+extern virNetworkDriver vbox25NetworkDriver;
#endif
int vboxRegister(void) {
virDriverPtrdriver;
+virNetworkDriverPtr networkDriver;
uint32_tuVersion;
/* vboxRegister() shouldn't fail as that will render libvirt unless.
* So, we use the v2.2 driver as a fallback/dummy.
*/
driver= vbox22Driver;
+networkDriver = vbox22NetworkDriver;
/* Init the glue and get the API version. */
if (VBoxCGlueInit() == 0) {
@@ -69,10 +73,12 @@ int vboxRegister(void) {
if (uVersion = 2001052 uVersion 2002051) {
DEBUG0(VirtualBox API version: 2.2);
driver= vbox22Driver;
+networkDriver = vbox22NetworkDriver;
#if 0
} else if (uVersion = 2002051 uVersion 2005051) {
DEBUG0(VirtualBox API version: 2.5);
driver= vbox25Driver;
+networkDriver = vbox25NetworkDriver;
#endif
} else {
DEBUG0(Unsupport VirtualBox API version);
@@ -84,6 +90,8 @@ int vboxRegister(void) {
if (virRegisterDriver(driver) 0)
return -1;
+if (virRegisterNetworkDriver(networkDriver) 0)
+return -1;
return 0;
}
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index a27eada..e2d0f03 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3633,6 +3633,1021 @@ cleanup:
return ret;
}
+/**
+ * The Network Functions here on
+ */
+static virDrvOpenStatus vboxNetworkOpen(virConnectPtr conn,
+virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+int flags ATTRIBUTE_UNUSED) {
+vboxGlobalData *data = conn-privateData;
+
+if (STRNEQ(conn-driver-name, VBOX))
+goto cleanup;
+
+if ((data-pFuncs == NULL) ||
+(data-vboxObj == NULL) ||
+(data-vboxSession == NULL))
+goto cleanup;
+
+DEBUG0(network intialized);
+/* conn-networkPrivateData = some network specific data */
+return VIR_DRV_OPEN_SUCCESS;
+
+cleanup:
+return VIR_DRV_OPEN_DECLINED;
+}
+
+static int vboxNetworkClose(virConnectPtr conn) {
+DEBUG0(network unintialized);
+conn-networkPrivateData = NULL;
+return 0;
+}
+
+static int vboxNumOfNetworks(virConnectPtr conn) {
+vboxGlobalData *data = conn-privateData;
+int numActive = 0;
+
+if (data-vboxObj) {
+IHost *host = NULL;
+
+data-vboxObj-vtbl-GetHost(data-vboxObj, host);
+if (host) {
+int i = 0;
+PRUint32 networkInterfacesSize = 0;
+IHostNetworkInterface **networkInterfaces = NULL;
+
+host-vtbl-GetNetworkInterfaces(host, networkInterfacesSize, networkInterfaces);
+
+for (i = 0; i networkInterfacesSize; i++) {
+if (networkInterfaces[i]) {
+PRUint32 interfaceType = 0;
+
+networkInterfaces[i]-vtbl-GetInterfaceType(networkInterfaces[i], interfaceType);
+if (interfaceType == HostNetworkInterfaceType_HostOnly) {
+PRUint32 status = HostNetworkInterfaceStatus_Unknown;
+
+networkInterfaces[i]-vtbl-GetStatus(networkInterfaces[i], status);
+
+if (status == HostNetworkInterfaceStatus_Up) {
+numActive++;
+}
+}
+
+networkInterfaces[i]-vtbl-nsisupports.Release((nsISupports *) networkInterfaces[i]);
+}
+}
+
+host-vtbl-nsisupports.Release((nsISupports *) host);
+}
+}
+
+DEBUG(numActive: %d, numActive);
+return numActive;
+}
+
+static int vboxListNetworks(virConnectPtr conn, char **const names, int nnames) {
+vboxGlobalData *data = conn-privateData;
+int numActive = 0;
+
+if (data-vboxObj) {
+IHost *host = NULL;
+
+data-vboxObj-vtbl-GetHost(data-vboxObj, host);
+if (host) {
+int i = 0;
+PRUint32 networkInterfacesSize = 0;
+IHostNetworkInterface **networkInterfaces = NULL;
+
+
Re: [libvirt] PATCH: Another attempt to fix vbox driver open
- If the user gives a URI with a vbox:/// prefix, we should always handle it, unless a 'server' is set when we leave it to the remote driver - If an invalid path is given we must give back a real error code - If after deciding the URI is for us, any initialization fails we must raise an error. - If the vbox glue layer is missing, we should still raise errors for requested URIs, so user knows their URI is correct. To do this, I've taken the approach of registering a dummy vbox driver if the glue layer is missing. This just parses the URI and always returns an error for any vbox:// URIs that would otherwise work ACK. works fine for 2.2.* as well as internal 2.5* development versions. Regards, Pritesh -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: ALlow for quiet build
This is a simpler version of http://www.redhat.com/archives/libvir-list/2009-April/msg00523.html just overriding the LIBTOOL variable in configure.in, instead of in each Makefile.am As before you still need 'make -s' to get full effect Daniel diff -r f94c0624da5c configure.in --- a/configure.in Fri May 08 11:41:29 2009 +0100 +++ b/configure.in Mon May 11 16:46:08 2009 +0100 @@ -57,6 +57,9 @@ dnl Support building Win32 DLLs (must ap AC_LIBTOOL_WIN32_DLL AM_PROG_LIBTOOL +dnl Override normal libtool in favour of our quiet version +LIBTOOL='$(SHELL) $(top_srcdir)/mylibtool' +AC_SUBST([LIBTOOL]) AM_PROG_CC_C_O diff -r f94c0624da5c mylibtool --- /dev/null Thu Jan 01 00:00:00 1970 + +++ b/mylibtool Mon May 11 16:46:08 2009 +0100 @@ -0,0 +1,58 @@ +#!/bin/sh + +mode=libtool +cfiles= +ofiles= +afiles= + +wantnext=0 +for v in $@ +do + case $v + in + --mode=compile) +mode=CC +;; + --mode=link) +mode=LD +;; + esac + + case $v + in +*.c) +cfiles=$cfiles $v +;; +*.o) +if [ $mode = LD -o $wantnext = 1 ]; then + ofiles=$ofiles $v +fi +;; +*.lo) +if [ $mode = LD -o $wantnext = 1 ]; then + ofiles=$ofiles $v +fi +;; + esac + + if [ $mode = LD -a $wantnext = 1 ]; then + afiles=$afiles $v + fi + + if [ $v = -o ]; then +wantnext=1 + else +wantnext=0 + fi +done + +args= +test -n $afiles args=$args -o$afiles +test -n $ofiles -a $mode = CC args=$args -o +test -n $ofiles args=$args$ofiles +test -n $cfiles args=$args$cfiles + +echo ($mode)$args + +here=`dirname $0` +exec $here/libtool --silent $@ -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to prevent rebooting from inside containers
Hi,
I needed to apply the following two small changes to get it compile.
On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but
a linux/capability.h header as part of the linux-libc-dev package.
The second change makes the code compile with -Werror, because vmDef
is not used in the lxcContainerDropCapabilities function.
diff --git a/src/lxc_container.c b/src/lxc_container.c
index 3687750..a2b3051 100644
--- a/src/lxc_container.c
+++ b/src/lxc_container.c
@@ -42,7 +42,7 @@
#include linux/fs.h
#include sys/prctl.h
-#include sys/capability.h
+#include linux/capability.h
#include virterror_internal.h
#include logging.h
@@ -642,7 +642,7 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef,
return lxcContainerSetupExtraMounts(vmDef);
}
-static int lxcContainerDropCapabilities( virDomainDefPtr vmDef )
+static int lxcContainerDropCapabilities( virDomainDefPtr vmDef
ATTRIBUTE_UNUSED )
{
int i;
const struct {
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to prevent rebooting from inside containers
Daniel Veillard wrote:
On Fri, May 08, 2009 at 09:04:35AM +0900, Ryota Ozaki wrote:
Hi,
Current lxc driver unexpectedly allows users inside containers to reboot
host physical machine. This patch prevents this by dropping CAP_SYS_BOOT
capability in the bounding set of the init processes in every containers.
Note that the patch intends to make it easy to add further capabilities
to drop if needed, although I'm not sure which capabilities should be
dropped. (We might need to drop CAP_SETFCAP as well to be strict...)
Thanks,
ozaki-r
Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com
From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001
From: Ryota Ozaki ozaki.ry...@gmail.com
Date: Fri, 8 May 2009 04:29:24 +0900
Subject: [PATCH] lxc: drop CAP_SYS_BOOT capability to prevent
rebooting from inside containers
Current lxc driver unexpectedly allows users inside containers to reboot
host physical machine. This patch prevents this by dropping CAP_SYS_BOOT
capability in the bounding set of the init processes in every containers.
---
src/lxc_container.c | 30 ++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/src/lxc_container.c b/src/lxc_container.c
index 3946b84..37ab216 100644
--- a/src/lxc_container.c
+++ b/src/lxc_container.c
@@ -32,6 +32,8 @@
#include sys/ioctl.h
#include sys/mount.h
#include sys/wait.h
+#include sys/prctl.h
+#include sys/capability.h
#include unistd.h
#include mntent.h
I had to move those 2 includes after #include linux/fs.h
otherwise MS_MOVE which is defined in the later would not be found
anymore. Weird but true !
@@ -639,6 +641,30 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef,
return lxcContainerSetupExtraMounts(vmDef);
}
+
+static int lxcContainerDropCapabilities( virDomainDefPtr vmDef )
+{
+int i;
+const struct {
+int id;
+const char *name;
+} caps[] = {
+#define ID_STRING(name) name, #name
+{ ID_STRING(CAP_SYS_BOOT) },
+};
+
+for (i = 0 ; i ARRAY_CARDINALITY(caps) ; i++) {
+if (prctl(PR_CAPBSET_DROP, caps[i].id, 0, 0, 0)) {
+lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ %s, _(failed to drop %s), caps[i].name);
Here the compiler complained about the args it really should be
lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_(failed to drop %s), caps[i].name);
+return -1;
+}
+}
+
+return 0;
+}
+
That said with the two fixes this looks like a good patch,
so applied and commited, thanks !
Daniel
I had a build failure today because of an unused parameter to
lxcContainerDropCapabilities. The attached oneliner fixes it. I don't
know the code, though, so sanity check it.
Dave
diff --git a/src/lxc_container.c b/src/lxc_container.c
index 3687750..314f293 100644
--- a/src/lxc_container.c
+++ b/src/lxc_container.c
@@ -642,7 +642,7 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef,
return lxcContainerSetupExtraMounts(vmDef);
}
-static int lxcContainerDropCapabilities( virDomainDefPtr vmDef )
+static int lxcContainerDropCapabilities( virDomainDefPtr vmDef
ATTRIBUTE_UNUSED )
{
int i;
const struct {
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to prevent rebooting from inside containers
On Mon, May 11, 2009 at 05:59:45PM +0200, Matthias Bolte wrote: Hi, I needed to apply the following two small changes to get it compile. On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package. That is because sys/capability.h is provided by libcap, not libc. I guess you don't have libcap-dev installed. $ rpm -qf /usr/include/sys/capability.h libcap-devel-2.06-4.fc9.i386 diff --git a/src/lxc_container.c b/src/lxc_container.c index 3687750..a2b3051 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -42,7 +42,7 @@ #include linux/fs.h #include sys/prctl.h -#include sys/capability.h +#include linux/capability.h #include virterror_internal.h #include logging.h NACK to this change. @@ -642,7 +642,7 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef, return lxcContainerSetupExtraMounts(vmDef); } -static int lxcContainerDropCapabilities( virDomainDefPtr vmDef ) +static int lxcContainerDropCapabilities( virDomainDefPtr vmDef ATTRIBUTE_UNUSED ) I committed this fix a little while ago... Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to prevent rebooting from inside containers
2009/5/11 Daniel P. Berrange berra...@redhat.com: On Mon, May 11, 2009 at 05:59:45PM +0200, Matthias Bolte wrote: Hi, I needed to apply the following two small changes to get it compile. On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package. That is because sys/capability.h is provided by libcap, not libc. I guess you don't have libcap-dev installed. $ rpm -qf /usr/include/sys/capability.h libcap-devel-2.06-4.fc9.i386 You guess was correct. With libcap-dev installed it compiles without problems. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to prevent rebooting from inside containers
Matthias Bolte wrote: 2009/5/11 Daniel P. Berrange berra...@redhat.com: On Mon, May 11, 2009 at 05:59:45PM +0200, Matthias Bolte wrote: Hi, I needed to apply the following two small changes to get it compile. On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package. That is because sys/capability.h is provided by libcap, not libc. I guess you don't have libcap-dev installed. $ rpm -qf /usr/include/sys/capability.h libcap-devel-2.06-4.fc9.i386 You guess was correct. With libcap-dev installed it compiles without problems. We should check for the presence of libcap-dev in the configure script. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to prevent rebooting from inside containers
On Mon, May 11, 2009 at 12:37:25PM -0400, Dave Allan wrote: Matthias Bolte wrote: 2009/5/11 Daniel P. Berrange berra...@redhat.com: On Mon, May 11, 2009 at 05:59:45PM +0200, Matthias Bolte wrote: Hi, I needed to apply the following two small changes to get it compile. On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package. That is because sys/capability.h is provided by libcap, not libc. I guess you don't have libcap-dev installed. $ rpm -qf /usr/include/sys/capability.h libcap-devel-2.06-4.fc9.i386 You guess was correct. With libcap-dev installed it compiles without problems. We should check for the presence of libcap-dev in the configure script. And also add a BuildRequires to the RPM specfile Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to prevent rebooting from inside containers
On Mon, May 11, 2009 at 05:22:15PM +0100, Daniel P. Berrange wrote: On Mon, May 11, 2009 at 05:59:45PM +0200, Matthias Bolte wrote: Hi, I needed to apply the following two small changes to get it compile. On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package. That is because sys/capability.h is provided by libcap, not libc. I guess you don't have libcap-dev installed. $ rpm -qf /usr/include/sys/capability.h libcap-devel-2.06-4.fc9.i386 diff --git a/src/lxc_container.c b/src/lxc_container.c index 3687750..a2b3051 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -42,7 +42,7 @@ #include linux/fs.h #include sys/prctl.h -#include sys/capability.h +#include linux/capability.h #include virterror_internal.h #include logging.h NACK to this change. Actually I take that back. We don't need anything in sys/capability.h, so its pointless adding a dep on libcap. Lets just use the linux/capability.h header to get the prctl() definition constants. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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 2/5] First level of plumbing for virInterface*.
On 05/11/2009 06:35 AM, Daniel P. Berrange wrote: On Fri, May 08, 2009 at 01:52:24PM -0400, Laine Stump wrote: From: Laine Stump la...@redhat.com --- include/libvirt/libvirt.h| 18 ++ include/libvirt/libvirt.h.in | 18 ++ include/libvirt/virterror.h |4 + src/datatypes.h | 25 ++ src/driver.h | 60 src/libvirt.c| 695 ++ src/util.h |2 - src/virterror.c | 21 ++ 8 files changed, 841 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index 91af6fd..b0d93a2 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -433,6 +433,24 @@ extern virConnectAuthPtr virConnectAuthPtrDefault; #define VIR_UUID_STRING_BUFLEN (36+1) +/** + * VIR_MAC_BUFLEN: + * + * This macro provides the length of the buffer required + * for an interface MAC address + */ + +#define VIR_MAC_BUFLEN (6) + +/** + * VIR_MAC_STRING_BUFLEN: + * + * This macro provides the length of the buffer required + * for virInterfaceGetMACString() + */ + +#define VIR_MAC_STRING_BUFLEN (VIR_MAC_BUFLEN * 3) + Since this is now in our public API, I'm wondering whether we shouldn't make the buflen longer. 6 bytes is fine with wired ethernet, but I'm not sure its sufficient for all network device types in general. eg, my wmaster0 device has a rather longer hardware address wmaster0 Link encap:UNSPEC HWaddr 00-13-02-B9-F9-D3-F4-1F-00-00-00-00-00-00-00-00 UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 RX packets:0 errors:0 dropped:0 overruns:0 frame:0 TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:0 (0.0 b) TX bytes:0 (0.0 b) Yes, tun interfaces too. Since this is binary data rather than a null-terminated string, we need to decide among the following three choices: 1) have a fixed length (how long? is 16 bytes long enough?) and zero-fill the shorter ones. 2) Add a macLen arg to any API function that uses mac address (this will need to be a return arg in some cases too) 3) Only provide the versions of the functions that accept/use ASCII mac address args. Also, this has ramifications on other code outside virInterface* using VIR_MAC_BUFLEN, so there will probably need to be a patch separate from this series that grows VIR_MAC_BUFLEN and fixes that other code accordingly Any opinion on how to proceed? -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [Qemu-devel] [PATCH 1/6] Allow multiple monitor devices (v2)
On Wed, 2009-04-08 at 13:34 -0500, Anthony Liguori wrote: Right now only one monitor device can be enabled at a time. In order to support asynchronous notification of events, I would like to introduce a 'wait' command that waits for an event to occur. This implies that we need an additional monitor session to allow commands to still be executed while waiting for an asynchronous notification. Was there any consensus reached in this thread? I'm once again looking for ways to communicate qemu watchdog events to libvirt. With these patches, libvirt could open a second monitor connection to qemu, and in the second one execute wait_event. When the watchdog triggers, the wait command would print the event, libvirt would get the fd data available notification, and create a domain event. Right? -- Hollis Blanchard IBM Linux Technology Center -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [Qemu-devel] [PATCH 1/6] Allow multiple monitor devices (v2)
Hollis Blanchard wrote: On Wed, 2009-04-08 at 13:34 -0500, Anthony Liguori wrote: Right now only one monitor device can be enabled at a time. In order to support asynchronous notification of events, I would like to introduce a 'wait' command that waits for an event to occur. This implies that we need an additional monitor session to allow commands to still be executed while waiting for an asynchronous notification. Was there any consensus reached in this thread? I'm once again looking for ways to communicate qemu watchdog events to libvirt. We can do multiple monitors as a debugging tool, but to support events, a proper machine monitor mode is a prerequisite. The real requirement is that events are obtainable via a single communication channel instead of requiring two separate communication channels. Internal implementation will look at lot like these patches. The reasoning for requiring a single channel is that coordinating between the two channels is expected to be prohibitively difficult. To have a single channel, we need a machine mode. It cannot be done in a human readable fashion. I think this summarizes the consensus we reached. I don't agree fully with the above but I'm okay with it. Would you agree Avi? -- Regards, Anthony Liguori -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
