12:16 p.m.
There is different spacing when listing active vs. inactive networks. Ex:
Name State Autostart
-----------------------------------------
default active yes
xxxxxx inactive no
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/virsh.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/virsh.c b/src/virsh.c
index fe4e8e6..e6241e6 100644
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -2798,7 +2798,7 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
else
autostartStr = autostart ? "yes" : "no";
- vshPrint(ctl, "%-20s %s %s\n",
+ vshPrint(ctl, "%-20s %-10s %-10s\n",
inactiveNames[i],
_("inactive"),
autostartStr);
--
1.6.0.6
12:16 p.m.
New subject: [libvirt] [PATCH] qemuDomainLookupByUUID: print correct UUID string on failed lookup.
Currently we print the raw UUID which isn't very useful in ascii format.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 40f5790..9bd7d03 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2043,7 +2043,7 @@ static virDomainPtr qemudDomainLookupByUUID(virConnectPtr conn,
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(uuid, uuidstr);
qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuid);
+ _("no domain with matching uuid '%s'"),
uuidstr);
goto cleanup;
}
--
1.6.0.6
1:22 p.m.
New subject: [libvirt] [PATCH] qemuDomainLookupByUUID: print correct UUID string on failed lookup.
On Thu, May 28, 2009 at 01:16:22PM -0400, Cole Robinson wrote:
Currently we print the raw UUID which isn't very useful in ascii
format.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 40f5790..9bd7d03 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2043,7 +2043,7 @@ static virDomainPtr qemudDomainLookupByUUID(virConnectPtr conn,
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(uuid, uuidstr);
qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuid);
+ _("no domain with matching uuid '%s'"),
uuidstr);
goto cleanup;
}
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
12:16 p.m.
New subject: [libvirt] [PATCH] qemu_driver: Fix another domain startup error reporting race.
Parse the command line output a bit earlier so we have a better chance
of reporting the full error output on failure.
I hit this when QEMU would try to boot an invalid kernel (virtinst bug).
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu_driver.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 9bd7d03..f62a46b 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -746,17 +746,21 @@ qemudReadLogOutput(virConnectPtr conn,
const char *what,
int timeout)
{
- int retries = timeout*10;
+ int retries = (timeout*10)+1;
int got = 0;
buf[0] = '\0';
while (retries) {
- ssize_t ret;
+ ssize_t func_ret, ret;
int isdead = 0;
+ func_ret = func(conn, vm, buf, fd);
+
if (kill(vm->pid, 0) == -1 && errno == ESRCH)
isdead = 1;
+ /* Any failures should be detected before we read the log, so we
+ * always have something useful to report on failure. */
ret = saferead(fd, buf+got, buflen-got-1);
if (ret < 0) {
virReportSystemError(conn, errno,
@@ -781,9 +785,8 @@ qemudReadLogOutput(virConnectPtr conn,
return -1;
}
- ret = func(conn, vm, buf, fd);
- if (ret <= 0)
- return ret;
+ if (func_ret <= 0)
+ return func_ret;
usleep(100*1000);
retries--;
--
1.6.0.6
1:24 p.m.
New subject: [libvirt] [PATCH] qemu_driver: Fix another domain startup error reporting race.
On Thu, May 28, 2009 at 01:16:23PM -0400, Cole Robinson wrote:
Parse the command line output a bit earlier so we have a better
chance
of reporting the full error output on failure.
I hit this when QEMU would try to boot an invalid kernel (virtinst bug).
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu_driver.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 9bd7d03..f62a46b 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -746,17 +746,21 @@ qemudReadLogOutput(virConnectPtr conn,
const char *what,
int timeout)
{
- int retries = timeout*10;
+ int retries = (timeout*10)+1;
Seems to be magical not really related to the problem ?
int got = 0;
buf[0] = '\0';
while (retries) {
- ssize_t ret;
+ ssize_t func_ret, ret;
int isdead = 0;
+ func_ret = func(conn, vm, buf, fd);
+
if (kill(vm->pid, 0) == -1 && errno == ESRCH)
isdead = 1;
+ /* Any failures should be detected before we read the log, so we
+ * always have something useful to report on failure. */
ret = saferead(fd, buf+got, buflen-got-1);
if (ret < 0) {
virReportSystemError(conn, errno,
@@ -781,9 +785,8 @@ qemudReadLogOutput(virConnectPtr conn,
return -1;
}
- ret = func(conn, vm, buf, fd);
- if (ret <= 0)
- return ret;
+ if (func_ret <= 0)
+ return func_ret;
usleep(100*1000);
retries--;
--
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
9:33 a.m.
New subject: [libvirt] [PATCH] qemu_driver: Fix another domain startup error reporting race.
On 05/28/2009 02:24 PM, Daniel P. Berrange wrote:
On Thu, May 28, 2009 at 01:16:23PM -0400, Cole Robinson wrote:
> Parse the command line output a bit earlier so we have a better chance
> of reporting the full error output on failure.
>
> I hit this when QEMU would try to boot an invalid kernel (virtinst bug).
>
> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
> ---
> src/qemu_driver.c | 13 ++++++++-----
> 1 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index 9bd7d03..f62a46b 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -746,17 +746,21 @@ qemudReadLogOutput(virConnectPtr conn,
> const char *what,
> int timeout)
> {
> - int retries = timeout*10;
> + int retries = (timeout*10)+1;
Seems to be magical not really related to the problem ?
Yeah, not really intended. I dropped this when I applied the patch.
- Cole
> int got = 0;
> buf[0] = '\0';
>
> while (retries) {
> - ssize_t ret;
> + ssize_t func_ret, ret;
> int isdead = 0;
>
> + func_ret = func(conn, vm, buf, fd);
> +
> if (kill(vm->pid, 0) == -1 && errno == ESRCH)
> isdead = 1;
>
> + /* Any failures should be detected before we read the log, so we
> + * always have something useful to report on failure. */
> ret = saferead(fd, buf+got, buflen-got-1);
> if (ret < 0) {
> virReportSystemError(conn, errno,
> @@ -781,9 +785,8 @@ qemudReadLogOutput(virConnectPtr conn,
> return -1;
> }
>
> - ret = func(conn, vm, buf, fd);
> - if (ret <= 0)
> - return ret;
> + if (func_ret <= 0)
> + return func_ret;
>
> usleep(100*1000);
> retries--;
> --
ACK
Daniel
12:16 p.m.
New subject: [libvirt] [PATCH] networkDestroy: Report error if network is inactive
This follows the same convention as domain drivers.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/network_driver.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/network_driver.c b/src/network_driver.c
index 29652d1..3518e01 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -1238,6 +1238,12 @@ static int networkDestroy(virNetworkPtr net) {
goto cleanup;
}
+ if (!virNetworkIsActive(network)) {
+ networkReportError(net->conn, NULL, net, VIR_ERR_INTERNAL_ERROR,
+ "%s", _("network is not active"));
+ goto cleanup;
+ }
+
ret = networkShutdownNetworkDaemon(net->conn, driver, network);
if (!network->persistent) {
virNetworkRemoveInactive(&driver->networks,
--
1.6.0.6
1:24 p.m.
New subject: [libvirt] [PATCH] networkDestroy: Report error if network is inactive
On Thu, May 28, 2009 at 01:16:24PM -0400, Cole Robinson wrote:
This follows the same convention as domain drivers.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/network_driver.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/network_driver.c b/src/network_driver.c
index 29652d1..3518e01 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -1238,6 +1238,12 @@ static int networkDestroy(virNetworkPtr net) {
goto cleanup;
}
+ if (!virNetworkIsActive(network)) {
+ networkReportError(net->conn, NULL, net, VIR_ERR_INTERNAL_ERROR,
+ "%s", _("network is not active"));
+ goto cleanup;
+ }
+
ret = networkShutdownNetworkDaemon(net->conn, driver, network);
if (!network->persistent) {
virNetworkRemoveInactive(&driver->networks,
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
12:16 p.m.
New subject: [libvirt] [PATCH] network driver: Fix regression defining colliding networks at daemon startup.
If two virtual networks have the same hardcoded bridge device (which
prevents them from being active simultaneously) we still want to define
them at daemon startup, so the user has a fighting chance of correcting
the XML error.
Add an extra flag to SetBridge to avoid reporting an error if there is
a bridge collision, and use this when loading network configs at startup.
This regressed via commit 6c2c73fc.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/network_conf.c | 17 +++++++++--------
src/network_conf.h | 3 ++-
src/network_driver.c | 4 ++--
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/network_conf.c b/src/network_conf.c
index b4da3fb..1e0cbb8 100644
--- a/src/network_conf.c
+++ b/src/network_conf.c
@@ -724,7 +724,6 @@ virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn,
virNetworkDefPtr def = NULL;
virNetworkObjPtr net;
int autostart;
- char *tmp;
if ((configFile = virNetworkConfigFile(conn, configDir, name)) == NULL)
goto error;
@@ -745,13 +744,10 @@ virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn,
goto error;
}
- /* Generate a bridge if none is found, but don't check for collisions
+ /* Generate a bridge if none is specified, but don't check for collisions
* if a bridge is hardcoded, so the network is at least defined
*/
- if ((tmp = virNetworkAllocateBridge(conn, nets, def->bridge)) != NULL) {
- VIR_FREE(def->bridge);
- def->bridge = tmp;
- } else
+ if (virNetworkSetBridgeName(conn, nets, def, 0))
goto error;
if (!(net = virNetworkAssignDef(conn, nets, def)))
@@ -913,12 +909,17 @@ char *virNetworkAllocateBridge(virConnectPtr conn,
int virNetworkSetBridgeName(virConnectPtr conn,
const virNetworkObjListPtr nets,
- virNetworkDefPtr def) {
+ virNetworkDefPtr def,
+ int check_collision) {
int ret = -1;
if (def->bridge && !strstr(def->bridge, "%d")) {
- if (virNetworkBridgeInUse(nets, def->bridge, def->name)) {
+ /* We may want to skip collision detection in this case (ex. when
+ * loading configs at daemon startup, so the network is at least
+ * defined. */
+ if (check_collision &&
+ virNetworkBridgeInUse(nets, def->bridge, def->name)) {
networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("bridge name '%s' already in use."),
def->bridge);
diff --git a/src/network_conf.h b/src/network_conf.h
index 365d469..1d51c83 100644
--- a/src/network_conf.h
+++ b/src/network_conf.h
@@ -179,7 +179,8 @@ char *virNetworkAllocateBridge(virConnectPtr conn,
int virNetworkSetBridgeName(virConnectPtr conn,
const virNetworkObjListPtr nets,
- virNetworkDefPtr def);
+ virNetworkDefPtr def,
+ int check_collision);
void virNetworkObjLock(virNetworkObjPtr obj);
void virNetworkObjUnlock(virNetworkObjPtr obj);
diff --git a/src/network_driver.c b/src/network_driver.c
index 3518e01..10d5fd3 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -1096,7 +1096,7 @@ static virNetworkPtr networkCreate(virConnectPtr conn, const char
*xml) {
if (!(def = virNetworkDefParseString(conn, xml)))
goto cleanup;
- if (virNetworkSetBridgeName(conn, &driver->networks, def))
+ if (virNetworkSetBridgeName(conn, &driver->networks, def, 1))
goto cleanup;
if (!(network = virNetworkAssignDef(conn,
@@ -1133,7 +1133,7 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char
*xml) {
if (!(def = virNetworkDefParseString(conn, xml)))
goto cleanup;
- if (virNetworkSetBridgeName(conn, &driver->networks, def))
+ if (virNetworkSetBridgeName(conn, &driver->networks, def, 1))
goto cleanup;
if (!(network = virNetworkAssignDef(conn,
--
1.6.0.6
1:24 p.m.
New subject: [libvirt] [PATCH] network driver: Fix regression defining colliding networks at daemon startup.
On Thu, May 28, 2009 at 01:16:25PM -0400, Cole Robinson wrote:
If two virtual networks have the same hardcoded bridge device (which
prevents them from being active simultaneously) we still want to define
them at daemon startup, so the user has a fighting chance of correcting
the XML error.
Add an extra flag to SetBridge to avoid reporting an error if there is
a bridge collision, and use this when loading network configs at startup.
This regressed via commit 6c2c73fc.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
ACK
diff --git a/src/network_conf.c b/src/network_conf.c
index b4da3fb..1e0cbb8 100644
--- a/src/network_conf.c
+++ b/src/network_conf.c
@@ -724,7 +724,6 @@ virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn,
virNetworkDefPtr def = NULL;
virNetworkObjPtr net;
int autostart;
- char *tmp;
if ((configFile = virNetworkConfigFile(conn, configDir, name)) == NULL)
goto error;
@@ -745,13 +744,10 @@ virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn,
goto error;
}
- /* Generate a bridge if none is found, but don't check for collisions
+ /* Generate a bridge if none is specified, but don't check for collisions
* if a bridge is hardcoded, so the network is at least defined
*/
- if ((tmp = virNetworkAllocateBridge(conn, nets, def->bridge)) != NULL) {
- VIR_FREE(def->bridge);
- def->bridge = tmp;
- } else
+ if (virNetworkSetBridgeName(conn, nets, def, 0))
goto error;
if (!(net = virNetworkAssignDef(conn, nets, def)))
@@ -913,12 +909,17 @@ char *virNetworkAllocateBridge(virConnectPtr conn,
int virNetworkSetBridgeName(virConnectPtr conn,
const virNetworkObjListPtr nets,
- virNetworkDefPtr def) {
+ virNetworkDefPtr def,
+ int check_collision) {
int ret = -1;
if (def->bridge && !strstr(def->bridge, "%d")) {
- if (virNetworkBridgeInUse(nets, def->bridge, def->name)) {
+ /* We may want to skip collision detection in this case (ex. when
+ * loading configs at daemon startup, so the network is at least
+ * defined. */
+ if (check_collision &&
+ virNetworkBridgeInUse(nets, def->bridge, def->name)) {
networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("bridge name '%s' already in use."),
def->bridge);
diff --git a/src/network_conf.h b/src/network_conf.h
index 365d469..1d51c83 100644
--- a/src/network_conf.h
+++ b/src/network_conf.h
@@ -179,7 +179,8 @@ char *virNetworkAllocateBridge(virConnectPtr conn,
int virNetworkSetBridgeName(virConnectPtr conn,
const virNetworkObjListPtr nets,
- virNetworkDefPtr def);
+ virNetworkDefPtr def,
+ int check_collision);
void virNetworkObjLock(virNetworkObjPtr obj);
void virNetworkObjUnlock(virNetworkObjPtr obj);
diff --git a/src/network_driver.c b/src/network_driver.c
index 3518e01..10d5fd3 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -1096,7 +1096,7 @@ static virNetworkPtr networkCreate(virConnectPtr conn, const char
*xml) {
if (!(def = virNetworkDefParseString(conn, xml)))
goto cleanup;
- if (virNetworkSetBridgeName(conn, &driver->networks, def))
+ if (virNetworkSetBridgeName(conn, &driver->networks, def, 1))
goto cleanup;
if (!(network = virNetworkAssignDef(conn,
@@ -1133,7 +1133,7 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char
*xml) {
if (!(def = virNetworkDefParseString(conn, xml)))
goto cleanup;
- if (virNetworkSetBridgeName(conn, &driver->networks, def))
+ if (virNetworkSetBridgeName(conn, &driver->networks, def, 1))
goto cleanup;
if (!(network = virNetworkAssignDef(conn,
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 :|
8:35 a.m.
New subject: [libvirt] [PATCH] network driver: Fix regression defining colliding networks at daemon startup.
On Thu, May 28, 2009 at 07:24:59PM +0100, Daniel P. Berrange wrote:
On Thu, May 28, 2009 at 01:16:25PM -0400, Cole Robinson wrote:
> If two virtual networks have the same hardcoded bridge device (which
> prevents them from being active simultaneously) we still want to define
> them at daemon startup, so the user has a fighting chance of correcting
> the XML error.
>
> Add an extra flag to SetBridge to avoid reporting an error if there is
> a bridge collision, and use this when loading network configs at startup.
>
> This regressed via commit 6c2c73fc.
>
> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
ACK
ACK to this series of 5 patches, cleanups or bug fixes, they all look
uncontroversial.
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/
9:32 a.m.
New subject: [libvirt] [PATCH] network driver: Fix regression defining colliding networks at daemon startup.
On 05/29/2009 09:35 AM, Daniel Veillard wrote:
On Thu, May 28, 2009 at 07:24:59PM +0100, Daniel P. Berrange wrote:
> On Thu, May 28, 2009 at 01:16:25PM -0400, Cole Robinson wrote:
>> If two virtual networks have the same hardcoded bridge device (which
>> prevents them from being active simultaneously) we still want to define
>> them at daemon startup, so the user has a fighting chance of correcting
>> the XML error.
>>
>> Add an extra flag to SetBridge to avoid reporting an error if there is
>> a bridge collision, and use this when loading network configs at startup.
>>
>> This regressed via commit 6c2c73fc.
>>
>> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
> ACK
ACK to this series of 5 patches, cleanups or bug fixes, they all look
uncontroversial.
Daniel
Thanks, I've committed them now.
- Cole
1:22 p.m.
On Thu, May 28, 2009 at 01:16:21PM -0400, Cole Robinson wrote:
There is different spacing when listing active vs. inactive networks.
Ex:
Name State Autostart
-----------------------------------------
default active yes
xxxxxx inactive no
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
5655
days inactive
5656
days old
12 comments
3 participants
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard