2:59 p.m.
This patch series is related to a discussion Eric Blake and I had a couple
of months back[1]. It essentially revamps the way that virsh does error
reporting so that errors from more calls are properly reported.
More details are in each individual patch.
[1] https://www.redhat.com/archives/libvir-list/2010-April/msg00240.html
2:59 p.m.
New subject: [libvirt] [PATCH 1/8] Remove the "showerror" parameter from vshConnectionUsability.
Nobody was using it anyway.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 270 ++++++++++++++++++++++++++++-----------------------------
1 files changed, 134 insertions(+), 136 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 1f33256..ee55a20 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -299,8 +299,7 @@ static void vshDebug(vshControl *ctl, int level, const char *format,
...)
static const char *vshDomainStateToString(int state);
static const char *vshDomainVcpuStateToString(int state);
-static int vshConnectionUsability(vshControl *ctl, virConnectPtr conn,
- int showerror);
+static int vshConnectionUsability(vshControl *ctl, virConnectPtr conn);
static char *editWriteToTempFile (vshControl *ctl, const char *doc);
static int editFile (vshControl *ctl, const char *filename);
@@ -526,7 +525,7 @@ cmdAutostart(vshControl *ctl, const vshCmd *cmd)
char *name;
int autostart;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
@@ -680,7 +679,7 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
int ret;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -722,7 +721,7 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
int maxname = 0;
inactive |= all;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (active) {
@@ -833,7 +832,7 @@ cmdDomstate(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -870,7 +869,7 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
char *name, *device;
struct _virDomainBlockStats stats;
- if (!vshConnectionUsability (ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability (ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain (ctl, cmd, &name)))
@@ -927,7 +926,7 @@ cmdDomIfstat (vshControl *ctl, const vshCmd *cmd)
char *name, *device;
struct _virDomainInterfaceStats stats;
- if (!vshConnectionUsability (ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability (ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain (ctl, cmd, &name)))
@@ -994,7 +993,7 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
struct _virDomainMemoryStat stats[VIR_DOMAIN_MEMORY_STAT_NR];
unsigned int nr_stats, i;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
@@ -1049,7 +1048,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
const char *device;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -1094,7 +1093,7 @@ cmdSuspend(vshControl *ctl, const vshCmd *cmd)
char *name;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
@@ -1142,7 +1141,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd)
#endif
unsigned int flags = VIR_DOMAIN_NONE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -1196,7 +1195,7 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *buffer;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -1243,7 +1242,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
int found;
int id;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
name = vshCommandOptString(cmd, "domain", &found);
@@ -1304,7 +1303,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd)
#endif
unsigned int flags = VIR_DOMAIN_NONE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomainBy(ctl, cmd, NULL, VSH_BYNAME)))
@@ -1359,7 +1358,7 @@ cmdSave(vshControl *ctl, const vshCmd *cmd)
char *to;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(to = vshCommandOptString(cmd, "file", NULL)))
@@ -1400,7 +1399,7 @@ cmdManagedSave(vshControl *ctl, const vshCmd *cmd)
char *name;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
@@ -1439,7 +1438,7 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd)
int ret = FALSE;
int hassave;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
@@ -1596,7 +1595,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd)
int i, ret;
int ret_val = FALSE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -1708,7 +1707,7 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd)
int found;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -1750,7 +1749,7 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
int flags = 0;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(to = vshCommandOptString(cmd, "file", NULL)))
@@ -1796,7 +1795,7 @@ cmdResume(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
@@ -1834,7 +1833,7 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
@@ -1872,7 +1871,7 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
@@ -1910,7 +1909,7 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
@@ -1953,7 +1952,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
unsigned int id;
char *str, uuid[VIR_UUID_STRING_BUFLEN];
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -2067,7 +2066,7 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -2148,7 +2147,7 @@ cmdDomjobabort(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -2182,7 +2181,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
int cell, cell_given;
unsigned long long memory;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
cell = vshCommandOptInt(cmd, "cellno", &cell_given);
@@ -2230,7 +2229,7 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd)
size_t cpumaplen;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -2321,7 +2320,7 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd)
int i;
enum { expect_num, expect_num_or_comma } state;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -2450,7 +2449,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
int maxcpu;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -2506,7 +2505,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
int kilobytes;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -2563,7 +2562,7 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
int kilobytes;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -2613,7 +2612,7 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
{
virNodeInfo info;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (virNodeGetInfo(ctl->conn, &info) < 0) {
@@ -2646,7 +2645,7 @@ cmdCapabilities (vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
{
char *caps;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if ((caps = virConnectGetCapabilities (ctl->conn)) == NULL) {
@@ -2694,7 +2693,7 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
if (update)
flags |= VIR_DOMAIN_XML_UPDATE_CPU;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -2737,7 +2736,7 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd)
char *xmlData;
int flags = 0;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
format = vshCommandOptString(cmd, "format", NULL);
@@ -2782,7 +2781,7 @@ cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
char *xmlData;
int flags = 0;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
format = vshCommandOptString(cmd, "format", NULL);
@@ -2821,7 +2820,7 @@ cmdDomname(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomainBy(ctl, cmd, NULL,
VSH_BYID|VSH_BYUUID)))
@@ -2852,7 +2851,7 @@ cmdDomid(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
unsigned int id;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomainBy(ctl, cmd, NULL,
VSH_BYNAME|VSH_BYUUID)))
@@ -2887,7 +2886,7 @@ cmdDomuuid(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
char uuid[VIR_UUID_STRING_BUFLEN];
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomainBy(ctl, cmd, NULL,
VSH_BYNAME|VSH_BYID)))
@@ -2937,7 +2936,7 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd)
const char *dname;
int flags = 0, found, ret = FALSE;
- if (!vshConnectionUsability (ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability (ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain (ctl, cmd, NULL)))
@@ -3030,7 +3029,7 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd)
int found;
int ret = FALSE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -3075,7 +3074,7 @@ cmdNetworkAutostart(vshControl *ctl, const vshCmd *cmd)
char *name;
int autostart;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(network = vshCommandOptNetwork(ctl, cmd, &name)))
@@ -3124,7 +3123,7 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *buffer;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -3172,7 +3171,7 @@ cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *buffer;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -3218,7 +3217,7 @@ cmdNetworkDestroy(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(network = vshCommandOptNetwork(ctl, cmd, &name)))
@@ -3257,7 +3256,7 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *dump;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(network = vshCommandOptNetwork(ctl, cmd, NULL)))
@@ -3301,7 +3300,7 @@ cmdInterfaceEdit (vshControl *ctl, const vshCmd *cmd)
char *doc_reread = NULL;
int flags = VIR_INTERFACE_XML_INACTIVE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
iface = vshCommandOptInterface (ctl, cmd, NULL);
@@ -3398,7 +3397,7 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
char **activeNames = NULL, **inactiveNames = NULL;
inactive |= all;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (active) {
@@ -3519,7 +3518,7 @@ cmdNetworkName(vshControl *ctl, const vshCmd *cmd)
{
virNetworkPtr network;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(network = vshCommandOptNetworkBy(ctl, cmd, NULL,
VSH_BYUUID)))
@@ -3551,7 +3550,7 @@ cmdNetworkStart(vshControl *ctl, const vshCmd *cmd)
virNetworkPtr network;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(network = vshCommandOptNetworkBy(ctl, cmd, NULL, VSH_BYNAME)))
@@ -3591,7 +3590,7 @@ cmdNetworkUndefine(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(network = vshCommandOptNetwork(ctl, cmd, &name)))
@@ -3629,7 +3628,7 @@ cmdNetworkUuid(vshControl *ctl, const vshCmd *cmd)
virNetworkPtr network;
char uuid[VIR_UUID_STRING_BUFLEN];
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(network = vshCommandOptNetworkBy(ctl, cmd, NULL,
@@ -3671,7 +3670,7 @@ cmdInterfaceList(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
char **activeNames = NULL, **inactiveNames = NULL;
inactive |= all;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (active) {
@@ -3778,7 +3777,7 @@ cmdInterfaceName(vshControl *ctl, const vshCmd *cmd)
{
virInterfacePtr iface;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(iface = vshCommandOptInterfaceBy(ctl, cmd, NULL,
VSH_BYMAC)))
@@ -3808,7 +3807,7 @@ cmdInterfaceMAC(vshControl *ctl, const vshCmd *cmd)
{
virInterfacePtr iface;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(iface = vshCommandOptInterfaceBy(ctl, cmd, NULL,
VSH_BYNAME)))
@@ -3846,7 +3845,7 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd)
if (inactive)
flags |= VIR_INTERFACE_XML_INACTIVE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(iface = vshCommandOptInterface(ctl, cmd, NULL)))
@@ -3887,7 +3886,7 @@ cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *buffer;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -3932,7 +3931,7 @@ cmdInterfaceUndefine(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(iface = vshCommandOptInterface(ctl, cmd, &name)))
@@ -3970,7 +3969,7 @@ cmdInterfaceStart(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(iface = vshCommandOptInterface(ctl, cmd, &name)))
@@ -4008,7 +4007,7 @@ cmdInterfaceDestroy(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(iface = vshCommandOptInterface(ctl, cmd, &name)))
@@ -4049,7 +4048,7 @@ cmdNWFilterDefine(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *buffer;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -4095,7 +4094,7 @@ cmdNWFilterUndefine(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(nwfilter = vshCommandOptNWFilter(ctl, cmd, &name)))
@@ -4134,7 +4133,7 @@ cmdNWFilterDumpXML(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *dump;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(nwfilter = vshCommandOptNWFilter(ctl, cmd, NULL)))
@@ -4172,7 +4171,7 @@ cmdNWFilterList(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
char **names;
char uuid[VIR_UUID_STRING_BUFLEN];
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
numfilters = virConnectNumOfNWFilters(ctl->conn);
@@ -4243,7 +4242,7 @@ cmdNWFilterEdit (vshControl *ctl, const vshCmd *cmd)
char *doc_edited = NULL;
char *doc_reread = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
nwfilter = vshCommandOptNWFilter (ctl, cmd, NULL);
@@ -4340,7 +4339,7 @@ cmdPoolAutostart(vshControl *ctl, const vshCmd *cmd)
char *name;
int autostart;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name)))
@@ -4390,7 +4389,7 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *buffer;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -4442,7 +4441,7 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *buffer;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -4493,7 +4492,7 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
int found = 0;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) {
+ if (!vshConnectionUsability(ctl, ctl->conn)) {
return FALSE;
}
@@ -4607,7 +4606,7 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
char *xml, *name;
int printXML = vshCommandOptBool(cmd, "print-xml");
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!buildPoolXML(cmd, &name, &xml))
@@ -4655,7 +4654,7 @@ cmdPoolDefine(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *buffer;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -4696,7 +4695,7 @@ cmdPoolDefineAs(vshControl *ctl, const vshCmd *cmd)
char *xml, *name;
int printXML = vshCommandOptBool(cmd, "print-xml");
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!buildPoolXML(cmd, &name, &xml))
@@ -4742,7 +4741,7 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name)))
@@ -4782,7 +4781,7 @@ cmdPoolDestroy(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name)))
@@ -4821,7 +4820,7 @@ cmdPoolDelete(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name)))
@@ -4860,7 +4859,7 @@ cmdPoolRefresh(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name)))
@@ -4899,7 +4898,7 @@ cmdPoolDumpXML(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *dump;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", NULL)))
@@ -4963,7 +4962,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
inactive |= all;
/* Check the connection to libvirtd daemon is still working */
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
/* Retrieve the number of active storage pools */
@@ -5358,7 +5357,7 @@ cmdPoolDiscoverSourcesAs(vshControl * ctl, const vshCmd * cmd
ATTRIBUTE_UNUSED)
if (!found)
host = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (host) {
@@ -5437,7 +5436,7 @@ cmdPoolDiscoverSources(vshControl * ctl, const vshCmd * cmd
ATTRIBUTE_UNUSED)
if (!found)
srcSpecFile = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (srcSpecFile && virFileReadAll(srcSpecFile, VIRSH_MAX_XML_FILE,
&srcSpec) < 0)
@@ -5480,7 +5479,7 @@ cmdPoolInfo(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char uuid[VIR_UUID_STRING_BUFLEN];
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", NULL)))
@@ -5572,7 +5571,7 @@ cmdPoolName(vshControl *ctl, const vshCmd *cmd)
{
virStoragePoolPtr pool;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL,
VSH_BYUUID)))
@@ -5604,7 +5603,7 @@ cmdPoolStart(vshControl *ctl, const vshCmd *cmd)
virStoragePoolPtr pool;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL, VSH_BYNAME)))
@@ -5683,7 +5682,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
unsigned long long capacity, allocation = 0;
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL,
@@ -5823,7 +5822,7 @@ cmdPoolUndefine(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name)))
@@ -5861,7 +5860,7 @@ cmdPoolUuid(vshControl *ctl, const vshCmd *cmd)
virStoragePoolPtr pool;
char uuid[VIR_UUID_STRING_BUFLEN];
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL,
@@ -5903,7 +5902,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *buffer;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL,
@@ -5964,7 +5963,7 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
int ret = FALSE;
char *buffer = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL, VSH_BYNAME)))
@@ -6063,7 +6062,7 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
int found;
int ret = FALSE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
if (!(origvol = vshCommandOptVol(ctl, cmd, "vol", "pool",
NULL)))
@@ -6136,7 +6135,7 @@ cmdVolDelete(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool",
&name))) {
@@ -6177,7 +6176,7 @@ cmdVolWipe(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *name;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool",
&name))) {
@@ -6218,7 +6217,7 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd)
virStorageVolPtr vol;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL)))
@@ -6269,7 +6268,7 @@ cmdVolDumpXML(vshControl *ctl, const vshCmd *cmd)
int ret = TRUE;
char *dump;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL)))
@@ -6309,7 +6308,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
int maxactive = 0, i;
char **activeNames = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", NULL)))
@@ -6385,7 +6384,7 @@ cmdVolName(vshControl *ctl, const vshCmd *cmd)
{
virStorageVolPtr vol;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(vol = vshCommandOptVolBy(ctl, cmd, "vol", "pool", NULL,
@@ -6421,7 +6420,7 @@ cmdVolPool(vshControl *ctl, const vshCmd *cmd)
char uuid[VIR_UUID_STRING_BUFLEN];
/* Check the connection to libvirtd daemon is still working */
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
/* Use the supplied string to locate the volume */
@@ -6475,7 +6474,7 @@ cmdVolKey(vshControl *ctl, const vshCmd *cmd)
{
virStorageVolPtr vol;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL)))
@@ -6509,7 +6508,7 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
virStorageVolPtr vol;
char *name = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool",
&name))) {
@@ -6543,7 +6542,7 @@ cmdSecretDefine(vshControl *ctl, const vshCmd *cmd)
virSecretPtr res;
char uuid[VIR_UUID_STRING_BUFLEN];
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", NULL);
@@ -6591,7 +6590,7 @@ cmdSecretDumpXML(vshControl *ctl, const vshCmd *cmd)
int ret = FALSE;
char *xml;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
secret = vshCommandOptSecret(ctl, cmd, NULL);
@@ -6633,7 +6632,7 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
char *base64, *value;
int found, res, ret = FALSE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
secret = vshCommandOptSecret(ctl, cmd, NULL);
@@ -6692,7 +6691,7 @@ cmdSecretGetValue(vshControl *ctl, const vshCmd *cmd)
size_t value_size;
int ret = FALSE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
secret = vshCommandOptSecret(ctl, cmd, NULL);
@@ -6742,7 +6741,7 @@ cmdSecretUndefine(vshControl *ctl, const vshCmd *cmd)
int ret = FALSE;
char *uuid;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
secret = vshCommandOptSecret(ctl, cmd, &uuid);
@@ -6776,7 +6775,7 @@ cmdSecretList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
int maxuuids = 0, i;
char **uuids = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
maxuuids = virConnectNumOfSecrets(ctl->conn);
@@ -6852,7 +6851,7 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
unsigned int minor;
unsigned int rel;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
hvType = virConnectGetType(ctl->conn);
@@ -7011,7 +7010,7 @@ cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
int found, num_devices, i;
int tree = vshCommandOptBool(cmd, "tree");
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
cap = vshCommandOptString(cmd, "cap", &found);
@@ -7098,7 +7097,7 @@ cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
char *xml;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(name = vshCommandOptString(cmd, "device", NULL)))
return FALSE;
@@ -7141,7 +7140,7 @@ cmdNodeDeviceDettach (vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(name = vshCommandOptString(cmd, "device", NULL)))
return FALSE;
@@ -7182,7 +7181,7 @@ cmdNodeDeviceReAttach (vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(name = vshCommandOptString(cmd, "device", NULL)))
return FALSE;
@@ -7223,7 +7222,7 @@ cmdNodeDeviceReset (vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
int ret = TRUE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(name = vshCommandOptString(cmd, "device", NULL)))
return FALSE;
@@ -7256,7 +7255,7 @@ cmdHostname (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
{
char *hostname;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
hostname = virConnectGetHostname (ctl->conn);
@@ -7285,7 +7284,7 @@ cmdURI (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
{
char *uri;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
uri = virConnectGetURI (ctl->conn);
@@ -7325,7 +7324,7 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd)
int port = 0;
char *doc;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -7399,7 +7398,7 @@ cmdTTYConsole(vshControl *ctl, const vshCmd *cmd)
int ret = FALSE;
char *doc;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -7462,7 +7461,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
int found;
unsigned int flags;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -7530,7 +7529,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
int found;
unsigned int flags;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -7598,7 +7597,7 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
int found;
unsigned int flags;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -7669,7 +7668,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
char *buf = NULL, *tmp = NULL;
unsigned int flags;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -7800,7 +7799,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
int i = 0, diff_mac, ret = FALSE;
unsigned int flags;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -7931,7 +7930,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
char *buf = NULL, *tmp = NULL;
unsigned int flags;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -8102,7 +8101,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
int i = 0, diff_tgt, ret = FALSE;
unsigned int flags;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -8220,7 +8219,7 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
char *buffer;
int result;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -8293,7 +8292,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
xmlXPathObjectPtr obj = NULL;
int res, i;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
from = vshCommandOptString(cmd, "file", &found);
@@ -8605,7 +8604,7 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd)
char *doc_reread = NULL;
int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
dom = vshCommandOptDomain (ctl, cmd, NULL);
@@ -8755,7 +8754,7 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
char *doc = NULL;
char *name = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
dom = vshCommandOptDomain(ctl, cmd, NULL);
@@ -8848,7 +8847,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
int current;
virDomainSnapshotPtr snapshot = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
dom = vshCommandOptDomain(ctl, cmd, NULL);
@@ -8915,7 +8914,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
char timestr[100];
struct tm time_info;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
dom = vshCommandOptDomain(ctl, cmd, NULL);
@@ -9024,7 +9023,7 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd)
virDomainSnapshotPtr snapshot = NULL;
char *xml = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
dom = vshCommandOptDomain(ctl, cmd, NULL);
@@ -9082,7 +9081,7 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd)
char *name;
virDomainSnapshotPtr snapshot = NULL;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
dom = vshCommandOptDomain(ctl, cmd, NULL);
@@ -9138,7 +9137,7 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd)
virDomainSnapshotPtr snapshot = NULL;
unsigned int flags = 0;
- if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
dom = vshCommandOptDomain(ctl, cmd, NULL);
@@ -10273,14 +10272,13 @@ vshDomainVcpuStateToString(int state)
}
static int
-vshConnectionUsability(vshControl *ctl, virConnectPtr conn, int showerror)
+vshConnectionUsability(vshControl *ctl, virConnectPtr conn)
{
/* TODO: use something like virConnectionState() to
* check usability of the connection
*/
if (!conn) {
- if (showerror)
- vshError(ctl, "%s", _("no valid connection"));
+ vshError(ctl, "%s", _("no valid connection"));
return FALSE;
}
return TRUE;
--
1.7.1.1
8:10 a.m.
New subject: [libvirt] [PATCH 1/8] Remove the "showerror" parameter from vshConnectionUsability.
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
Nobody was using it anyway.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 270 ++++++++++++++++++++++++++++-----------------------------
1 files changed, 134 insertions(+), 136 deletions(-)
Mostly search & replace.
ACK.
Matthias
8:15 a.m.
New subject: [libvirt] [PATCH 1/8] Remove the "showerror" parameter from vshConnectionUsability.
On 07/15/10 - 03:10:25PM, Matthias Bolte wrote:
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
> Nobody was using it anyway.
>
> Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
> ---
> tools/virsh.c | 270 ++++++++++++++++++++++++++++-----------------------------
> 1 files changed, 134 insertions(+), 136 deletions(-)
>
Mostly search & replace.
ACK.
Thanks, pushed.
--
Chris Lalancette
2:59 p.m.
New subject: [libvirt] [PATCH 2/8] Remove error checking after using vshMalloc.
vshMalloc and friends always exit() on allocation failure,
so there is no reason to do checking for NULL in the code
that uses it.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 38 +-------------------------------------
1 files changed, 1 insertions(+), 37 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index ee55a20..d539fbc 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1614,8 +1614,6 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd)
if (nparams) {
params = vshMalloc(ctl, sizeof(virSchedParameter)* nparams);
- if (params == NULL)
- goto cleanup;
memset(params, 0, sizeof(virSchedParameter)* nparams);
ret = virDomainGetSchedulerParameters(dom, params, &nparams);
@@ -7695,51 +7693,40 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
/* Make XML of interface */
tmp = vshMalloc(ctl, 1);
- if (!tmp) goto cleanup;
buf = vshMalloc(ctl, strlen(type) + 25);
- if (!buf) goto cleanup;
sprintf(buf, " <interface type='%s'>\n" , type);
tmp = vshRealloc(ctl, tmp, strlen(source) + 28);
- if (!tmp) goto cleanup;
if (typ == 1) {
sprintf(tmp, " <source network='%s'/>\n", source);
} else if (typ == 2) {
sprintf(tmp, " <source bridge='%s'/>\n", source);
}
buf = vshRealloc(ctl, buf, strlen(buf) + strlen(tmp) + 1);
- if (!buf) goto cleanup;
strcat(buf, tmp);
if (target != NULL) {
tmp = vshRealloc(ctl, tmp, strlen(target) + 24);
- if (!tmp) goto cleanup;
sprintf(tmp, " <target dev='%s'/>\n", target);
buf = vshRealloc(ctl, buf, strlen(buf) + strlen(tmp) + 1);
- if (!buf) goto cleanup;
strcat(buf, tmp);
}
if (mac != NULL) {
tmp = vshRealloc(ctl, tmp, strlen(mac) + 25);
- if (!tmp) goto cleanup;
sprintf(tmp, " <mac address='%s'/>\n", mac);
buf = vshRealloc(ctl, buf, strlen(buf) + strlen(tmp) + 1);
- if (!buf) goto cleanup;
strcat(buf, tmp);
}
if (script != NULL) {
tmp = vshRealloc(ctl, tmp, strlen(script) + 25);
- if (!tmp) goto cleanup;
sprintf(tmp, " <script path='%s'/>\n", script);
buf = vshRealloc(ctl, buf, strlen(buf) + strlen(tmp) + 1);
- if (!buf) goto cleanup;
strcat(buf, tmp);
}
buf = vshRealloc(ctl, buf, strlen(buf) + 19);
- if (!buf) goto cleanup;
strcat(buf, " </interface>\n");
if (vshCommandOptBool(cmd, "persistent")) {
@@ -7967,9 +7954,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
/* Make XML of disk */
tmp = vshMalloc(ctl, 1);
- if (!tmp) goto cleanup;
buf = vshMalloc(ctl, 23);
- if (!buf) goto cleanup;
if (isFile) {
sprintf(buf, " <disk type='file'");
} else {
@@ -7978,72 +7963,56 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
if (type) {
tmp = vshRealloc(ctl, tmp, strlen(type) + 13);
- if (!tmp) goto cleanup;
sprintf(tmp, " device='%s'>\n", type);
} else {
tmp = vshRealloc(ctl, tmp, 3);
- if (!tmp) goto cleanup;
sprintf(tmp, ">\n");
}
buf = vshRealloc(ctl, buf, strlen(buf) + strlen(tmp) + 1);
- if (!buf) goto cleanup;
strcat(buf, tmp);
if (driver) {
tmp = vshRealloc(ctl, tmp, strlen(driver) + 22);
- if (!tmp) goto cleanup;
sprintf(tmp, " <driver name='%s'", driver);
} else {
tmp = vshRealloc(ctl, tmp, 25);
- if (!tmp) goto cleanup;
sprintf(tmp, " <driver name='phy'");
}
buf = vshRealloc(ctl, buf, strlen(buf) + strlen(tmp) + 1);
- if (!buf) goto cleanup;
strcat(buf, tmp);
if (subdriver) {
tmp = vshRealloc(ctl, tmp, strlen(subdriver) + 12);
- if (!tmp) goto cleanup;
sprintf(tmp, " type='%s'/>\n", subdriver);
} else {
tmp = vshRealloc(ctl, tmp, 4);
- if (!tmp) goto cleanup;
sprintf(tmp, "/>\n");
}
buf = vshRealloc(ctl, buf, strlen(buf) + strlen(tmp) + 1);
- if (!buf) goto cleanup;
strcat(buf, tmp);
tmp = vshRealloc(ctl, tmp, strlen(source) + 25);
- if (!tmp) goto cleanup;
if (isFile) {
sprintf(tmp, " <source file='%s'/>\n", source);
} else {
sprintf(tmp, " <source dev='%s'/>\n", source);
}
buf = vshRealloc(ctl, buf, strlen(buf) + strlen(tmp) + 1);
- if (!buf) goto cleanup;
strcat(buf, tmp);
tmp = vshRealloc(ctl, tmp, strlen(target) + 24);
- if (!tmp) goto cleanup;
sprintf(tmp, " <target dev='%s'/>\n", target);
buf = vshRealloc(ctl, buf, strlen(buf) + strlen(tmp) + 1);
- if (!buf) goto cleanup;
strcat(buf, tmp);
if (mode != NULL) {
tmp = vshRealloc(ctl, tmp, strlen(mode) + 11);
- if (!tmp) goto cleanup;
sprintf(tmp, " <%s/>\n", mode);
buf = vshRealloc(ctl, buf, strlen(buf) + strlen(tmp) + 1);
- if (!buf) goto cleanup;
strcat(buf, tmp);
}
buf = vshRealloc(ctl, buf, strlen(buf) + 13);
- if (!buf) goto cleanup;
strcat(buf, " </disk>\n");
if (vshCommandOptBool(cmd, "persistent")) {
@@ -8386,12 +8355,7 @@ editWriteToTempFile (vshControl *ctl, const char *doc)
const char *tmpdir;
int fd;
- ret = malloc (PATH_MAX);
- if (!ret) {
- vshError(ctl, _("malloc: failed to allocate temporary file name: %s"),
- strerror(errno));
- return NULL;
- }
+ ret = vshMalloc(ctl, PATH_MAX);
tmpdir = getenv ("TMPDIR");
if (!tmpdir) tmpdir = "/tmp";
--
1.7.1.1
8:10 a.m.
New subject: [libvirt] [PATCH 2/8] Remove error checking after using vshMalloc.
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
vshMalloc and friends always exit() on allocation failure,
so there is no reason to do checking for NULL in the code
that uses it.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
ACK.
Matthias
8:15 a.m.
New subject: [libvirt] [PATCH 2/8] Remove error checking after using vshMalloc.
On 07/15/10 - 03:10:49PM, Matthias Bolte wrote:
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
> vshMalloc and friends always exit() on allocation failure,
> so there is no reason to do checking for NULL in the code
> that uses it.
>
> Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
> ---
ACK.
Thanks, pushed.
--
Chris Lalancette
2:59 p.m.
New subject: [libvirt] [PATCH 3/8] Remove unused and bitrotting vshCommandOptStringList
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 29 -----------------------------
1 files changed, 0 insertions(+), 29 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index d539fbc..c1451d8 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -227,9 +227,6 @@ static char *vshCommandOptString(const vshCmd *cmd, const char *name,
int *found);
static long long vshCommandOptLongLong(const vshCmd *cmd, const char *name,
int *found);
-#if 0
-static int vshCommandOptStringList(const vshCmd *cmd, const char *name, char ***data);
-#endif
static int vshCommandOptBool(const vshCmd *cmd, const char *name);
#define VSH_BYID (1 << 1)
@@ -9564,32 +9561,6 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name, int
*found)
return res;
}
-#if 0
-static int
-vshCommandOptStringList(const vshCmd *cmd, const char *name, char ***data)
-{
- vshCmdOpt *arg = cmd->opts;
- char **val = NULL;
- int nval = 0;
-
- while (arg) {
- if (arg->def && STREQ(arg->def->name, name)) {
- char **tmp = realloc(val, sizeof(*tmp) * (nval+1));
- if (!tmp) {
- VIR_FREE(val);
- return -1;
- }
- val = tmp;
- val[nval++] = arg->data;
- }
- arg = arg->next;
- }
-
- *data = val;
- return nval;
-}
-#endif
-
/*
* Returns TRUE/FALSE if the option exists
*/
--
1.7.1.1
8:11 a.m.
New subject: [libvirt] [PATCH 3/8] Remove unused and bitrotting vshCommandOptStringList
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 29 -----------------------------
1 files changed, 0 insertions(+), 29 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index d539fbc..c1451d8 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -227,9 +227,6 @@ static char *vshCommandOptString(const vshCmd *cmd, const char
*name,
int *found);
static long long vshCommandOptLongLong(const vshCmd *cmd, const char *name,
int *found);
-#if 0
-static int vshCommandOptStringList(const vshCmd *cmd, const char *name, char ***data);
-#endif
static int vshCommandOptBool(const vshCmd *cmd, const char *name);
#define VSH_BYID (1 << 1)
@@ -9564,32 +9561,6 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name, int
*found)
return res;
}
-#if 0
-static int
-vshCommandOptStringList(const vshCmd *cmd, const char *name, char ***data)
-{
- vshCmdOpt *arg = cmd->opts;
- char **val = NULL;
- int nval = 0;
-
- while (arg) {
- if (arg->def && STREQ(arg->def->name, name)) {
- char **tmp = realloc(val, sizeof(*tmp) * (nval+1));
- if (!tmp) {
- VIR_FREE(val);
- return -1;
- }
- val = tmp;
- val[nval++] = arg->data;
- }
- arg = arg->next;
- }
-
- *data = val;
- return nval;
-}
-#endif
-
/*
* Returns TRUE/FALSE if the option exists
*/
--
1.7.1.1
ACK.
Matthias
8:16 a.m.
New subject: [libvirt] [PATCH 3/8] Remove unused and bitrotting vshCommandOptStringList
On 07/15/10 - 03:11:02PM, Matthias Bolte wrote:
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
> Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
> ---
> tools/virsh.c | 29 -----------------------------
> 1 files changed, 0 insertions(+), 29 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index d539fbc..c1451d8 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -227,9 +227,6 @@ static char *vshCommandOptString(const vshCmd *cmd, const char
*name,
> int *found);
> static long long vshCommandOptLongLong(const vshCmd *cmd, const char *name,
> int *found);
> -#if 0
> -static int vshCommandOptStringList(const vshCmd *cmd, const char *name, char
***data);
> -#endif
> static int vshCommandOptBool(const vshCmd *cmd, const char *name);
>
> #define VSH_BYID (1 << 1)
> @@ -9564,32 +9561,6 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name,
int *found)
> return res;
> }
>
> -#if 0
> -static int
> -vshCommandOptStringList(const vshCmd *cmd, const char *name, char ***data)
> -{
> - vshCmdOpt *arg = cmd->opts;
> - char **val = NULL;
> - int nval = 0;
> -
> - while (arg) {
> - if (arg->def && STREQ(arg->def->name, name)) {
> - char **tmp = realloc(val, sizeof(*tmp) * (nval+1));
> - if (!tmp) {
> - VIR_FREE(val);
> - return -1;
> - }
> - val = tmp;
> - val[nval++] = arg->data;
> - }
> - arg = arg->next;
> - }
> -
> - *data = val;
> - return nval;
> -}
> -#endif
> -
> /*
> * Returns TRUE/FALSE if the option exists
> */
> --
> 1.7.1.1
>
ACK.
Thanks, pushed.
--
Chris Lalancette
2:59 p.m.
New subject: [libvirt] [PATCH 4/8] Remove all uses of virshReportError except "vshError".
With the current virsh code, error reporting is a bit fragile
because the reporting is decoupled from the place where the
error occurred. There are two places where this can be a problem:
1) In utility functions that don't properly dispatch errors
like the main API functions do. In this case, the error from
the utility function initially gets stored, but then may get
blown away during a cleanup function.
2) If a cleanup function has an additional error. In this case,
what will happen is that we will report the error from the cleanup
function, not the original error, which may make the problem
harder to diagnose.
What this patch does is to report errors exactly at the point
that they occurred, in vshError(). That way the individual
cmd* functions can do whatever cleanup is necessary without
fear of blowing away the appropriate error.
With this in place, it's now the responsibility of the
individual cmd* functions to use vshError when an error
occurs. However, this should make error reporting more consistent.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 22 +++++-----------------
1 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index c1451d8..015d038 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -377,7 +377,7 @@ virshErrorHandler(void *unused ATTRIBUTE_UNUSED, virErrorPtr error)
* and it's IMHO a bug that libvirt does that sometimes.
*/
static void
-virshReportError(vshControl *ctl)
+virshReportError(void)
{
if (last_error == NULL) {
/* Calling directly into libvirt util functions won't trigger the
@@ -391,11 +391,11 @@ virshReportError(vshControl *ctl)
}
if (last_error->code == VIR_ERR_OK) {
- vshError(ctl, "%s", _("unknown error"));
+ fprintf(stderr, "error: %s\n", _("unknown error"));
goto out;
}
- vshError(ctl, "%s", last_error->message);
+ fprintf(stderr, "error: %s\n", last_error->message);
out:
virFreeError(last_error);
@@ -5911,7 +5911,6 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
}
if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
- virshReportError(ctl);
virStoragePoolFree(pool);
return FALSE;
}
@@ -5973,7 +5972,6 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
- virshReportError(ctl);
goto cleanup;
}
@@ -7470,7 +7468,6 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
}
if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
- virshReportError(ctl);
virDomainFree(dom);
return FALSE;
}
@@ -7538,7 +7535,6 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
}
if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
- virshReportError(ctl);
virDomainFree(dom);
return FALSE;
}
@@ -7606,7 +7602,6 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
}
if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
- virshReportError(ctl);
virDomainFree(dom);
return FALSE;
}
@@ -8727,11 +8722,6 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
buffer = strdup("<domainsnapshot/>");
else {
if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
- /* we have to report the error here because during cleanup
- * we'll run through virDomainFree(), which loses the
- * last error
- */
- virshReportError(ctl);
goto cleanup;
}
}
@@ -9910,9 +9900,6 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd)
if (enable_timing)
GETTIMEOFDAY(&after);
- if (ret == FALSE)
- virshReportError(ctl);
-
/* try to automatically catch disconnections */
if ((ret == FALSE) &&
((disconnected != 0) ||
@@ -10268,6 +10255,8 @@ vshError(vshControl *ctl, const char *format, ...)
va_end(ap);
fputc('\n', stderr);
+
+ virshReportError();
}
static void *
@@ -10348,7 +10337,6 @@ vshInit(vshControl *ctl)
* such as "help".
*/
if (!ctl->conn) {
- virshReportError(ctl);
vshError(ctl, "%s", _("failed to connect to the
hypervisor"));
return FALSE;
}
--
1.7.1.1
11:07 a.m.
New subject: [libvirt] [PATCH 4/8] Remove all uses of virshReportError except "vshError".
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
With the current virsh code, error reporting is a bit fragile
because the reporting is decoupled from the place where the
error occurred. There are two places where this can be a problem:
1) In utility functions that don't properly dispatch errors
like the main API functions do. In this case, the error from
the utility function initially gets stored, but then may get
blown away during a cleanup function.
2) If a cleanup function has an additional error. In this case,
what will happen is that we will report the error from the cleanup
function, not the original error, which may make the problem
harder to diagnose.
What this patch does is to report errors exactly at the point
that they occurred, in vshError(). That way the individual
cmd* functions can do whatever cleanup is necessary without
fear of blowing away the appropriate error.
So this patch changes the internal handling but should not affect the
actual error output of virsh? I'm asking because we are losing error
messages this way with this patch applied.
With this in place, it's now the responsibility of the
individual cmd* functions to use vshError when an error
occurs. However, this should make error reporting more consistent.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 22 +++++-----------------
1 files changed, 5 insertions(+), 17 deletions(-)
@@ -8727,11 +8722,6 @@ cmdSnapshotCreate(vshControl *ctl, const
vshCmd *cmd)
buffer = strdup("<domainsnapshot/>");
else {
if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
- /* we have to report the error here because during cleanup
- * we'll run through virDomainFree(), which loses the
- * last error
- */
- virshReportError(ctl);
For example this one. I tested it with and without this patch.
Without this patch we get
virsh # snapshot-create test1 this-file-does-not-exist.xml
error: Failed to open file 'this-file-does-not-exist.xml': No such
file or directory
virsh #
With this patch applied we get
virsh # snapshot-create test1 this-file-does-not-exist.xml
virsh #
Making it look like it succeeded. I assume it's the same pattern in
all places where you removed the call to virshReportError on
virFileReadAll failure.
Ah, okay. Now I see that you fixed this in patch 6/8. Now the error is
more verbose, but that's okay I think.
virsh # snapshot-create test1 this-file-does-not-exist.xml
error: Failed to read contents of 'this-file-does-not-exist.xml'
error: Failed to open file 'this-file-does-not-exist.xml': No such
file or directory
virsh #
Maybe you should merge patch 4 and 6 into one. That way error
reporting for virFileReadAll isn't broken in between those two
commits.
ACK.
Matthias
7:49 a.m.
New subject: [libvirt] [PATCH 4/8] Remove all uses of virshReportError except "vshError".
On 07/17/10 - 06:07:00PM, Matthias Bolte wrote:
For example this one. I tested it with and without this patch.
Without this patch we get
virsh # snapshot-create test1 this-file-does-not-exist.xml
error: Failed to open file 'this-file-does-not-exist.xml': No such
file or directory
virsh #
With this patch applied we get
virsh # snapshot-create test1 this-file-does-not-exist.xml
virsh #
Making it look like it succeeded. I assume it's the same pattern in
all places where you removed the call to virshReportError on
virFileReadAll failure.
Ah, okay. Now I see that you fixed this in patch 6/8. Now the error is
more verbose, but that's okay I think.
virsh # snapshot-create test1 this-file-does-not-exist.xml
error: Failed to read contents of 'this-file-does-not-exist.xml'
error: Failed to open file 'this-file-does-not-exist.xml': No such
file or directory
virsh #
Maybe you should merge patch 4 and 6 into one. That way error
reporting for virFileReadAll isn't broken in between those two
commits.
Yep, you hit it exactly. I was trying to get the error reporting to be more
explicit, but I did it in two stages. Merging this and 6 is not a bad idea,
I'll do that when I commit.
ACK.
Thanks,
--
Chris Lalancette
2:59 p.m.
New subject: [libvirt] [PATCH 5/8] Add additional error reporting to virsh.
With the change to make vshError() responsible for printing
all errors, there were some places in the code that would no
longer properly print errors. The good news is that the vast
majority of virsh was already printing errors, so this patch
just cleans up the rest of the users to make them consistent.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 228 +++++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 174 insertions(+), 54 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 015d038..3c48eee 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -638,18 +638,24 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom)
}
doc = virDomainGetXMLDesc(dom, 0);
- if (!doc)
+ if (!doc) {
+ vshError(ctl, "%s", _("Failed to get the domain XML"));
goto cleanup;
+ }
xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL,
XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOWARNING);
VIR_FREE(doc);
- if (!xml)
+ if (!xml) {
+ vshError(ctl, "%s", _("Failed to parse the domain XML"));
goto cleanup;
+ }
ctxt = xmlXPathNewContext(xml);
- if (!ctxt)
+ if (!ctxt) {
+ vshError(ctl, "%s", _("Failed to allocate memory"));
goto cleanup;
+ }
tty = virXPathString("string(/domain/devices/console/@tty)", ctxt);
if (tty) {
@@ -838,8 +844,10 @@ cmdDomstate(vshControl *ctl, const vshCmd *cmd)
if (virDomainGetInfo(dom, &info) == 0)
vshPrint(ctl, "%s\n",
_(vshDomainStateToString(info.state)));
- else
+ else {
+ vshError(ctl, "%s", _("Failed to get domain info"));
ret = FALSE;
+ }
virDomainFree(dom);
return ret;
@@ -1057,6 +1065,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
}
if (virDomainGetBlockInfo(dom, device, &info, 0) < 0) {
+ vshError(ctl, _("Failed to get block info for device '%s'"),
device);
virDomainFree(dom);
return FALSE;
}
@@ -1614,8 +1623,10 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd)
memset(params, 0, sizeof(virSchedParameter)* nparams);
ret = virDomainGetSchedulerParameters(dom, params, &nparams);
- if (ret == -1)
+ if (ret == -1) {
+ vshError(ctl, "%s", _("Failed to get scheduler
parameters"));
goto cleanup;
+ }
/* See if any params are being set */
for (i = 0; i < nparams; i++){
@@ -1630,12 +1641,16 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd)
/* Update parameters & refresh data */
if (update) {
ret = virDomainSetSchedulerParameters(dom, params, nparams);
- if (ret == -1)
+ if (ret == -1) {
+ vshError(ctl, "%s", _("Failed to set scheduler
parameters"));
goto cleanup;
+ }
ret = virDomainGetSchedulerParameters(dom, params, &nparams);
- if (ret == -1)
+ if (ret == -1) {
+ vshError(ctl, "%s", _("Failed to update scheduler
parameters"));
goto cleanup;
+ }
} else {
/* See if we've tried to --set var=val. If so, the fact that
we reach this point (with update == 0) means that "var" did
@@ -1993,6 +2008,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
info.memory);
} else {
+ vshError(ctl, "%s", _("Failed to get domain info"));
ret = FALSE;
}
@@ -2014,6 +2030,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
memset(&secmodel, 0, sizeof secmodel);
if (virNodeGetSecurityModel(ctl->conn, &secmodel) == -1) {
if (last_error->code != VIR_ERR_NO_SUPPORT) {
+ vshError(ctl, "%s", _("Failed to get security model"));
virDomainFree(dom);
return FALSE;
}
@@ -2026,6 +2043,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
/* Security labels are only valid for active domains */
memset(&seclabel, 0, sizeof seclabel);
if (virDomainGetSecurityLabel(dom, &seclabel) == -1) {
+ vshError(ctl, "%s", _("Failed to get domain security
label"));
virDomainFree(dom);
return FALSE;
} else {
@@ -2115,6 +2133,7 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%-17s %-.3lf %s\n", _("File total:"), val,
unit);
}
} else {
+ vshError(ctl, "%s", _("Failed to get domain job info"));
ret = FALSE;
}
cleanup:
@@ -2148,8 +2167,10 @@ cmdDomjobabort(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return FALSE;
- if (virDomainAbortJob(dom) < 0)
+ if (virDomainAbortJob(dom) < 0) {
+ vshError(ctl, "%s", _("Failed to abort domain job"));
ret = FALSE;
+ }
virDomainFree(dom);
return ret;
@@ -2182,12 +2203,16 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
cell = vshCommandOptInt(cmd, "cellno", &cell_given);
if (!cell_given) {
memory = virNodeGetFreeMemory(ctl->conn);
- if (memory == 0)
+ if (memory == 0) {
+ vshError(ctl, "%s", _("Failed to get node free
memory"));
return FALSE;
+ }
} else {
ret = virNodeGetCellsFreeMemory(ctl->conn, &memory, cell, 1);
- if (ret != 1)
+ if (ret != 1) {
+ vshError(ctl, "%s", _("Failed to get node cells free
memory"));
return FALSE;
+ }
}
if (cell == -1)
@@ -2231,11 +2256,13 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd)
return FALSE;
if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) {
+ vshError(ctl, "%s", _("Failed to get node info"));
virDomainFree(dom);
return FALSE;
}
if (virDomainGetInfo(dom, &info) != 0) {
+ vshError(ctl, "%s", _("Failed to get domain info"));
virDomainFree(dom);
return FALSE;
}
@@ -2335,6 +2362,7 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd)
}
if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) {
+ vshError(ctl, "%s", _("Failed to get node info"));
virDomainFree(dom);
return FALSE;
}
@@ -2413,6 +2441,7 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd)
} while (cpulist);
if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) != 0) {
+ vshError(ctl, "%s", _("Failed to pin domain vcpus"));
ret = FALSE;
}
@@ -2459,6 +2488,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
maxcpu = virDomainGetMaxVcpus(dom);
if (maxcpu <= 0) {
+ vshError(ctl, "%s", _("Failed to get domain maximum
vcpus"));
virDomainFree(dom);
return FALSE;
}
@@ -2470,6 +2500,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
}
if (virDomainSetVcpus(dom, count) != 0) {
+ vshError(ctl, "%s", _("Failed to set domain vcpus"));
ret = FALSE;
}
@@ -2527,6 +2558,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
}
if (virDomainSetMemory(dom, kilobytes) != 0) {
+ vshError(ctl, "%s", _("Failed to set domain memory"));
ret = FALSE;
}
@@ -2699,6 +2731,7 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s", dump);
VIR_FREE(dump);
} else {
+ vshError(ctl, "%s", _("Failed to dump domain xml"));
ret = FALSE;
}
@@ -2745,6 +2778,7 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s", xmlData);
VIR_FREE(xmlData);
} else {
+ vshError(ctl, "%s", _("Failed to convert native to domain
xml"));
ret = FALSE;
}
@@ -2790,6 +2824,7 @@ cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s", configData);
VIR_FREE(configData);
} else {
+ vshError(ctl, "%s", _("Failed to convert domain xml to
native"));
ret = FALSE;
}
@@ -2986,7 +3021,10 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd)
virDomainPtr ddom = NULL;
dconn = virConnectOpenAuth (desturi, virConnectAuthPtrDefault, 0);
- if (!dconn) goto done;
+ if (!dconn) {
+ vshError(ctl, _("Failed to open connection to '%s'"),
desturi);
+ goto done;
+ }
ddom = virDomainMigrate (dom, dconn, flags, dname, migrateuri, 0);
if (ddom) {
@@ -3036,8 +3074,10 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd)
goto done;
}
- if (virDomainMigrateSetMaxDowntime(dom, downtime, 0))
+ if (virDomainMigrateSetMaxDowntime(dom, downtime, 0) < 0) {
+ vshError(ctl, "%s", _("Failed to set migration maximum
downtime"));
goto done;
+ }
ret = TRUE;
@@ -3262,6 +3302,7 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s", dump);
VIR_FREE(dump);
} else {
+ vshError(ctl, "%s", _("Failed to dumpxml for network"));
ret = FALSE;
}
@@ -3304,8 +3345,10 @@ cmdInterfaceEdit (vshControl *ctl, const vshCmd *cmd)
/* Get the XML configuration of the interface. */
doc = virInterfaceGetXMLDesc (iface, flags);
- if (!doc)
+ if (!doc) {
+ vshError(ctl, "%s", _("Failed to get xml for interface"));
goto cleanup;
+ }
/* Create and open the temporary file. */
tmp = editWriteToTempFile (ctl, doc);
@@ -3331,8 +3374,10 @@ cmdInterfaceEdit (vshControl *ctl, const vshCmd *cmd)
* losing a connection or the interface going away.
*/
doc_reread = virInterfaceGetXMLDesc (iface, flags);
- if (!doc_reread)
+ if (!doc_reread) {
+ vshError(ctl, "%s", _("Failed to re-read interface xml"));
goto cleanup;
+ }
if (STRNEQ (doc, doc_reread)) {
vshError(ctl, "%s",
@@ -3343,8 +3388,10 @@ cmdInterfaceEdit (vshControl *ctl, const vshCmd *cmd)
/* Everything checks out, so redefine the interface. */
virInterfaceFree (iface);
iface = virInterfaceDefineXML (ctl->conn, doc_edited, 0);
- if (!iface)
+ if (!iface) {
+ vshError(ctl, "%s", _("Failed to define new interface
XML"));
goto cleanup;
+ }
vshPrint (ctl, _("Interface %s XML configuration edited.\n"),
virInterfaceGetName(iface));
@@ -3851,6 +3898,7 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s", dump);
VIR_FREE(dump);
} else {
+ vshError(ctl, "%s", _("Failed to get xml for interface"));
ret = FALSE;
}
@@ -4139,6 +4187,7 @@ cmdNWFilterDumpXML(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s", dump);
VIR_FREE(dump);
} else {
+ vshError(ctl, "%s", _("Failed to get nwfilter XML"));
ret = FALSE;
}
@@ -4246,8 +4295,10 @@ cmdNWFilterEdit (vshControl *ctl, const vshCmd *cmd)
/* Get the XML configuration of the interface. */
doc = virNWFilterGetXMLDesc (nwfilter, 0);
- if (!doc)
+ if (!doc) {
+ vshError(ctl, "%s", _("Failed to get nwfilter XML"));
goto cleanup;
+ }
/* Create and open the temporary file. */
tmp = editWriteToTempFile (ctl, doc);
@@ -4273,8 +4324,10 @@ cmdNWFilterEdit (vshControl *ctl, const vshCmd *cmd)
* losing a connection or the interface going away.
*/
doc_reread = virNWFilterGetXMLDesc (nwfilter, 0);
- if (!doc_reread)
+ if (!doc_reread) {
+ vshError(ctl, "%s", _("Failed to re-read nwfilter XML"));
goto cleanup;
+ }
if (STRNEQ (doc, doc_reread)) {
vshError(ctl, "%s",
@@ -4285,8 +4338,10 @@ cmdNWFilterEdit (vshControl *ctl, const vshCmd *cmd)
/* Everything checks out, so redefine the interface. */
virNWFilterFree (nwfilter);
nwfilter = virNWFilterDefineXML (ctl->conn, doc_edited);
- if (!nwfilter)
+ if (!nwfilter) {
+ vshError(ctl, "%s", _("Failed to define new nwfilter XML"));
goto cleanup;
+ }
vshPrint (ctl, _("Network filter %s XML configuration edited.\n"),
virNWFilterGetName(nwfilter));
@@ -4904,6 +4959,7 @@ cmdPoolDumpXML(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s", dump);
VIR_FREE(dump);
} else {
+ vshError(ctl, "%s", _("Failed to get XML for pool"));
ret = FALSE;
}
@@ -5539,6 +5595,7 @@ cmdPoolInfo(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%-15s %2.2lf %s\n", _("Available:"), val,
unit);
}
} else {
+ vshError(ctl, "%s", _("Failed to get pool info"));
ret = FALSE;
}
@@ -5772,7 +5829,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
virBufferAddLit(&buf, "</volume>\n");
if (virBufferError(&buf)) {
- vshPrint(ctl, "%s", _("Failed to allocate XML buffer"));
+ vshError(ctl, "%s", _("Failed to allocate XML buffer"));
return FALSE;
}
xml = virBufferContentAndReset(&buf);
@@ -6072,12 +6129,14 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
origxml = virStorageVolGetXMLDesc(origvol, 0);
- if (!origxml)
+ if (!origxml) {
+ vshError(ctl, "%s", _("Failed to get storage volume XML"));
goto cleanup;
+ }
newxml = makeCloneXML(origxml, name);
if (!newxml) {
- vshPrint(ctl, "%s", _("Failed to allocate XML buffer"));
+ vshError(ctl, "%s", _("Failed to allocate XML buffer"));
goto cleanup;
}
@@ -6231,6 +6290,7 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd)
val = prettyCapacity(info.allocation, &unit);
vshPrint(ctl, "%-15s %2.2lf %s\n", _("Allocation:"), val,
unit);
} else {
+ vshError(ctl, "%s", _("Failed to get storage volume info"));
ret = FALSE;
}
@@ -6272,6 +6332,7 @@ cmdVolDumpXML(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s", dump);
VIR_FREE(dump);
} else {
+ vshError(ctl, "%s", _("Failed to get storage volume XML"));
ret = FALSE;
}
@@ -6591,8 +6652,10 @@ cmdSecretDumpXML(vshControl *ctl, const vshCmd *cmd)
return FALSE;
xml = virSecretGetXMLDesc(secret, 0);
- if (xml == NULL)
+ if (xml == NULL) {
+ vshError(ctl, "%s", _("Failed to get secret XML"));
goto cleanup;
+ }
vshPrint(ctl, "%s", xml);
VIR_FREE(xml);
ret = TRUE;
@@ -6692,8 +6755,10 @@ cmdSecretGetValue(vshControl *ctl, const vshCmd *cmd)
return FALSE;
value = virSecretGetValue(secret, &value_size, 0);
- if (value == NULL)
+ if (value == NULL) {
+ vshError(ctl, "%s", _("Failed to get secret value"));
goto cleanup;
+ }
base64_encode_alloc((char *)value, value_size, &base64);
memset(value, 0, value_size);
@@ -7101,6 +7166,7 @@ cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd)
xml = virNodeDeviceGetXMLDesc(device, 0);
if (!xml) {
+ vshError(ctl, "%s", _("Failed to get node device XML"));
virNodeDeviceFree(device);
return FALSE;
}
@@ -7324,18 +7390,24 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd)
return FALSE;
doc = virDomainGetXMLDesc(dom, 0);
- if (!doc)
+ if (!doc) {
+ vshError(ctl, "%s", _("Failed to get domain XML"));
goto cleanup;
+ }
xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL,
XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOWARNING);
VIR_FREE(doc);
- if (!xml)
+ if (!xml) {
+ vshError(ctl, "%s", _("Failed to parse domain XML"));
goto cleanup;
+ }
ctxt = xmlXPathNewContext(xml);
- if (!ctxt)
+ if (!ctxt) {
+ vshError(ctl, "%s", _("Failed to allocate memory"));
goto cleanup;
+ }
obj = xmlXPathEval(BAD_CAST
"string(/domain/devices/graphics[@type='vnc']/@port)", ctxt);
if ((obj == NULL) || (obj->type != XPATH_STRING) ||
@@ -7398,18 +7470,24 @@ cmdTTYConsole(vshControl *ctl, const vshCmd *cmd)
return FALSE;
doc = virDomainGetXMLDesc(dom, 0);
- if (!doc)
+ if (!doc) {
+ vshError(ctl, "%s", _("Failed to get domain XML"));
goto cleanup;
+ }
xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL,
XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOWARNING);
VIR_FREE(doc);
- if (!xml)
+ if (!xml) {
+ vshError(ctl, "%s", _("Failed to parse domain XML"));
goto cleanup;
+ }
ctxt = xmlXPathNewContext(xml);
- if (!ctxt)
+ if (!ctxt) {
+ vshError(ctl, "%s", _("Failed to allocate memory"));
goto cleanup;
+ }
obj = xmlXPathEval(BAD_CAST "string(/domain/devices/console/@tty)", ctxt);
if ((obj == NULL) || (obj->type != XPATH_STRING) ||
@@ -7790,20 +7868,22 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
mac = vshCommandOptString(cmd, "mac", NULL);
doc = virDomainGetXMLDesc(dom, 0);
- if (!doc)
+ if (!doc) {
+ vshError(ctl, "%s", _("Failed to get domain XML"));
goto cleanup;
+ }
xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL,
XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOWARNING);
VIR_FREE(doc);
if (!xml) {
- vshError(ctl, "%s", _("Failed to get interface
information"));
+ vshError(ctl, "%s", _("Failed to parse domain XML"));
goto cleanup;
}
ctxt = xmlXPathNewContext(xml);
if (!ctxt) {
- vshError(ctl, "%s", _("Failed to get interface
information"));
+ vshError(ctl, "%s", _("Failed to allocate memory"));
goto cleanup;
}
@@ -8072,20 +8152,22 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
doc = virDomainGetXMLDesc(dom, 0);
- if (!doc)
+ if (!doc) {
+ vshError(ctl, "%s", _("Failed to get domain XML"));
goto cleanup;
+ }
xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL,
XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOWARNING);
VIR_FREE(doc);
if (!xml) {
- vshError(ctl, "%s", _("Failed to get disk information"));
+ vshError(ctl, "%s", _("Failed to parse domain XML"));
goto cleanup;
}
ctxt = xmlXPathNewContext(xml);
if (!ctxt) {
- vshError(ctl, "%s", _("Failed to get disk information"));
+ vshError(ctl, "%s", _("Failed to allocate memory"));
goto cleanup;
}
@@ -8569,8 +8651,10 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd)
/* Get the XML configuration of the domain. */
doc = virDomainGetXMLDesc (dom, flags);
- if (!doc)
+ if (!doc) {
+ vshError(ctl, "%s", _("Failed to get domain XML"));
goto cleanup;
+ }
/* Create and open the temporary file. */
tmp = editWriteToTempFile (ctl, doc);
@@ -8596,8 +8680,10 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd)
* losing a connection or the domain going away.
*/
doc_reread = virDomainGetXMLDesc (dom, flags);
- if (!doc_reread)
+ if (!doc_reread) {
+ vshError(ctl, "%s", _("Failed to re-read domain XML"));
goto cleanup;
+ }
if (STRNEQ (doc, doc_reread)) {
vshError(ctl,
@@ -8608,8 +8694,10 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd)
/* Everything checks out, so redefine the domain. */
virDomainFree (dom);
dom = virDomainDefineXML (ctl->conn, doc_edited);
- if (!dom)
+ if (!dom) {
+ vshError(ctl, "%s", _("Failed to define new domain XML"));
goto cleanup;
+ }
vshPrint (ctl, _("Domain %s XML configuration edited.\n"),
virDomainGetName(dom));
@@ -8731,21 +8819,29 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
}
snapshot = virDomainSnapshotCreateXML(dom, buffer, 0);
- if (snapshot == NULL)
+ if (snapshot == NULL) {
+ vshError(ctl, "%s", _("Failed to create domain snapshot"));
goto cleanup;
+ }
doc = virDomainSnapshotGetXMLDesc(snapshot, 0);
- if (!doc)
+ if (!doc) {
+ vshError(ctl, "%s", _("Failed to get domain snapshot XML"));
goto cleanup;
+ }
xml = xmlReadDoc((const xmlChar *) doc, "domainsnapshot.xml", NULL,
XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOWARNING);
- if (!xml)
+ if (!xml) {
+ vshError(ctl, "%s", _("Failed to parse domain snapshot
XML"));
goto cleanup;
+ }
ctxt = xmlXPathNewContext(xml);
- if (!ctxt)
+ if (!ctxt) {
+ vshError(ctl, "%s", _("Failed to allocate memory"));
goto cleanup;
+ }
name = virXPathString("string(/domainsnapshot/name)", ctxt);
if (!name) {
@@ -8806,17 +8902,23 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
current = virDomainHasCurrentSnapshot(dom, 0);
- if (current < 0)
+ if (current < 0) {
+ vshError(ctl, "%s", _("Failed to get domain current
snapshot"));
goto cleanup;
+ }
else if (current) {
char *xml;
- if (!(snapshot = virDomainSnapshotCurrent(dom, 0)))
+ if (!(snapshot = virDomainSnapshotCurrent(dom, 0))) {
+ vshError(ctl, "%s", _("Failed to fetch domain current
snapshot"));
goto cleanup;
+ }
xml = virDomainSnapshotGetXMLDesc(snapshot, 0);
- if (!xml)
+ if (!xml) {
+ vshError(ctl, "%s", _("Failed to get domain snapshot
XML"));
goto cleanup;
+ }
vshPrint(ctl, "%s", xml);
VIR_FREE(xml);
@@ -8874,19 +8976,25 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
numsnaps = virDomainSnapshotNum(dom, 0);
- if (numsnaps < 0)
+ if (numsnaps < 0) {
+ vshError(ctl, "%s", _("Failed to get number of domain
snapshots"));
goto cleanup;
+ }
vshPrint(ctl, " %-20s %-25s %s\n", _("Name"), _("Creation
Time"), _("State"));
vshPrint(ctl, "---------------------------------------------------\n");
if (numsnaps) {
- if (VIR_ALLOC_N(names, numsnaps) < 0)
+ if (VIR_ALLOC_N(names, numsnaps) < 0) {
+ vshError(ctl, "%s", _("Failed to allocate memory"));
goto cleanup;
+ }
actual = virDomainSnapshotListNames(dom, names, numsnaps, 0);
- if (actual < 0)
+ if (actual < 0) {
+ vshError(ctl, "%s", _("Failed to list domain
snapshots"));
goto cleanup;
+ }
qsort(&names[0], actual, sizeof(char*), namesorter);
@@ -8988,12 +9096,16 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd)
}
snapshot = virDomainSnapshotLookupByName(dom, name, 0);
- if (snapshot == NULL)
+ if (snapshot == NULL) {
+ vshError(ctl, "%s", _("Failed to lookup domain snapshot"));
goto cleanup;
+ }
xml = virDomainSnapshotGetXMLDesc(snapshot, 0);
- if (!xml)
+ if (!xml) {
+ vshError(ctl, "%s", _("Failed to get domain snapshot XML"));
goto cleanup;
+ }
vshPrint(ctl, "%s", xml);
@@ -9046,11 +9158,15 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd)
}
snapshot = virDomainSnapshotLookupByName(dom, name, 0);
- if (snapshot == NULL)
+ if (snapshot == NULL) {
+ vshError(ctl, "%s", _("Failed to lookup domain snapshot"));
goto cleanup;
+ }
- if (virDomainRevertToSnapshot(snapshot, 0) < 0)
+ if (virDomainRevertToSnapshot(snapshot, 0) < 0) {
+ vshError(ctl, "%s", _("Failed to revert to domain
snapshot"));
goto cleanup;
+ }
ret = TRUE;
@@ -9105,11 +9221,15 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN;
snapshot = virDomainSnapshotLookupByName(dom, name, 0);
- if (snapshot == NULL)
+ if (snapshot == NULL) {
+ vshError(ctl, "%s", _("Failed to lookup domain snapshot"));
goto cleanup;
+ }
- if (virDomainSnapshotDelete(snapshot, flags) < 0)
+ if (virDomainSnapshotDelete(snapshot, flags) < 0) {
+ vshError(ctl, "%s", _("Failed to delete domain snapshot"));
goto cleanup;
+ }
ret = TRUE;
--
1.7.1.1
11:07 a.m.
New subject: [libvirt] [PATCH 5/8] Add additional error reporting to virsh.
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
With the change to make vshError() responsible for printing
all errors, there were some places in the code that would no
longer properly print errors. The good news is that the vast
majority of virsh was already printing errors, so this patch
just cleans up the rest of the users to make them consistent.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 228 +++++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 174 insertions(+), 54 deletions(-)
The patch as is look good, but I wonder how you spot all those places
that missed a vshError call. Did you manually go over the whole
virsh.c file and checked all libvirt API calls?
I just spot calls to virDomainMigrate and virDomainMigrateToURI
(around line 3040) that are not matched with a call to vshError, you
might want to fold that into your patch.
ACK.
Matthias
4:58 a.m.
New subject: [libvirt] [PATCH 5/8] Add additional error reporting to virsh.
On Sat, Jul 17, 2010 at 06:07:30PM +0200, Matthias Bolte wrote:
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
> With the change to make vshError() responsible for printing
> all errors, there were some places in the code that would no
> longer properly print errors. The good news is that the vast
> majority of virsh was already printing errors, so this patch
> just cleans up the rest of the users to make them consistent.
>
> Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
> ---
> tools/virsh.c | 228 +++++++++++++++++++++++++++++++++++++++++++--------------
> 1 files changed, 174 insertions(+), 54 deletions(-)
>
The patch as is look good, but I wonder how you spot all those places
that missed a vshError call. Did you manually go over the whole
virsh.c file and checked all libvirt API calls?
I just spot calls to virDomainMigrate and virDomainMigrateToURI
(around line 3040) that are not matched with a call to vshError, you
might want to fold that into your patch.
I sounds like there should be a final catch all case. In 'main' check
the return value of 'vshCommandRun' and if it is FALSE and vshError
hasn't been invoked then report the error.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
7:52 a.m.
New subject: [libvirt] [PATCH 5/8] Add additional error reporting to virsh.
On 07/17/10 - 06:07:30PM, Matthias Bolte wrote:
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
> With the change to make vshError() responsible for printing
> all errors, there were some places in the code that would no
> longer properly print errors. The good news is that the vast
> majority of virsh was already printing errors, so this patch
> just cleans up the rest of the users to make them consistent.
>
> Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
> ---
> tools/virsh.c | 228 +++++++++++++++++++++++++++++++++++++++++++--------------
> 1 files changed, 174 insertions(+), 54 deletions(-)
>
The patch as is look good, but I wonder how you spot all those places
that missed a vshError call. Did you manually go over the whole
virsh.c file and checked all libvirt API calls?
I just spot calls to virDomainMigrate and virDomainMigrateToURI
(around line 3040) that are not matched with a call to vshError, you
might want to fold that into your patch.
Yeah, I did just go over the file visually, which is why I probably missed a
couple :). Dan's suggestion for a catch-all case is not bad; it's a
programming error when it is hit, but at least we will get *something* out on
an error. I'll look at doing that.
--
Chris Lalancette
2:59 p.m.
New subject: [libvirt] [PATCH 6/8] Add vshFileReadAll() helper.
This helper calls virFileReadAll(), and then outputs a
proper error if it fails for whatever reason.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 65 ++++++++++++++++++++++++++++++++------------------------
1 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 3c48eee..02ab7d3 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -294,6 +294,9 @@ static void vshDebug(vshControl *ctl, int level, const char *format,
...)
/* XXX: add batch support */
#define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__)
+static int vshFileReadAll(vshControl *ctl, const char *filename, int size,
+ char **buffer);
+
static const char *vshDomainStateToString(int state);
static const char *vshDomainVcpuStateToString(int state);
static int vshConnectionUsability(vshControl *ctl, virConnectPtr conn);
@@ -1154,7 +1157,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd)
if (!found)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
if (vshCommandOptBool(cmd, "paused"))
@@ -1208,7 +1211,7 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd)
if (!found)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
dom = virDomainDefineXML(ctl->conn, buffer);
@@ -2770,7 +2773,7 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd)
format = vshCommandOptString(cmd, "format", NULL);
configFile = vshCommandOptString(cmd, "config", NULL);
- if (virFileReadAll(configFile, 1024*1024, &configData) < 0)
+ if (vshFileReadAll(ctl, configFile, 1024*1024, &configData) < 0)
return FALSE;
xmlData = virConnectDomainXMLFromNative(ctl->conn, format, configData, flags);
@@ -2816,7 +2819,7 @@ cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
format = vshCommandOptString(cmd, "format", NULL);
xmlFile = vshCommandOptString(cmd, "xml", NULL);
- if (virFileReadAll(xmlFile, 1024*1024, &xmlData) < 0)
+ if (vshFileReadAll(ctl, xmlFile, 1024*1024, &xmlData) < 0)
return FALSE;
configData = virConnectDomainXMLToNative(ctl->conn, format, xmlData, flags);
@@ -3165,7 +3168,7 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd)
if (!found)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
network = virNetworkCreateXML(ctl->conn, buffer);
@@ -3213,7 +3216,7 @@ cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd)
if (!found)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
network = virNetworkDefineXML(ctl->conn, buffer);
@@ -3936,7 +3939,7 @@ cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd)
if (!found)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
iface = virInterfaceDefineXML(ctl->conn, buffer, 0);
@@ -4098,7 +4101,7 @@ cmdNWFilterDefine(vshControl *ctl, const vshCmd *cmd)
if (!found)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
nwfilter = virNWFilterDefineXML(ctl->conn, buffer);
@@ -4446,7 +4449,7 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
if (!found)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
pool = virStoragePoolCreateXML(ctl->conn, buffer, 0);
@@ -4499,7 +4502,7 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd)
return FALSE;
}
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
dev = virNodeDeviceCreateXML(ctl->conn, buffer, 0);
@@ -4711,7 +4714,7 @@ cmdPoolDefine(vshControl *ctl, const vshCmd *cmd)
if (!found)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
pool = virStoragePoolDefineXML(ctl->conn, buffer, 0);
@@ -5490,7 +5493,7 @@ cmdPoolDiscoverSources(vshControl * ctl, const vshCmd * cmd
ATTRIBUTE_UNUSED)
if (!vshConnectionUsability(ctl, ctl->conn))
return FALSE;
- if (srcSpecFile && virFileReadAll(srcSpecFile, VIRSH_MAX_XML_FILE,
&srcSpec) < 0)
+ if (srcSpecFile && vshFileReadAll(ctl, srcSpecFile, VIRSH_MAX_XML_FILE,
&srcSpec) < 0)
return FALSE;
srcList = virConnectFindStoragePoolSources(ctl->conn, type, srcSpec, 0);
@@ -5967,7 +5970,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
return FALSE;
}
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
virStoragePoolFree(pool);
return FALSE;
}
@@ -6028,9 +6031,8 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
if (!(inputvol = vshCommandOptVol(ctl, cmd, "vol", "inputpool",
NULL)))
goto cleanup;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
goto cleanup;
- }
newvol = virStorageVolCreateXMLFrom(pool, buffer, inputvol, 0);
@@ -6603,7 +6605,7 @@ cmdSecretDefine(vshControl *ctl, const vshCmd *cmd)
if (!from)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
res = virSecretDefineXML(ctl->conn, buffer, 0);
@@ -7545,7 +7547,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
return FALSE;
}
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
virDomainFree(dom);
return FALSE;
}
@@ -7612,7 +7614,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
return FALSE;
}
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
virDomainFree(dom);
return FALSE;
}
@@ -7679,7 +7681,7 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
return FALSE;
}
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
virDomainFree(dom);
return FALSE;
}
@@ -8269,7 +8271,7 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
if (!found)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
result = virConnectCompareCPU(ctl->conn, buffer, 0);
@@ -8342,7 +8344,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
if (!found)
return FALSE;
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
doc = xmlNewDoc(NULL);
@@ -8521,12 +8523,8 @@ editReadBackFile (vshControl *ctl, const char *filename)
{
char *ret;
- if (virFileReadAll (filename, VIRSH_MAX_XML_FILE, &ret) == -1) {
- vshError(ctl,
- _("%s: failed to read temporary file: %s"),
- filename, strerror(errno));
+ if (vshFileReadAll (ctl, filename, VIRSH_MAX_XML_FILE, &ret) == -1)
return NULL;
- }
return ret;
}
@@ -8809,9 +8807,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
if (from == NULL)
buffer = strdup("<domainsnapshot/>");
else {
- if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
+ if (vshFileReadAll(ctl, from, VIRSH_MAX_XML_FILE, &buffer) < 0)
goto cleanup;
- }
}
if (buffer == NULL) {
vshError(ctl, "%s", _("Out of memory"));
@@ -10430,6 +10427,18 @@ _vshStrdup(vshControl *ctl, const char *s, const char *filename,
int line)
exit(EXIT_FAILURE);
}
+static int
+vshFileReadAll(vshControl *ctl, const char *filename, int size, char **buffer)
+{
+ int ret;
+
+ ret = virFileReadAll(filename, size, buffer);
+ if (ret < 0)
+ vshError(ctl, _("Failed to read contents of '%s'"), filename);
+
+ return ret;
+}
+
/*
* Initialize connection.
*/
--
1.7.1.1
11:07 a.m.
New subject: [libvirt] [PATCH 6/8] Add vshFileReadAll() helper.
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
This helper calls virFileReadAll(), and then outputs a
proper error if it fails for whatever reason.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 65 ++++++++++++++++++++++++++++++++------------------------
1 files changed, 37 insertions(+), 28 deletions(-)
See comments on patch 4/8.
ACK.
Matthias
2:59 p.m.
New subject: [libvirt] [PATCH 7/8] Fix up inconsistent virsh option error reporting.
The virsh option error reporting was not being used
consistently; some commands would spit out errors on
missing required options while others would just silently fail.
However, vshCommandOptString knows which ones are required
and which ones aren't, so make it spit out an error where
appropriate. The rest of the patch is just cleaning up
the uses of vshCommandOptString to deal with the new error
reporting.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 66 ++++++++++++++++++++------------------------------------
1 files changed, 24 insertions(+), 42 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 02ab7d3..6a22071 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -571,6 +571,7 @@ static int
cmdConnect(vshControl *ctl, const vshCmd *cmd)
{
int ro = vshCommandOptBool(cmd, "readonly");
+ char *name;
if (ctl->conn) {
if (virConnectClose(ctl->conn) != 0) {
@@ -581,7 +582,10 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
}
VIR_FREE(ctl->name);
- ctl->name = vshStrdup(ctl, vshCommandOptString(cmd, "name", NULL));
+ name = vshCommandOptString(cmd, "name", NULL);
+ if (!name)
+ return FALSE;
+ ctl->name = vshStrdup(ctl, name);
if (!ro) {
ctl->readonly = 0;
@@ -2359,7 +2363,6 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd)
}
if (!(cpulist = vshCommandOptString(cmd, "cpulist", NULL))) {
- vshError(ctl, "%s", _("vcpupin: Missing cpulist"));
virDomainFree(dom);
return FALSE;
}
@@ -2976,10 +2979,8 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd)
return FALSE;
desturi = vshCommandOptString (cmd, "desturi", &found);
- if (!found) {
- vshError(ctl, "%s", _("migrate: Missing desturi"));
+ if (!found)
goto done;
- }
migrateuri = vshCommandOptString (cmd, "migrateuri", NULL);
@@ -7542,7 +7543,6 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
from = vshCommandOptString(cmd, "file", &found);
if (!found) {
- vshError(ctl, "%s", _("attach-device: Missing <file>
option"));
virDomainFree(dom);
return FALSE;
}
@@ -7609,7 +7609,6 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
from = vshCommandOptString(cmd, "file", &found);
if (!found) {
- vshError(ctl, "%s", _("detach-device: Missing <file>
option"));
virDomainFree(dom);
return FALSE;
}
@@ -7676,7 +7675,6 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
from = vshCommandOptString(cmd, "file", &found);
if (!found) {
- vshError(ctl, "%s", _("update-device: Missing <file>
option"));
virDomainFree(dom);
return FALSE;
}
@@ -9087,10 +9085,8 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
name = vshCommandOptString(cmd, "snapshotname", NULL);
- if (name == NULL) {
- vshError(ctl, "%s", _("missing snapshotname"));
+ if (name == NULL)
goto cleanup;
- }
snapshot = virDomainSnapshotLookupByName(dom, name, 0);
if (snapshot == NULL) {
@@ -9149,10 +9145,8 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
name = vshCommandOptString(cmd, "snapshotname", NULL);
- if (name == NULL) {
- vshError(ctl, "%s", _("missing snapshotname"));
+ if (name == NULL)
goto cleanup;
- }
snapshot = virDomainSnapshotLookupByName(dom, name, 0);
if (snapshot == NULL) {
@@ -9209,10 +9203,8 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
name = vshCommandOptString(cmd, "snapshotname", NULL);
- if (name == NULL) {
- vshError(ctl, "%s", _("missing snapshotname"));
+ if (name == NULL)
goto cleanup;
- }
if (vshCommandOptBool(cmd, "children"))
flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN;
@@ -9647,7 +9639,13 @@ vshCommandOptString(const vshCmd *cmd, const char *name, int
*found)
if (found)
*found = arg ? TRUE : FALSE;
- return arg && arg->data && *arg->data ? arg->data : NULL;
+ if (arg && arg->data && *arg->data)
+ return arg->data;
+
+ if ((arg->def->flag) & VSH_OFLAG_REQ)
+ vshError(NULL, _("Missing required option '%s'"), name);
+
+ return NULL;
}
/*
@@ -9711,10 +9709,8 @@ vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd,
if (!cmd_has_option (ctl, cmd, optname))
return NULL;
- if (!(n = vshCommandOptString(cmd, optname, NULL))) {
- vshError(ctl, "%s", _("undefined domain name or id"));
+ if (!(n = vshCommandOptString(cmd, optname, NULL)))
return NULL;
- }
vshDebug(ctl, 5, "%s: found option <%s>: %s\n",
cmd->def->name, optname, n);
@@ -9759,10 +9755,8 @@ vshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd,
if (!cmd_has_option (ctl, cmd, optname))
return NULL;
- if (!(n = vshCommandOptString(cmd, optname, NULL))) {
- vshError(ctl, "%s", _("undefined network name"));
+ if (!(n = vshCommandOptString(cmd, optname, NULL)))
return NULL;
- }
vshDebug(ctl, 5, "%s: found option <%s>: %s\n",
cmd->def->name, optname, n);
@@ -9800,10 +9794,8 @@ vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd,
if (!cmd_has_option (ctl, cmd, optname))
return NULL;
- if (!(n = vshCommandOptString(cmd, optname, NULL))) {
- vshError(ctl, "%s", _("undefined nwfilter name"));
+ if (!(n = vshCommandOptString(cmd, optname, NULL)))
return NULL;
- }
vshDebug(ctl, 5, "%s: found option <%s>: %s\n",
cmd->def->name, optname, n);
@@ -9840,10 +9832,8 @@ vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd,
if (!cmd_has_option (ctl, cmd, optname))
return NULL;
- if (!(n = vshCommandOptString(cmd, optname, NULL))) {
- vshError(ctl, "%s", _("undefined interface identifier"));
+ if (!(n = vshCommandOptString(cmd, optname, NULL)))
return NULL;
- }
vshDebug(ctl, 5, "%s: found option <%s>: %s\n",
cmd->def->name, optname, n);
@@ -9877,10 +9867,8 @@ vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char
*optname,
virStoragePoolPtr pool = NULL;
char *n;
- if (!(n = vshCommandOptString(cmd, optname, NULL))) {
- vshError(ctl, "%s", _("undefined pool name"));
+ if (!(n = vshCommandOptString(cmd, optname, NULL)))
return NULL;
- }
vshDebug(ctl, 5, "%s: found option <%s>: %s\n",
cmd->def->name, optname, n);
@@ -9918,15 +9906,11 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
char *n, *p;
int found;
- if (!(n = vshCommandOptString(cmd, optname, NULL))) {
- vshError(ctl, "%s", _("undefined vol name"));
+ if (!(n = vshCommandOptString(cmd, optname, NULL)))
return NULL;
- }
- if (!(p = vshCommandOptString(cmd, pooloptname, &found)) && found) {
- vshError(ctl, "%s", _("undefined pool name"));
+ if (!(p = vshCommandOptString(cmd, pooloptname, &found)) && found)
return NULL;
- }
if (p)
pool = vshCommandOptPoolBy(ctl, cmd, pooloptname, name, flag);
@@ -9976,10 +9960,8 @@ vshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, char
**name)
return NULL;
n = vshCommandOptString(cmd, optname, NULL);
- if (n == NULL) {
- vshError(ctl, "%s", _("undefined secret UUID"));
+ if (n == NULL)
return NULL;
- }
vshDebug(ctl, 5, "%s: found option <%s>: %s\n", cmd->def->name,
optname, n);
--
1.7.1.1
11:08 a.m.
New subject: [libvirt] [PATCH 7/8] Fix up inconsistent virsh option error reporting.
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
The virsh option error reporting was not being used
consistently; some commands would spit out errors on
missing required options while others would just silently fail.
However, vshCommandOptString knows which ones are required
and which ones aren't, so make it spit out an error where
appropriate. The rest of the patch is just cleaning up
the uses of vshCommandOptString to deal with the new error
reporting.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 66 ++++++++++++++++++++------------------------------------
1 files changed, 24 insertions(+), 42 deletions(-)
ACK.
Matthias
1:51 p.m.
New subject: [libvirt] [PATCH 7/8] Fix up inconsistent virsh option error reporting.
On 07/17/10 - 06:08:03PM, Matthias Bolte wrote:
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
> The virsh option error reporting was not being used
> consistently; some commands would spit out errors on
> missing required options while others would just silently fail.
> However, vshCommandOptString knows which ones are required
> and which ones aren't, so make it spit out an error where
> appropriate. The rest of the patch is just cleaning up
> the uses of vshCommandOptString to deal with the new error
> reporting.
>
> Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
> ---
> tools/virsh.c | 66 ++++++++++++++++++++------------------------------------
> 1 files changed, 24 insertions(+), 42 deletions(-)
>
ACK.
Thanks, I pushed this one.
--
Chris Lalancette
2:59 p.m.
New subject: [libvirt] [PATCH 8/8] Always clear out the last_error in virshReportError.
Otherwise you can get bogus "unknown error" printouts on
subsequent commands.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 6a22071..6b7580b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -390,7 +390,7 @@ virshReportError(void)
* no error was ever raised, so just ignore */
last_error = virSaveLastError();
if (!last_error || last_error->code == VIR_ERR_OK)
- return;
+ goto out;
}
if (last_error->code == VIR_ERR_OK) {
--
1.7.1.1
11:08 a.m.
New subject: [libvirt] [PATCH 8/8] Always clear out the last_error in virshReportError.
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
Otherwise you can get bogus "unknown error" printouts on
subsequent commands.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
tools/virsh.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 6a22071..6b7580b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -390,7 +390,7 @@ virshReportError(void)
* no error was ever raised, so just ignore */
last_error = virSaveLastError();
if (!last_error || last_error->code == VIR_ERR_OK)
- return;
+ goto out;
}
if (last_error->code == VIR_ERR_OK) {
--
1.7.1.1
ACK.
Matthias
1:56 p.m.
New subject: [libvirt] [PATCH 8/8] Always clear out the last_error in virshReportError.
On 07/17/10 - 06:08:17PM, Matthias Bolte wrote:
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
> Otherwise you can get bogus "unknown error" printouts on
> subsequent commands.
>
> Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
> ---
> tools/virsh.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 6a22071..6b7580b 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -390,7 +390,7 @@ virshReportError(void)
> * no error was ever raised, so just ignore */
> last_error = virSaveLastError();
> if (!last_error || last_error->code == VIR_ERR_OK)
> - return;
> + goto out;
> }
>
> if (last_error->code == VIR_ERR_OK) {
> --
> 1.7.1.1
ACK.
Thanks, I also pushed this one.
--
Chris Lalancette
5240
days inactive
5251
days old
24 comments
3 participants
participants (3)
-
Chris Lalancette -
Daniel P. Berrange -
Matthias Bolte