4:42 a.m.
> I came up with the attached patch to fix this for me, but
> a) I'm not sure if it is a clean use of the api to call virConnectOpen()
> from within a state-initializer function
> b) This is just for qemu/kvm, I haven't looked at any other drivers
>
> It would be nice if an experienced libvirt-developer could take a look at
> this. Thank you very much.
Thanks for the bug report & patch.
Did this patch actually make it work for you ? AFAICT, there's a problem
even earlier, which is that we are doing autostart of the virtual machines,
before autostart of the networks and storage pools, so I'm not sure that
this patch is sufficient.
This patch alone not, but this patch + the one in my first mail
(see https://www.redhat.com/archives/libvir-list/2008-November/msg00457.html)
together make it work for me. The first patch fixes the autostart order, the
second one adds the necessary conn structure.
Kind regards,
Gerd
--
Address (better: trap) for people I really don't want to get mail from:
james(a)cactusamerica.com
4:54 a.m.
New subject: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
On Mon, Dec 01, 2008 at 11:42:28AM +0100, Gerd v. Egidy wrote:
> > I came up with the attached patch to fix this for me, but
> > a) I'm not sure if it is a clean use of the api to call virConnectOpen()
> > from within a state-initializer function
> > b) This is just for qemu/kvm, I haven't looked at any other drivers
> >
> > It would be nice if an experienced libvirt-developer could take a look at
> > this. Thank you very much.
>
> Thanks for the bug report & patch.
>
> Did this patch actually make it work for you ? AFAICT, there's a problem
> even earlier, which is that we are doing autostart of the virtual machines,
> before autostart of the networks and storage pools, so I'm not sure that
> this patch is sufficient.
This patch alone not, but this patch + the one in my first mail
(see https://www.redhat.com/archives/libvir-list/2008-November/msg00457.html)
together make it work for me. The first patch fixes the autostart order, the
second one adds the necessary conn structure.
Oh yes, I totally missed the patch in your first mail. The first patch
is definitely correct and I'll apply that shortly. While your second
patch is also functionally OK, I'm not entirely happy with creating a
connection object deep inside the QEMU driver code. So I'm going to
think about whether there's a better way todo that bit.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
5:23 a.m.
New subject: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
Hi Daniel,
> This patch alone not, but this patch + the one in my first mail
> (see
> https://www.redhat.com/archives/libvir-list/2008-November/msg00457.html)
> together make it work for me. The first patch fixes the autostart order,
> the second one adds the necessary conn structure.
Oh yes, I totally missed the patch in your first mail. The first patch
is definitely correct and I'll apply that shortly.
I just added some documentation (same as in virInitialize) to make sure this
bug does not get introduced again. New version attached.
While your second
patch is also functionally OK, I'm not entirely happy with creating a
connection object deep inside the QEMU driver code.
Yeah, that was exactly my thought too.
So I'm going to
think about whether there's a better way todo that bit.
I looked through the rest of the qemu-initialization and it looks like this is
the only point where the conn-object is needed. So a solution would be to
directly access the network driver functions. But on the other hand these
functions seem all to imply that there is a valid conn available. So we would
have to change that, at least for the lookup-by-name case.
But I'm feeling too new in libvirt to actually do design decisions, so it's
your call...
Kind regards,
Gerd
diff -r -u libvirt-0.5.0.orig/qemud/qemud.c libvirt-0.5.0/qemud/qemud.c
--- libvirt-0.5.0.orig/qemud/qemud.c 2008-11-21 13:47:32.000000000 +0100
+++ libvirt-0.5.0/qemud/qemud.c 2008-12-01 12:21:37.000000000 +0100
@@ -755,28 +755,23 @@
virInitialize();
+ /*
+ * Note that the order is important: the first ones have a higher
+ * priority when calling virStateInitialize.
+ */
#ifdef WITH_DRIVER_MODULES
/* We don't care if any of these fail, because the whole point
* is to allow users to only install modules they want to use.
* If they try to use a open a connection for a module that
* is not loaded they'll get a suitable error at that point
*/
- virDriverLoadModule("qemu");
- virDriverLoadModule("lxc");
- virDriverLoadModule("uml");
virDriverLoadModule("network");
virDriverLoadModule("storage");
virDriverLoadModule("nodedev");
+ virDriverLoadModule("qemu");
+ virDriverLoadModule("lxc");
+ virDriverLoadModule("uml");
#else
-#ifdef WITH_QEMU
- qemuRegister();
-#endif
-#ifdef WITH_LXC
- lxcRegister();
-#endif
-#ifdef WITH_UML
- umlRegister();
-#endif
#ifdef WITH_NETWORK
networkRegister();
#endif
@@ -786,6 +781,15 @@
#if defined(HAVE_HAL) || defined(HAVE_DEVKIT)
nodedevRegister();
#endif
+#ifdef WITH_QEMU
+ qemuRegister();
+#endif
+#ifdef WITH_LXC
+ lxcRegister();
+#endif
+#ifdef WITH_UML
+ umlRegister();
+#endif
#endif
virEventRegisterImpl(virEventAddHandleImpl,
--
Address (better: trap) for people I really don't want to get mail from:
james(a)cactusamerica.com
5:38 a.m.
New subject: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
On Mon, Dec 01, 2008 at 12:23:12PM +0100, Gerd v. Egidy wrote:
Hi Daniel,
> > This patch alone not, but this patch + the one in my first mail
> > (see
> > https://www.redhat.com/archives/libvir-list/2008-November/msg00457.html)
> > together make it work for me. The first patch fixes the autostart order,
> > the second one adds the necessary conn structure.
>
> Oh yes, I totally missed the patch in your first mail. The first patch
> is definitely correct and I'll apply that shortly.
I just added some documentation (same as in virInitialize) to make sure this
bug does not get introduced again. New version attached.
> While your second
> patch is also functionally OK, I'm not entirely happy with creating a
> connection object deep inside the QEMU driver code.
Yeah, that was exactly my thought too.
> So I'm going to
> think about whether there's a better way todo that bit.
I looked through the rest of the qemu-initialization and it looks like this is
the only point where the conn-object is needed. So a solution would be to
directly access the network driver functions. But on the other hand these
functions seem all to imply that there is a valid conn available. So we would
have to change that, at least for the lookup-by-name case.
I've committed this patch now. I'll send a suggestion for the second patch
shortly...
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
5:54 a.m.
New subject: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
On Tue, Dec 02, 2008 at 11:38:36AM +0000, Daniel P. Berrange wrote:
On Mon, Dec 01, 2008 at 12:23:12PM +0100, Gerd v. Egidy wrote:
> Hi Daniel,
>
> > > This patch alone not, but this patch + the one in my first mail
> > > (see
> > > https://www.redhat.com/archives/libvir-list/2008-November/msg00457.html)
> > > together make it work for me. The first patch fixes the autostart order,
> > > the second one adds the necessary conn structure.
> >
> > Oh yes, I totally missed the patch in your first mail. The first patch
> > is definitely correct and I'll apply that shortly.
>
> I just added some documentation (same as in virInitialize) to make sure this
> bug does not get introduced again. New version attached.
>
> > While your second
> > patch is also functionally OK, I'm not entirely happy with creating a
> > connection object deep inside the QEMU driver code.
>
> Yeah, that was exactly my thought too.
>
> > So I'm going to
> > think about whether there's a better way todo that bit.
>
> I looked through the rest of the qemu-initialization and it looks like this is
> the only point where the conn-object is needed. So a solution would be to
> directly access the network driver functions. But on the other hand these
> functions seem all to imply that there is a valid conn available. So we would
> have to change that, at least for the lookup-by-name case.
I've committed this patch now. I'll send a suggestion for the second patch
shortly...
Ok, I think your suggested patch is the best we can manage at this point
in time. Here's an update which also handle it for UML driver, and adds
a check for NULL on connection open.
Daniel
Index: src/qemu_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.162
diff -u -p -r1.162 qemu_driver.c
--- src/qemu_driver.c 28 Nov 2008 12:03:20 -0000 1.162
+++ src/qemu_driver.c 2 Dec 2008 11:52:28 -0000
@@ -138,12 +138,26 @@ static struct qemud_driver *qemu_driver
static void
qemudAutostartConfigs(struct qemud_driver *driver) {
unsigned int i;
+ /* XXX: Figure out a better way todo this. The domain
+ * startup code needs a connection handle in order
+ * to lookup the bridge associated with a virtual
+ * network
+ */
+ virConnectPtr conn = virConnectOpen(getuid() ?
+ "qemu:///session" :
+ "qemu:///system");
+
+ if (!conn) {
+ qemudLog(QEMUD_ERR, "%s",
+ _("Cannot autostart domains, failed to open connection"));
+ return;
+ }
for (i = 0 ; i < driver->domains.count ; i++) {
virDomainObjPtr vm = driver->domains.objs[i];
if (vm->autostart &&
!virDomainIsActive(vm)) {
- int ret = qemudStartVMDaemon(NULL, driver, vm, NULL);
+ int ret = qemudStartVMDaemon(conn, driver, vm, NULL);
if (ret < 0) {
virErrorPtr err = virGetLastError();
qemudLog(QEMUD_ERR, _("Failed to autostart VM '%s':
%s\n"),
@@ -155,6 +169,8 @@ qemudAutostartConfigs(struct qemud_drive
}
}
}
+
+ virConnectClose(conn);
}
/**
Index: src/uml_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/uml_driver.c,v
retrieving revision 1.6
diff -u -p -r1.6 uml_driver.c
--- src/uml_driver.c 2 Dec 2008 11:23:27 -0000 1.6
+++ src/uml_driver.c 2 Dec 2008 11:52:28 -0000
@@ -117,16 +117,32 @@ static struct uml_driver *uml_driver = N
static void
umlAutostartConfigs(struct uml_driver *driver) {
unsigned int i;
+ /* XXX: Figure out a better way todo this. The domain
+ * startup code needs a connection handle in order
+ * to lookup the bridge associated with a virtual
+ * network
+ */
+ virConnectPtr conn = virConnectOpen(getuid() ?
+ "uml:///session" :
+ "uml:///system");
+
+ if (!conn) {
+ umlLog(UML_ERR, "%s",
+ _("Cannot autostart domains, failed to open connection"));
+ return;
+ }
for (i = 0 ; i < driver->domains.count ; i++) {
if (driver->domains.objs[i]->autostart &&
!virDomainIsActive(driver->domains.objs[i]) &&
- umlStartVMDaemon(NULL, driver, driver->domains.objs[i]) < 0) {
+ umlStartVMDaemon(conn, driver, driver->domains.objs[i]) < 0) {
virErrorPtr err = virGetLastError();
umlLog(UML_ERR, _("Failed to autostart VM '%s': %s\n"),
driver->domains.objs[i]->def->name, err->message);
}
}
+
+ virConnectClose(conn);
}
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
6:44 a.m.
New subject: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
Hi Daniel,
thanks for looking into this.
+ virConnectPtr conn = virConnectOpen(getuid() ?
+ "qemu:///session" :
+ "qemu:///system");
+
+ if (!conn) {
+ qemudLog(QEMUD_ERR, "%s",
+ _("Cannot autostart domains, failed to open
connection")); + return;
+ }
I thought about that check too, but decided against it, because
a) currently we always call it with conn=NULL, it only fails if you don't use
a directly bridged network
b) it currently won't do any harm calling with conn=NULL, creating the vm
fails later on because it can't find the network device.
But if some code relying on the conn-structure is added later on without
sanity-checks, this check will make sure no bigger harm is done.
Kind regards,
Gerd
--
Address (better: trap) for people I really don't want to get mail from:
james(a)cactusamerica.com
7 a.m.
New subject: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
Hi Daniel,
> I've committed this patch now. I'll send a suggestion
for the second
> patch shortly...
Ok, I think your suggested patch is the best we can manage at this point
in time. Here's an update which also handle it for UML driver, and adds
a check for NULL on connection open.
sorry for disturbing you again...
I've seen that my first patch is included in 0.5.1 but not this one. Is there
any special reason for not including it yet (e.g. you want to wait if you
maybe find a better solution) or did you just forget it, didn't have time,...?
Kind regards,
Gerd
8:25 a.m.
New subject: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
On Fri, Dec 05, 2008 at 02:00:00PM +0100, Gerd v. Egidy wrote:
Hi Daniel,
> > I've committed this patch now. I'll send a suggestion for the second
> > patch shortly...
>
> Ok, I think your suggested patch is the best we can manage at this point
> in time. Here's an update which also handle it for UML driver, and adds
> a check for NULL on connection open.
sorry for disturbing you again...
I've seen that my first patch is included in 0.5.1 but not this one. Is there
any special reason for not including it yet (e.g. you want to wait if you
maybe find a better solution) or did you just forget it, didn't have time,...?
Just for completeness, yes this last patch is actually in CVS head,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
5800
days inactive
5835
days old
7 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Gerd v. Egidy