[libvirt] [PATCH] virsh: Use consistent spacing for net-list
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@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
Currently we print the raw UUID which isn't very useful in ascii format. Signed-off-by: Cole Robinson <crobinso@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
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@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 :|
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@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
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@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 :|
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@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
This follows the same convention as domain drivers. Signed-off-by: Cole Robinson <crobinso@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
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@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 :|
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@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
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@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 :|
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@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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
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@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
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@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 :|
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard