[libvirt] [PATCH] Don't treat pci_system_init failure as fatal if no PCI bus is present
Xen PV domU's have no PCI bus. node_device_udev.c calls pci_system_init
which looks for /sys/bus/pci. If it does not find /sys/bus/pci (which it
won't in a Xen PV domU) it returns unsuccesfully (ENOENT), which libvirt
considers fatal. This makes libvirt unusable in this environment, even
though there are plenty of valid virtualisation options that work
there (LXC, UML, and QEmu spring to mind)
https://bugzilla.redhat.com/show_bug.cgi?id=709471
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/node_device/node_device_udev.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index badf241..08ef856 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1620,7 +1620,7 @@ static int udevDeviceMonitorStartup(int privileged)
/* Ignore failure as non-root; udev is not as helpful in that
* situation, but a non-privileged user won't benefit much
* from udev in the first place. */
-if (privileged || errno != EACCES) {
+if (errno != ENOENT (privileged || errno != EACCES)) {
char ebuf[256];
VIR_ERROR(_(Failed to initialize libpciaccess: %s),
virStrerror(pciret, ebuf, sizeof ebuf));
--
1.7.5.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Pass virSecurityManagerPtr to virSecurityDAC{Set, Restore}ChardevCallback
virSecurityDAC{Set,Restore}ChardevCallback expect virSecurityManagerPtr,
but are passed virDomainObjPtr instead. This makes
virSecurityDACSetChardevLabel set a wrong uid/gid on chardevs. This
patch fixes this behaviour.
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/security/security_dac.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 1c1a037..b8de232 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -509,7 +509,7 @@ virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr
mgr,
if (virDomainChrDefForeach(vm-def,
false,
virSecurityDACRestoreChardevCallback,
- vm) 0)
+ mgr) 0)
rc = -1;
if (vm-def-os.kernel
@@ -565,7 +565,7 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr,
if (virDomainChrDefForeach(vm-def,
true,
virSecurityDACSetChardevCallback,
- vm) 0)
+ mgr) 0)
return -1;
if (vm-def-os.kernel
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [RFC] nwfilter: resolve deadlock between VM operations and filter update
I had trouble applying the patch (I think maybe Thunderbird may have fiddled with the formatting :( ), but after doing it manually, it works excellently. Thanks! -- Soren Hansen Ubuntu Developerhttp://www.ubuntu.com/ OpenStack Developer http://www.openstack.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add nwfilter support to UML driver
Extend user-mode-linux driver to support nwfilter.
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/uml/uml_conf.c | 16 +---
src/uml/uml_driver.c |8 +++-
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index 4906192..f2eaef5 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -46,6 +46,7 @@
#include verify.h
#include bridge.h
#include logging.h
+#include domain_nwfilter.h
#define VIR_FROM_THIS VIR_FROM_UML
@@ -108,7 +109,8 @@ virCapsPtr umlCapsInit(void) {
static int
-umlConnectTapDevice(virDomainNetDefPtr net,
+umlConnectTapDevice(virConnectPtr conn,
+virDomainNetDefPtr net,
const char *bridge)
{
brControl *brctl = NULL;
@@ -164,6 +166,14 @@ umlConnectTapDevice(virDomainNetDefPtr net,
goto error;
}
+if (net-filter) {
+if (virDomainConfNWFilterInstantiate(conn, net)) {
+if (template_ifname)
+VIR_FREE(net-ifname);
+goto error;
+}
+}
+
brShutdown(brctl);
return 0;
@@ -239,7 +249,7 @@ umlBuildCommandLineNet(virConnectPtr conn,
goto error;
}
-if (umlConnectTapDevice(def, bridge) 0) {
+if (umlConnectTapDevice(conn, def, bridge) 0) {
VIR_FREE(bridge);
goto error;
}
@@ -250,7 +260,7 @@ umlBuildCommandLineNet(virConnectPtr conn,
}
case VIR_DOMAIN_NET_TYPE_BRIDGE:
-if (umlConnectTapDevice(def, def-data.bridge.brname) 0)
+if (umlConnectTapDevice(conn, def, def-data.bridge.brname) 0)
goto error;
/* ethNNN=tuntap,tapname,macaddr,gateway */
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 0a5c829..40345d5 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -58,6 +58,7 @@
#include domain_conf.h
#include datatypes.h
#include logging.h
+#include domain_nwfilter.h
#define VIR_FROM_THIS VIR_FROM_UML
@@ -876,6 +877,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
if (umlBuildCommandLine(conn, driver, vm, keepfd,
argv, progenv) 0) {
close(logfd);
+virDomainConfVMNWFilterTeardown(vm);
umlCleanupTapDevices(conn, vm);
return -1;
}
@@ -928,8 +930,11 @@ static int umlStartVMDaemon(virConnectPtr conn,
VIR_FREE(progenv[i]);
VIR_FREE(progenv);
-if (ret 0)
+if (ret 0) {
+virDomainConfVMNWFilterTeardown(vm);
umlCleanupTapDevices(conn, vm);
+}
+
/* NB we don't mark it running here - we do that async
with inotify */
@@ -965,6 +970,7 @@ static void umlShutdownVMDaemon(virConnectPtr conn
ATTRIBUTE_UNUSED,
vm-def-id = -1;
vm-state = VIR_DOMAIN_SHUTOFF;
+virDomainConfVMNWFilterTeardown(vm);
umlCleanupTapDevices(conn, vm);
if (vm-newDef) {
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add nwfilter support to UML driver
On 07-09-2010 10:32, Daniel Veillard wrote: We are supposed to be in feature freeze mode this week, Apologies. I didn't realise. Where could I have learned this? -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Default to qemu:///system if accessible
On 06-09-2010 11:17, Daniel P. Berrange wrote: Our goal is to improve qemu://session's networking such that this isn't a reason to use qemu://system anymore Fair enough, but when that happens, I'm supposing people won't have access to the system-wide UNIX socket anymore. Enabing use of qemu://session is key to solving a number of important security problems, not least that a user should be able to just keep the disk iso images in their home directory and not have to worry about their ownership/permissions. True. In addition by running VMs directly under the user's session things like pulseaudio, SDL can directly access the X GNOME sessions without further special config. I was under the impression that currently targeted solution to at least the audio problem was tunelling through VNC? This is ignoring two important use cases which are common in the corporate world. Shared development servers where many users are on one server, and personal workstations where the users are not allowed to have root. I disagree. In both of those cases, I'd be surprised if people were able to access the privileged libvirtd socket. In the former case, if people generally had access to the systemwide libvirtd instance, I'd assume that was because that was the one they were supposed to use for their shared development stuff. In the latter case, with that sort of access, I could have full root shell access within minutes, so that'd be a pretty big security fail. The thing about heuristics is that they're never correct for everyone. Your patch is making it more correct for one group of people, and less correct for a different group of people. Further making a significant functional change to libvirt that will break things for people that are relying on the existing behaviour. I understand the difficulty of heuristics. That's exactly why I think this discussion is useful: It seeks to determine the accuracy of the current heuristic, which I claim is inaccurate in all but extraordinary cases. If someone wants to save typing they need merely set LIBVIRT_CONNECT_URI to whatever they want and thus avoid the default connection logic completely. As you point out to Eric elsewhere in this thread, this is about adjusting the behaviour of the heuristic, not about just providing a default URI. I admit, though, that I did not know of the environment variable to do this, which is called LIBVIRT_DEFAULT_URI, by the way :) -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Default to qemu:///system if accessible
On 06-09-2010 12:24, Daniel P. Berrange wrote: On Mon, Sep 06, 2010 at 12:16:40PM +0200, Soren Hansen wrote: On 06-09-2010 11:17, Daniel P. Berrange wrote: Our goal is to improve qemu://session's networking such that this isn't a reason to use qemu://system anymore Fair enough, but when that happens, I'm supposing people won't have access to the system-wide UNIX socket anymore. No, we won't change access to the system instance, the policy for that is already configurable per-host by admins if they so desire. Yes, that's what I meant. If qemu:///session turns useful enough, admins won't give users access to the privileged UNIX socket. I disagree. In both of those cases, I'd be surprised if people were able to access the privileged libvirtd socket. In the former case, if people generally had access to the systemwide libvirtd instance, I'd assume that was because that was the one they were supposed to use for their shared development stuff. In the latter case, with that sort of access, I could have full root shell access within minutes, so that'd be a pretty big security fail. You are equating access to the UNIX socket, with authorization to the unix socket. Indeed I am. With PolicyKit auth enabled by default, the UNIX socket is mode 0777 at all times, but this does not imply that all users are able to use it. They can connect, but if PolicyKit denies them, their connection will be dropped by the server. Fascinating. I had no idea. That's a very convincing argument :) What if I could come up with a check for whether the user would be authorized to access the socket? Like including a auth_unix_rw == none condition in the check? Would that change your view at all? -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Default to qemu:///system if accessible
On 06-09-2010 13:22, Daniel P. Berrange wrote: On Mon, Sep 06, 2010 at 01:07:34PM +0200, Soren Hansen wrote: On 06-09-2010 12:24, Daniel P. Berrange wrote: On Mon, Sep 06, 2010 at 12:16:40PM +0200, Soren Hansen wrote: On 06-09-2010 11:17, Daniel P. Berrange wrote: Our goal is to improve qemu://session's networking such that this isn't a reason to use qemu://system anymore Fair enough, but when that happens, I'm supposing people won't have access to the system-wide UNIX socket anymore. No, we won't change access to the system instance, the policy for that is already configurable per-host by admins if they so desire. Yes, that's what I meant. If qemu:///session turns useful enough, admins won't give users access to the privileged UNIX socket. Or they'll leave the permissions 0777 and simply change the policykit rule to deny access, or they'll not change anything, and simply not tell the user the root password, which is what's required to be entered with policykit to access qemu:///system. I understand. I wrote the above under the (false) assumption that having write access to the UNIX socket implied having privileges to the system's libvirtd. I disagree. In both of those cases, I'd be surprised if people were able to access the privileged libvirtd socket. In the former case, if people generally had access to the systemwide libvirtd instance, I'd assume that was because that was the one they were supposed to use for their shared development stuff. In the latter case, with that sort of access, I could have full root shell access within minutes, so that'd be a pretty big security fail. You are equating access to the UNIX socket, with authorization to the unix socket. Indeed I am. Therein lies the flaw in this approach. Yes, I learned that a few lines further down. :) With PolicyKit auth enabled by default, the UNIX socket is mode 0777 at all times, but this does not imply that all users are able to use it. They can connect, but if PolicyKit denies them, their connection will be dropped by the server. Fascinating. I had no idea. That's a very convincing argument :) What if I could come up with a check for whether the user would be authorized to access the socket? Like including a auth_unix_rw == none condition in the check? Would that change your view at all? PolicyKit is just one example I happened to pick, auth_unix_rw == none was also just an example. the same logic applies to any of the other authentication/authorization methods that libvirtd applies. Of course. Any check based on socket permissions is fundamentally flawed, I feel the same way about one that is based on uid, to be honest. even with auth_unix_rw=none (which you can't check because a non-root user can't read /etc/libvirt/libvirtd.conf), On Ubuntu, /etc/libvirt/libvirtd.conf is mode 0644? Should I be worried about that? A quick glance in there doesn't reveal anything that I'm uncomfortable disclosing. when we add role based access control, you may be able to connect to the 'rw' socket, but have no more privileges than on the 'ro' socket. In that particular case, I could also check for whether RBAC was enabled, but that's really beside the point right now. My question was a general one: Assuming I can determine that a given user is authorized to manage the systemwide libvirtd, would you agree that that is the one they're most likely to want to access? I simply cannot think up a realistic example use case where someone has this privilege, but actually wants to access qemu:///session. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Default to qemu:///system if accessible
On 06-09-2010 14:18, Daniel P. Berrange wrote: On Ubuntu, /etc/libvirt/libvirtd.conf is mode 0644? Should I be worried about that? A quick glance in there doesn't reveal anything that I'm uncomfortable disclosing. The /etc/libvirt directory itself should be 0700 though, Nope, it's 0755. :( I'll look into getting that fixed. since various files under that location include passwords (qemu.conf, secrets/*, qemu/*xml, lxc/*xml, uml/*xml). We don't currently have any passwords in libvirtd.conf itself, but its certainly possible this might change in the future. While it is possible to rely on getting each individual file there to have correct permissions, IMHO it is safer to make the /etc/libvirt directory 0700 Makes sense. Thanks for pointing this out. I've never used passwords in any of these files myself, so I never really gave it much thought :( Assuming I can determine that a given user is authorized to manage the systemwide libvirtd, would you agree that that is the one they're most likely to want to access? I simply cannot think up a realistic example use case where someone has this privilege, but actually wants to access qemu:///session. No, I don't agree. I already mentioned the reasons why it is desirable to run within the user session - SDL, audio, disk permissions, and to add another one gnome-keyring integration for qcow2 passwords which is a future feature we'd like for the secrets driver. IMHO if we are to get better integration into the user's desktop experiance, then having both libvirtd the VMs running in the user's context, rather than a separate context is key. Yes, of course, when qemu:///session gets this smart and cool you will want to access qemu:///session by default. At /exactly/ the same time, the motivation for setting yourself up with access to qemu:///system disappears. When that motivation is gone, any admin worth his salt will immediately revoke said access (in the shared scenario) (since it's a gaping security hole) and voilà, libvirt will go back to using qemu:///session by default. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Default to qemu:///system if accessible
If no uri is passed to one of the virConnectOpen-ish calls, libvirt
attempts to autoprobe a sensible URI. Part of the current logic checks
for getuid() 0, and if that check is succesful, a libvirtd daemon is
spawned. This patch replaces this check with a call to
access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK) so that users with access to the
qemu:///system UNIX socket connect to that one by default instead of
spawning a new libvirtd daemon.
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/remote/remote_driver.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index a945710..17e6ead 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1086,7 +1086,7 @@ remoteOpen (virConnectPtr conn,
if (!conn-uri) {
DEBUG0(Auto-probe remote URI);
#ifndef __sun
-if (getuid() 0) {
+if (access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK)) {
DEBUG0(Auto-spawn user daemon instance);
rflags |= VIR_DRV_OPEN_REMOTE_USER;
if (!autostart ||
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Default to qemu:///system if accessible
On 03-09-2010 16:08, Daniel P. Berrange wrote: If no uri is passed to one of the virConnectOpen-ish calls, libvirt attempts to autoprobe a sensible URI. Part of the current logic checks for getuid() 0, and if that check is succesful, a libvirtd daemon is spawned. This patch replaces this check with a call to access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK) so that users with access to the qemu:///system UNIX socket connect to that one by default instead of spawning a new libvirtd daemon. NACK, I don't think we should be changing this. If the user is unprivileged, it should always default to the unprivileged libvirtd, regardless of whether they are also authorized to connect to the privileged libvirtd (via socket permissions or policykit, or kerberos). If the unprivileged user still wants the privileged libvirtd, they should given an explicit URI. Hm... I didn't think this was going to be controversial :) I firmly believe that if a user has access to the privileged libvirt daemon, that's the one he'll want to interact with in all but extraordinary cases. I consider it very analogous to how virt-manager doesn't even offer usermode networking if you're connected to qemu:///system, since if you aren't stuck with usermode networking, there's no reason to use it (other than to prove this statement wrong). Ubuntu has carried patches for this for virsh, virt-manager, and virt-viewer for a while now (at least a year and a half). I'm not sure if they've been submitted here (or to et-mgmt-tools) before (and if not, why not), but that's the lay of the land, and I've had nothing but good feedback on it. Now I just wanted to fix it properly (directly in libvirt) and get it upstream. It's certainly saved me a lot of typing. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Default to qemu:///system if accessible
On 03-09-2010 15:59, Daniel Veillard wrote:
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index a945710..17e6ead 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1086,7 +1086,7 @@ remoteOpen (virConnectPtr conn,
if (!conn-uri) {
DEBUG0(Auto-probe remote URI);
#ifndef __sun
-if (getuid() 0) {
+if (access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK)) {
DEBUG0(Auto-spawn user daemon instance);
rflags |= VIR_DRV_OPEN_REMOTE_USER;
if (!autostart ||
Hum, that sounds like a change of semantic.
Depends. I think of the (getuid() 0) as a check for whether you can
access the privileged UNIX socket that simply doesn't take into account
that users other than root may have been given this privilege by way of
membership of the group owning the socket.
Before as non root you would span a local daemon, now you use the system
one. I'm not saying it's a bad thing in most cases but it's a serious
change, and we try to avoid those in general.
As I suggested in another e-mail a short while ago in this thread, I'm
making this change to maintain the status quo in Ubuntu. We already do
this in virsh and virt-viewer and something very, very similar in
virt-manager. Someone recently dropped the virt-viewer patch by mistake,
and I decided to make the change directly in libvirt to not change how
libvirt has behaved for us in the past couple of years, so I can
certainly relate to your desire to not change the behaviour.
Personally, at least, I find this behaviour much more pleasant. On all
my servers, I just grant whoever needs to be able to deal with the VM's
on it access to the socket (by adding them to the libvirtd group), and
that's it. I don't need to instruct them to set per-application
environment variables or whatever.
Generally, I use libvirt on two classes of machine: Workstations/laptops
(where I'm functionally the only user), or on servers whose job is to
run virtual machines and nothing else.
In the former case, obviously I want to use the systemwide libvirtd to
get access to all the fancy networking things. I have absolutely no
motivation to use qemu:///session on there anymore.
In the latter case, only people who are supposed to manage the virtual
machines will have access to the server, and IMO they have no business
running stuff under qemu:///session on that box.
The only situation where I actually need qemu:///session is if I for
some reason want to run a VM on a machine that isn't mine (neither in
terms of ownership, nor responsibility). So far, that's never happened.
It's cool that it's possible(!), but I've never needed it.
--
Soren Hansen
Ubuntu Developer
http://www.ubuntu.com/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Explicitly pass uml_dir argument to user-mode-linux
On 25-08-2010 11:03, Soren Hansen wrote: uml_dir overrides user-mode-linux's default of ~/.uml. This is needed for a couple of different reasons: Any comments on this patch? It seems perfectly reasonable to me to get this in regardless of the outcome of this discussion: https://www.redhat.com/archives/libvir-list/2010-August/msg00672.html -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Use global directory as UML's monitorDir for privileged connections
On 31-08-2010 12:39, Daniel P. Berrange wrote: Can we make this '%s/lib/libvirt/uml' You want transient stuff like UNIX sockets in /var/lib? That seems odd to me. Not hugely, but I don't want it to overlap with the directories used by libvirtd for transient stuff. If we make it '%/run/libvirt/uml-guest' instead that would be OK Excellent. I understand your desire to be consistent, but I really think integrating well with existing management tools for the hypervisor (in this case uml-utilities) is more important. Agreed, I hadn't seen your reply about uml-utilities when writing this. Excellent again. I'll make these adjustments. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Use global directory as UML's monitorDir for privileged connections
For privileged UML connections (uml:///system), we shouldn't use root's
home dir, but rather somewhere in /var/run/libvirt/uml-guest.
https://bugzilla.redhat.com/show_bug.cgi?id=499536
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/uml/uml_driver.c | 12
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 8b129b7..0a5c829 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -373,6 +373,10 @@ umlStartup(int privileged) {
if ((base = strdup (SYSCONF_DIR /libvirt)) == NULL)
goto out_of_memory;
+
+if (virAsprintf(uml_driver-monitorDir,
+%s/run/libvirt/uml-guest, LOCAL_STATE_DIR) == -1)
+goto out_of_memory;
} else {
if (virAsprintf(uml_driver-logDir,
@@ -381,11 +385,11 @@ umlStartup(int privileged) {
if (virAsprintf(base, %s/.libvirt, userdir) == -1)
goto out_of_memory;
-}
-if (virAsprintf(uml_driver-monitorDir,
-%s/.uml, userdir) == -1)
-goto out_of_memory;
+if (virAsprintf(uml_driver-monitorDir,
+%s/.uml, userdir) == -1)
+goto out_of_memory;
+}
/* Configuration paths are either ~/.libvirt/uml/... (session) or
* /etc/libvirt/uml/... (system).
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Explicitly pass uml_dir argument to user-mode-linux
uml_dir overrides user-mode-linux's default of ~/.uml. This is needed for a couple of different reasons: libvirt expects this to default to virGetUserDirectory(geteuid()) + '/.uml'. However, user-mode-linux actually uses the HOME environment variable to determine where to look for the uml sockets, but if running libvirtd under sudo (which I routinely do during development), $HOME is pointing at my user's homedir, while my euid is 0, so libvirt looks in /root. Also (and this was my actual motivation for this patch), if HOME isn't set at all, user-mode-linux utterly fails. Looking at the code, it seems it's meant to emit a warning, but alas, it doesn't for some reason. If running libvirtd from upstart, HOME is not set, so any system using upstart will need this change. Signed-off-by: Soren Hansen so...@linux2go.dk --- src/uml/uml_conf.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 65b06c5..4906192 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -409,7 +409,7 @@ static char *umlNextArg(char *args) * for a given virtual machine. */ int umlBuildCommandLine(virConnectPtr conn, -struct uml_driver *driver ATTRIBUTE_UNUSED, +struct uml_driver *driver, virDomainObjPtr vm, fd_set *keepfd, const char ***retargv, @@ -499,7 +499,6 @@ int umlBuildCommandLine(virConnectPtr conn, ADD_ENV_COPY(LD_PRELOAD); ADD_ENV_COPY(LD_LIBRARY_PATH); ADD_ENV_COPY(PATH); -ADD_ENV_COPY(HOME); ADD_ENV_COPY(USER); ADD_ENV_COPY(LOGNAME); ADD_ENV_COPY(TMPDIR); @@ -508,6 +507,7 @@ int umlBuildCommandLine(virConnectPtr conn, //ADD_ARG_PAIR(con0, fd:0,fd:1); ADD_ARG_PAIR(mem, memory); ADD_ARG_PAIR(umid, vm-def-name); +ADD_ARG_PAIR(uml_dir, driver-monitorDir); if (vm-def-os.root) ADD_ARG_PAIR(root, vm-def-os.root); -- 1.7.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Explicitly pass uml_dir argument to user-mode-linux
On 25-08-2010 16:07, Cole Robinson wrote: On 08/25/2010 05:03 AM, Soren Hansen wrote: uml_dir overrides user-mode-linux's default of ~/.uml. This is needed for a couple of different reasons: I think this should also solve this long standing fedora/selinux bug: https://bugzilla.redhat.com/show_bug.cgi?id=499536 Well, halfway. The other half should be easy, though. Gimme a few minutes. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Use global directory as UML's monitorDir for privileged connections
For privileged UML connections (uml:///system), we shouldn't use root's
home dir, but rather somewhre in /var/run/libvirt/uml.
https://bugzilla.redhat.com/show_bug.cgi?id=499536
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/uml/uml_driver.c | 12
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 8b129b7..c8b9997 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -373,6 +373,10 @@ umlStartup(int privileged) {
if ((base = strdup (SYSCONF_DIR /libvirt)) == NULL)
goto out_of_memory;
+
+if (virAsprintf(uml_driver-monitorDir,
+%s/run/libvirt/uml, LOCAL_STATE_DIR) == -1)
+goto out_of_memory;
} else {
if (virAsprintf(uml_driver-logDir,
@@ -381,11 +385,11 @@ umlStartup(int privileged) {
if (virAsprintf(base, %s/.libvirt, userdir) == -1)
goto out_of_memory;
-}
-if (virAsprintf(uml_driver-monitorDir,
-%s/.uml, userdir) == -1)
-goto out_of_memory;
+if (virAsprintf(uml_driver-monitorDir,
+%s/.uml, userdir) == -1)
+goto out_of_memory;
+}
/* Configuration paths are either ~/.libvirt/uml/... (session) or
* /etc/libvirt/uml/... (system).
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Explicitly pass uml_dir argument to user-mode-linux
On 25-08-2010 16:18, Daniel P. Berrange wrote: Almost. We still need to change 'driver-monitorDir' so that it gets initialized to '$LOCAL_STATE_DIR/lib/libvirt/uml' like we do with QEMU (or $HOME/.libvirt/uml/lib for session mode) I think leaving it as $HOME/.uml is preferable for uml:///session. It integrates well with the existing, well-known uml-utilities (e.g. uml_mconsole domain name Just Works[tm]). -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Use global directory as UML's monitorDir for privileged connections
On 25-08-2010 18:07, Daniel P. Berrange wrote:
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -373,6 +373,10 @@ umlStartup(int privileged) {
if ((base = strdup (SYSCONF_DIR /libvirt)) == NULL)
goto out_of_memory;
+
+if (virAsprintf(uml_driver-monitorDir,
+%s/run/libvirt/uml, LOCAL_STATE_DIR) == -1)
+goto out_of_memory;
Can we make this '%s/lib/libvirt/uml'
You want transient stuff like UNIX sockets in /var/lib? That seems odd
to me.
The FHS even explicitly says: System programs that maintain transient
UNIX-domain sockets must place them in this directory. where this
directory is /var/run.
} else {
if (virAsprintf(uml_driver-logDir,
@@ -381,11 +385,11 @@ umlStartup(int privileged) {
if (virAsprintf(base, %s/.libvirt, userdir) == -1)
goto out_of_memory;
-}
-if (virAsprintf(uml_driver-monitorDir,
-%s/.uml, userdir) == -1)
-goto out_of_memory;
+if (virAsprintf(uml_driver-monitorDir,
+%s/.uml, userdir) == -1)
+goto out_of_memory;
+}
And '%s/.libvirt/uml/lib'
I understand your desire to be consistent, but I really think
integrating well with existing management tools for the hypervisor (in
this case uml-utilities) is more important.
--
Soren Hansen
Ubuntu Developer
http://www.ubuntu.com/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Support virDomainAttachDevice and virDomainDetachDevice for disks in UML
On 24-08-2010 22:20, Matthias Bolte wrote: +if (ret 0) +goto error; I was about to push this patch, but compile testing gave this error: uml/uml_driver.c: In function 'umlDomainAttachUmlDisk': uml/uml_driver.c:1729: error: 'ret' may be used uninitialized in this function [-Wuninitialized] I think if (ret 0) goto error; and int ret; can just be removed completely from this function, but I would like have your ACK on this before doing so. Please do so. I haven't a clue where that came from, and due to the magic of having git rebased every time, I'll never know. :) -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
Like the comment suggested, we just open the file and pass the file
descriptor to uml. The input stream is set to null, since I couldn't
find any useful way to actually use a file for input for a chardev and
this also mimics what e.g. QEmu does internally.
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/uml/uml_conf.c | 31 ++-
src/uml/uml_conf.h |1 +
src/uml/uml_driver.c | 12 +++-
3 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index 42193e4..65b06c5 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -295,7 +295,8 @@ error:
static char *
umlBuildCommandLineChr(virDomainChrDefPtr def,
- const char *dev)
+ const char *dev,
+ fd_set *keepfd)
{
char *ret = NULL;
@@ -344,8 +345,27 @@ umlBuildCommandLineChr(virDomainChrDefPtr def,
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
-case VIR_DOMAIN_CHR_TYPE_PIPE:
-/* XXX could open the file/pipe just pass the FDs */
+ {
+int fd_out;
+
+if ((fd_out = open(def-data.file.path,
+ O_WRONLY | O_APPEND | O_CREAT, 0660)) 0) {
+virReportSystemError(errno,
+ _(failed to open chardev file: %s),
+ def-data.file.path);
+return NULL;
+}
+if (virAsprintf(ret, %s%d=null,fd:%d, dev, def-target.port,
fd_out) 0) {
+virReportOOMError();
+close(fd_out);
+return NULL;
+}
+FD_SET(fd_out, keepfd);
+}
+break;
+ case VIR_DOMAIN_CHR_TYPE_PIPE:
+/* XXX could open the pipe just pass the FDs. Be wary of
+ * the effects of blocking I/O, though. */
case VIR_DOMAIN_CHR_TYPE_VC:
case VIR_DOMAIN_CHR_TYPE_UDP:
@@ -391,6 +411,7 @@ static char *umlNextArg(char *args)
int umlBuildCommandLine(virConnectPtr conn,
struct uml_driver *driver ATTRIBUTE_UNUSED,
virDomainObjPtr vm,
+fd_set *keepfd,
const char ***retargv,
const char ***retenv)
{
@@ -513,7 +534,7 @@ int umlBuildCommandLine(virConnectPtr conn,
for (i = 0 ; i UML_MAX_CHAR_DEVICE ; i++) {
char *ret = NULL;
if (i == 0 vm-def-console)
-ret = umlBuildCommandLineChr(vm-def-console, con);
+ret = umlBuildCommandLineChr(vm-def-console, con, keepfd);
if (!ret)
if (virAsprintf(ret, con%d=none, i) 0)
goto no_memory;
@@ -527,7 +548,7 @@ int umlBuildCommandLine(virConnectPtr conn,
if (vm-def-serials[j]-target.port == i)
chr = vm-def-serials[j];
if (chr)
-ret = umlBuildCommandLineChr(chr, ssl);
+ret = umlBuildCommandLineChr(chr, ssl, keepfd);
if (!ret)
if (virAsprintf(ret, ssl%d=none, i) 0)
goto no_memory;
diff --git a/src/uml/uml_conf.h b/src/uml/uml_conf.h
index b33acc8..d8b2349 100644
--- a/src/uml/uml_conf.h
+++ b/src/uml/uml_conf.h
@@ -71,6 +71,7 @@ virCapsPtr umlCapsInit (void);
int umlBuildCommandLine (virConnectPtr conn,
struct uml_driver *driver,
virDomainObjPtr dom,
+ fd_set *keepfd,
const char ***retargv,
const char ***retenv);
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 9cad7f1..e926a9f 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -811,6 +811,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
char *logfile;
int logfd = -1;
struct stat sb;
+int openmax;
fd_set keepfd;
char ebuf[1024];
umlDomainObjPrivatePtr priv = vm-privateData;
@@ -869,7 +870,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
return -1;
}
-if (umlBuildCommandLine(conn, driver, vm,
+if (umlBuildCommandLine(conn, driver, vm, keepfd,
argv, progenv) 0) {
close(logfd);
umlCleanupTapDevices(conn, vm);
@@ -908,6 +909,15 @@ static int umlStartVMDaemon(virConnectPtr conn,
NULL, NULL, NULL);
close(logfd);
+/*
+ * At the moment, the only thing that populates keepfd is
+ * umlBuildCommandLineChr. We want to close every fd it opens.
+ */
+openmax = sysconf (_SC_OPEN_MAX);
+for (i = 0; i openmax; i++)
+if (FD_ISSET(i, keepfd))
+close(i);
+
for (i = 0 ; argv[i] ; i++)
VIR_FREE(argv[i]);
VIR_FREE(argv);
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On 23-08-2010 12:42, Daniel P. Berrange wrote: +/* + * At the moment, the only thing that populates keepfd is + * umlBuildCommandLineChr. We want to close every fd it opens. + */ +openmax = sysconf (_SC_OPEN_MAX); +for (i = 0; i openmax; i++) +if (FD_ISSET(i, keepfd)) +close(i); + Unfortunately fdset is one of those limited types that can't represent all possible values. So you need to use FD_SETSIZE instead of _SC_OPEN_MAX here Ok, I'll fix that up, but just so that I understand: Your concern is that there might be an open file descriptor between FD_SETSIZE and _SC_OPEN_MAX that we don't want to close? -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On 23-08-2010 13:04, Daniel P. Berrange wrote: Ok, I'll fix that up, but just so that I understand: Your concern is that there might be an open file descriptor between FD_SETSIZE and _SC_OPEN_MAX that we don't want to close? No, its that if you try to run FD_ISSET for i FD_SETSIZE, you'll likely have an array overflow / out of bounds, so just stop at FD_SETSIZE. Oh, of course. How silly of me. Thanks :) -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
Like the comment suggested, we just open the file and pass the file
descriptor to uml. The input stream is set to null, since I couldn't
find any useful way to actually use a file for input for a chardev and
this also mimics what e.g. QEmu does internally.
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/uml/uml_conf.c | 31 ++-
src/uml/uml_conf.h |1 +
src/uml/uml_driver.c | 10 +-
3 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index 42193e4..65b06c5 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -295,7 +295,8 @@ error:
static char *
umlBuildCommandLineChr(virDomainChrDefPtr def,
- const char *dev)
+ const char *dev,
+ fd_set *keepfd)
{
char *ret = NULL;
@@ -344,8 +345,27 @@ umlBuildCommandLineChr(virDomainChrDefPtr def,
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
-case VIR_DOMAIN_CHR_TYPE_PIPE:
-/* XXX could open the file/pipe just pass the FDs */
+ {
+int fd_out;
+
+if ((fd_out = open(def-data.file.path,
+ O_WRONLY | O_APPEND | O_CREAT, 0660)) 0) {
+virReportSystemError(errno,
+ _(failed to open chardev file: %s),
+ def-data.file.path);
+return NULL;
+}
+if (virAsprintf(ret, %s%d=null,fd:%d, dev, def-target.port,
fd_out) 0) {
+virReportOOMError();
+close(fd_out);
+return NULL;
+}
+FD_SET(fd_out, keepfd);
+}
+break;
+ case VIR_DOMAIN_CHR_TYPE_PIPE:
+/* XXX could open the pipe just pass the FDs. Be wary of
+ * the effects of blocking I/O, though. */
case VIR_DOMAIN_CHR_TYPE_VC:
case VIR_DOMAIN_CHR_TYPE_UDP:
@@ -391,6 +411,7 @@ static char *umlNextArg(char *args)
int umlBuildCommandLine(virConnectPtr conn,
struct uml_driver *driver ATTRIBUTE_UNUSED,
virDomainObjPtr vm,
+fd_set *keepfd,
const char ***retargv,
const char ***retenv)
{
@@ -513,7 +534,7 @@ int umlBuildCommandLine(virConnectPtr conn,
for (i = 0 ; i UML_MAX_CHAR_DEVICE ; i++) {
char *ret = NULL;
if (i == 0 vm-def-console)
-ret = umlBuildCommandLineChr(vm-def-console, con);
+ret = umlBuildCommandLineChr(vm-def-console, con, keepfd);
if (!ret)
if (virAsprintf(ret, con%d=none, i) 0)
goto no_memory;
@@ -527,7 +548,7 @@ int umlBuildCommandLine(virConnectPtr conn,
if (vm-def-serials[j]-target.port == i)
chr = vm-def-serials[j];
if (chr)
-ret = umlBuildCommandLineChr(chr, ssl);
+ret = umlBuildCommandLineChr(chr, ssl, keepfd);
if (!ret)
if (virAsprintf(ret, ssl%d=none, i) 0)
goto no_memory;
diff --git a/src/uml/uml_conf.h b/src/uml/uml_conf.h
index b33acc8..d8b2349 100644
--- a/src/uml/uml_conf.h
+++ b/src/uml/uml_conf.h
@@ -71,6 +71,7 @@ virCapsPtr umlCapsInit (void);
int umlBuildCommandLine (virConnectPtr conn,
struct uml_driver *driver,
virDomainObjPtr dom,
+ fd_set *keepfd,
const char ***retargv,
const char ***retenv);
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 9cad7f1..6582d95 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -869,7 +869,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
return -1;
}
-if (umlBuildCommandLine(conn, driver, vm,
+if (umlBuildCommandLine(conn, driver, vm, keepfd,
argv, progenv) 0) {
close(logfd);
umlCleanupTapDevices(conn, vm);
@@ -908,6 +908,14 @@ static int umlStartVMDaemon(virConnectPtr conn,
NULL, NULL, NULL);
close(logfd);
+/*
+ * At the moment, the only thing that populates keepfd is
+ * umlBuildCommandLineChr. We want to close every fd it opens.
+ */
+for (i = 0; i FD_SETSIZE; i++)
+if (FD_ISSET(i, keepfd))
+close(i);
+
for (i = 0 ; argv[i] ; i++)
VIR_FREE(argv[i]);
VIR_FREE(argv);
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Support virDomainAttachDevice and virDomainDetachDevice for disks in UML
On 19-08-2010 15:22, Daniel P. Berrange wrote:
+static inline void umlShrinkDisks(virDomainDefPtr def, size_t i)
+{
+if (def-ndisks 1) {
+memmove(def-disks + i,
+def-disks + i + 1,
+sizeof(*def-disks) *
+(def-ndisks - (i + 1)));
+def-ndisks--;
+if (VIR_REALLOC_N(def-disks, def-ndisks) 0) {
+/* ignore, harmless */
+}
+} else {
+VIR_FREE(def-disks);
+def-ndisks = 0;
+}
+}
Since this code is already used in the QEMU driver, I think we
should just put it straight into src/conf/domain_conf.c so we
can share it. 'virDomainDiskRemove' is probably a more accurate
name than ShrinkDisks.
Sounds good to me. I wasn't sure where to stick it, so I just left it
here and figured you'd probably tell me something like this :) I'll move it.
+
+
+static int umlDomainAttachUmlDisk(struct uml_driver *driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
+{
+int i, ret;
+char *cmd = NULL;
+char *reply;
+
+for (i = 0 ; i vm-def-ndisks ; i++) {
+if (STREQ(vm-def-disks[i]-dst, disk-dst)) {
+umlReportError(VIR_ERR_OPERATION_FAILED,
+ _(target %s already exists), disk-dst);
+return -1;
+}
+}
+
+if (!disk-src) {
+umlReportError(VIR_ERR_INTERNAL_ERROR,
+ %s, _(disk source path is missing));
+goto error;
+}
+
+if (virAsprintf(cmd, config %s=%s, disk-dst, disk-src) 0) {
+virReportOOMError();
+return -1;
+}
+
+if (umlMonitorCommand(driver, vm, cmd, reply) 0)
+return -1;
Needs to be a 'goto error' to avoid leaking 'cmd'
Ah, yes, good catch. I also need to free the reply. :)
@@ -1900,9 +2119,9 @@ static virDriver umlDriver = {
umlDomainStartWithFlags, /* domainCreateWithFlags */
umlDomainDefine, /* domainDefineXML */
umlDomainUndefine, /* domainUndefine */
-NULL, /* domainAttachDevice */
+umlDomainAttachDevice, /* domainAttachDevice */
NULL, /* domainAttachDeviceFlags */
-NULL, /* domainDetachDevice */
+umlDomainDetachDevice, /* domainDetachDevice */
NULL, /* domainDetachDeviceFlags */
You should also implement the DeviceFlags variants at
the same time. You can just do a dumb wrapper like QEMU
driver does, for simplicity.
Right you are. I misunderstood what those were for.
--
Soren Hansen
Ubuntu Developer
http://www.ubuntu.com/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On 19-08-2010 15:41, Daniel P. Berrange wrote: + case VIR_DOMAIN_CHR_TYPE_PIPE: +/* XXX could open the pipe just pass the FDs */ Any reason not to let the code deal with PIPE too ? It seems like the code should work equally well for both PIPE FILE. (well drop the O_CREATE|O_APPEND - assume the user has got the pipe pre-created with 'mkfifo'. Not in particular. I was just in a hurry, wanted to share the patch sooner rather than later and then attack the pipes later on. I'll fix it up, but perhaps I won't get to it today. I think this leaks file descriptors in libvirtd. There's no existing code which ever closes the FDs in keepfd after spawning UML later on in the function. True. I somehow thought virExecWhatever took care of that, but I see now that it doesn't, and I guess that would be troublesome. I'll handle this in the uml driver. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Add new API for accessing remote guest text console
On 17-08-2010 19:02, Daniel P. Berrange wrote: The 'virsh console' command has been an oddity that only works when run locally, as the same UID as the QEMU instance. This is because it directly opens /dev/pty/XXX. This introduces a formal API for accessing consoles that uses the virStreamPtr APIs. Now any app can open consoles anywhere it can connect to libvirt I don't (right now, at least) have any comments on the patches themselves, but I can't help but wonder what other wonderful improvements you've got in your pipeline. I spent at least a couple of hours on something like this a couple of weeks ago, but had I known that you were already doing it, I wouldn't have wasted my time. So, in an effort to not duplicate efforts, perhaps everyone who's working on something reasonably big (certainly stuff like this, but also smaller change sets) could put a list up somewhere for all to see? Perhaps such a list already exists and I just don't know about it? -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow chardev of type 'file' for UML domains.
On 18-08-2010 12:45, so...@linux2go.dk wrote:
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 04493ba..5f73ce2 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -737,10 +737,6 @@ static int umlMonitorCommand(const struct uml_driver
*driver,
virReportSystemError(errno, _(cannot read reply %s), cmd);
goto error;
}
-if (nbytes sizeof res) {
-virReportSystemError(0, _(incomplete reply %s), cmd);
-goto error;
-}
if (sizeof res.data res.length) {
virReportSystemError(0, _(invalid length in reply %s), cmd);
goto error;
Whoops, this bit wasn't meant to be included. /me reposts without it.
--
Soren Hansen
Ubuntu Developer
http://www.ubuntu.com/
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On Sun, Aug 15, 2010 at 11:08:43PM +0200, Soren Hansen wrote:
Unless multiple threads are reading from the fd (?), I'm quite
confident in my data. I dumped the whole monitor_response buffer right
before and right after the recvfrom call along with the return value
of recvfrom. I'll rerun the tests tomorrow and show you the results.
I'm running this on another kernel right now and I'm not seeing the
problem. I'll try again with the kernel I used a couple of days ago.
I'm not sure if I pointed this out in my initial e-mail, but even if I
didn't have this recvfrom problem, the check seems odd to my eye. Is
the monitor really going to send a max sized datagram each time? I
would have expected it to only send a datagram the size of the header
+ the length of the data.
My hunch here seems correct though.
I've applied this diff:
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 9cf669f..c289ad3 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -729,8 +733,13 @@ static int umlMonitorCommand(const struct uml_driver
*driver,
do {
ssize_t nbytes;
addrlen = sizeof(addr);
+VIR_DEBUG(res.error: %d, res.extra: %d, res.length: %d, res.data:
%.*s,
+ res.error, res.extra, res.length, res.length,
res.data);
nbytes = recvfrom(priv-monitor, res, sizeof res, 0,
(struct sockaddr *)addr, addrlen);
+VIR_DEBUG(nbytes: %d, nbytes);
+VIR_DEBUG(res.error: %d, res.extra: %d, res.length: %d, res.data:
%.*s,
+ res.error, res.extra, res.length, res.length,
res.data);
if (nbytes 0) {
if (errno == EAGAIN || errno == EINTR)
continue;
And I get this output:
11:28:14.602: debug : umlMonitorCommand:702 : Run command 'config con0'
11:28:14.602: debug : umlMonitorCommand:737 : res.error: 5, res.extra: 0,
res.length: 4096, res.data:
11:28:14.602: debug : umlMonitorCommand:740 : nbytes: 28
11:28:14.602: debug : umlMonitorCommand:742 : res.error: 0, res.extra: 0,
res.length: 16, res.data: pts:/dev/pts/31
11:28:14.602: debug : umlMonitorCommand:771 : Command reply is 'pts:/dev/pts/31'
So the size of the response datagram isn't sizeof(res) as the check in
uml_driver.c expects, but rather sizeof(res.error) + sizeof(res.extra) +
sizeof(res.length) + res.length.
--
Soren Hansen so...@linux2go.dk
Systems Architect, The Rackspace Cloud
Ubuntu Developer
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On Mon, Aug 16, 2010 at 11:37:07AM +0200, Soren Hansen wrote: I'm running this on another kernel right now and I'm not seeing the problem. I'll try again with the kernel I used a couple of days ago. Ok, found the other kernel. Same diff as in my previous e-mail, same action. These are my results: 09:41:01.134: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.134: debug : umlMonitorCommand:733 : res.error: 6, res.extra: 0, res.length: 4096, res.data: 09:41:01.134: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.134: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.134: debug : umlMonitorCommand:761 : Command reply is 'pts' 09:41:01.144: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.144: debug : umlMonitorCommand:733 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.144: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.144: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.144: debug : umlMonitorCommand:761 : Command reply is 'pts' 09:41:01.154: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.154: debug : umlMonitorCommand:733 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.155: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.155: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.155: debug : umlMonitorCommand:761 : Command reply is 'pts' 09:41:01.165: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.165: debug : umlMonitorCommand:733 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.169: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.169: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.169: debug : umlMonitorCommand:761 : Command reply is 'pts' 09:41:01.179: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.179: debug : umlMonitorCommand:733 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.179: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.179: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 15, res.data: pts:/dev/pts/7 09:41:01.179: debug : umlMonitorCommand:761 : Command reply is 'pts:/dev/pts/7' This one's a 2.6.34.1 kernel. The one where I didn't see the problem is a 2.6.32.something-or-the-other kernel. Mindboggling. -- Soren Hansen so...@linux2go.dk Systems Architect, The Rackspace Cloud Ubuntu Developer -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On 16-08-2010 18:08, Eric Blake wrote: On 08/16/2010 03:54 AM, Soren Hansen wrote: 09:41:01.134: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.134: debug : umlMonitorCommand:733 : res.error: 6, res.extra: 0, res.length: 4096, res.data: 09:41:01.134: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.134: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts This one's a 2.6.34.1 kernel. The one where I didn't see the problem is a 2.6.32.something-or-the-other kernel. Mindboggling. Oh my. This really does look like a kernel bug. Can you confirm it with an strace? Annoyingly, no. It doesn't happen in the primary libvirt thread, so I have to pass -f to strace, and uml doesn't let you PTRACE it, so I can't trigger the problem. Have you reported this regression to the right kernel folks? It doesn't happen in 2.6.35, so I seems isolated to that particular kernel. It's really very, very odd. I guess it would help if we could write a simpler test program to isolate whether this recvfrom bug exists in a bare minimum number of syscalls. Meanwhile, I have no idea how to work around a buggy recvfrom that doesn't return the correct number of bytes. No, you're right. Whatever we do to work around this is going to suck somehow. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On 16-08-2010 18:08, Eric Blake wrote:
Ok, found the other kernel. Same diff as in my previous e-mail, same
action. These are my results:
09:41:01.134: debug : umlMonitorCommand:698 : Run command 'config con0'
09:41:01.134: debug : umlMonitorCommand:733 : res.error: 6, res.extra: 0,
res.length: 4096, res.data:
09:41:01.134: debug : umlMonitorCommand:736 : nbytes: 0
09:41:01.134: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0,
res.length: 4, res.data: pts
*blush*
Ok, we can officially stop chasing this now. While working on some of
the other patches I've sent, I apparantly managed to copy a bit of code
from one place to another and by luck it was syntactically correct, so
the compiler didn't complain. The offending *two characters* were:
nbytes = recvfrom(priv-monitor, res, sizeof res, 0,
- (struct sockaddr *)addr, addrlen);
+ (struct sockaddr *)addr, addrlen)0;
if (nbytes 0) {
Henceforth, nbytes was 0.
Sorry about the wasted brain cycles on this.
--
Soren Hansen
Ubuntu Developer
http://www.ubuntu.com/
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On 16-08-2010 18:04, Eric Blake wrote: So the size of the response datagram isn't sizeof(res) as the check in uml_driver.c expects, but rather sizeof(res.error) + sizeof(res.extra) + sizeof(res.length) + res.length. I agree with this analysis. In other words, the check should be more like this (two conditions - did we get enough bytes to even have a valid res.length, and did we get enough bytes to match with what res.length stated): if (nbytes offsetof(struct monitor_request, data) || nbytes res.length + offsetof(struct monitor_request, data)) incomplete reply Yup, this looks good. But before I write such a patch, I'm going to look in more details at your other reply. Let's just forget all about that one, shall we? Please? :) -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On Sat, Aug 14, 2010 at 11:11:40AM -0600, Eric Blake wrote: First of all, for me, the call to recvfrom returns 0, even though the buffer actually is populated with the response from the monitor. I'm not sure about this one. The recvfrom is called in a loop, and POSIX says that recvfrom shall return the size if a message was read, or 0 if no messages were available but the other end of the connection did an orderly shutdown. Yes, it does. That's part of why it took me so long to figure out. I really didn't expect data to come back when recvfrom returned 0 :) I think we need a bit more understanding of why you are getting a 0 return, and whether it only happens after a successful iteration of the loop (in which case the contents of res are leftover from the prior iteration). Unless multiple threads are reading from the fd (?), I'm quite confident in my data. I dumped the whole monitor_response buffer right before and right after the recvfrom call along with the return value of recvfrom. I'll rerun the tests tomorrow and show you the results. If my results are accurate and it is indeed a kernel bug (well, or eglibc or whatnot), I believe it's in the spirit of libvirt to accept that one of the other parts of the equation is doing something silly and work around it. :) I'm happy to provide more evidence, though. I'm not sure if I pointed this out in my initial e-mail, but even if I didn't have this recvfrom problem, the check seems odd to my eye. Is the monitor really going to send a max sized datagram each time? I would have expected it to only send a datagram the size of the header + the length of the data. -- Soren Hansen so...@linux2go.dk Systems Architect, The Rackspace Cloud Ubuntu Developer -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] Close fd's of persistent tap devices
When passing a NULL tapfd argument to brAddTap, we need to close the fd of the tap device. If we don't, libvirt will keep the fd open indefinitely and renders the the guest unable to configure its side of the tap device. Signed-off-by: Soren Hansen so...@linux2go.dk --- src/util/bridge.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/util/bridge.c b/src/util/bridge.c index 7d0caae..da62c5e 100644 --- a/src/util/bridge.c +++ b/src/util/bridge.c @@ -538,6 +538,8 @@ brAddTap(brControl *ctl, goto error; if (tapfd) *tapfd = fd; +else +close(fd); return 0; error: -- 1.7.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Make umlConnectTapDevice ask brAddTap for a persistent tap device.
This patch does two things:
* It makes umlConnectTapDevice ask brAddTap for a persistent tap by
passing it a NULL tapfd argument.
* Stops umlConnectTapDevice from immediately dismantling the bridge
it just set up.
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/uml/uml_conf.c |6 +-
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index bc8cbce..06543cb 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -112,7 +112,6 @@ umlConnectTapDevice(virDomainNetDefPtr net,
const char *bridge)
{
brControl *brctl = NULL;
-int tapfd = -1;
int template_ifname = 0;
int err;
unsigned char tapmac[VIR_MAC_BUFLEN];
@@ -140,7 +139,7 @@ umlConnectTapDevice(virDomainNetDefPtr net,
net-ifname,
tapmac,
0,
-tapfd))) {
+NULL))) {
if (err == ENOTSUP) {
/* In this particular case, give a better diagnostic. */
umlReportError(VIR_ERR_INTERNAL_ERROR,
@@ -164,9 +163,6 @@ umlConnectTapDevice(virDomainNetDefPtr net,
VIR_FREE(net-ifname);
goto error;
}
-close(tapfd);
-
-brShutdown(brctl);
return 0;
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] A couple of UML-related fixes
I've been trying to get UML working for the last couple of days. As it stands, at least bridged networking doesn't work for me. I've got a fix, but I'm not sure I'm doing it right. I can get it to fail in three distinct ways: * With the current code, it seems that the tap device disappears almost immediately. This happens becuase the call to brAddTap includes an (int *)tapfd argument, so the tap device isn't persistent. A few lines after brAddTap, close(tapfd) is called and the tap device goes missing. * Passing a NULL tapfd argument to brAddTap and removing the call to close(tapfd) makes the tapfd persist, but when the uml domain attempts to bring up the interface, it fails with EBUSY. This turns out to be because brAddTap marks the tap device persistent, but doesn't close the fd, so both libvirt and uml have the fd open and that's doesn't work out well. * Finally, I tried passing the tapfd result back up the call chain so that I have a list of them that I could tell virExecDaemonize to keep open and then close them in libvirt after the fork. I think this is when I got EPERM when trying to bring the interface up in UML domain, but I'm not sure right now. Also, I've had problems connecting to the domain's console. These three patches seem to fix everything for me. Soren Hansen (3): Close fd's of persistent tap devices Make umlConnectTapDevice ask brAddTap for a persistent tap device. Remove wrong check for uml monitor response size src/uml/uml_conf.c |6 +- src/uml/uml_driver.c |4 src/util/bridge.c|2 ++ 3 files changed, 3 insertions(+), 9 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
The current check for the size of the response from the uml monitor is
problematic for a couple of reasons:
First of all, for me, the call to recvfrom returns 0, even though the
buffer actually is populated with the response from the monitor.
Second, it seems to me that (assuming recvfrom actually returned the
number of bytes read) it should be compared against the size of the
response header plus the actual data length rather than the max size of
the datagram.
At any rate, the only way I can get anything useful to happen here is to
completely remove the check, so this patch does just that.
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/uml/uml_driver.c |4
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 04493ba..9cf669f 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -737,10 +737,6 @@ static int umlMonitorCommand(const struct uml_driver
*driver,
virReportSystemError(errno, _(cannot read reply %s), cmd);
goto error;
}
-if (nbytes sizeof res) {
-virReportSystemError(0, _(incomplete reply %s), cmd);
-goto error;
-}
if (sizeof res.data res.length) {
virReportSystemError(0, _(invalid length in reply %s), cmd);
goto error;
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Make umlConnectTapDevice ask brAddTap for a persistent tap device.
On 12-08-2010 14:20, Daniel P. Berrange wrote: -brShutdown(brctl); brShutdown doesn't touch the bridge device at all. This simply closes the socket file handle we used for talking to the kernel to create the bridge. Removing this causes an FD leak Hm, so it does. I'll put it back. Regards, Soren. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add ubd to the list of disk prefixes
On Tue, Aug 10, 2010 at 07:18:06AM -0600, Eric Blake wrote: Pushed, along with a .mailmap update to satisfy 'make syntax-check'. Soren, this patch introduced a third distinct author address for you - let me know if you want AUTHORS updated to swap which one is listed as your primary address. so...@canonical.com is no longer valid, so I won't be submitting any more patches from that address. Let's go with so...@linux2go.dk as the primary one and leave so...@ubuntu.com as an alias. -- Soren Hansen so...@linux2go.dk Systems Architect, The Rackspace Cloud Ubuntu Developer -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add ubd to the list of disk prefixes
virDiskNameToIndex has a list of disk name prefixes that it uses in the
process of finding the disk's index. This list is missing ubd which
is the disk prefix used for UML domains.
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/util/util.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 8f2a17e..c173e49 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2367,7 +2367,7 @@ const char *virEnumToString(const char *const*types,
int virDiskNameToIndex(const char *name) {
const char *ptr = NULL;
int idx = 0;
-static char const* const drive_prefix[] = {fd, hd, vd, sd, xvd};
+static char const* const drive_prefix[] = {fd, hd, vd, sd, xvd,
ubd};
unsigned int i;
for (i = 0; i ARRAY_CARDINALITY(drive_prefix); i++) {
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Make virDomainGetXMLDesc output cache settings even if no special driverName is set
If a special cache strategy for a disk has been specified in a domain
definition, but no driverName has been set, virDomainGetXMLDesc will not
include the driver tag at all.
This patch makes sure that the driver tag is included if any of the
settings it can contain has been set.
Signed-off-by: Soren Hansen so...@linux2go.dk
---
src/conf/domain_conf.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 96ba0b0..56c5239 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4718,8 +4718,10 @@ virDomainDiskDefFormat(virBufferPtr buf,
disk type='%s' device='%s'\n,
type, device);
-if (def-driverName) {
-virBufferVSprintf(buf, driver name='%s', def-driverName);
+if (def-driverName || def-driverType || def-cachemode) {
+virBufferVSprintf(buf, driver);
+if (def-driverName)
+virBufferVSprintf(buf, name='%s', def-driverName);
if (def-driverType)
virBufferVSprintf(buf, type='%s', def-driverType);
if (def-cachemode)
--
1.7.0
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Eventtest unit test failure
On Mon, Nov 30, 2009 at 10:26:12AM +, Daniel P. Berrange wrote: I've changed the libvirt package in Ubuntu to run the test suite with every build. However, on our build daemons, the eventtest unit test seems to almost[1] consistently fail. You can see an example build log here: http://launchpadlibrarian.net/36196282/buildlog_ubuntu-lucid-amd64.libvirt_0.7.0-1ubuntu15_FAILEDTOBUILD.txt.gz Does this look familiar to anyone? Any idea what might be causing this? FWIW, I cannot reproduce it locally, so it's likely environmental, but at this point, I don't really know where to begin to look. What HZ setting are the kernels on the problem host running with ? IIRC we had someone else report a problem with this test suite on a kernel built with HZ=100, rather than the more common HZ=1000 Ah, indeed they are running with HZ=100. So this is a known problem? Is there a known fix? -- Soren Hansen | Lead virtualisation engineer | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Eventtest unit test failure
Hi, guys. I've changed the libvirt package in Ubuntu to run the test suite with every build. However, on our build daemons, the eventtest unit test seems to almost[1] consistently fail. You can see an example build log here: http://launchpadlibrarian.net/36196282/buildlog_ubuntu-lucid-amd64.libvirt_0.7.0-1ubuntu15_FAILEDTOBUILD.txt.gz Does this look familiar to anyone? Any idea what might be causing this? FWIW, I cannot reproduce it locally, so it's likely environmental, but at this point, I don't really know where to begin to look. [1]: It actually succeeds on sparc and armel :-/ -- Soren Hansen | Lead virtualisation engineer | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Supporting dynamic bridge names
I'm fairly sure we've discussed this before, but I couldn't find it in
my archives..
A rather long time ago (0.4.0 timeframe, I think) we switched the
default network in Ubuntu to use a bridge whose name was defined as
virbr%d. This was done to be able to actually supply a default,
enabled network without the risk of interfering with an existing bridge
called virbr0 (ignoring the possible implications of unconditionally
using 192.168.122.0/24 (about which, I might add, I've never had /any/
complaints)).
Now, since 0.5.0 (or thereabouts, I think), libvirt doesn't support
this, which
a) makes it rather difficult to achieve our goal of not clashing with
existing bridges named virbr0, and
b) causes some amount of grief for updates.
To overcome this, I've changed virNetworkAllocateBridge like so:
--- libvirt-0.6.1.orig/src/network_conf.c 2009-03-03 09:23:22.0
+0100
+++ libvirt-0.6.1/src/network_conf.c2009-04-16 20:36:43.660996644 +0200
@@ -875,16 +875,20 @@
}
char *virNetworkAllocateBridge(virConnectPtr conn,
- const virNetworkObjListPtr nets)
+ const virNetworkObjListPtr nets,
+ const char *template)
{
int id = 0;
char *newname;
+ if (!template)
+ template = virbr%d;
+
do {
char try[50];
-snprintf(try, sizeof(try), virbr%d, id);
+snprintf(try, sizeof(try), template, id);
if (!virNetworkBridgeInUse(nets, try, NULL)) {
if (!(newname = strdup(try))) {
--- libvirt-0.6.1.orig/src/network_conf.h 2009-03-03 09:23:22.0
+0100
+++ libvirt-0.6.1/src/network_conf.h2009-04-16 15:36:46.975452201 +0200
@@ -174,7 +174,8 @@
const char *skipname);
char *virNetworkAllocateBridge(virConnectPtr conn,
- const virNetworkObjListPtr nets);
+ const virNetworkObjListPtr nets,
+ const char *template);
int virNetworkSetBridgeName(virConnectPtr conn,
const virNetworkObjListPtr nets,
And virNetworkSetBridgeName like so:
--- libvirt-0.6.1.orig/src/network_conf.c 2009-03-03 09:23:22.0
+0100
+++ libvirt-0.6.1/src/network_conf.c2009-04-16 20:36:43.660996644 +0200
@@ -909,7 +913,7 @@
int ret = -1;
-if (def-bridge) {
+if (def-bridge !strstr(def-bridge, %d)) {
if (virNetworkBridgeInUse(nets, def-bridge, def-name)) {
networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_(bridge name '%s' already in use.),
@@ -918,7 +922,7 @@
}
} else {
/* Allocate a bridge name */
-if (!(def-bridge = virNetworkAllocateBridge(conn, nets)))
+if (!(def-bridge = virNetworkAllocateBridge(conn, nets, def-bridge)))
goto error;
}
And finally virNetworkLoadConfig like so:
--- libvirt-0.6.1.orig/src/network_conf.c 2009-03-03 09:23:22.0
+0100
+++ libvirt-0.6.1/src/network_conf.c2009-04-16 20:36:43.660996644 +0200
@@ -747,7 +747,7 @@
/* Generate a bridge if none is found, but don't check for collisions
* if a bridge is hardcoded, so the network is at least defined
*/
-if (!def-bridge !(def-bridge = virNetworkAllocateBridge(conn, nets)))
+if (!(def-bridge = virNetworkAllocateBridge(conn, nets, def-bridge)))
goto error;
if (!(net = virNetworkAssignDef(conn, nets, def)))
This is far from perfect, though.
a) As used to be the case for the VNC port, virsh net-dumpxml will
give you /current/ bridge name, so e.g. virsh net-edit will cause you
to hardcode the bridge name if you're not careful to replace the current
name with one that says '%d' somewhere. This can be fixed by e.g. either
adding a template or current attribute on the bridge element in the
network XML (depending on whether you want the name attribute to be
set to the current value or the template value).
b) Even if you /do/ remember to put '%d' in place of the assigned number
during editing, the bridge number will be increased, because
virNetworkBridgeInUse thinks the bridge name is in use. This should be
easy to fix, though.
I'm perfectly willing to work on this, but I'd like to get you guys' gut
feeling on this first.
--
Soren Hansen |
Lead Virtualisation Engineer | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Supporting dynamic bridge names
On Fri, Apr 17, 2009 at 10:01:38AM +0100, Daniel P. Berrange wrote:
The problem with this approach is that the bridge name potentially
ends up being different every time the virtual network is started. The
bridge name needs to be stable,
Why? I can't remember ever having to refer to it, and if I did, I can
get that information at runtime by parsing the output from virsh
net-dumpxml name of the network. What am I not thinking of? :)
if (!def-bridge !(def-bridge = virNetworkAllocateBridge(conn,
nets)))
To check for %d, as well as NULL, and pass in the template name as your
patch more or less does
Right, with my patch applied virNetworkAllocateBridge takes the
!def-bridge case into account.
I guess that if this is how you want it, the patch should be good as it
is. Here it is in its entirety:
Index: libvirt-0.6.1/src/network_conf.c
===
--- libvirt-0.6.1.orig/src/network_conf.c 2009-04-16 20:49:28.851655724
+0200
+++ libvirt-0.6.1/src/network_conf.c2009-04-17 09:42:33.075084537 +0200
@@ -747,7 +747,7 @@
/* Generate a bridge if none is found, but don't check for collisions
* if a bridge is hardcoded, so the network is at least defined
*/
-if (!def-bridge !(def-bridge = virNetworkAllocateBridge(conn, nets)))
+if (!(def-bridge = virNetworkAllocateBridge(conn, nets, def-bridge)))
goto error;
if (!(net = virNetworkAssignDef(conn, nets, def)))
@@ -875,16 +875,20 @@
}
char *virNetworkAllocateBridge(virConnectPtr conn,
- const virNetworkObjListPtr nets)
+ const virNetworkObjListPtr nets,
+ const char *template)
{
int id = 0;
char *newname;
+ if (!template)
+ template = virbr%d;
+
do {
char try[50];
-snprintf(try, sizeof(try), virbr%d, id);
+snprintf(try, sizeof(try), template, id);
if (!virNetworkBridgeInUse(nets, try, NULL)) {
if (!(newname = strdup(try))) {
@@ -909,7 +913,7 @@
int ret = -1;
-if (def-bridge) {
+if (def-bridge !strstr(def-bridge, %d)) {
if (virNetworkBridgeInUse(nets, def-bridge, def-name)) {
networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_(bridge name '%s' already in use.),
@@ -918,7 +922,7 @@
}
} else {
/* Allocate a bridge name */
-if (!(def-bridge = virNetworkAllocateBridge(conn, nets)))
+if (!(def-bridge = virNetworkAllocateBridge(conn, nets, def-bridge)))
goto error;
}
Index: libvirt-0.6.1/src/network_conf.h
===
--- libvirt-0.6.1.orig/src/network_conf.h 2009-04-16 20:49:28.899655820
+0200
+++ libvirt-0.6.1/src/network_conf.h2009-04-17 09:42:33.075084537 +0200
@@ -174,7 +174,8 @@
const char *skipname);
char *virNetworkAllocateBridge(virConnectPtr conn,
- const virNetworkObjListPtr nets);
+ const virNetworkObjListPtr nets,
+ const char *template);
int virNetworkSetBridgeName(virConnectPtr conn,
const virNetworkObjListPtr nets,
--
Soren Hansen |
Lead Virtualisation Engineer | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Supporting dynamic bridge names
On Fri, Apr 17, 2009 at 10:45:19AM +0100, Daniel P. Berrange wrote:
The problem with this approach is that the bridge name potentially
ends up being different every time the virtual network is started.
The bridge name needs to be stable,
Why? I can't remember ever having to refer to it, and if I did, I can
get that information at runtime by parsing the output from virsh
net-dumpxml name of the network. What am I not thinking of? :)
The non-QEMU drivers I'm afraid.
Ah, I see. I've not spent much time with those. Longer term, would it
perhaps make sense to let them specify the name of the network they want
to use like we do for the QEMU driver?
Index: libvirt-0.6.1/src/network_conf.c
===
--- libvirt-0.6.1.orig/src/network_conf.c 2009-04-16 20:49:28.851655724
+0200
+++ libvirt-0.6.1/src/network_conf.c2009-04-17 09:42:33.075084537
+0200
@@ -747,7 +747,7 @@
/* Generate a bridge if none is found, but don't check for collisions
* if a bridge is hardcoded, so the network is at least defined
*/
-if (!def-bridge !(def-bridge = virNetworkAllocateBridge(conn,
nets)))
+if (!(def-bridge = virNetworkAllocateBridge(conn, nets, def-bridge)))
goto error;
I think this would leak memory - the def-bridge string being passed in
is overwritten upon return, if its a non-null string.
Good catch.
Updated patch:
Index: libvirt-0.6.1/src/network_conf.c
===
--- libvirt-0.6.1.orig/src/network_conf.c 2009-04-16 20:49:28.851655724
+0200
+++ libvirt-0.6.1/src/network_conf.c2009-04-17 13:11:54.494770980 +0200
@@ -724,6 +724,7 @@
virNetworkDefPtr def = NULL;
virNetworkObjPtr net;
int autostart;
+char *tmp;
if ((configFile = virNetworkConfigFile(conn, configDir, name)) == NULL)
goto error;
@@ -747,7 +748,10 @@
/* Generate a bridge if none is found, but don't check for collisions
* if a bridge is hardcoded, so the network is at least defined
*/
-if (!def-bridge !(def-bridge = virNetworkAllocateBridge(conn, nets)))
+if (tmp = virNetworkAllocateBridge(conn, nets, def-bridge)) {
+VIR_FREE(def-bridge);
+def-bridge = tmp;
+} else
goto error;
if (!(net = virNetworkAssignDef(conn, nets, def)))
@@ -875,16 +879,20 @@
}
char *virNetworkAllocateBridge(virConnectPtr conn,
- const virNetworkObjListPtr nets)
+ const virNetworkObjListPtr nets,
+ const char *template)
{
int id = 0;
char *newname;
+ if (!template)
+ template = virbr%d;
+
do {
char try[50];
-snprintf(try, sizeof(try), virbr%d, id);
+snprintf(try, sizeof(try), template, id);
if (!virNetworkBridgeInUse(nets, try, NULL)) {
if (!(newname = strdup(try))) {
@@ -909,7 +917,7 @@
int ret = -1;
-if (def-bridge) {
+if (def-bridge !strstr(def-bridge, %d)) {
if (virNetworkBridgeInUse(nets, def-bridge, def-name)) {
networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_(bridge name '%s' already in use.),
@@ -918,7 +926,7 @@
}
} else {
/* Allocate a bridge name */
-if (!(def-bridge = virNetworkAllocateBridge(conn, nets)))
+if (!(def-bridge = virNetworkAllocateBridge(conn, nets, def-bridge)))
goto error;
}
Index: libvirt-0.6.1/src/network_conf.h
===
--- libvirt-0.6.1.orig/src/network_conf.h 2009-04-16 20:49:28.899655820
+0200
+++ libvirt-0.6.1/src/network_conf.h2009-04-17 09:42:33.075084537 +0200
@@ -174,7 +174,8 @@
const char *skipname);
char *virNetworkAllocateBridge(virConnectPtr conn,
- const virNetworkObjListPtr nets);
+ const virNetworkObjListPtr nets,
+ const char *template);
int virNetworkSetBridgeName(virConnectPtr conn,
const virNetworkObjListPtr nets,
--
Soren Hansen |
Lead Virtualisation Engineer | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Improve heuristic for default guest architecture
In libvirt 0.6.1, if you create a domain description of type 'kvm'
without an arch set on an x86-64 host, you would get an i686 qemu guest
rather than the expected x86-64 kvm guest.
This is because virCapabilitiesDefaultGuestArch doesn't take the domain
type into consideration, so it just returned the first hvm architecutre
that has been registered, which is i686.
After applying Dan P's patch,
http://www.redhat.com/archives/libvir-list/2009-March/msg00281.html,
I now get a i686 kvm guest, since kvm now can do i686 guests from
libvirt. This is certainly an improvement, but I think a more reasonable
default is to attempt to match the host's architecture.
This patch makes virCapabilitiesDefaultGuestArch also check the domain
type, and also gives preference to a guest architecture that matches the
host's architecture.
Index: libvirt-0.6.1/src/capabilities.c
===
--- libvirt-0.6.1.orig/src/capabilities.c 2009-03-19 15:18:09.483317579
+0100
+++ libvirt-0.6.1/src/capabilities.c2009-03-19 15:42:31.027341187 +0100
@@ -468,14 +468,26 @@
*/
extern const char *
virCapabilitiesDefaultGuestArch(virCapsPtr caps,
-const char *ostype)
+const char *ostype,
+const char *domain)
{
-int i;
+int i, j;
+const char *arch = NULL;
for (i = 0 ; i caps-nguests ; i++) {
-if (STREQ(caps-guests[i]-ostype, ostype))
-return caps-guests[i]-arch.name;
+if (STREQ(caps-guests[i]-ostype, ostype)) {
+for (j = 0 ; j caps-guests[i]-arch.ndomains ; j++) {
+if (STREQ(caps-guests[i]-arch.domains[j]-type, domain)) {
+/* Use the first match... */
+if (!arch)
+arch = caps-guests[i]-arch.name;
+/* ...unless we can match the host's architecture. */
+if (STREQ(caps-guests[i]-arch.name, caps-host.arch))
+return caps-guests[i]-arch.name;
+}
+}
+}
}
-return NULL;
+return arch;
}
/**
Index: libvirt-0.6.1/src/capabilities.h
===
--- libvirt-0.6.1.orig/src/capabilities.h 2009-03-19 15:18:09.507338228
+0100
+++ libvirt-0.6.1/src/capabilities.h2009-03-19 15:42:31.027341187 +0100
@@ -177,7 +177,8 @@
extern const char *
virCapabilitiesDefaultGuestArch(virCapsPtr caps,
-const char *ostype);
+const char *ostype,
+const char *domain);
extern const char *
virCapabilitiesDefaultGuestMachine(virCapsPtr caps,
const char *ostype,
Index: libvirt-0.6.1/src/domain_conf.c
===
--- libvirt-0.6.1.orig/src/domain_conf.c2009-03-19 15:18:09.531341976
+0100
+++ libvirt-0.6.1/src/domain_conf.c 2009-03-19 15:42:31.031345327 +0100
@@ -2146,7 +2146,7 @@
goto error;
}
} else {
-const char *defaultArch = virCapabilitiesDefaultGuestArch(caps,
def-os.type);
+const char *defaultArch = virCapabilitiesDefaultGuestArch(caps,
def-os.type, virDomainVirtTypeToString(def-virtType));
if (defaultArch == NULL) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
_(no supported architecture for os type
'%s'),
Index: libvirt-0.6.1/src/xm_internal.c
===
--- libvirt-0.6.1.orig/src/xm_internal.c2009-03-19 15:18:09.559316828
+0100
+++ libvirt-0.6.1/src/xm_internal.c 2009-03-19 15:42:45.807318313 +0100
@@ -695,7 +695,7 @@
if (!(def-os.type = strdup(hvm ? hvm : xen)))
goto no_memory;
-defaultArch = virCapabilitiesDefaultGuestArch(priv-caps, def-os.type);
+defaultArch = virCapabilitiesDefaultGuestArch(priv-caps, def-os.type,
virDomainVirtTypeToString(def-virtType));
if (defaultArch == NULL) {
xenXMError(conn, VIR_ERR_INTERNAL_ERROR,
_(no supported architecture for os type '%s'),
--
Soren Hansen |
Lead Virtualisation Engineer | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Pass -name argument to QEMU
On Thu, May 15, 2008 at 10:24:35PM +0100, Daniel P. Berrange wrote: Xenner doesn't start its help output right at the start of line, Ah. :/ and there isn't any other output there that causes ambiguity in current qemu help output: # qemu-kvm | grep -- -drive -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][index=i] If one happens to be added in the future, that's fine because -drive will be enabled anyway for future releases. I wasn't only worried about future releases, but also previous ones. I didn't want to have to check back through qemu's vcs for instances of -drive, so I coded defensively instead. :) What versions of QEmu does libvirt actually claim to support? If we only support recent versions, then we're probably fine. -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Pass -name argument to QEMU
On Tue, May 13, 2008 at 12:21:02AM +0100, Daniel P. Berrange wrote: -if (strstr(help, \n-drive)) -*flags |= QEMUD_CMD_FLAG_DRIVE_OPT; [...] +if (strstr(help, -drive)) +*flags |= QEMUD_CMD_FLAG_DRIVE; Why this change? I put the \n in front to prevent something like disk-drive in a descriptive text making libvirt think that -drive was a valid option. -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [Libvir] [PATCH] Add bus attribute to disk target definition
On Tue, May 06, 2008 at 10:02:21PM +0100, Daniel P. Berrange wrote: sorry for the delay. Here's the newest version of the patch which should address all the issues that have been raised so far. Yes no. It didn't address the redundant re-ordering of -drive parameters when using boot=on, The reasoning here was (I mentioned this in a previous mail, too) that when qemu/kvm some day grows the ability to have more than one boot device specified with boot=on (using extboot or whatever), we're going to have to change things *anyway*. Ordering the devices according to boot priority seems like a reasonable guess as to what would be required to do, so I figured I'd leave it as is. nor re-add the -boot param to make PXE work again. Yes and no. In the latest patch I provided I only set use_extboot if there's only one boot device defined, and it's a virtio device, so PXE booting would use the old-style -boot n syntax. I literallly woke up this morning and instantly smacked my forehead due to another problem this introduced, so I'm happy you changed it. :) One further complication is that QEMU 0.9.1 has -drive support but not the extboot support, so boot=on can't be used there. It rather annoyingly complains qemu: unknowm parameter 'boot' in 'file=/home/berrange/boot.iso,media=cdrom,boot=on' Ah, figures. +if (!bus) +disk-bus = QEMUD_DISK_BUS_IDE; This was giving floppy disks a bus of 'ide' which isn't technically correct - they're really their own bus type - I reckon we should call it 'fdc'. Ah. Yes, I must admit that floppy disks were completely off my radar. This double loop is redundant - there's no need to pull bootable drives to the front of the list - this is why there's an explicit flag rather than relying on ordering. I did this for two reasons: a) I wanted to avoid the bootDisk, bootFloppy, bootCD variables approach you used. It just didn't appeal to me. *shrug* b) As I mentioned further up, this was also done in an attempt to match what would be needed when it becomes possible to specify ,boot=on for more than one device, but we can revisit this when that day comes. Your patch looks fine to me. Oh, and thanks for doing all the test cases as well. I didn't want to get started on those until we had agreed on the logic that should be applied. -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] Add bus attribute to disk target definition
;
+break;
+}
+}
+
+/* Pick up the rest of the devices */
+j=0;
+while (disk) {
+if (!handledDisks[j]) {
+handledDisks[j] = 1;
+if (!((*argv)[++n] = strdup(-drive)))
+goto no_memory;
+if (!((*argv)[++n] = qemudDriveOpt(disk, 0)))
+goto no_memory;
+}
+disk = disk-next;
+j++;
+}
+} else {
+while (disk) {
+char dev[NAME_MAX];
+char file[PATH_MAX];
+
+if (!strcmp(disk-dst, hdc)
+disk-device == QEMUD_DISK_CDROM)
+ if (disk-src[0])
+ snprintf(dev, NAME_MAX, -%s, cdrom);
+ else {
+ disk = disk-next;
+ continue;
+ }
+else
+snprintf(dev, NAME_MAX, -%s, disk-dst);
+snprintf(file, PATH_MAX, %s, disk-src);
+
+if (!((*argv)[++n] = strdup(dev)))
+goto no_memory;
+if (!((*argv)[++n] = strdup(file)))
+goto no_memory;
+
+disk = disk-next;
+}
}
if (!net) {
@@ -3587,6 +3737,7 @@
virBufferVSprintf(buf, source %s='%s'/\n,
typeAttrs[disk-type], disk-src);
+virBufferVSprintf(buf, target dev='%s' bus='%s'/\n,
disk-dst, qemudBusIdToName(disk-bus));
virBufferVSprintf(buf, target dev='%s'/\n, disk-dst);
if (disk-readonly)
=== modified file 'src/qemu_conf.h'
--- old/src/qemu_conf.h 2008-04-30 12:30:55 +
+++ new/src/qemu_conf.h 2008-05-06 17:52:17 +
@@ -56,10 +56,17 @@
QEMUD_DISK_FLOPPY,
};
+enum qemud_vm_disk_bus {
+QEMUD_DISK_BUS_IDE,
+QEMUD_DISK_BUS_SCSI,
+QEMUD_DISK_BUS_VIRTIO
+};
+
/* Stores the virtual disk configuration */
struct qemud_vm_disk_def {
int type;
int device;
+int bus;
char src[PATH_MAX];
char dst[NAME_MAX];
int readonly;
@@ -223,6 +230,7 @@
QEMUD_CMD_FLAG_KQEMU = 1,
QEMUD_CMD_FLAG_VNC_COLON = 2,
QEMUD_CMD_FLAG_NO_REBOOT = 4,
+QEMUD_CMD_FLAG_DRIVE_OPT = 8,
};
=== modified file 'src/util.c'
--- old/src/util.c 2008-04-25 14:53:05 +
+++ new/src/util.c 2008-05-06 17:52:17 +
@@ -771,3 +771,44 @@
return -1;
}
+
+/* Translates a device name of the form (regex) [fhv]d[a-z]+ into
+ * the corresponding index (e.g. sda = 1, hdz = 26, vdaa = 27)
+ * @param name The name of the device
+ * @return name's index, or 0 on failure
+ */
+int virDiskNameToIndex(const char *name) {
+const char *ptr = NULL;
+int idx = 0;
+
+if (strlen(name) 3)
+return 0;
+
+switch (*name) {
+case 'f':
+case 'h':
+case 'v':
+case 's':
+break;
+default:
+return 0;
+}
+
+if (*(name + 1) != 'd')
+return 0;
+
+ptr = name+2;
+
+while (*ptr) {
+idx = idx * 26;
+
+if ('a' *ptr || 'z' *ptr)
+return 0;
+
+idx += *ptr - 'a' + 1;
+ptr++;
+}
+
+return idx;
+}
+
=== modified file 'src/util.h'
--- old/src/util.h 2008-04-25 14:53:05 +
+++ new/src/util.h 2008-05-06 17:52:18 +
@@ -92,4 +92,6 @@
int virParseMacAddr(const char* str, unsigned char *addr);
+int virDiskNameToIndex(const char* str);
+
#endif /* __VIR_UTIL_H__ */
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [Libvir] make syntax-check fails with bzr checkouts
On Wed, Apr 30, 2008 at 12:19:51PM +0200, Jim Meyering wrote: Unfortunately, the above no longer applies, due to upstream (gnulib) changes to deal with non-srcdir (aka VPATH) builds. I updated libvirt from gnulib just yesterday, and will again, later today. Here's the new patch that seems to do the trick: === modified file 'build-aux/vc-list-files' --- old/build-aux/vc-list-files 2008-04-30 16:11:08 + +++ new/build-aux/vc-list-files 2008-05-06 12:25:50 + @@ -75,6 +75,9 @@ eval exec git ls-files '$dir' $postprocess elif test -d .hg; then eval exec hg locate '$dir/*' $postprocess +elif test -d .bzr; then + test $postprocess = '' postprocess=| sed 's|^\./||' + eval exec bzr ls --versioned '$dir' $postprocess elif test -d CVS; then test $postprocess = '' postprocess=| sed 's|^\./||' if test -x build-aux/cvsu; then -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] make syntax-check fails with bzr checkouts
The bzr problem was obviously that vc-list-files didn't support bzr.
Failing with CVS was caused by a bashism in vc-list-files. In Ubuntu,
/bin/sh points to dash instead of bash, but vc-list-files had
#!/bin/sh.
This patch fixes both issues:
=== modified file 'build-aux/vc-list-files'
--- build-aux/vc-list-files 2008-02-01 19:47:07 +
+++ build-aux/vc-list-files 2008-04-30 07:43:06 +
@@ -39,6 +39,8 @@
exec git ls-files $dir
elif test -d .hg; then
exec hg locate $dir/*
+elif test -d .bzr; then
+ exec bzr ls --versioned --kind=file ${*:---from-root}
elif test -d CVS; then
if test -x build-aux/cvsu; then
build-aux/cvsu --find --types=AFGM $dir
@@ -49,7 +51,7 @@
sub(/CVS\/Entries/, , f); \
print f $2; \
}}' \
- $(find ${*-*} -name Entries -print) /dev/null;
+ $(find ${*:-*} -name Entries -print) /dev/null;
fi
else
echo $0: Failed to determine type of version control used in `pwd` 12
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] make syntax-check fails with bzr checkouts
On Wed, Apr 30, 2008 at 12:19:51PM +0200, Jim Meyering wrote:
Unfortunately, the above no longer applies, due to upstream (gnulib)
changes to deal with non-srcdir (aka VPATH) builds. I updated libvirt
from gnulib just yesterday, and will again, later today.
Can you adapt your patch to make bzr work with the newer version?
I'll do it when you're done updating gnulib.
}}' \
- $(find ${*-*} -name Entries -print) /dev/null;
+ $(find ${*:-*} -name Entries -print) /dev/null;
Thanks for reporting that.
Note though that POSIX appears to require the behavior that bash exhibits,
so calling this a bashism doesn't seem right.
No. POSIX requires that ${*-*} should expand to * if $* is unset (and
not if it's null). $* is defined to expand to the positional parameters
starting from 1. The spec is not *entirely* clear, I'll give you that,
but considering $* to be unset (instead of just null) messes with my
sanity. At any rate, relying on bash's interpretation of the spec is a
text book example of a bashism, if you ask me.
but there is no such exemption for the ${parameter-word} syntax. All
that to say that this looks more like an infelicity in dash, and since
Ubuntu relies on it, you may want to investigate further.
Well, if anyone insists on not changing something that only works with
bash to something that works with any shell (${*:-*} vs. ${*-*}) , but
also refuses to put /bin/bash explicitly as the interpreter is just
being pointlessly difficult, IMO.
However, back to vc-list-files, that awk-based code is so hard to
reach for me -- I avoid CVS, and when I do use it, I have cvsu in my
path -- that it is rarely tested.
Anyhow the use of * there is unnecessary, and in addition,
there was a subtle (but probably never problematic) quoting bug.
Here's the patch I've pushed to gnulib:
http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=173a9f0c48a16c3507f8
That patch will make the cvs case fail, too. It will prepend a ./ to all
path names which will screw with the comparison, AFAICS.
If you care about the CVS-oriented cases, there are two more patches.
Not really. :) It just popped up when I was trying to determine why my
bzr case was failing.
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] PATCH: Support network interface model in Xen and QEMU driver
On Wed, Apr 30, 2008 at 12:43:06PM +0100, Daniel P. Berrange wrote: And Ubuntu have already shipped a product with a patch using this syntax applied, so we can't reasonably change it. Ironically, I'm with Daniel Veillard on this one. Sure, it'd be nice if it was factored into the decision to some extent, but I'd be sad if I have somehow short-circuited the development process by forcing this decision onto the rest of you guys. I clearly read too much into the fact that Richard had posted a patch that used this syntax and noone objected. My bad entirely, and I'll deal with the mess it causes. Somehow. :) -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] PATCH: Support network interface model in Xen and QEMU driver
On Wed, Apr 30, 2008 at 11:36:33PM +0100, Daniel P. Berrange wrote: WRT to the network interface type attribute, I advised Soren at the virt summit in Austin, that since Rich Jones had already posted the patch and we'd all basically agreed on syntax it was reasonably to include the patch in Ubuntu. It was only a matter of time before we merged it - as I have done today. Thanks very much. That strikes that bit off of my Stuff I might need to worry about list. :) Now, the disk model syntax supporting virtio is where I agree with Daniel that it should have been posted upstream before inclusion in a product Even if the code was just a quick hack, not in a state fit for merging - it is always beneficial to post as early as possible just for the sake of visibility comment. This is good advice. Thanks. This said I believe the proposed 'bus' atribute for disks is the optimal way to handle virtio for disks. I agree. A model type='foo' / element in the disk definition could still be used to specify which particular SCSI controller you'd like. Just for future enhancements please post ideas to this list asap. I'll keep that in mind. I'm truly sorry for the stir I've caused and I have every intention of making sure it won't happen again. I myself have posted ideas more than 1 year before actually getting around to implementing them, so there's no requirement to follow through with code immediately :-) :) -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[Libvir] [PATCH] Ancient libparted
Some of us are stuck with an ancient libparted, which doesn't know about
PED_PARTITION_PROTECTED. This patch allows us to compile libvirt.
=== modified file 'src/parthelper.c'
--- src/parthelper.c2008-04-10 16:53:29 +
+++ src/parthelper.c2008-04-29 07:47:08 +
@@ -67,8 +67,10 @@
content = free;
else if (part-type PED_PARTITION_METADATA)
content = metadata;
+#ifdef PED_PARTITION_PROTECTED
else if (part-type PED_PARTITION_PROTECTED)
content = protected;
+#endif
else
content = data;
} else if (part-type == PED_PARTITION_EXTENDED) {
@@ -80,8 +82,10 @@
content = free;
else if (part-type PED_PARTITION_METADATA)
content = metadata;
+#ifdef PED_PARTITION_PROTECTED
else if (part-type PED_PARTITION_PROTECTED)
content = protected;
+#endif
else
content = data;
}
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[Libvir] [PATCH] Add bus attribute to disk target definition
{
+ disk = disk-next;
+ continue;
+ }
+else
+snprintf(dev, NAME_MAX, -%s, disk-dst);
+snprintf(file, PATH_MAX, %s, disk-src);
+
+if (!((*argv)[++n] = strdup(dev)))
+goto no_memory;
+if (!((*argv)[++n] = strdup(file)))
+goto no_memory;
+
+disk = disk-next;
+}
}
if (!net) {
@@ -3565,6 +3701,7 @@
virBufferVSprintf(buf, source %s='%s'/\n,
typeAttrs[disk-type], disk-src);
+virBufferVSprintf(buf, target dev='%s' bus='%s'/\n,
disk-dst, qemudBusIdToName(disk-bus));
virBufferVSprintf(buf, target dev='%s'/\n, disk-dst);
if (disk-readonly)
=== modified file 'src/qemu_conf.h'
--- src/qemu_conf.h 2008-04-25 20:46:13 +
+++ src/qemu_conf.h 2008-04-29 07:13:16 +
@@ -56,10 +56,17 @@
QEMUD_DISK_FLOPPY,
};
+enum qemud_vm_disk_bus {
+QEMUD_DISK_BUS_IDE,
+QEMUD_DISK_BUS_SCSI,
+QEMUD_DISK_BUS_VIRTIO
+};
+
/* Stores the virtual disk configuration */
struct qemud_vm_disk_def {
int type;
int device;
+int bus;
char src[PATH_MAX];
char dst[NAME_MAX];
int readonly;
=== modified file 'src/util.c'
--- src/util.c 2008-04-25 14:53:05 +
+++ src/util.c 2008-04-29 06:59:49 +
@@ -771,3 +771,43 @@
return -1;
}
+
+/* Translates a device name of the form (regex) [fhv]d[a-z]+ into
+ * the corresponding index (e.g. sda = 1, hdz = 26, vdaa = 27)
+ * @param name The name of the device
+ * @return name's index, or 0 on failure
+ */
+int virDiskNameToIndex(const char *name) {
+const char *ptr = NULL;
+int idx = 0;
+
+if (strlen(name) 3)
+return 0;
+
+switch (*name) {
+case 'f':
+case 'h':
+case 'v':
+break;
+default:
+return 0;
+}
+
+if (*(name + 1) != 'd')
+return 0;
+
+ptr = name+2;
+
+while (*ptr) {
+idx = idx * 26;
+
+if ('a' *ptr || 'z' *ptr)
+return 0;
+
+idx += *ptr - 'a' + 1;
+ptr++;
+}
+
+return idx;
+}
+
=== modified file 'src/util.h'
--- src/util.h 2008-04-25 14:53:05 +
+++ src/util.h 2008-04-29 06:59:57 +
@@ -92,4 +92,6 @@
int virParseMacAddr(const char* str, unsigned char *addr);
+int virDiskNameToIndex(const char* str);
+
#endif /* __VIR_UTIL_H__ */
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] Allow selection of the NIC model in QEMU/KVM
;
unsigned char mac[QEMUD_MAC_ADDRESS_LEN];
+char model[QEMUD_MODEL_MAX_LEN];
union {
struct {
char ifname[BR_IFNAME_MAXLEN];
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] Add bus attribute to disk target definition
On Tue, Apr 29, 2008 at 02:27:00PM +0100, Daniel P. Berrange wrote:
+strcmp((const char *)target, hdd)
+strcmp((const char *)target, hdd)
These two lines test the same thing !
Quite so. My bad.
+strncmp((const char *)target, vd, 2)) {
Its probably a better idea to just compress the explicit hda - hdd
comparisons down into a single 'hd' prefix comparison, as you did
with 'vd'.
Hm.. Yes, I suppose that if you're using -drive notation, hd[e-z]
starts making sense. Fixed.
+if (!bus)
+disk-bus = QEMUD_DISK_BUS_IDE;
+else if (!strcmp((const char *)bus, ide))
+disk-bus = QEMUD_DISK_BUS_IDE;
+else if (!strcmp((const char *)bus, scsi))
+disk-bus = QEMUD_DISK_BUS_SCSI;
+else if (!strcmp((const char *)bus, virtio))
+disk-bus = QEMUD_DISK_BUS_VIRTIO;
Can you use the STREQ macro here instead of strcmp.
Erm... I *could*.. I'm curious, though, why e.g. the similar code right
above it doesn't use STREQ if that's the preferred way to do it?
IMO, it's better to change this sort of things all in one go, and keep
everything consistent until then.
+else {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, Invalid
bus type: %s, bus);
+goto error;
+}
This line needs to be marked for translation, with _(...).
Fixed.
You can run 'make syntax-check' for check for such issues.
Yes, in theory :) In the real world, however, make syntax-check fails
horribly here. I'll be fixing that up next.
+static char *qemudAddBootDrive(virConnectPtr conn,
+struct qemud_vm_def *def,
+ char *handledDisks,
+ int type) {
+int j = 0;
+struct qemud_vm_disk_def *disk = def-disks;
+
+while (disk) {
+if (!handledDisks[j] disk-device == type) {
+handledDisks[j] = 1;
+return qemudDriveOpt(disk, 1);
+}
+j++;
+disk = disk-next;
+}
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ Requested boot device type %d, but no such
device defined., type);
+return 0;
+}
@@ -2207,30 +2295,32 @@
goto no_memory;
}
-for (i = 0 ; i vm-def-os.nBootDevs ; i++) {
-switch (vm-def-os.bootDevs[i]) {
-case QEMUD_BOOT_CDROM:
-boot[i] = 'd';
-break;
-case QEMUD_BOOT_FLOPPY:
-boot[i] = 'a';
-break;
-case QEMUD_BOOT_DISK:
-boot[i] = 'c';
-break;
-case QEMUD_BOOT_NET:
-boot[i] = 'n';
-break;
-default:
-boot[i] = 'c';
-break;
+if (vm-def-virtType != QEMUD_VIRT_KVM) {
This is wrong - both QEMU and KVM can support the -drive parameter,
Oh, I wasn't aware. Hmm..
and not all KVM support it. You need to add a check in the method
qemudExtractVersionInfo() to probe for availability of the -drive
option, as we do with other opts - see QEMUD_CMD_FLAG_NO_REBOOT for
example.
Interesting. I'll add that.
Even if the -drive parameter is supported, it should still pass the
-boot a/c/d/n parameter in.
Why? And how would you boot from a virtio device this way?
There is nothing in the -drive parameter handling, AFAICT, that
requires the boot drive to be listed first on the command line. So
this first loop is not needed, and this second loop is sufficient,
simply turn on the boot= flag on the first match drive type when
iterating over the list.
If you want to specify more than one boot device, the only way to
determine the priority is by specifying them ordered by priority. At
least, that's how it *ought* to be, but now I see that extboot only
actually supports one boot device. :/ I could have sworn I had seen it
work with both hard drive and cdrom.
+int virDiskNameToIndex(const char *name) {
+const char *ptr = NULL;
+int idx = 0;
+
+if (strlen(name) 3)
+return 0;
+
+switch (*name) {
+case 'f':
+case 'h':
+case 'v':
You need a case 's' there to allow for SCSI...
Good point. I suppose I do so in qemudParseDiskXml as well (when
checking the target).
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] Add bus attribute to disk target definition
On Tue, Apr 29, 2008 at 03:17:14PM +0100, Daniel P. Berrange wrote:
+if (!bus)
+disk-bus = QEMUD_DISK_BUS_IDE;
+else if (!strcmp((const char *)bus, ide))
+disk-bus = QEMUD_DISK_BUS_IDE;
+else if (!strcmp((const char *)bus, scsi))
+disk-bus = QEMUD_DISK_BUS_SCSI;
+else if (!strcmp((const char *)bus, virtio))
+disk-bus = QEMUD_DISK_BUS_VIRTIO;
Can you use the STREQ macro here instead of strcmp.
Erm... I *could*.. I'm curious, though, why e.g. the similar code right
above it doesn't use STREQ if that's the preferred way to do it?
We've been slowly updating code to match these new standards when doing
patches.
Well, if that's the way you do it, I'll follow suit.. However, I have
to say that I pity the person that reads the code and finds these two
sections of code that seem to do rather similar things, but use
different functions to do it, and then has to work out what on earth the
difference between the two might be.
You can run 'make syntax-check' for check for such issues.
Yes, in theory :) In the real world, however, make syntax-check
fails horribly here. I'll be fixing that up next.
If you send details to the list, Jim will no doubt be able to point
you in the right direction on this...
I'll do that in a minute. Thanks.
Even if the -drive parameter is supported, it should still pass the
-boot a/c/d/n parameter in.
Why? And how would you boot from a virtio device this way?
It is needed for PXE boot at least, and IMHO,
Good point.
QEMU should treat 'boot c' as if 'boot=on' were set for the first
-drive parameter for back compat.
Yes, indeed it should. Sadly, though, it doesn't.
kvm -drive file=root.qcow,if=virtio -boot c
fails miserably, while
kvm -drive file=root.qcow,if=virtio,boot=on
works beautifully.
This logic is going to look horrible :( Something like:
if boot == hd (one of the disks is a virtio disk)
then
use new style -drive foo,boot=on notation
else
use old style -boot [acdn] notation
?
There is nothing in the -drive parameter handling, AFAICT, that
requires the boot drive to be listed first on the command line. So
this first loop is not needed, and this second loop is sufficient,
simply turn on the boot= flag on the first match drive type when
iterating over the list.
If you want to specify more than one boot device, the only way to
determine the priority is by specifying them ordered by priority. At
least, that's how it *ought* to be, but now I see that extboot only
actually supports one boot device. :/ I could have sworn I had seen
it work with both hard drive and cdrom.
The QEMU code only allows a single boot device will abort if 1 has
it
if (extboot_drive != -1) {
fprintf(stderr, qemu: two bootable drives specified\n);
return -1;
}
Yes, that's what I noticed earlier today, although I geniunely hope this
is something that will be fixed at some point, and when that happy day
comes, I've either guessed correctly as to how it will derive boot
priorities and we won't have to fix anything, or I've guessed wrong, and
then we'll be no worse off than if we didn't adopt this approach right
now, AFAICS.
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[Libvir] make syntax-check fails with bzr checkouts
I seem to be completely unable to get make syntax-checks to function
properly with my bzr checkout of libvirt[1]. I've attached the output as
as-is.txt. I tried adding hacking bzr support into vc-list-files (see
vc-list-files-bzr.patch), but that didn't quite seem to do the trick, as
you can see in in vc-list-files-maybe-fixed.log, which is the output
after I patched vc-list-files.
I tried CVS, too, and that also fails (see cvs-syntax-check.log).
Is it only meant to work with git?
[1]: http://bazaar.launchpad.net/~vcs-imports/libvirt/trunk
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
if test -f po/POTFILES.in; then \
grep -E -v '^(#|$)' po/POTFILES.in\
| grep -v '^src/false\.c$' | sort po-check-1;
\
files=; \
for file in $(build-aux/vc-list-files | if test -f .x-po-check; then
grep -vEf .x-po-check; else grep -v ChangeLog; fi); do
\
echo $file; \
case $file in \
djgpp/* | man/*) continue;; \
*/c99-to-c89.diff) continue;; \
esac; \
case $file in \
*.[ch]) \
base=`expr $file : ' \(.*\)\..'`; \
{ test -f $base.l || test -f $base.y; } continue;; \
*) continue;; \
esac; \
files=$files $file; \
done; \
grep -E -l '\b(N?_|gettext *)\([^)]*(|$)' $files\
| sort -u po-check-2;
\
diff -u po-check-1 po-check-2 || exit 1;
\
rm -f po-check-1 po-check-2;
\
fi
build-aux/vc-list-files: Failed to determine type of version control used in
/home/soren/src/projects/Virtualisation/libvirt/disk-bus
make: *** [po-check] Interrupt
=== modified file 'build-aux/vc-list-files'
--- build-aux/vc-list-files 2008-02-01 19:47:07 +
+++ build-aux/vc-list-files 2008-04-29 14:59:20 +
@@ -39,6 +39,13 @@
exec git ls-files $dir
elif test -d .hg; then
exec hg locate $dir/*
+elif test -d .bzr; then
+ if [ $dir = . ]
+ then
+exec bzr ls
+ else
+exec bzr ls $dir
+ fi
elif test -d CVS; then
if test -x build-aux/cvsu; then
build-aux/cvsu --find --types=AFGM $dir
if test -f po/POTFILES.in; then \
grep -E -v '^(#|$)' po/POTFILES.in\
| grep -v '^src/false\.c$' | sort po-check-1;
\
files=; \
for file in $(build-aux/vc-list-files | if test -f .x-po-check; then
grep -vEf .x-po-check; else grep -v ChangeLog; fi); do
\
echo $file; \
case $file in \
djgpp/* | man/*) continue;; \
*/c99-to-c89.diff) continue;; \
esac; \
case $file in \
*.[ch]) \
base=`expr $file : ' \(.*\)\..'`; \
{ test -f $base.l || test -f $base.y; } continue;; \
*) continue;; \
esac; \
files=$files $file; \
done; \
grep -E -l '\b(N?_|gettext *)\([^)]*(|$)' $files\
| sort -u po-check-2;
\
diff -u po-check-1 po-check-2 || exit 1;
\
rm -f po-check-1 po-check-2;
\
fi
.cvsignore
.shelf
.x-sc_avoid_if_before_free
.x-sc_avoid_write
.x-sc_no_have_config_h
.x-sc_require_config_h
.x-sc_trailing_blank
ABOUT-NLS
AUTHORS
COPYING
COPYING.LIB
GNUmakefile
HACKING
INSTALL
Makefile
Makefile.am
Makefile.cfg
Makefile.in
Makefile.maint
NEWS
README
RENAMES
TODO
acinclude.m4
aclocal.m4
Re: [Libvir] Patch for routed virtual networks
On Fri, Mar 28, 2008 at 08:40:19PM +, Daniel P. Berrange wrote: In theory I think it might be possible to avoid the need to configure the local LAN router, by messing with proxy_arp, but I've not got it to work so far. Unless I've misunderstood something, this won't work. If a machine on the LAN tries to reach one of the virtual machines (which is on a different subnet) it won't send out an ARP request for the IP in question, but just stick the MAC address of the the gateway onto the packet and let the gateway deal with it. AIUI proxy ARP is for when you have a subnet that's physically separated in which case the bridge will need to proxy the arp requests for hosts on the one side to hosts on the other side (and vice versa, obviously). -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] Keyboard layout support for QEMU/KVM
On Mon, Jan 14, 2008 at 07:09:23PM +0100, Daniel Hokka Zakrisson wrote: This patch adds support for qemu's -k option, which is required to run virt-manager with a non-US keymap. Without it, keys are remapped a number of times, ending up with a completely unusable layout. Even with this patch I am having problems using the AltGr key, returning a scancode of 00. This seems to be a qemu problem though. Note that a different solution to the same problem has been proposed by Anthony Liguori. http://sourceforge.net/mailarchive/forum.php?thread_name=478A6BFA.80009%40codemonkey.wsforum_name=gtk-vnc-devel -- Soren Hansen Ubuntu Server Team http://www.ubuntu.com/ signature.asc Description: Digital signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
