12:07 p.m.
This patchset fixes retrieval of string arguments in virsh to report sane
errors in all the places it's used. A new string option retrieval function is
added that supports error reporting and changes appropriate places to use it.
Along with that this series cleans up a few places with old code and bad style.
*** BLURB HERE ***
Peter Krempa (17):
virsh-snapshot: Fix coding style and remove dead code
virsh-volume: Rename helper function makeCloneXML to vshMakeCloneXML
virsh-pool: Rename helper function buildPoolXML to vshBuildPoolXML
virsh: Add helper to request string arguments with error reporting
virsh-pool: Update pool commands to use vshCommandOptStringReq
virsh-volume: Update volume commands to use vshCommandOptStringReq
virsh-domain-monitor: Update domain commands to use
vshCommandOptStringReq
virsh-domain: Update domain commands to use vshCommandOptStringReq
virsh-host: Update host commands to use vshCommandOptStringReq
virsh-network: Update network commands to use vshCommandOptStringReq
virsh-interface: Update interface commands to use
vshCommandOptStringReq
virsh-nodedev: Update node device commands to use
vshCommandOptStringReq
virsh-nodedev: Refactor error paths, error messages and whitespace
virsh-snapshot: Refactor cmdSnapshotDumpXML
virsh-snapshot: Update snapshot commands to use vshCommandOptStringReq
virsh-nwfilter: Update nwfilter commands to use vshCommandOptStringReq
virsh-secret: Refactor error paths
tools/virsh-domain-monitor.c | 44 +++----
tools/virsh-domain.c | 299 +++++++++++++++++--------------------------
tools/virsh-host.c | 9 +-
tools/virsh-interface.c | 8 +-
tools/virsh-network.c | 19 +--
tools/virsh-nodedev.c | 44 ++++---
tools/virsh-nwfilter.c | 4 +-
tools/virsh-pool.c | 56 ++++----
tools/virsh-secret.c | 30 +++--
tools/virsh-snapshot.c | 55 +++-----
tools/virsh-volume.c | 73 +++++------
tools/virsh.c | 51 ++++++++
tools/virsh.h | 4 +
13 files changed, 320 insertions(+), 376 deletions(-)
--
1.8.1.1
12:07 p.m.
New subject: [libvirt] [PATCH 01/17] virsh-snapshot: Fix coding style and remove dead code
buffer won't be NULL in any case when reaching the check and the first
block of the if statement was lacking parentheses
---
tools/virsh-snapshot.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index e4745d6..f76c40e 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -204,26 +204,17 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "live"))
flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE;
- dom = vshCommandOptDomain(ctl, cmd, NULL);
- if (dom == NULL)
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "xmlfile", &from) <= 0)
+ if (vshCommandOptString(cmd, "xmlfile", &from) <= 0) {
buffer = vshStrdup(ctl, "<domainsnapshot/>");
- else {
+ } else {
if (virFileReadAll(from, VSH_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
- */
- vshReportError(ctl);
+ vshSaveLibvirtError();
goto cleanup;
}
}
- if (buffer == NULL) {
- vshError(ctl, "%s", _("Out of memory"));
- goto cleanup;
- }
ret = vshSnapshotCreate(ctl, dom, buffer, flags, from);
--
1.8.1.1
10:43 p.m.
New subject: [libvirt] [PATCH 01/17] virsh-snapshot: Fix coding style and remove dead code
On 2013年01月22日 02:07, Peter Krempa wrote:
buffer won't be NULL in any case when reaching the check and the
first
block of the if statement was lacking parentheses
---
tools/virsh-snapshot.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index e4745d6..f76c40e 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -204,26 +204,17 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "live"))
flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE;
- dom = vshCommandOptDomain(ctl, cmd, NULL);
- if (dom == NULL)
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "xmlfile",&from)<= 0)
+ if (vshCommandOptString(cmd, "xmlfile",&from)<= 0) {
buffer = vshStrdup(ctl, "<domainsnapshot/>");
- else {
+ } else {
if (virFileReadAll(from, VSH_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
- */
- vshReportError(ctl);
+ vshSaveLibvirtError();
goto cleanup;
}
}
- if (buffer == NULL) {
- vshError(ctl, "%s", _("Out of memory"));
- goto cleanup;
- }
ret = vshSnapshotCreate(ctl, dom, buffer, flags, from);
ACK.
12:07 p.m.
New subject: [libvirt] [PATCH 02/17] virsh-volume: Rename helper function makeCloneXML to vshMakeCloneXML
---
tools/virsh-volume.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index e17d946..466188c 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -464,7 +464,7 @@ cleanup:
}
static xmlChar *
-makeCloneXML(const char *origxml, const char *newname)
+vshMakeCloneXML(const char *origxml, const char *newname)
{
xmlDocPtr doc = NULL;
@@ -555,7 +555,7 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
if (!origxml)
goto cleanup;
- newxml = makeCloneXML(origxml, name);
+ newxml = vshMakeCloneXML(origxml, name);
if (!newxml) {
vshPrint(ctl, "%s", _("Failed to allocate XML buffer"));
goto cleanup;
--
1.8.1.1
10:44 p.m.
New subject: [libvirt] [PATCH 02/17] virsh-volume: Rename helper function makeCloneXML to vshMakeCloneXML
On 2013年01月22日 02:07, Peter Krempa wrote:
---
tools/virsh-volume.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index e17d946..466188c 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -464,7 +464,7 @@ cleanup:
}
static xmlChar *
-makeCloneXML(const char *origxml, const char *newname)
+vshMakeCloneXML(const char *origxml, const char *newname)
{
xmlDocPtr doc = NULL;
@@ -555,7 +555,7 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
if (!origxml)
goto cleanup;
- newxml = makeCloneXML(origxml, name);
+ newxml = vshMakeCloneXML(origxml, name);
if (!newxml) {
vshPrint(ctl, "%s", _("Failed to allocate XML buffer"));
goto cleanup;
ACK
12:07 p.m.
New subject: [libvirt] [PATCH 03/17] virsh-pool: Rename helper function buildPoolXML to vshBuildPoolXML
---
tools/virsh-pool.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index cb9866f..bc400a1 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -226,7 +226,7 @@ static const vshCmdOptDef opts_pool_X_as[] = {
{.name = NULL}
};
-static int buildPoolXML(const vshCmd *cmd, const char **retname, char **xml) {
+static int vshBuildPoolXML(const vshCmd *cmd, const char **retname, char **xml) {
const char *name = NULL, *type = NULL, *srcHost = NULL, *srcPath = NULL,
*srcDev = NULL, *srcName = NULL, *srcFormat = NULL, *target = NULL;
@@ -303,7 +303,7 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
char *xml;
bool printXML = vshCommandOptBool(cmd, "print-xml");
- if (!buildPoolXML(cmd, &name, &xml))
+ if (!vshBuildPoolXML(cmd, &name, &xml))
return false;
if (printXML) {
@@ -387,7 +387,7 @@ cmdPoolDefineAs(vshControl *ctl, const vshCmd *cmd)
char *xml;
bool printXML = vshCommandOptBool(cmd, "print-xml");
- if (!buildPoolXML(cmd, &name, &xml))
+ if (!vshBuildPoolXML(cmd, &name, &xml))
return false;
if (printXML) {
--
1.8.1.1
10:45 p.m.
New subject: [libvirt] [PATCH 03/17] virsh-pool: Rename helper function buildPoolXML to vshBuildPoolXML
On 2013年01月22日 02:07, Peter Krempa wrote:
---
tools/virsh-pool.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index cb9866f..bc400a1 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -226,7 +226,7 @@ static const vshCmdOptDef opts_pool_X_as[] = {
{.name = NULL}
};
-static int buildPoolXML(const vshCmd *cmd, const char **retname, char **xml) {
+static int vshBuildPoolXML(const vshCmd *cmd, const char **retname, char **xml) {
Deserved to changed the style meanwhile:
static int
vshBuildPoolXML (...)
const char *name = NULL, *type = NULL, *srcHost = NULL, *srcPath = NULL,
*srcDev = NULL, *srcName = NULL, *srcFormat = NULL, *target = NULL;
@@ -303,7 +303,7 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
char *xml;
bool printXML = vshCommandOptBool(cmd, "print-xml");
- if (!buildPoolXML(cmd,&name,&xml))
+ if (!vshBuildPoolXML(cmd,&name,&xml))
return false;
if (printXML) {
@@ -387,7 +387,7 @@ cmdPoolDefineAs(vshControl *ctl, const vshCmd *cmd)
char *xml;
bool printXML = vshCommandOptBool(cmd, "print-xml");
- if (!buildPoolXML(cmd,&name,&xml))
+ if (!vshBuildPoolXML(cmd,&name,&xml))
return false;
if (printXML) {
ACK with the style change.
12:07 p.m.
New subject: [libvirt] [PATCH 04/17] virsh: Add helper to request string arguments with error reporting
This patch adds a helper function with similar semantics to
vshCommandOptString that requests a string argument, but does some error
reporting without the need to do it in the functions themselves.
The error reporting also provides information about the parameter whose
retrieval failed.
---
tools/virsh.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
tools/virsh.h | 4 ++++
2 files changed, 55 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c
index 908c6a1..1a3cab0 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1417,6 +1417,57 @@ vshCommandOptString(const vshCmd *cmd, const char *name, const char
**value)
}
/**
+ * vshCommandOptStringReq: Get a required string argumment
+ * @ctl virsh control structure
+ * @cmd command structure
+ * @name option name
+ * @value result (updated to NULL or the actual value)
+ *
+ * Returns option as string. Prints error message if needed
+ * Return value:
+ * 0 if option is correct (available or not required)
+ * -1 and error message printed on error
+ */
+int
+vshCommandOptStringReq(vshControl *ctl,
+ const vshCmd *cmd,
+ const char *name,
+ const char **value)
+{
+ vshCmdOpt *arg;
+ int ret;
+ const char *error = NULL;
+
+ /* clear out the value */
+ *value = NULL;
+
+ ret = vshCommandOpt(cmd, name, &arg);
+ /* option is not required and not present */
+ if (ret == 0)
+ return 0;
+ /* this should not be propagated here, just to be sure */
+ if (ret == -1)
+ error = N_("Mandatory option not present");
+
+ if (ret == -2)
+ error = N_("Programming error: Invalid option name");
+
+ if (!arg->data)
+ error = N_("Programming error: Requested option is a boolean");
+
+ if (!*arg->data && !(arg->def->flags & VSH_OFLAG_EMPTY_OK))
+ error = N_("Option argument is empty");
+
+ if (error) {
+ vshError(ctl, _("Failed to get option '%s': %s"), name,
_(error));
+ return -1;
+ }
+
+ *value = arg->data;
+ return 0;
+}
+
+/**
* vshCommandOptLongLong:
* @cmd command reference
* @name option name
diff --git a/tools/virsh.h b/tools/virsh.h
index ab7161f..fec9785 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -275,6 +275,10 @@ int vshCommandOptUL(const vshCmd *cmd, const char *name,
int vshCommandOptString(const vshCmd *cmd, const char *name,
const char **value)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
+int vshCommandOptStringReq(vshControl *ctl, const vshCmd *cmd,
+ const char *name, const char **value)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK;
int vshCommandOptLongLong(const vshCmd *cmd, const char *name,
long long *value)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
--
1.8.1.1
11:18 p.m.
New subject: [libvirt] [PATCH 04/17] virsh: Add helper to request string arguments with error reporting
On 2013年01月22日 02:07, Peter Krempa wrote:
This patch adds a helper function with similar semantics to
vshCommandOptString that requests a string argument, but does some error
reporting without the need to do it in the functions themselves.
The error reporting also provides information about the parameter whose
retrieval failed.
---
tools/virsh.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
tools/virsh.h | 4 ++++
2 files changed, 55 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c
index 908c6a1..1a3cab0 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1417,6 +1417,57 @@ vshCommandOptString(const vshCmd *cmd, const char *name, const
char **value)
}
/**
+ * vshCommandOptStringReq: Get a required string argumment
Trivial, but we usually describe what the function does at [1].
+ * @ctl virsh control structure
And have a ":" after @foo.
+ * @cmd command structure
+ * @name option name
+ * @value result (updated to NULL or the actual value)
s/actual value/option argument/,
+ *
[1]. Right here.
+ * Returns option as string. Prints error message if needed
I think you mean option argument here.
+ * Return value:
+ * 0 if option is correct (available or not required)
+ * -1 and error message printed on error
Not to be captious. But how about rewording above like:
Returns 0 on success or the option is not present and not
requirent, *value is set to the option argument if success;
Or -1 on failure, with error message printed.
+ */
+int
+vshCommandOptStringReq(vshControl *ctl,
+ const vshCmd *cmd,
+ const char *name,
+ const char **value)
+{
+ vshCmdOpt *arg;
+ int ret;
+ const char *error = NULL;
+
+ /* clear out the value */
+ *value = NULL;
+
+ ret = vshCommandOpt(cmd, name,&arg);
+ /* option is not required and not present */
+ if (ret == 0)
+ return 0;
+ /* this should not be propagated here, just to be sure */
+ if (ret == -1)
+ error = N_("Mandatory option not present");
+
+ if (ret == -2)
+ error = N_("Programming error: Invalid option name");
+
+ if (!arg->data)
+ error = N_("Programming error: Requested option is a boolean");
+
+ if (!*arg->data&& !(arg->def->flags& VSH_OFLAG_EMPTY_OK))
+ error = N_("Option argument is empty");
+
+ if (error) {
+ vshError(ctl, _("Failed to get option '%s': %s"), name,
_(error));
+ return -1;
+ }
+
+ *value = arg->data;
+ return 0;
+}
+
+/**
* vshCommandOptLongLong:
* @cmd command reference
* @name option name
diff --git a/tools/virsh.h b/tools/virsh.h
index ab7161f..fec9785 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -275,6 +275,10 @@ int vshCommandOptUL(const vshCmd *cmd, const char *name,
int vshCommandOptString(const vshCmd *cmd, const char *name,
const char **value)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
+int vshCommandOptStringReq(vshControl *ctl, const vshCmd *cmd,
+ const char *name, const char **value)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK;
int vshCommandOptLongLong(const vshCmd *cmd, const char *name,
long long *value)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
ACK with the small nits fixed.
4:47 a.m.
New subject: [libvirt] [PATCH 04/17] virsh: Add helper to request string arguments with error reporting
On 01/31/13 06:18, Osier Yang wrote:
On 2013年01月22日 02:07, Peter Krempa wrote:
> This patch adds a helper function with similar semantics to
> vshCommandOptString that requests a string argument, but does some error
> reporting without the need to do it in the functions themselves.
>
> The error reporting also provides information about the parameter whose
> retrieval failed.
> ---
> tools/virsh.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/virsh.h | 4 ++++
> 2 files changed, 55 insertions(+)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 908c6a1..1a3cab0 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -1417,6 +1417,57 @@ vshCommandOptString(const vshCmd *cmd, const
> char *name, const char **value)
> }
>
> /**
> + * vshCommandOptStringReq: Get a required string argumment
Trivial, but we usually describe what the function does at [1].
> + * @ctl virsh control structure
And have a ":" after @foo.
Not in virsh.c. All surrounding functions don't have the colon in the
comment. This is worth cleaning up separately instead of doing it in
multiple ways in a single file.
> + * @cmd command structure
> + * @name option name
> + * @value result (updated to NULL or the actual value)
s/actual value/option argument/,
> + *
[1]. Right here.
Peter
5:28 a.m.
New subject: [libvirt] [PATCH 04/17] virsh: Add helper to request string arguments with error reporting
On 2013年02月04日 18:47, Peter Krempa wrote:
On 01/31/13 06:18, Osier Yang wrote:
> On 2013年01月22日 02:07, Peter Krempa wrote:
>> This patch adds a helper function with similar semantics to
>> vshCommandOptString that requests a string argument, but does some error
>> reporting without the need to do it in the functions themselves.
>>
>> The error reporting also provides information about the parameter whose
>> retrieval failed.
>> ---
>> tools/virsh.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> tools/virsh.h | 4 ++++
>> 2 files changed, 55 insertions(+)
>>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index 908c6a1..1a3cab0 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -1417,6 +1417,57 @@ vshCommandOptString(const vshCmd *cmd, const
>> char *name, const char **value)
>> }
>>
>> /**
>> + * vshCommandOptStringReq: Get a required string argumment
>
> Trivial, but we usually describe what the function does at [1].
>
>> + * @ctl virsh control structure
>
> And have a ":" after @foo.
Not in virsh.c. All surrounding functions don't have the colon in the
comment. This is worth cleaning up separately instead of doing it in
multiple ways in a single file.
Agreed, a follow up patch will be good then. But others still stands.
>
>> + * @cmd command structure
>> + * @name option name
>> + * @value result (updated to NULL or the actual value)
>
> s/actual value/option argument/,
>
>> + *
>
> [1]. Right here.
>
Peter
5:32 a.m.
New subject: [libvirt] [PATCH 04/17] virsh: Add helper to request string arguments with error reporting
On 02/04/13 12:28, Osier Yang wrote:
On 2013年02月04日 18:47, Peter Krempa wrote:
> On 01/31/13 06:18, Osier Yang wrote:
>> On 2013年01月22日 02:07, Peter Krempa wrote:
>>> This patch adds a helper function with similar semantics to
>>> vshCommandOptString that requests a string argument, but does some
>>> error
>>> reporting without the need to do it in the functions themselves.
>>>
>>> The error reporting also provides information about the parameter whose
>>> retrieval failed.
>>> ---
>>> tools/virsh.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>> tools/virsh.h | 4 ++++
>>> 2 files changed, 55 insertions(+)
>>>
>>> diff --git a/tools/virsh.c b/tools/virsh.c
>>> index 908c6a1..1a3cab0 100644
>>> --- a/tools/virsh.c
>>> +++ b/tools/virsh.c
>>> @@ -1417,6 +1417,57 @@ vshCommandOptString(const vshCmd *cmd, const
>>> char *name, const char **value)
>>> }
>>>
>>> /**
>>> + * vshCommandOptStringReq: Get a required string argumment
>>
>> Trivial, but we usually describe what the function does at [1].
>>
>>> + * @ctl virsh control structure
>>
>> And have a ":" after @foo.
>
> Not in virsh.c. All surrounding functions don't have the colon in the
> comment. This is worth cleaning up separately instead of doing it in
> multiple ways in a single file.
Agreed, a follow up patch will be good then. But others still stands.
I changed the comment to:
/**
+ * vshCommandOptStringReq:
+ * @ctl virsh control structure
+ * @cmd command structure
+ * @name option name
+ * @value result (updated to NULL or the option argument)
+ *
+ * Gets a option argument as string. This function reports errors.
+ *
+ * Returns 0 on success or when the option is not present and not
+ * required, *value is set to the option argument. On error -1 is
+ * returned and error message printed.
+ */
Peter
5:35 a.m.
New subject: [libvirt] [PATCH 04/17] virsh: Add helper to request string arguments with error reporting
On 2013年02月04日 19:32, Peter Krempa wrote:
On 02/04/13 12:28, Osier Yang wrote:
> On 2013年02月04日 18:47, Peter Krempa wrote:
>> On 01/31/13 06:18, Osier Yang wrote:
>>> On 2013年01月22日 02:07, Peter Krempa wrote:
>>>> This patch adds a helper function with similar semantics to
>>>> vshCommandOptString that requests a string argument, but does some
>>>> error
>>>> reporting without the need to do it in the functions themselves.
>>>>
>>>> The error reporting also provides information about the parameter
>>>> whose
>>>> retrieval failed.
>>>> ---
>>>> tools/virsh.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> tools/virsh.h | 4 ++++
>>>> 2 files changed, 55 insertions(+)
>>>>
>>>> diff --git a/tools/virsh.c b/tools/virsh.c
>>>> index 908c6a1..1a3cab0 100644
>>>> --- a/tools/virsh.c
>>>> +++ b/tools/virsh.c
>>>> @@ -1417,6 +1417,57 @@ vshCommandOptString(const vshCmd *cmd, const
>>>> char *name, const char **value)
>>>> }
>>>>
>>>> /**
>>>> + * vshCommandOptStringReq: Get a required string argumment
>>>
>>> Trivial, but we usually describe what the function does at [1].
>>>
>>>> + * @ctl virsh control structure
>>>
>>> And have a ":" after @foo.
>>
>> Not in virsh.c. All surrounding functions don't have the colon in the
>> comment. This is worth cleaning up separately instead of doing it in
>> multiple ways in a single file.
>
> Agreed, a follow up patch will be good then. But others still stands.
I changed the comment to:
/**
+ * vshCommandOptStringReq:
+ * @ctl virsh control structure
+ * @cmd command structure
+ * @name option name
+ * @value result (updated to NULL or the option argument)
+ *
+ * Gets a option argument as string. This function reports errors.
I think "The function reports errors" can just be removed. As it
doesn't report any error on success.
+ *
+ * Returns 0 on success or when the option is not present and not
+ * required, *value is set to the option argument. On error -1 is
+ * returned and error message printed.
+ */
Peter
ACK with that.
7:20 a.m.
New subject: [libvirt] [PATCH 04/17] virsh: Add helper to request string arguments with error reporting
On 02/04/13 12:35, Osier Yang wrote:
On 2013年02月04日 19:32, Peter Krempa wrote:
> On 02/04/13 12:28, Osier Yang wrote:
>> On 2013年02月04日 18:47, Peter Krempa wrote:
>>> On 01/31/13 06:18, Osier Yang wrote:
>>>> On 2013年01月22日 02:07, Peter Krempa wrote:
>>>>> This patch adds a helper function with similar semantics to
>>>>> vshCommandOptString that requests a string argument, but does some
>>>>> error
>>>>> reporting without the need to do it in the functions themselves.
>>>>>
>>>>> The error reporting also provides information about the parameter
>>>>> whose
>>>>> retrieval failed.
>>>>> ---
>>>>> tools/virsh.c | 51
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> tools/virsh.h | 4 ++++
>>>>> 2 files changed, 55 insertions(+)
>>>>>
>>>>> diff --git a/tools/virsh.c b/tools/virsh.c
>>>>> index 908c6a1..1a3cab0 100644
>>>>> --- a/tools/virsh.c
>>>>> +++ b/tools/virsh.c
>>>>> @@ -1417,6 +1417,57 @@ vshCommandOptString(const vshCmd *cmd, const
>>>>> char *name, const char **value)
>>>>> }
>>>>>
>>>>> /**
>>>>> + * vshCommandOptStringReq: Get a required string argumment
>>>>
>>>> Trivial, but we usually describe what the function does at [1].
>>>>
>>>>> + * @ctl virsh control structure
>>>>
>>>> And have a ":" after @foo.
>>>
>>> Not in virsh.c. All surrounding functions don't have the colon in the
>>> comment. This is worth cleaning up separately instead of doing it in
>>> multiple ways in a single file.
>>
>> Agreed, a follow up patch will be good then. But others still stands.
>
>
> I changed the comment to:
> /**
> + * vshCommandOptStringReq:
> + * @ctl virsh control structure
> + * @cmd command structure
> + * @name option name
> + * @value result (updated to NULL or the option argument)
> + *
> + * Gets a option argument as string. This function reports errors.
I think "The function reports errors" can just be removed. As it
doesn't report any error on success.
Okay.
> + *
> + * Returns 0 on success or when the option is not present and not
> + * required, *value is set to the option argument. On error -1 is
> + * returned and error message printed.
> + */
>
> Peter
ACK with that.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
12:07 p.m.
New subject: [libvirt] [PATCH 05/17] virsh-pool: Update pool commands to use vshCommandOptStringReq
Rework most of option string retrievals to the new helper.
---
tools/virsh-pool.c | 56 +++++++++++++++++++++++++-----------------------------
1 file changed, 26 insertions(+), 30 deletions(-)
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index bc400a1..5a7c8dd 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -46,7 +46,7 @@ vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char
*optname,
const char *n = NULL;
virCheckFlags(VSH_BYUUID | VSH_BYNAME, NULL);
- if (vshCommandOptString(cmd, optname, &n) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0)
return NULL;
vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n",
@@ -154,7 +154,7 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
@@ -226,26 +226,28 @@ static const vshCmdOptDef opts_pool_X_as[] = {
{.name = NULL}
};
-static int vshBuildPoolXML(const vshCmd *cmd, const char **retname, char **xml) {
-
+static int
+vshBuildPoolXML(vshControl *ctl,
+ const vshCmd *cmd,
+ const char **retname,
+ char **xml)
+{
const char *name = NULL, *type = NULL, *srcHost = NULL, *srcPath = NULL,
*srcDev = NULL, *srcName = NULL, *srcFormat = NULL, *target = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (vshCommandOptString(cmd, "name", &name) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
goto cleanup;
- if (vshCommandOptString(cmd, "type", &type) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
goto cleanup;
- if (vshCommandOptString(cmd, "source-host", &srcHost) < 0 ||
- vshCommandOptString(cmd, "source-path", &srcPath) < 0 ||
- vshCommandOptString(cmd, "source-dev", &srcDev) < 0 ||
- vshCommandOptString(cmd, "source-name", &srcName) < 0 ||
- vshCommandOptString(cmd, "source-format", &srcFormat) < 0 ||
- vshCommandOptString(cmd, "target", &target) < 0) {
- vshError(NULL, "%s", _("missing argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "source-host", &srcHost) < 0
||
+ vshCommandOptStringReq(ctl, cmd, "source-path", &srcPath) < 0
||
+ vshCommandOptStringReq(ctl, cmd, "source-dev", &srcDev) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "source-name", &srcName) < 0
||
+ vshCommandOptStringReq(ctl, cmd, "source-format", &srcFormat) <
0 ||
+ vshCommandOptStringReq(ctl, cmd, "target", &target) < 0)
goto cleanup;
- }
virBufferAsprintf(&buf, "<pool type='%s'>\n", type);
virBufferAsprintf(&buf, " <name>%s</name>\n", name);
@@ -303,7 +305,7 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
char *xml;
bool printXML = vshCommandOptBool(cmd, "print-xml");
- if (!vshBuildPoolXML(cmd, &name, &xml))
+ if (!vshBuildPoolXML(ctl, cmd, &name, &xml))
return false;
if (printXML) {
@@ -350,7 +352,7 @@ cmdPoolDefine(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
@@ -387,7 +389,7 @@ cmdPoolDefineAs(vshControl *ctl, const vshCmd *cmd)
char *xml;
bool printXML = vshCommandOptBool(cmd, "print-xml");
- if (!vshBuildPoolXML(cmd, &name, &xml))
+ if (!vshBuildPoolXML(ctl, cmd, &name, &xml))
return false;
if (printXML) {
@@ -959,10 +961,8 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
if (vshCommandOptBool(cmd, "transient"))
flags |= VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT;
- if (vshCommandOptString(cmd, "type", &type) < 0) {
- vshError(ctl, "%s", _("Invalid argument for
'type'"));
+ if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
return false;
- }
if (type) {
int poolType = -1;
@@ -1352,18 +1352,16 @@ cmdPoolDiscoverSourcesAs(vshControl * ctl, const vshCmd * cmd
ATTRIBUTE_UNUSED)
char *srcList;
const char *initiator = NULL;
- if (vshCommandOptString(cmd, "type", &type) <= 0 ||
- vshCommandOptString(cmd, "host", &host) < 0 ||
- vshCommandOptString(cmd, "initiator", &initiator) < 0) {
- vshError(ctl,"%s", _("missing argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "host", &host) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "initiator", &initiator) < 0)
return false;
- }
if (host) {
const char *port = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (vshCommandOptString(cmd, "port", &port) < 0) {
+ if (vshCommandOptStringReq(ctl, cmd, "port", &port) < 0) {
vshError(ctl, "%s", _("missing argument"));
virBufferFreeAndReset(&buf);
return false;
@@ -1427,13 +1425,11 @@ cmdPoolDiscoverSources(vshControl * ctl, const vshCmd * cmd
ATTRIBUTE_UNUSED)
const char *type = NULL, *srcSpecFile = NULL;
char *srcSpec = NULL, *srcList;
- if (vshCommandOptString(cmd, "type", &type) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
return false;
- if (vshCommandOptString(cmd, "srcSpec", &srcSpecFile) < 0) {
- vshError(ctl, "%s", _("missing option"));
+ if (vshCommandOptStringReq(ctl, cmd, "srcSpec", &srcSpecFile) < 0)
return false;
- }
if (srcSpecFile && virFileReadAll(srcSpecFile, VSH_MAX_XML_FILE,
&srcSpec) < 0)
--
1.8.1.1
11:22 p.m.
New subject: [libvirt] [PATCH 05/17] virsh-pool: Update pool commands to use vshCommandOptStringReq
On 2013年01月22日 02:07, Peter Krempa wrote:
Rework most of option string retrievals to the new helper.
---
tools/virsh-pool.c | 56 +++++++++++++++++++++++++-----------------------------
1 file changed, 26 insertions(+), 30 deletions(-)
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index bc400a1..5a7c8dd 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -46,7 +46,7 @@ vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char
*optname,
const char *n = NULL;
virCheckFlags(VSH_BYUUID | VSH_BYNAME, NULL);
- if (vshCommandOptString(cmd, optname,&n)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname,&n)< 0)
return NULL;
vshDebug(ctl, VSH_ERR_INFO, "%s: found option<%s>: %s\n",
@@ -154,7 +154,7 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
@@ -226,26 +226,28 @@ static const vshCmdOptDef opts_pool_X_as[] = {
{.name = NULL}
};
-static int vshBuildPoolXML(const vshCmd *cmd, const char **retname, char **xml) {
-
+static int
+vshBuildPoolXML(vshControl *ctl,
+ const vshCmd *cmd,
+ const char **retname,
+ char **xml)
Okay, you change the style here. :-)
+{
const char *name = NULL, *type = NULL, *srcHost = NULL, *srcPath = NULL,
*srcDev = NULL, *srcName = NULL, *srcFormat = NULL, *target = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (vshCommandOptString(cmd, "name",&name)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "name",&name)< 0)
goto cleanup;
- if (vshCommandOptString(cmd, "type",&type)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "type",&type)< 0)
goto cleanup;
- if (vshCommandOptString(cmd, "source-host",&srcHost)< 0 ||
- vshCommandOptString(cmd, "source-path",&srcPath)< 0 ||
- vshCommandOptString(cmd, "source-dev",&srcDev)< 0 ||
- vshCommandOptString(cmd, "source-name",&srcName)< 0 ||
- vshCommandOptString(cmd, "source-format",&srcFormat)< 0 ||
- vshCommandOptString(cmd, "target",&target)< 0) {
- vshError(NULL, "%s", _("missing argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "source-host",&srcHost)< 0
||
+ vshCommandOptStringReq(ctl, cmd, "source-path",&srcPath)< 0
||
+ vshCommandOptStringReq(ctl, cmd, "source-dev",&srcDev)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "source-name",&srcName)< 0
||
+ vshCommandOptStringReq(ctl, cmd, "source-format",&srcFormat)<
0 ||
+ vshCommandOptStringReq(ctl, cmd, "target",&target)< 0)
goto cleanup;
- }
virBufferAsprintf(&buf, "<pool type='%s'>\n", type);
virBufferAsprintf(&buf, "<name>%s</name>\n", name);
@@ -303,7 +305,7 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
char *xml;
bool printXML = vshCommandOptBool(cmd, "print-xml");
- if (!vshBuildPoolXML(cmd,&name,&xml))
+ if (!vshBuildPoolXML(ctl, cmd,&name,&xml))
return false;
if (printXML) {
@@ -350,7 +352,7 @@ cmdPoolDefine(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
@@ -387,7 +389,7 @@ cmdPoolDefineAs(vshControl *ctl, const vshCmd *cmd)
char *xml;
bool printXML = vshCommandOptBool(cmd, "print-xml");
- if (!vshBuildPoolXML(cmd,&name,&xml))
+ if (!vshBuildPoolXML(ctl, cmd,&name,&xml))
return false;
if (printXML) {
@@ -959,10 +961,8 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
if (vshCommandOptBool(cmd, "transient"))
flags |= VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT;
- if (vshCommandOptString(cmd, "type",&type)< 0) {
- vshError(ctl, "%s", _("Invalid argument for
'type'"));
+ if (vshCommandOptStringReq(ctl, cmd, "type",&type)< 0)
return false;
- }
if (type) {
int poolType = -1;
@@ -1352,18 +1352,16 @@ cmdPoolDiscoverSourcesAs(vshControl * ctl, const vshCmd * cmd
ATTRIBUTE_UNUSED)
char *srcList;
const char *initiator = NULL;
- if (vshCommandOptString(cmd, "type",&type)<= 0 ||
- vshCommandOptString(cmd, "host",&host)< 0 ||
- vshCommandOptString(cmd, "initiator",&initiator)< 0) {
- vshError(ctl,"%s", _("missing argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "type",&type)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "host",&host)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "initiator",&initiator)< 0)
return false;
- }
if (host) {
const char *port = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (vshCommandOptString(cmd, "port",&port)< 0) {
+ if (vshCommandOptStringReq(ctl, cmd, "port",&port)< 0) {
vshError(ctl, "%s", _("missing argument"));
virBufferFreeAndReset(&buf);
return false;
@@ -1427,13 +1425,11 @@ cmdPoolDiscoverSources(vshControl * ctl, const vshCmd * cmd
ATTRIBUTE_UNUSED)
const char *type = NULL, *srcSpecFile = NULL;
char *srcSpec = NULL, *srcList;
- if (vshCommandOptString(cmd, "type",&type)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "type",&type)< 0)
return false;
- if (vshCommandOptString(cmd, "srcSpec",&srcSpecFile)< 0) {
- vshError(ctl, "%s", _("missing option"));
+ if (vshCommandOptStringReq(ctl, cmd, "srcSpec",&srcSpecFile)< 0)
return false;
- }
if (srcSpecFile&& virFileReadAll(srcSpecFile, VSH_MAX_XML_FILE,
&srcSpec)< 0)
ACK
4:59 a.m.
New subject: [libvirt] [PATCH 05/17] virsh-pool: Update pool commands to use vshCommandOptStringReq
On 01/31/13 06:22, Osier Yang wrote:
On 2013年01月22日 02:07, Peter Krempa wrote:
> Rework most of option string retrievals to the new helper.
> ---
> tools/virsh-pool.c | 56
> +++++++++++++++++++++++++-----------------------------
> 1 file changed, 26 insertions(+), 30 deletions(-)
>
> diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
> index bc400a1..5a7c8dd 100644
> --- a/tools/virsh-pool.c
> +++ b/tools/virsh-pool.c
[...]
> @@ -226,26 +226,28 @@ static const vshCmdOptDef opts_pool_X_as[]
= {
> {.name = NULL}
> };
>
> -static int vshBuildPoolXML(const vshCmd *cmd, const char **retname,
> char **xml) {
> -
> +static int
> +vshBuildPoolXML(vshControl *ctl,
> + const vshCmd *cmd,
> + const char **retname,
> + char **xml)
Okay, you change the style here. :-)
Yep, part of this hunk belongs to the patch before.
ACK
Thanks.
Peter
12:07 p.m.
New subject: [libvirt] [PATCH 06/17] virsh-volume: Update volume commands to use vshCommandOptStringReq
---
tools/virsh-volume.c | 69 +++++++++++++++++++++-------------------------------
1 file changed, 28 insertions(+), 41 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 466188c..0575166 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -52,13 +52,12 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
const char *n = NULL, *p = NULL;
virCheckFlags(VSH_BYUUID | VSH_BYNAME, NULL);
- if (vshCommandOptString(cmd, optname, &n) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0)
return NULL;
- if (pooloptname != NULL && vshCommandOptString(cmd, pooloptname, &p) <
0) {
- vshError(ctl, "%s", _("missing option"));
+ if (pooloptname != NULL &&
+ vshCommandOptStringReq(ctl, cmd, pooloptname, &p) < 0)
return NULL;
- }
if (p)
pool = vshCommandOptPoolBy(ctl, cmd, pooloptname, name, flags);
@@ -182,10 +181,10 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
VSH_BYNAME)))
return false;
- if (vshCommandOptString(cmd, "name", &name) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
goto cleanup;
- if (vshCommandOptString(cmd, "capacity", &capacityStr) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "capacity", &capacityStr) < 0)
goto cleanup;
if (vshVolSize(capacityStr, &capacity) < 0) {
@@ -199,14 +198,11 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (vshCommandOptString(cmd, "format", &format) < 0 ||
- vshCommandOptString(cmd, "backing-vol", &snapshotStrVol) < 0 ||
- vshCommandOptString(cmd, "backing-vol-format",
- &snapshotStrFormat) < 0) {
- vshError(ctl, "%s", _("missing argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "backing-vol", &snapshotStrVol)
< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "backing-vol-format",
+ &snapshotStrFormat) < 0)
goto cleanup;
- }
-
virBufferAddLit(&buf, "<volume>\n");
virBufferAsprintf(&buf, " <name>%s</name>\n", name);
@@ -340,9 +336,9 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
virStoragePoolPtr pool;
virStorageVolPtr vol;
const char *from = NULL;
- bool ret = true;
+ bool ret = false;
unsigned int flags = 0;
- char *buffer;
+ char *buffer = NULL;
if (vshCommandOptBool(cmd, "prealloc-metadata"))
flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
@@ -350,29 +346,26 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
VSH_BYNAME)))
return false;
- if (vshCommandOptString(cmd, "file", &from) <= 0) {
- virStoragePoolFree(pool);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
+ goto cleanup;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
- vshReportError(ctl);
- virStoragePoolFree(pool);
- return false;
+ vshSaveLibvirtError();
+ goto cleanup;
}
- vol = virStorageVolCreateXML(pool, buffer, flags);
- VIR_FREE(buffer);
- virStoragePoolFree(pool);
-
- if (vol != NULL) {
+ if ((vol = virStorageVolCreateXML(pool, buffer, flags))) {
vshPrint(ctl, _("Vol %s created from %s\n"),
virStorageVolGetName(vol), from);
virStorageVolFree(vol);
+ ret = true;
} else {
vshError(ctl, _("Failed to create vol from %s"), from);
- ret = false;
}
+
+cleanup:
+ VIR_FREE(buffer);
+ virStoragePoolFree(pool);
return ret;
}
@@ -429,9 +422,9 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "prealloc-metadata"))
flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
- if (vshCommandOptString(cmd, "file", &from) <= 0) {
+
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
goto cleanup;
- }
if (!(inputvol = vshCommandOptVol(ctl, cmd, "vol", "inputpool",
NULL)))
goto cleanup;
@@ -548,7 +541,7 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (vshCommandOptString(cmd, "newname", &name) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "newname", &name) < 0)
goto cleanup;
origxml = virStorageVolGetXMLDesc(origvol, 0);
@@ -658,10 +651,8 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
return false;
}
- if (vshCommandOptString(cmd, "file", &file) < 0) {
- vshError(ctl, _("file must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0)
goto cleanup;
- }
if ((fd = open(file, O_RDONLY)) < 0) {
vshError(ctl, _("cannot read %s"), file);
@@ -764,10 +755,8 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool",
&name)))
return false;
- if (vshCommandOptString(cmd, "file", &file) < 0) {
- vshError(ctl, _("file must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0)
goto cleanup;
- }
if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) < 0) {
if (errno != EEXIST ||
@@ -906,10 +895,8 @@ cmdVolWipe(vshControl *ctl, const vshCmd *cmd)
return false;
}
- if (vshCommandOptString(cmd, "algorithm", &algorithm_str) < 0) {
- vshError(ctl, "%s", _("missing argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "algorithm", &algorithm_str) <
0)
goto out;
- }
if (algorithm_str &&
(algorithm = virStorageVolWipeAlgorithmTypeFromString(algorithm_str)) < 0) {
@@ -1072,7 +1059,7 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd)
if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL)))
return false;
- if (vshCommandOptString(cmd, "capacity", &capacityStr) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "capacity", &capacityStr) <=
0)
goto cleanup;
virSkipSpaces(&capacityStr);
if (*capacityStr == '-') {
--
1.8.1.1
11:25 p.m.
New subject: [libvirt] [PATCH 06/17] virsh-volume: Update volume commands to use vshCommandOptStringReq
On 2013年01月22日 02:07, Peter Krempa wrote:
---
tools/virsh-volume.c | 69 +++++++++++++++++++++-------------------------------
1 file changed, 28 insertions(+), 41 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 466188c..0575166 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -52,13 +52,12 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
const char *n = NULL, *p = NULL;
virCheckFlags(VSH_BYUUID | VSH_BYNAME, NULL);
- if (vshCommandOptString(cmd, optname,&n)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname,&n)< 0)
return NULL;
- if (pooloptname != NULL&& vshCommandOptString(cmd, pooloptname,&p)<
0) {
- vshError(ctl, "%s", _("missing option"));
+ if (pooloptname != NULL&&
+ vshCommandOptStringReq(ctl, cmd, pooloptname,&p)< 0)
return NULL;
- }
if (p)
pool = vshCommandOptPoolBy(ctl, cmd, pooloptname, name, flags);
@@ -182,10 +181,10 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
VSH_BYNAME)))
return false;
- if (vshCommandOptString(cmd, "name",&name)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "name",&name)< 0)
goto cleanup;
- if (vshCommandOptString(cmd, "capacity",&capacityStr)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "capacity",&capacityStr)< 0)
goto cleanup;
if (vshVolSize(capacityStr,&capacity)< 0) {
@@ -199,14 +198,11 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (vshCommandOptString(cmd, "format",&format)< 0 ||
- vshCommandOptString(cmd, "backing-vol",&snapshotStrVol)< 0 ||
- vshCommandOptString(cmd, "backing-vol-format",
-&snapshotStrFormat)< 0) {
- vshError(ctl, "%s", _("missing argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "format",&format)< 0 ||
+ vshCommandOptStringReq(ctl, cmd,
"backing-vol",&snapshotStrVol)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "backing-vol-format",
+&snapshotStrFormat)< 0)
goto cleanup;
- }
-
virBufferAddLit(&buf, "<volume>\n");
virBufferAsprintf(&buf, "<name>%s</name>\n", name);
@@ -340,9 +336,9 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
virStoragePoolPtr pool;
virStorageVolPtr vol;
const char *from = NULL;
- bool ret = true;
+ bool ret = false;
unsigned int flags = 0;
- char *buffer;
+ char *buffer = NULL;
if (vshCommandOptBool(cmd, "prealloc-metadata"))
flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
@@ -350,29 +346,26 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
VSH_BYNAME)))
return false;
- if (vshCommandOptString(cmd, "file",&from)<= 0) {
- virStoragePoolFree(pool);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
+ goto cleanup;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0) {
- vshReportError(ctl);
- virStoragePoolFree(pool);
- return false;
+ vshSaveLibvirtError();
+ goto cleanup;
}
- vol = virStorageVolCreateXML(pool, buffer, flags);
- VIR_FREE(buffer);
- virStoragePoolFree(pool);
-
- if (vol != NULL) {
+ if ((vol = virStorageVolCreateXML(pool, buffer, flags))) {
vshPrint(ctl, _("Vol %s created from %s\n"),
virStorageVolGetName(vol), from);
virStorageVolFree(vol);
+ ret = true;
} else {
vshError(ctl, _("Failed to create vol from %s"), from);
- ret = false;
}
+
+cleanup:
+ VIR_FREE(buffer);
+ virStoragePoolFree(pool);
return ret;
}
@@ -429,9 +422,9 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "prealloc-metadata"))
flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
- if (vshCommandOptString(cmd, "file",&from)<= 0) {
+
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
goto cleanup;
- }
if (!(inputvol = vshCommandOptVol(ctl, cmd, "vol", "inputpool",
NULL)))
goto cleanup;
@@ -548,7 +541,7 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (vshCommandOptString(cmd, "newname",&name)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "newname",&name)< 0)
goto cleanup;
origxml = virStorageVolGetXMLDesc(origvol, 0);
@@ -658,10 +651,8 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
return false;
}
- if (vshCommandOptString(cmd, "file",&file)< 0) {
- vshError(ctl, _("file must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "file",&file)< 0)
goto cleanup;
- }
if ((fd = open(file, O_RDONLY))< 0) {
vshError(ctl, _("cannot read %s"), file);
@@ -764,10 +755,8 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
if (!(vol = vshCommandOptVol(ctl, cmd, "vol",
"pool",&name)))
return false;
- if (vshCommandOptString(cmd, "file",&file)< 0) {
- vshError(ctl, _("file must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "file",&file)< 0)
goto cleanup;
- }
if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666))< 0) {
if (errno != EEXIST ||
@@ -906,10 +895,8 @@ cmdVolWipe(vshControl *ctl, const vshCmd *cmd)
return false;
}
- if (vshCommandOptString(cmd, "algorithm",&algorithm_str)< 0) {
- vshError(ctl, "%s", _("missing argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "algorithm",&algorithm_str)<
0)
goto out;
- }
if (algorithm_str&&
(algorithm = virStorageVolWipeAlgorithmTypeFromString(algorithm_str))< 0)
{
@@ -1072,7 +1059,7 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd)
if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL)))
return false;
- if (vshCommandOptString(cmd, "capacity",&capacityStr)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "capacity",&capacityStr)<= 0)
goto cleanup;
virSkipSpaces(&capacityStr);
if (*capacityStr == '-') {
ACK
5:13 a.m.
New subject: [libvirt] [PATCH 06/17] virsh-volume: Update volume commands to use vshCommandOptStringReq
On 01/31/13 06:25, Osier Yang wrote:
On 2013年01月22日 02:07, Peter Krempa wrote:
> ---
> tools/virsh-volume.c | 69
> +++++++++++++++++++++-------------------------------
> 1 file changed, 28 insertions(+), 41 deletions(-)
>
> diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
> return false;
>
> - if (vshCommandOptString(cmd, "capacity",&capacityStr)<= 0)
> + if (vshCommandOptStringReq(ctl, cmd, "capacity",&capacityStr)<=
0)
The comparison operator needs to be changed to "<" here, as you pointed
out in other patches.
> goto cleanup;
> virSkipSpaces(&capacityStr);
> if (*capacityStr == '-') {
ACK
Thanks.
Peter
12:07 p.m.
New subject: [libvirt] [PATCH 07/17] virsh-domain-monitor: Update domain commands to use vshCommandOptStringReq
---
tools/virsh-domain-monitor.c | 44 +++++++++++++++++++-------------------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 1e8b8cf..7e105fd 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -373,26 +373,25 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
{
virDomainBlockInfo info;
virDomainPtr dom;
- bool ret = true;
+ bool ret = false;
const char *device = NULL;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "device", &device) <= 0) {
- virDomainFree(dom);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "device", &device) < 0)
+ goto cleanup;
- if (virDomainGetBlockInfo(dom, device, &info, 0) < 0) {
- virDomainFree(dom);
- return false;
- }
+ if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)
+ goto cleanup;
vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info.capacity);
vshPrint(ctl, "%-15s %llu\n", _("Allocation:"),
info.allocation);
vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info.physical);
+ ret = true;
+
+cleanup:
virDomainFree(dom);
return ret;
}
@@ -670,16 +669,13 @@ cmdDomIfGetLink(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "interface", &iface) <= 0) {
- virDomainFree(dom);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0)
+ goto cleanup;
if (vshCommandOptBool(cmd, "config"))
flags = VIR_DOMAIN_XML_INACTIVE;
- desc = virDomainGetXMLDesc(dom, flags);
- if (desc == NULL) {
+ if (!(desc = virDomainGetXMLDesc(dom, flags))) {
vshError(ctl, _("Failed to get domain description xml"));
goto cleanup;
}
@@ -752,8 +748,7 @@ cleanup:
xmlXPathFreeObject(obj);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
return ret;
}
@@ -891,7 +886,7 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (vshCommandOptString(cmd, "device", &device) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device", &device) <= 0)
goto cleanup;
rc = virDomainBlockStatsFlags(dom, device, NULL, &nparams, 0);
@@ -1016,19 +1011,17 @@ cmdDomIfstat(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
const char *name = NULL, *device = NULL;
struct _virDomainInterfaceStats stats;
+ bool ret = false;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (vshCommandOptString(cmd, "interface", &device) <= 0) {
- virDomainFree(dom);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "interface", &device) < 0)
+ goto cleanup;
if (virDomainInterfaceStats(dom, device, &stats, sizeof(stats)) == -1) {
vshError(ctl, _("Failed to get interface stats %s %s"), name, device);
- virDomainFree(dom);
- return false;
+ goto cleanup;
}
if (stats.rx_bytes >= 0)
@@ -1055,8 +1048,9 @@ cmdDomIfstat(vshControl *ctl, const vshCmd *cmd)
if (stats.tx_drop >= 0)
vshPrint(ctl, "%s tx_drop %lld\n", device, stats.tx_drop);
+cleanup:
virDomainFree(dom);
- return true;
+ return ret;
}
/*
--
1.8.1.1
12:50 a.m.
New subject: [libvirt] [PATCH 07/17] virsh-domain-monitor: Update domain commands to use vshCommandOptStringReq
On 2013年01月22日 02:07, Peter Krempa wrote:
---
tools/virsh-domain-monitor.c | 44 +++++++++++++++++++-------------------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 1e8b8cf..7e105fd 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -373,26 +373,25 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
{
virDomainBlockInfo info;
virDomainPtr dom;
- bool ret = true;
+ bool ret = false;
const char *device = NULL;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "device",&device)<= 0) {
- virDomainFree(dom);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "device",&device)< 0)
+ goto cleanup;
- if (virDomainGetBlockInfo(dom, device,&info, 0)< 0) {
- virDomainFree(dom);
- return false;
- }
+ if (virDomainGetBlockInfo(dom, device,&info, 0)< 0)
+ goto cleanup;
vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info.capacity);
vshPrint(ctl, "%-15s %llu\n", _("Allocation:"),
info.allocation);
vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info.physical);
+ ret = true;
+
+cleanup:
virDomainFree(dom);
return ret;
}
@@ -670,16 +669,13 @@ cmdDomIfGetLink(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "interface",&iface)<= 0) {
- virDomainFree(dom);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "interface",&iface)< 0)
+ goto cleanup;
if (vshCommandOptBool(cmd, "config"))
flags = VIR_DOMAIN_XML_INACTIVE;
- desc = virDomainGetXMLDesc(dom, flags);
- if (desc == NULL) {
+ if (!(desc = virDomainGetXMLDesc(dom, flags))) {
vshError(ctl, _("Failed to get domain description xml"));
goto cleanup;
}
@@ -752,8 +748,7 @@ cleanup:
xmlXPathFreeObject(obj);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
return ret;
}
@@ -891,7 +886,7 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd,&name)))
return false;
- if (vshCommandOptString(cmd, "device",&device)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device",&device)<= 0)
I think you mean < 0 here.
goto cleanup;
rc = virDomainBlockStatsFlags(dom, device, NULL,&nparams, 0);
@@ -1016,19 +1011,17 @@ cmdDomIfstat(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
const char *name = NULL, *device = NULL;
struct _virDomainInterfaceStats stats;
+ bool ret = false;
if (!(dom = vshCommandOptDomain(ctl, cmd,&name)))
return false;
- if (vshCommandOptString(cmd, "interface",&device)<= 0) {
- virDomainFree(dom);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "interface",&device)< 0)
+ goto cleanup;
if (virDomainInterfaceStats(dom, device,&stats, sizeof(stats)) == -1) {
vshError(ctl, _("Failed to get interface stats %s %s"), name,
device);
- virDomainFree(dom);
- return false;
+ goto cleanup;
}
if (stats.rx_bytes>= 0)
@@ -1055,8 +1048,9 @@ cmdDomIfstat(vshControl *ctl, const vshCmd *cmd)
if (stats.tx_drop>= 0)
vshPrint(ctl, "%s tx_drop %lld\n", device, stats.tx_drop);
+cleanup:
virDomainFree(dom);
- return true;
+ return ret;
Is "ret" set to true yet if success?
5:19 a.m.
New subject: [libvirt] [PATCH 07/17] virsh-domain-monitor: Update domain commands to use vshCommandOptStringReq
On 01/31/13 07:50, Osier Yang wrote:
Thanks.
Peter
On 2013年01月22日 02:07, Peter Krempa wrote:
> ---
> tools/virsh-domain-monitor.c | 44
> +++++++++++++++++++-------------------------
> 1 file changed, 19 insertions(+), 25 deletions(-)
> diff --git a/tools/virsh-domain-monitor.c
b/tools/virsh-domain-monitor.c
> index 1e8b8cf..7e105fd 100644
[...]
> @@ -891,7 +886,7 @@ cmdDomblkstat(vshControl *ctl, const vshCmd
*cmd)
> if (!(dom = vshCommandOptDomain(ctl, cmd,&name)))
> return false;
> - if (vshCommandOptString(cmd,
"device",&device)<= 0)
> + if (vshCommandOptStringReq(ctl, cmd, "device",&device)<= 0)
I think you mean < 0 here.
right ....
> goto cleanup;
> rc = virDomainBlockStatsFlags(dom, device,
NULL,&nparams, 0);
[...]
> @@ -1055,8 +1048,9 @@ cmdDomIfstat(vshControl *ctl, const vshCmd
*cmd)
> if (stats.tx_drop>= 0)
> vshPrint(ctl, "%s tx_drop %lld\n", device, stats.tx_drop);
> +cleanup:
> virDomainFree(dom);
> - return true;
> + return ret;
Is "ret" set to true yet if success?
I forgot to put "ret = true;" before the cleanup label.
12:07 p.m.
New subject: [libvirt] [PATCH 08/17] virsh-domain: Update domain commands to use vshCommandOptStringReq
---
tools/virsh-domain.c | 299 ++++++++++++++++++++-------------------------------
1 file changed, 116 insertions(+), 183 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f4b6622..e3c43d4 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -74,7 +74,7 @@ vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd,
if (!vshCmdHasOption(ctl, cmd, optname))
return NULL;
- if (vshCommandOptString(cmd, optname, &n) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0)
return NULL;
vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n",
@@ -188,43 +188,42 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
const char *from = NULL;
char *buffer;
- int ret;
+ int rv;
+ bool ret = false;
unsigned int flags;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "file", &from) <= 0) {
- virDomainFree(dom);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
+ goto cleanup;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
vshReportError(ctl);
- virDomainFree(dom);
- return false;
+ goto cleanup;
}
if (vshCommandOptBool(cmd, "config")) {
flags = VIR_DOMAIN_AFFECT_CONFIG;
if (virDomainIsActive(dom) == 1)
flags |= VIR_DOMAIN_AFFECT_LIVE;
- ret = virDomainAttachDeviceFlags(dom, buffer, flags);
+ rv = virDomainAttachDeviceFlags(dom, buffer, flags);
} else {
- ret = virDomainAttachDevice(dom, buffer);
+ rv = virDomainAttachDevice(dom, buffer);
}
VIR_FREE(buffer);
- if (ret < 0) {
+ if (rv < 0) {
vshError(ctl, _("Failed to attach device from %s"), from);
- virDomainFree(dom);
- return false;
- } else {
- vshPrint(ctl, "%s", _("Device attached successfully\n"));
+ goto cleanup;
}
+ vshPrint(ctl, "%s", _("Device attached successfully\n"));
+ ret = true;
+
+cleanup:
virDomainFree(dom);
- return true;
+ return ret;
}
/*
@@ -486,28 +485,18 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "source", &source) <= 0)
- goto cleanup;
- /* Allow empty string as a placeholder that implies no source, for
- * use in adding a cdrom drive with no disk. */
- if (!*source)
- source = NULL;
-
- if (vshCommandOptString(cmd, "target", &target) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "driver", &driver) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "subdriver", &subdriver) < 0
||
+ vshCommandOptStringReq(ctl, cmd, "type", &type) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "cache", &cache) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "serial", &serial) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "address", &straddr) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0)
goto cleanup;
- if (vshCommandOptString(cmd, "driver", &driver) < 0 ||
- vshCommandOptString(cmd, "subdriver", &subdriver) < 0 ||
- vshCommandOptString(cmd, "type", &type) < 0 ||
- vshCommandOptString(cmd, "mode", &mode) < 0 ||
- vshCommandOptString(cmd, "cache", &cache) < 0 ||
- vshCommandOptString(cmd, "serial", &serial) < 0 ||
- vshCommandOptString(cmd, "address", &straddr) < 0 ||
- vshCommandOptString(cmd, "sourcetype", &stype) < 0) {
- vshError(ctl, "%s", _("missing option"));
- goto cleanup;
- }
-
if (!stype) {
if (driver && (STREQ(driver, "file") || STREQ(driver,
"tap"))) {
isFile = true;
@@ -766,20 +755,16 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "type", &type) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "script", &script) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "model", &model) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "inbound", &inboundStr) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "outbound", &outboundStr) < 0)
goto cleanup;
- if (vshCommandOptString(cmd, "source", &source) < 0 ||
- vshCommandOptString(cmd, "target", &target) < 0 ||
- vshCommandOptString(cmd, "mac", &mac) < 0 ||
- vshCommandOptString(cmd, "script", &script) < 0 ||
- vshCommandOptString(cmd, "model", &model) < 0 ||
- vshCommandOptString(cmd, "inbound", &inboundStr) < 0 ||
- vshCommandOptString(cmd, "outbound", &outboundStr) < 0) {
- vshError(ctl, "missing argument");
- goto cleanup;
- }
-
/* check interface type */
if (STREQ(type, "network")) {
typ = 1;
@@ -1070,7 +1055,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
goto cleanup;
- if (vshCommandOptString(cmd, "device", &disk) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device", &disk) < 0)
goto cleanup;
if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec", &value)) <
0) {
@@ -1349,7 +1334,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
goto cleanup;
- if (vshCommandOptString(cmd, "path", &path) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
goto cleanup;
if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) {
@@ -1372,7 +1357,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
ret = virDomainBlockJobSetSpeed(dom, path, bandwidth, 0);
break;
case VSH_CMD_BLOCK_JOB_PULL:
- if (vshCommandOptString(cmd, "base", &base) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0)
goto cleanup;
if (base)
ret = virDomainBlockRebase(dom, path, base, bandwidth, 0);
@@ -1380,8 +1365,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
ret = virDomainBlockPull(dom, path, bandwidth, 0);
break;
case VSH_CMD_BLOCK_JOB_COMMIT:
- if (vshCommandOptString(cmd, "base", &base) < 0 ||
- vshCommandOptString(cmd, "top", &top) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "top", &top) < 0)
goto cleanup;
if (vshCommandOptBool(cmd, "shallow"))
flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW;
@@ -1397,7 +1382,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
flags |= VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT;
if (vshCommandOptBool(cmd, "raw"))
flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW;
- if (vshCommandOptString(cmd, "dest", &base) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "dest", &base) < 0)
goto cleanup;
ret = virDomainBlockRebase(dom, path, base, bandwidth, flags);
}
@@ -1541,7 +1526,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
}
timeout *= 1000;
}
- if (vshCommandOptString(cmd, "path", &path) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
return false;
if (vshCommandOptBool(cmd, "async"))
abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
@@ -1741,7 +1726,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
}
timeout *= 1000;
}
- if (vshCommandOptString(cmd, "path", &path) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
return false;
if (vshCommandOptBool(cmd, "async"))
abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
@@ -2017,7 +2002,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
}
timeout *= 1000;
}
- if (vshCommandOptString(cmd, "path", &path) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
return false;
if (vshCommandOptBool(cmd, "async"))
abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
@@ -2138,10 +2123,8 @@ cmdBlockResize(vshControl *ctl, const vshCmd *cmd)
unsigned int flags = 0;
int ret = false;
- if (vshCommandOptString(cmd, "path", (const char **) &path) < 0) {
- vshError(ctl, "%s", _("Path must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "path", (const char **) &path)
< 0)
return false;
- }
if (vshCommandOptScaledInt(cmd, "size", &size, 1024, ULLONG_MAX) <
0) {
vshError(ctl, "%s", _("Unable to parse integer"));
@@ -2250,10 +2233,8 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "devname", &name) < 0) {
- vshError(ctl, "%s", _("Invalid devname"));
+ if (vshCommandOptStringReq(ctl, cmd, "devname", &name) < 0)
goto cleanup;
- }
if (force)
flags |= VIR_DOMAIN_CONSOLE_FORCE;
@@ -2329,10 +2310,8 @@ cmdDomIfSetLink(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "interface", &iface) <= 0)
- goto cleanup;
-
- if (vshCommandOptString(cmd, "state", &state) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "state", &state) < 0)
goto cleanup;
config = vshCommandOptBool(cmd, "config");
@@ -2542,14 +2521,12 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (vshCommandOptString(cmd, "interface", &device) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "interface", &device) < 0)
goto cleanup;
- if (vshCommandOptString(cmd, "inbound", &inboundStr) < 0 ||
- vshCommandOptString(cmd, "outbound", &outboundStr) < 0) {
- vshError(ctl, "missing argument");
+ if (vshCommandOptStringReq(ctl, cmd, "inbound", &inboundStr) < 0 ||
+ vshCommandOptStringReq(ctl,cmd, "outbound", &outboundStr) < 0)
goto cleanup;
- }
memset(&inbound, 0, sizeof(inbound));
memset(&outbound, 0, sizeof(outbound));
@@ -2746,10 +2723,8 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (vshCommandOptString(cmd, "target", &target) < 0) {
- vshError(ctl, _("Invalid target argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0)
goto cleanup;
- }
if (STREQ(target, "mem"))
suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM;
@@ -3374,7 +3349,7 @@ doSave(void *opaque)
if (pthread_sigmask(SIG_BLOCK, &sigmask, &oldsigmask) < 0)
goto out_sig;
- if (vshCommandOptString(cmd, "file", &to) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0)
goto out;
if (vshCommandOptBool(cmd, "bypass-cache"))
@@ -3384,10 +3359,8 @@ doSave(void *opaque)
if (vshCommandOptBool(cmd, "paused"))
flags |= VIR_DOMAIN_SAVE_PAUSED;
- if (vshCommandOptString(cmd, "xml", &xmlfile) < 0) {
- vshError(ctl, "%s", _("malformed xml argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0)
goto out;
- }
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
goto out;
@@ -3523,7 +3496,7 @@ cmdSave(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (vshCommandOptString(cmd, "file", &to) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0)
goto cleanup;
if (vshCommandOptBool(cmd, "verbose"))
@@ -3590,7 +3563,7 @@ cmdSaveImageDumpxml(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "security-info"))
flags |= VIR_DOMAIN_XML_SECURE;
- if (vshCommandOptString(cmd, "file", &file) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0)
return false;
xml = virDomainSaveImageGetXMLDesc(ctl->conn, file, flags);
@@ -3652,13 +3625,11 @@ cmdSaveImageDefine(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "paused"))
flags |= VIR_DOMAIN_SAVE_PAUSED;
- if (vshCommandOptString(cmd, "file", &file) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0)
return false;
- if (vshCommandOptString(cmd, "xml", &xmlfile) <= 0) {
- vshError(ctl, "%s", _("malformed or missing xml argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) <= 0)
return false;
- }
if (virFileReadAll(xmlfile, 8192, &xml) < 0)
goto cleanup;
@@ -3726,7 +3697,7 @@ cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd)
return false;
}
- if (vshCommandOptString(cmd, "file", &file) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0)
return false;
#define EDIT_GET_XML \
@@ -4213,7 +4184,7 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd)
const char *xmlfile = NULL;
char *xml = NULL;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (vshCommandOptBool(cmd, "bypass-cache"))
@@ -4223,10 +4194,8 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "paused"))
flags |= VIR_DOMAIN_SAVE_PAUSED;
- if (vshCommandOptString(cmd, "xml", &xmlfile) < 0) {
- vshError(ctl, "%s", _("malformed xml argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0)
return false;
- }
if (xmlfile &&
virFileReadAll(xmlfile, 8192, &xml) < 0)
@@ -4318,7 +4287,7 @@ doDump(void *opaque)
if (pthread_sigmask(SIG_BLOCK, &sigmask, &oldsigmask) < 0)
goto out_sig;
- if (vshCommandOptString(cmd, "file", &to) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0)
goto out;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
@@ -4364,7 +4333,7 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (vshCommandOptString(cmd, "file", &to) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0)
return false;
if (vshCommandOptBool(cmd, "verbose"))
@@ -4474,10 +4443,8 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
bool generated = false;
char *mime = NULL;
- if (vshCommandOptString(cmd, "file", (const char **) &file) < 0) {
- vshError(ctl, "%s", _("file must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "file", (const char **) &file)
< 0)
return false;
- }
if (vshCommandOptUInt(cmd, "screen", &screen) < 0) {
vshError(ctl, "%s", _("invalid screen ID"));
@@ -4615,10 +4582,8 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd)
int rv;
char **modes = NULL, **tmp;
- if (vshCommandOptString(cmd, "mode", &mode) < 0) {
- vshError(ctl, "%s", _("Invalid type"));
+ if (vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0)
return false;
- }
if (mode && !(modes = virStringSplit(mode, ",", 0))) {
vshError(ctl, "%s", _("Cannot parse mode string"));
@@ -4699,10 +4664,8 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd)
int flags = 0;
char **modes = NULL, **tmp;
- if (vshCommandOptString(cmd, "mode", &mode) < 0) {
- vshError(ctl, "%s", _("Invalid type"));
+ if (vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0)
return false;
- }
if (mode && !(modes = virStringSplit(mode, ",", 0))) {
vshError(ctl, "%s", _("Cannot parse mode string"));
@@ -4926,14 +4889,12 @@ cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
const char *type = NULL;
int vcpus;
- if (vshCommandOptString(cmd, "type", &type) < 0) {
- vshError(ctl, "%s", _("Invalid type"));
+ if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
return false;
- }
- vcpus = virConnectGetMaxVcpus(ctl->conn, type);
- if (vcpus < 0)
+ if ((vcpus = virConnectGetMaxVcpus(ctl->conn, type)) < 0)
return false;
+
vshPrint(ctl, "%d\n", vcpus);
return true;
@@ -5385,14 +5346,12 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
flags = -1;
}
- if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+ if (vshCommandOptStringReq(ctl, cmd, "cpulist", &cpulist) < 0)
return false;
- if (vshCommandOptString(cmd, "cpulist", &cpulist) < 0) {
- vshError(ctl, "%s", _("vcpupin: Missing cpulist."));
- virDomainFree(dom);
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- }
+
query = !cpulist;
/* In query mode, "vcpu" is optional */
@@ -5623,8 +5582,7 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "cpulist", &cpulist) < 0) {
- vshError(ctl, "%s", _("emulatorpin: Missing cpulist."));
+ if (vshCommandOptStringReq(ctl, cmd, "cpulist", &cpulist) < 0) {
virDomainFree(dom);
return false;
}
@@ -5896,14 +5854,11 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
xmlBufferPtr xml_buf = NULL;
xmlNodePtr node;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
- if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
- vshError(ctl, _("Failed to read file '%s' to compare"),
- from);
+ if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
return false;
- }
/* try to extract the CPU element from as it would appear in a domain XML*/
if (!(xml = virXMLParseStringCtxt(buffer, from, &ctxt)))
@@ -6000,7 +5955,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
virBuffer buf = VIR_BUFFER_INITIALIZER;
int i;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
@@ -6287,7 +6242,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd)
#endif
unsigned int flags = VIR_DOMAIN_NONE;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
@@ -6342,7 +6297,7 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
@@ -6813,15 +6768,11 @@ cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "pid", &pidstr) <= 0) {
- vshError(ctl, "%s", _("missing argument"));
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "pid", &pidstr) < 0)
+ goto cleanup;
- if (vshCommandOptString(cmd, "signame", &signame) <= 0) {
- vshError(ctl, "%s", _("missing argument"));
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "signame", &signame) < 0)
+ goto cleanup;
if (virStrToLong_ll(pidstr, NULL, 10, &pid_value) < 0) {
vshError(ctl, _("malformed PID value: %s"), pidstr);
@@ -7334,19 +7285,17 @@ cmdNumatune(vshControl * ctl, const vshCmd * cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "nodeset", &nodeset) < 0) {
- vshError(ctl, "%s", _("Unable to parse nodeset."));
+ if (vshCommandOptStringReq(ctl, cmd, "nodeset", &nodeset) < 0)
goto cleanup;
- }
+
if (nodeset &&
virTypedParamsAddString(¶ms, &nparams, &maxparams,
VIR_DOMAIN_NUMA_NODESET, nodeset) < 0)
goto save_error;
- if (vshCommandOptString(cmd, "mode", &mode) < 0) {
- vshError(ctl, "%s", _("Unable to parse mode."));
+ if (vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0)
goto cleanup;
- }
+
if (mode) {
int m;
/* Accept string or integer, in case server understands newer
@@ -7865,8 +7814,8 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd)
char *xmlData;
unsigned int flags = 0;
- if (vshCommandOptString(cmd, "format", &format) < 0 ||
- vshCommandOptString(cmd, "config", &configFile) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "config", &configFile) < 0)
return false;
if (virFileReadAll(configFile, 1024*1024, &configData) < 0)
@@ -7917,8 +7866,8 @@ cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
char *xmlData;
unsigned int flags = 0;
- if (vshCommandOptString(cmd, "format", &format) < 0
- || vshCommandOptString(cmd, "xml", &xmlFile) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "xml", &xmlFile) < 0)
return false;
if (virFileReadAll(xmlFile, 1024*1024, &xmlData) < 0)
@@ -8179,17 +8128,11 @@ doMigrate(void *opaque)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto out;
- if (vshCommandOptString(cmd, "desturi", &desturi) <= 0 ||
- vshCommandOptString(cmd, "migrateuri", &migrateuri) < 0 ||
- vshCommandOptString(cmd, "dname", &dname) < 0) {
- vshError(ctl, "%s", _("missing argument"));
- goto out;
- }
-
- if (vshCommandOptString(cmd, "xml", &xmlfile) < 0) {
- vshError(ctl, "%s", _("malformed xml argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "desturi", &desturi) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "migrateuri", &migrateuri) < 0
||
+ vshCommandOptStringReq(ctl, cmd, "dname", &dname) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0)
goto out;
- }
if (vshCommandOptBool(cmd, "live"))
flags |= VIR_MIGRATE_LIVE;
@@ -9115,7 +9058,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
goto cleanup;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
@@ -9189,22 +9132,19 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
const char *from = NULL;
- char *buffer;
- int ret;
+ char *buffer = NULL;
+ bool ret = false;
unsigned int flags;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "file", &from) <= 0) {
- virDomainFree(dom);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
+ goto cleanup;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
vshReportError(ctl);
- virDomainFree(dom);
- return false;
+ goto cleanup;
}
if (vshCommandOptBool(cmd, "config")) {
@@ -9218,19 +9158,18 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "force"))
flags |= VIR_DOMAIN_DEVICE_MODIFY_FORCE;
- ret = virDomainUpdateDeviceFlags(dom, buffer, flags);
- VIR_FREE(buffer);
-
- if (ret < 0) {
+ if (virDomainUpdateDeviceFlags(dom, buffer, flags) < 0) {
vshError(ctl, _("Failed to update device from %s"), from);
- virDomainFree(dom);
- return false;
- } else {
- vshPrint(ctl, "%s", _("Device updated successfully\n"));
+ goto cleanup;
}
+ vshPrint(ctl, "%s", _("Device updated successfully\n"));
+ ret = true;
+
+cleanup:
+ VIR_FREE(buffer);
virDomainFree(dom);
- return true;
+ return ret;
}
/*
@@ -9289,15 +9228,13 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
unsigned int flags;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
- goto cleanup;
+ return false;
- if (vshCommandOptString(cmd, "type", &type) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
goto cleanup;
- if (vshCommandOptString(cmd, "mac", &mac) < 0) {
- vshError(ctl, "%s", _("missing option"));
+ if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)
goto cleanup;
- }
doc = virDomainGetXMLDesc(dom, 0);
if (!doc)
@@ -9389,8 +9326,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
}
cleanup:
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
xmlXPathFreeObject(obj);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
@@ -9660,9 +9596,9 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
xmlNodePtr disk_node = NULL;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
- goto cleanup;
+ return false;
- if (vshCommandOptString(cmd, "target", &target) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0)
goto cleanup;
doc = virDomainGetXMLDesc(dom, 0);
@@ -9698,8 +9634,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
xmlFreeNode(disk_node);
VIR_FREE(disk_xml);
VIR_FREE(doc);
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
return functionReturn;
}
@@ -9887,10 +9822,10 @@ cmdChangeMedia(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "path", &path) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
goto cleanup;
- if (vshCommandOptString(cmd, "source", &source) < 0)
+ if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0)
goto cleanup;
if (insert && !source) {
@@ -9970,10 +9905,8 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (vshCommandOptString(cmd, "mountpoint", &mountPoint) < 0) {
- vshError(ctl, _("Unable to parse mountpoint parameter"));
+ if (vshCommandOptStringReq(ctl, cmd, "mountpoint", &mountPoint) <
0)
goto cleanup;
- }
if (virDomainFSTrim(dom, mountPoint, minimum, flags) < 0) {
vshError(ctl, _("Unable to invoke fstrim"));
--
1.8.1.1
12:58 a.m.
New subject: [libvirt] [PATCH 08/17] virsh-domain: Update domain commands to use vshCommandOptStringReq
On 2013年01月22日 02:07, Peter Krempa wrote:
---
tools/virsh-domain.c | 299 ++++++++++++++++++++-------------------------------
1 file changed, 116 insertions(+), 183 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f4b6622..e3c43d4 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -74,7 +74,7 @@ vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd,
if (!vshCmdHasOption(ctl, cmd, optname))
return NULL;
- if (vshCommandOptString(cmd, optname,&n)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname,&n)< 0)
return NULL;
vshDebug(ctl, VSH_ERR_INFO, "%s: found option<%s>: %s\n",
@@ -188,43 +188,42 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
const char *from = NULL;
char *buffer;
- int ret;
+ int rv;
+ bool ret = false;
unsigned int flags;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "file",&from)<= 0) {
- virDomainFree(dom);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
+ goto cleanup;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0) {
vshReportError(ctl);
- virDomainFree(dom);
- return false;
+ goto cleanup;
}
if (vshCommandOptBool(cmd, "config")) {
flags = VIR_DOMAIN_AFFECT_CONFIG;
if (virDomainIsActive(dom) == 1)
flags |= VIR_DOMAIN_AFFECT_LIVE;
- ret = virDomainAttachDeviceFlags(dom, buffer, flags);
+ rv = virDomainAttachDeviceFlags(dom, buffer, flags);
} else {
- ret = virDomainAttachDevice(dom, buffer);
+ rv = virDomainAttachDevice(dom, buffer);
}
VIR_FREE(buffer);
- if (ret< 0) {
+ if (rv< 0) {
vshError(ctl, _("Failed to attach device from %s"), from);
- virDomainFree(dom);
- return false;
- } else {
- vshPrint(ctl, "%s", _("Device attached successfully\n"));
+ goto cleanup;
}
+ vshPrint(ctl, "%s", _("Device attached successfully\n"));
+ ret = true;
+
+cleanup:
virDomainFree(dom);
- return true;
+ return ret;
}
/*
@@ -486,28 +485,18 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "source",&source)<= 0)
- goto cleanup;
- /* Allow empty string as a placeholder that implies no source, for
- * use in adding a cdrom drive with no disk. */
- if (!*source)
- source = NULL;
-
- if (vshCommandOptString(cmd, "target",&target)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "source",&source)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "target",&target)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "driver",&driver)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "subdriver",&subdriver)< 0
||
+ vshCommandOptStringReq(ctl, cmd, "type",&type)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "mode",&mode)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "cache",&cache)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "serial",&serial)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "address",&straddr)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "sourcetype",&stype)< 0)
goto cleanup;
- if (vshCommandOptString(cmd, "driver",&driver)< 0 ||
- vshCommandOptString(cmd, "subdriver",&subdriver)< 0 ||
- vshCommandOptString(cmd, "type",&type)< 0 ||
- vshCommandOptString(cmd, "mode",&mode)< 0 ||
- vshCommandOptString(cmd, "cache",&cache)< 0 ||
- vshCommandOptString(cmd, "serial",&serial)< 0 ||
- vshCommandOptString(cmd, "address",&straddr)< 0 ||
- vshCommandOptString(cmd, "sourcetype",&stype)< 0) {
- vshError(ctl, "%s", _("missing option"));
- goto cleanup;
- }
-
if (!stype) {
if (driver&& (STREQ(driver, "file") || STREQ(driver,
"tap"))) {
isFile = true;
@@ -766,20 +755,16 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "type",&type)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "type",&type)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "source",&source)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "target",&target)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "mac",&mac)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "script",&script)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "model",&model)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "inbound",&inboundStr)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "outbound",&outboundStr)< 0)
goto cleanup;
- if (vshCommandOptString(cmd, "source",&source)< 0 ||
- vshCommandOptString(cmd, "target",&target)< 0 ||
- vshCommandOptString(cmd, "mac",&mac)< 0 ||
- vshCommandOptString(cmd, "script",&script)< 0 ||
- vshCommandOptString(cmd, "model",&model)< 0 ||
- vshCommandOptString(cmd, "inbound",&inboundStr)< 0 ||
- vshCommandOptString(cmd, "outbound",&outboundStr)< 0) {
- vshError(ctl, "missing argument");
- goto cleanup;
- }
-
/* check interface type */
if (STREQ(type, "network")) {
typ = 1;
@@ -1070,7 +1055,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd,&name)))
goto cleanup;
- if (vshCommandOptString(cmd, "device",&disk)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device",&disk)< 0)
goto cleanup;
if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec",&value))<
0) {
@@ -1349,7 +1334,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
if (!(dom = vshCommandOptDomain(ctl, cmd,&name)))
goto cleanup;
- if (vshCommandOptString(cmd, "path",&path)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "path",&path)< 0)
goto cleanup;
if (vshCommandOptUL(cmd, "bandwidth",&bandwidth)< 0) {
@@ -1372,7 +1357,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
ret = virDomainBlockJobSetSpeed(dom, path, bandwidth, 0);
break;
case VSH_CMD_BLOCK_JOB_PULL:
- if (vshCommandOptString(cmd, "base",&base)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "base",&base)< 0)
goto cleanup;
if (base)
ret = virDomainBlockRebase(dom, path, base, bandwidth, 0);
@@ -1380,8 +1365,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
ret = virDomainBlockPull(dom, path, bandwidth, 0);
break;
case VSH_CMD_BLOCK_JOB_COMMIT:
- if (vshCommandOptString(cmd, "base",&base)< 0 ||
- vshCommandOptString(cmd, "top",&top)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "base",&base)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "top",&top)< 0)
goto cleanup;
if (vshCommandOptBool(cmd, "shallow"))
flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW;
@@ -1397,7 +1382,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
flags |= VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT;
if (vshCommandOptBool(cmd, "raw"))
flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW;
- if (vshCommandOptString(cmd, "dest",&base)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "dest",&base)< 0)
goto cleanup;
ret = virDomainBlockRebase(dom, path, base, bandwidth, flags);
}
@@ -1541,7 +1526,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
}
timeout *= 1000;
}
- if (vshCommandOptString(cmd, "path",&path)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "path",&path)< 0)
return false;
if (vshCommandOptBool(cmd, "async"))
abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
@@ -1741,7 +1726,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
}
timeout *= 1000;
}
- if (vshCommandOptString(cmd, "path",&path)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "path",&path)< 0)
return false;
if (vshCommandOptBool(cmd, "async"))
abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
@@ -2017,7 +2002,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
}
timeout *= 1000;
}
- if (vshCommandOptString(cmd, "path",&path)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "path",&path)< 0)
return false;
if (vshCommandOptBool(cmd, "async"))
abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
@@ -2138,10 +2123,8 @@ cmdBlockResize(vshControl *ctl, const vshCmd *cmd)
unsigned int flags = 0;
int ret = false;
- if (vshCommandOptString(cmd, "path", (const char **)&path)< 0) {
- vshError(ctl, "%s", _("Path must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "path", (const char **)&path)<
0)
return false;
- }
if (vshCommandOptScaledInt(cmd, "size",&size, 1024, ULLONG_MAX)<
0) {
vshError(ctl, "%s", _("Unable to parse integer"));
@@ -2250,10 +2233,8 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "devname",&name)< 0) {
- vshError(ctl, "%s", _("Invalid devname"));
+ if (vshCommandOptStringReq(ctl, cmd, "devname",&name)< 0)
goto cleanup;
- }
if (force)
flags |= VIR_DOMAIN_CONSOLE_FORCE;
@@ -2329,10 +2310,8 @@ cmdDomIfSetLink(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "interface",&iface)<= 0)
- goto cleanup;
-
- if (vshCommandOptString(cmd, "state",&state)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "interface",&iface)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "state",&state)< 0)
goto cleanup;
config = vshCommandOptBool(cmd, "config");
@@ -2542,14 +2521,12 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd,&name)))
return false;
- if (vshCommandOptString(cmd, "interface",&device)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "interface",&device)< 0)
goto cleanup;
- if (vshCommandOptString(cmd, "inbound",&inboundStr)< 0 ||
- vshCommandOptString(cmd, "outbound",&outboundStr)< 0) {
- vshError(ctl, "missing argument");
+ if (vshCommandOptStringReq(ctl, cmd, "inbound",&inboundStr)< 0 ||
+ vshCommandOptStringReq(ctl,cmd, "outbound",&outboundStr)< 0)
goto cleanup;
- }
memset(&inbound, 0, sizeof(inbound));
memset(&outbound, 0, sizeof(outbound));
@@ -2746,10 +2723,8 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (vshCommandOptString(cmd, "target",&target)< 0) {
- vshError(ctl, _("Invalid target argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "target",&target)< 0)
goto cleanup;
- }
if (STREQ(target, "mem"))
suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM;
@@ -3374,7 +3349,7 @@ doSave(void *opaque)
if (pthread_sigmask(SIG_BLOCK,&sigmask,&oldsigmask)< 0)
goto out_sig;
- if (vshCommandOptString(cmd, "file",&to)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&to)< 0)
goto out;
if (vshCommandOptBool(cmd, "bypass-cache"))
@@ -3384,10 +3359,8 @@ doSave(void *opaque)
if (vshCommandOptBool(cmd, "paused"))
flags |= VIR_DOMAIN_SAVE_PAUSED;
- if (vshCommandOptString(cmd, "xml",&xmlfile)< 0) {
- vshError(ctl, "%s", _("malformed xml argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "xml",&xmlfile)< 0)
goto out;
- }
if (!(dom = vshCommandOptDomain(ctl, cmd,&name)))
goto out;
@@ -3523,7 +3496,7 @@ cmdSave(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd,&name)))
return false;
- if (vshCommandOptString(cmd, "file",&to)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&to)< 0)
goto cleanup;
if (vshCommandOptBool(cmd, "verbose"))
@@ -3590,7 +3563,7 @@ cmdSaveImageDumpxml(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "security-info"))
flags |= VIR_DOMAIN_XML_SECURE;
- if (vshCommandOptString(cmd, "file",&file)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&file)< 0)
return false;
xml = virDomainSaveImageGetXMLDesc(ctl->conn, file, flags);
@@ -3652,13 +3625,11 @@ cmdSaveImageDefine(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "paused"))
flags |= VIR_DOMAIN_SAVE_PAUSED;
- if (vshCommandOptString(cmd, "file",&file)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&file)< 0)
return false;
- if (vshCommandOptString(cmd, "xml",&xmlfile)<= 0) {
- vshError(ctl, "%s", _("malformed or missing xml
argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "xml",&xmlfile)<= 0)
Again, think you mean "< 0"
return false;
- }
if (virFileReadAll(xmlfile, 8192,&xml)< 0)
goto cleanup;
@@ -3726,7 +3697,7 @@ cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd)
return false;
}
- if (vshCommandOptString(cmd, "file",&file)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&file)< 0)
return false;
#define EDIT_GET_XML \
@@ -4213,7 +4184,7 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd)
const char *xmlfile = NULL;
char *xml = NULL;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (vshCommandOptBool(cmd, "bypass-cache"))
@@ -4223,10 +4194,8 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "paused"))
flags |= VIR_DOMAIN_SAVE_PAUSED;
- if (vshCommandOptString(cmd, "xml",&xmlfile)< 0) {
- vshError(ctl, "%s", _("malformed xml argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "xml",&xmlfile)< 0)
return false;
- }
if (xmlfile&&
virFileReadAll(xmlfile, 8192,&xml)< 0)
@@ -4318,7 +4287,7 @@ doDump(void *opaque)
if (pthread_sigmask(SIG_BLOCK,&sigmask,&oldsigmask)< 0)
goto out_sig;
- if (vshCommandOptString(cmd, "file",&to)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&to)< 0)
goto out;
if (!(dom = vshCommandOptDomain(ctl, cmd,&name)))
@@ -4364,7 +4333,7 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd,&name)))
return false;
- if (vshCommandOptString(cmd, "file",&to)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&to)< 0)
return false;
if (vshCommandOptBool(cmd, "verbose"))
@@ -4474,10 +4443,8 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
bool generated = false;
char *mime = NULL;
- if (vshCommandOptString(cmd, "file", (const char **)&file)< 0) {
- vshError(ctl, "%s", _("file must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "file", (const char **)&file)<
0)
return false;
- }
if (vshCommandOptUInt(cmd, "screen",&screen)< 0) {
vshError(ctl, "%s", _("invalid screen ID"));
@@ -4615,10 +4582,8 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd)
int rv;
char **modes = NULL, **tmp;
- if (vshCommandOptString(cmd, "mode",&mode)< 0) {
- vshError(ctl, "%s", _("Invalid type"));
+ if (vshCommandOptStringReq(ctl, cmd, "mode",&mode)< 0)
return false;
- }
if (mode&& !(modes = virStringSplit(mode, ",", 0))) {
vshError(ctl, "%s", _("Cannot parse mode string"));
@@ -4699,10 +4664,8 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd)
int flags = 0;
char **modes = NULL, **tmp;
- if (vshCommandOptString(cmd, "mode",&mode)< 0) {
- vshError(ctl, "%s", _("Invalid type"));
+ if (vshCommandOptStringReq(ctl, cmd, "mode",&mode)< 0)
return false;
- }
if (mode&& !(modes = virStringSplit(mode, ",", 0))) {
vshError(ctl, "%s", _("Cannot parse mode string"));
@@ -4926,14 +4889,12 @@ cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
const char *type = NULL;
int vcpus;
- if (vshCommandOptString(cmd, "type",&type)< 0) {
- vshError(ctl, "%s", _("Invalid type"));
+ if (vshCommandOptStringReq(ctl, cmd, "type",&type)< 0)
return false;
- }
- vcpus = virConnectGetMaxVcpus(ctl->conn, type);
- if (vcpus< 0)
+ if ((vcpus = virConnectGetMaxVcpus(ctl->conn, type))< 0)
return false;
+
vshPrint(ctl, "%d\n", vcpus);
return true;
@@ -5385,14 +5346,12 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
flags = -1;
}
- if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+ if (vshCommandOptStringReq(ctl, cmd, "cpulist",&cpulist)< 0)
return false;
- if (vshCommandOptString(cmd, "cpulist",&cpulist)< 0) {
- vshError(ctl, "%s", _("vcpupin: Missing cpulist."));
- virDomainFree(dom);
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- }
+
query = !cpulist;
/* In query mode, "vcpu" is optional */
@@ -5623,8 +5582,7 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "cpulist",&cpulist)< 0) {
- vshError(ctl, "%s", _("emulatorpin: Missing cpulist."));
+ if (vshCommandOptStringReq(ctl, cmd, "cpulist",&cpulist)< 0) {
virDomainFree(dom);
return false;
}
@@ -5896,14 +5854,11 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
xmlBufferPtr xml_buf = NULL;
xmlNodePtr node;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
- if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0) {
- vshError(ctl, _("Failed to read file '%s' to compare"),
- from);
+ if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
return false;
- }
/* try to extract the CPU element from as it would appear in a domain XML*/
if (!(xml = virXMLParseStringCtxt(buffer, from,&ctxt)))
@@ -6000,7 +5955,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
virBuffer buf = VIR_BUFFER_INITIALIZER;
int i;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
@@ -6287,7 +6242,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd)
#endif
unsigned int flags = VIR_DOMAIN_NONE;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
@@ -6342,7 +6297,7 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
@@ -6813,15 +6768,11 @@ cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "pid",&pidstr)<= 0) {
- vshError(ctl, "%s", _("missing argument"));
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "pid",&pidstr)< 0)
+ goto cleanup;
- if (vshCommandOptString(cmd, "signame",&signame)<= 0) {
- vshError(ctl, "%s", _("missing argument"));
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "signame",&signame)< 0)
+ goto cleanup;
if (virStrToLong_ll(pidstr, NULL, 10,&pid_value)< 0) {
vshError(ctl, _("malformed PID value: %s"), pidstr);
@@ -7334,19 +7285,17 @@ cmdNumatune(vshControl * ctl, const vshCmd * cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "nodeset",&nodeset)< 0) {
- vshError(ctl, "%s", _("Unable to parse nodeset."));
+ if (vshCommandOptStringReq(ctl, cmd, "nodeset",&nodeset)< 0)
goto cleanup;
- }
+
if (nodeset&&
virTypedParamsAddString(¶ms,&nparams,&maxparams,
VIR_DOMAIN_NUMA_NODESET, nodeset)< 0)
goto save_error;
- if (vshCommandOptString(cmd, "mode",&mode)< 0) {
- vshError(ctl, "%s", _("Unable to parse mode."));
+ if (vshCommandOptStringReq(ctl, cmd, "mode",&mode)< 0)
goto cleanup;
- }
+
if (mode) {
int m;
/* Accept string or integer, in case server understands newer
@@ -7865,8 +7814,8 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd)
char *xmlData;
unsigned int flags = 0;
- if (vshCommandOptString(cmd, "format",&format)< 0 ||
- vshCommandOptString(cmd, "config",&configFile)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "format",&format)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "config",&configFile)< 0)
return false;
if (virFileReadAll(configFile, 1024*1024,&configData)< 0)
@@ -7917,8 +7866,8 @@ cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
char *xmlData;
unsigned int flags = 0;
- if (vshCommandOptString(cmd, "format",&format)< 0
- || vshCommandOptString(cmd, "xml",&xmlFile)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "format",&format)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "xml",&xmlFile)< 0)
return false;
if (virFileReadAll(xmlFile, 1024*1024,&xmlData)< 0)
@@ -8179,17 +8128,11 @@ doMigrate(void *opaque)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto out;
- if (vshCommandOptString(cmd, "desturi",&desturi)<= 0 ||
- vshCommandOptString(cmd, "migrateuri",&migrateuri)< 0 ||
- vshCommandOptString(cmd, "dname",&dname)< 0) {
- vshError(ctl, "%s", _("missing argument"));
- goto out;
- }
-
- if (vshCommandOptString(cmd, "xml",&xmlfile)< 0) {
- vshError(ctl, "%s", _("malformed xml argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "desturi",&desturi)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "migrateuri",&migrateuri)< 0
||
+ vshCommandOptStringReq(ctl, cmd, "dname",&dname)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "xml",&xmlfile)< 0)
goto out;
- }
if (vshCommandOptBool(cmd, "live"))
flags |= VIR_MIGRATE_LIVE;
@@ -9115,7 +9058,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
goto cleanup;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0) {
@@ -9189,22 +9132,19 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
const char *from = NULL;
- char *buffer;
- int ret;
+ char *buffer = NULL;
+ bool ret = false;
unsigned int flags;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "file",&from)<= 0) {
- virDomainFree(dom);
- return false;
- }
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
+ goto cleanup;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0) {
vshReportError(ctl);
- virDomainFree(dom);
- return false;
+ goto cleanup;
}
if (vshCommandOptBool(cmd, "config")) {
@@ -9218,19 +9158,18 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "force"))
flags |= VIR_DOMAIN_DEVICE_MODIFY_FORCE;
- ret = virDomainUpdateDeviceFlags(dom, buffer, flags);
- VIR_FREE(buffer);
-
- if (ret< 0) {
+ if (virDomainUpdateDeviceFlags(dom, buffer, flags)< 0) {
vshError(ctl, _("Failed to update device from %s"), from);
- virDomainFree(dom);
- return false;
- } else {
- vshPrint(ctl, "%s", _("Device updated successfully\n"));
+ goto cleanup;
}
+ vshPrint(ctl, "%s", _("Device updated successfully\n"));
+ ret = true;
+
+cleanup:
+ VIR_FREE(buffer);
virDomainFree(dom);
- return true;
+ return ret;
}
/*
@@ -9289,15 +9228,13 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
unsigned int flags;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
- goto cleanup;
+ return false;
- if (vshCommandOptString(cmd, "type",&type)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "type",&type)< 0)
goto cleanup;
- if (vshCommandOptString(cmd, "mac",&mac)< 0) {
- vshError(ctl, "%s", _("missing option"));
+ if (vshCommandOptStringReq(ctl, cmd, "mac",&mac)< 0)
goto cleanup;
- }
doc = virDomainGetXMLDesc(dom, 0);
if (!doc)
@@ -9389,8 +9326,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
}
cleanup:
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
xmlXPathFreeObject(obj);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
@@ -9660,9 +9596,9 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
xmlNodePtr disk_node = NULL;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
- goto cleanup;
+ return false;
- if (vshCommandOptString(cmd, "target",&target)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "target",&target)< 0)
goto cleanup;
doc = virDomainGetXMLDesc(dom, 0);
@@ -9698,8 +9634,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
xmlFreeNode(disk_node);
VIR_FREE(disk_xml);
VIR_FREE(doc);
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
return functionReturn;
}
@@ -9887,10 +9822,10 @@ cmdChangeMedia(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "path",&path)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "path",&path)< 0)
goto cleanup;
- if (vshCommandOptString(cmd, "source",&source)< 0)
+ if (vshCommandOptStringReq(ctl, cmd, "source",&source)< 0)
goto cleanup;
if (insert&& !source) {
@@ -9970,10 +9905,8 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (vshCommandOptString(cmd, "mountpoint",&mountPoint)< 0) {
- vshError(ctl, _("Unable to parse mountpoint parameter"));
+ if (vshCommandOptStringReq(ctl, cmd, "mountpoint",&mountPoint)<
0)
goto cleanup;
- }
if (virDomainFSTrim(dom, mountPoint, minimum, flags)< 0) {
vshError(ctl, _("Unable to invoke fstrim"));
ACK with the one "<= 0" fixed.
12:07 p.m.
New subject: [libvirt] [PATCH 09/17] virsh-host: Update host commands to use vshCommandOptStringReq
---
tools/virsh-host.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index d05e435..5c33c2d 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -103,10 +103,9 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
}
VIR_FREE(ctl->name);
- if (vshCommandOptString(cmd, "name", &name) < 0) {
- vshError(ctl, "%s", _("Please specify valid connection
URI"));
+ if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
return false;
- }
+
ctl->name = vshStrdup(ctl, name);
ctl->useGetInfo = false;
@@ -550,10 +549,8 @@ cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd)
unsigned int suspendTarget;
long long duration;
- if (vshCommandOptString(cmd, "target", &target) < 0) {
- vshError(ctl, _("Invalid target argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0)
return false;
- }
if (vshCommandOptLongLong(cmd, "duration", &duration) < 0) {
vshError(ctl, _("Invalid duration argument"));
--
1.8.1.1
12:59 a.m.
New subject: [libvirt] [PATCH 09/17] virsh-host: Update host commands to use vshCommandOptStringReq
On 2013年01月22日 02:07, Peter Krempa wrote:
---
tools/virsh-host.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index d05e435..5c33c2d 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -103,10 +103,9 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
}
VIR_FREE(ctl->name);
- if (vshCommandOptString(cmd, "name",&name)< 0) {
- vshError(ctl, "%s", _("Please specify valid connection
URI"));
+ if (vshCommandOptStringReq(ctl, cmd, "name",&name)< 0)
return false;
- }
+
ctl->name = vshStrdup(ctl, name);
ctl->useGetInfo = false;
@@ -550,10 +549,8 @@ cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd)
unsigned int suspendTarget;
long long duration;
- if (vshCommandOptString(cmd, "target",&target)< 0) {
- vshError(ctl, _("Invalid target argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "target",&target)< 0)
return false;
- }
if (vshCommandOptLongLong(cmd, "duration",&duration)< 0) {
vshError(ctl, _("Invalid duration argument"));
ACK
12:07 p.m.
New subject: [libvirt] [PATCH 10/17] virsh-network: Update network commands to use vshCommandOptStringReq
---
tools/virsh-network.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 9ba3ca8..0843a6b 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -50,7 +50,7 @@ vshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd,
if (!vshCmdHasOption(ctl, cmd, optname))
return NULL;
- if (vshCommandOptString(cmd, optname, &n) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0)
return NULL;
vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n",
@@ -158,7 +158,7 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
@@ -204,7 +204,7 @@ cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
@@ -889,10 +889,8 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd)
if (!(network = vshCommandOptNetwork(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "command", &commandStr) < 0) {
- vshError(ctl, "%s", _("missing or malformed command
argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "command", &commandStr) < 0)
goto cleanup;
- }
if (STREQ(commandStr, "add")) {
/* "add" is a synonym for "add-last" */
@@ -905,10 +903,9 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd)
}
}
- if (vshCommandOptString(cmd, "section", §ionStr) < 0) {
- vshError(ctl, "%s", _("missing or malformed section
argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "section", §ionStr) < 0)
goto cleanup;
- }
+
section = virNetworkSectionTypeFromString(sectionStr);
if (section <= 0 || section >= VIR_NETWORK_SECTION_LAST) {
vshError(ctl, _("unrecognized section name '%s'"),
sectionStr);
@@ -927,10 +924,8 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd)
* the desired xml.
*/
- if (vshCommandOptString(cmd, "xml", &xml) < 0) {
- vshError(ctl, "%s", _("malformed or missing xml argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "xml", &xml) < 0)
goto cleanup;
- }
if (*xml != '<') {
/* contents of xmldata is actually the name of a file that
--
1.8.1.1
12:59 a.m.
New subject: [libvirt] [PATCH 10/17] virsh-network: Update network commands to use vshCommandOptStringReq
On 2013年01月22日 02:07, Peter Krempa wrote:
---
tools/virsh-network.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 9ba3ca8..0843a6b 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -50,7 +50,7 @@ vshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd,
if (!vshCmdHasOption(ctl, cmd, optname))
return NULL;
- if (vshCommandOptString(cmd, optname,&n)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname,&n)< 0)
return NULL;
vshDebug(ctl, VSH_ERR_INFO, "%s: found option<%s>: %s\n",
@@ -158,7 +158,7 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
@@ -204,7 +204,7 @@ cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
@@ -889,10 +889,8 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd)
if (!(network = vshCommandOptNetwork(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "command",&commandStr)< 0) {
- vshError(ctl, "%s", _("missing or malformed command
argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "command",&commandStr)< 0)
goto cleanup;
- }
if (STREQ(commandStr, "add")) {
/* "add" is a synonym for "add-last" */
@@ -905,10 +903,9 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd)
}
}
- if (vshCommandOptString(cmd, "section",§ionStr)< 0) {
- vshError(ctl, "%s", _("missing or malformed section
argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "section",§ionStr)< 0)
goto cleanup;
- }
+
section = virNetworkSectionTypeFromString(sectionStr);
if (section<= 0 || section>= VIR_NETWORK_SECTION_LAST) {
vshError(ctl, _("unrecognized section name '%s'"),
sectionStr);
@@ -927,10 +924,8 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd)
* the desired xml.
*/
- if (vshCommandOptString(cmd, "xml",&xml)< 0) {
- vshError(ctl, "%s", _("malformed or missing xml
argument"));
+ if (vshCommandOptStringReq(ctl, cmd, "xml",&xml)< 0)
goto cleanup;
- }
if (*xml != '<') {
/* contents of xmldata is actually the name of a file that
ACK
12:07 p.m.
New subject: [libvirt] [PATCH 11/17] virsh-interface: Update interface commands to use vshCommandOptStringReq
---
tools/virsh-interface.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index 57cfc4d..5337f5e 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -51,7 +51,7 @@ vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd,
if (!vshCmdHasOption(ctl, cmd, optname))
return NULL;
- if (vshCommandOptString(cmd, optname, &n) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0)
return NULL;
vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n",
@@ -503,7 +503,7 @@ cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
@@ -778,10 +778,8 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd)
}
/* Name for new bridge device */
- if (vshCommandOptString(cmd, "bridge", &br_name) <= 0) {
- vshError(ctl, "%s", _("Missing bridge device name in
command"));
+ if (vshCommandOptStringReq(ctl, cmd, "bridge", &br_name) < 0)
goto cleanup;
- }
/* make sure "new" device doesn't already exist */
if ((br_handle = virInterfaceLookupByName(ctl->conn, br_name))) {
--
1.8.1.1
1 a.m.
New subject: [libvirt] [PATCH 11/17] virsh-interface: Update interface commands to use vshCommandOptStringReq
On 2013年01月22日 02:07, Peter Krempa wrote:
---
tools/virsh-interface.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index 57cfc4d..5337f5e 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -51,7 +51,7 @@ vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd,
if (!vshCmdHasOption(ctl, cmd, optname))
return NULL;
- if (vshCommandOptString(cmd, optname,&n)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname,&n)< 0)
return NULL;
vshDebug(ctl, VSH_ERR_INFO, "%s: found option<%s>: %s\n",
@@ -503,7 +503,7 @@ cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
@@ -778,10 +778,8 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd)
}
/* Name for new bridge device */
- if (vshCommandOptString(cmd, "bridge",&br_name)<= 0) {
- vshError(ctl, "%s", _("Missing bridge device name in
command"));
+ if (vshCommandOptStringReq(ctl, cmd, "bridge",&br_name)< 0)
goto cleanup;
- }
/* make sure "new" device doesn't already exist */
if ((br_handle = virInterfaceLookupByName(ctl->conn, br_name))) {
ACK
12:07 p.m.
New subject: [libvirt] [PATCH 12/17] virsh-nodedev: Update node device commands to use vshCommandOptStringReq
---
tools/virsh-nodedev.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 11135ab..8b4c75b 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -67,7 +67,7 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
@@ -115,7 +115,7 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
const char *name = NULL;
- if (vshCommandOptString(cmd, "name", &name) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
return false;
dev = virNodeDeviceLookupByName(ctl->conn, name);
@@ -488,7 +488,7 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
char *xml;
- if (vshCommandOptString(cmd, "device", &name) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0)
return false;
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
@@ -533,7 +533,7 @@ cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
bool ret = true;
- if (vshCommandOptString(cmd, "device", &name) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0)
return false;
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
@@ -578,7 +578,7 @@ cmdNodeDeviceReAttach(vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
bool ret = true;
- if (vshCommandOptString(cmd, "device", &name) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0)
return false;
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
@@ -621,7 +621,7 @@ cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
bool ret = true;
- if (vshCommandOptString(cmd, "device", &name) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0)
return false;
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
--
1.8.1.1
1 a.m.
New subject: [libvirt] [PATCH 12/17] virsh-nodedev: Update node device commands to use vshCommandOptStringReq
On 2013年01月22日 02:07, Peter Krempa wrote:
---
tools/virsh-nodedev.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 11135ab..8b4c75b 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -67,7 +67,7 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
@@ -115,7 +115,7 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
const char *name = NULL;
- if (vshCommandOptString(cmd, "name",&name)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "name",&name)< 0)
return false;
dev = virNodeDeviceLookupByName(ctl->conn, name);
@@ -488,7 +488,7 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
char *xml;
- if (vshCommandOptString(cmd, "device",&name)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device",&name)< 0)
return false;
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
@@ -533,7 +533,7 @@ cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
bool ret = true;
- if (vshCommandOptString(cmd, "device",&name)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device",&name)< 0)
return false;
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
@@ -578,7 +578,7 @@ cmdNodeDeviceReAttach(vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
bool ret = true;
- if (vshCommandOptString(cmd, "device",&name)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device",&name)< 0)
return false;
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
@@ -621,7 +621,7 @@ cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd)
virNodeDevicePtr device;
bool ret = true;
- if (vshCommandOptString(cmd, "device",&name)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "device",&name)< 0)
return false;
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
ACK
12:07 p.m.
New subject: [libvirt] [PATCH 13/17] virsh-nodedev: Refactor error paths, error messages and whitespace
This patch adds some empty lines to separate blocks of code, cleans up
unnecessary error message constructs in cmdNodeDeviceDetach,
cmdNodeDeviceReAttach, cmdNodeDeviceReset and refactors error paths in
cmdNodeDeviceDumpXML.
---
tools/virsh-nodedev.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 8b4c75b..f85bded 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -485,26 +485,28 @@ static bool
cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd)
{
const char *name = NULL;
- virNodeDevicePtr device;
- char *xml;
+ virNodeDevicePtr device = NULL;
+ char *xml = NULL;
+ bool ret = false;
if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0)
return false;
+
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
- vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
+ vshError(ctl, _("Could not find matching device '%s'"), name);
return false;
}
- xml = virNodeDeviceGetXMLDesc(device, 0);
- if (!xml) {
- virNodeDeviceFree(device);
- return false;
- }
+ if (!(xml = virNodeDeviceGetXMLDesc(device, 0)))
+ goto cleanup;
vshPrint(ctl, "%s\n", xml);
+ ret = true;
+
+cleanup:
VIR_FREE(xml);
virNodeDeviceFree(device);
- return true;
+ return ret;
}
/*
@@ -535,8 +537,9 @@ cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0)
return false;
+
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
- vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
+ vshError(ctl, _("Could not find matching device '%s'"), name);
return false;
}
@@ -548,6 +551,7 @@ cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("Failed to detach device %s"), name);
ret = false;
}
+
virNodeDeviceFree(device);
return ret;
}
@@ -580,8 +584,9 @@ cmdNodeDeviceReAttach(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0)
return false;
+
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
- vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
+ vshError(ctl, _("Could not find matching device '%s'"), name);
return false;
}
@@ -591,6 +596,7 @@ cmdNodeDeviceReAttach(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("Failed to re-attach device %s"), name);
ret = false;
}
+
virNodeDeviceFree(device);
return ret;
}
@@ -623,8 +629,9 @@ cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0)
return false;
+
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
- vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
+ vshError(ctl, _("Could not find matching device '%s'"), name);
return false;
}
@@ -634,6 +641,7 @@ cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("Failed to reset device %s"), name);
ret = false;
}
+
virNodeDeviceFree(device);
return ret;
}
--
1.8.1.1
1:04 a.m.
New subject: [libvirt] [PATCH 13/17] virsh-nodedev: Refactor error paths, error messages and whitespace
On 2013年01月22日 02:07, Peter Krempa wrote:
This patch adds some empty lines to separate blocks of code, cleans
up
unnecessary error message constructs in cmdNodeDeviceDetach,
cmdNodeDeviceReAttach, cmdNodeDeviceReset and refactors error paths in
cmdNodeDeviceDumpXML.
---
tools/virsh-nodedev.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 8b4c75b..f85bded 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -485,26 +485,28 @@ static bool
cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd)
{
const char *name = NULL;
- virNodeDevicePtr device;
- char *xml;
+ virNodeDevicePtr device = NULL;
+ char *xml = NULL;
+ bool ret = false;
if (vshCommandOptStringReq(ctl, cmd, "device",&name)< 0)
return false;
+
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
- vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
+ vshError(ctl, _("Could not find matching device '%s'"),
name);
return false;
}
- xml = virNodeDeviceGetXMLDesc(device, 0);
- if (!xml) {
- virNodeDeviceFree(device);
- return false;
- }
+ if (!(xml = virNodeDeviceGetXMLDesc(device, 0)))
+ goto cleanup;
vshPrint(ctl, "%s\n", xml);
+ ret = true;
+
+cleanup:
VIR_FREE(xml);
virNodeDeviceFree(device);
- return true;
+ return ret;
}
/*
@@ -535,8 +537,9 @@ cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "device",&name)< 0)
return false;
+
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
- vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
+ vshError(ctl, _("Could not find matching device '%s'"),
name);
return false;
}
@@ -548,6 +551,7 @@ cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("Failed to detach device %s"), name);
ret = false;
}
+
virNodeDeviceFree(device);
return ret;
}
@@ -580,8 +584,9 @@ cmdNodeDeviceReAttach(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "device",&name)< 0)
return false;
+
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
- vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
+ vshError(ctl, _("Could not find matching device '%s'"),
name);
return false;
}
@@ -591,6 +596,7 @@ cmdNodeDeviceReAttach(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("Failed to re-attach device %s"), name);
ret = false;
}
+
virNodeDeviceFree(device);
return ret;
}
@@ -623,8 +629,9 @@ cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "device",&name)< 0)
return false;
+
if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) {
- vshError(ctl, "%s '%s'", _("Could not find matching
device"), name);
+ vshError(ctl, _("Could not find matching device '%s'"),
name);
return false;
}
@@ -634,6 +641,7 @@ cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("Failed to reset device %s"), name);
ret = false;
}
+
virNodeDeviceFree(device);
return ret;
}
ACK
12:07 p.m.
New subject: [libvirt] [PATCH 14/17] virsh-snapshot: Refactor cmdSnapshotDumpXML
This patch simplifies error paths and switches to use
vshCommandOptStringReq for argument retrieval in cmdSnapshotDumpXML
---
tools/virsh-snapshot.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index f76c40e..c980b5b 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -1775,31 +1775,26 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "security-info"))
flags |= VIR_DOMAIN_XML_SECURE;
- dom = vshCommandOptDomain(ctl, cmd, NULL);
- if (dom == NULL)
- goto cleanup;
+ if (vshCommandOptStringReq(ctl, cmd, "snapshotname", &name) < 0)
+ return false;
- if (vshCommandOptString(cmd, "snapshotname", &name) <= 0)
- goto cleanup;
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+ return false;
- snapshot = virDomainSnapshotLookupByName(dom, name, 0);
- if (snapshot == NULL)
+ if (!(snapshot = virDomainSnapshotLookupByName(dom, name, 0)))
goto cleanup;
- xml = virDomainSnapshotGetXMLDesc(snapshot, flags);
- if (!xml)
+ if (!(xml = virDomainSnapshotGetXMLDesc(snapshot, flags)))
goto cleanup;
vshPrint(ctl, "%s", xml);
-
ret = true;
cleanup:
VIR_FREE(xml);
if (snapshot)
virDomainSnapshotFree(snapshot);
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
return ret;
}
--
1.8.1.1
1:07 a.m.
New subject: [libvirt] [PATCH 14/17] virsh-snapshot: Refactor cmdSnapshotDumpXML
On 2013年01月22日 02:07, Peter Krempa wrote:
This patch simplifies error paths and switches to use
vshCommandOptStringReq for argument retrieval in cmdSnapshotDumpXML
---
tools/virsh-snapshot.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index f76c40e..c980b5b 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -1775,31 +1775,26 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "security-info"))
flags |= VIR_DOMAIN_XML_SECURE;
- dom = vshCommandOptDomain(ctl, cmd, NULL);
- if (dom == NULL)
- goto cleanup;
+ if (vshCommandOptStringReq(ctl, cmd, "snapshotname",&name)< 0)
+ return false;
- if (vshCommandOptString(cmd, "snapshotname",&name)<= 0)
- goto cleanup;
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+ return false;
- snapshot = virDomainSnapshotLookupByName(dom, name, 0);
- if (snapshot == NULL)
+ if (!(snapshot = virDomainSnapshotLookupByName(dom, name, 0)))
goto cleanup;
- xml = virDomainSnapshotGetXMLDesc(snapshot, flags);
- if (!xml)
+ if (!(xml = virDomainSnapshotGetXMLDesc(snapshot, flags)))
goto cleanup;
vshPrint(ctl, "%s", xml);
-
ret = true;
cleanup:
VIR_FREE(xml);
if (snapshot)
virDomainSnapshotFree(snapshot);
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
return ret;
}
ACK
12:07 p.m.
New subject: [libvirt] [PATCH 15/17] virsh-snapshot: Update snapshot commands to use vshCommandOptStringReq
also avoids potential NULL pointer dereference:
$ virsh snapshot-current asdf ""
error: invalid snapshotname argument '(null)'
by removing the error message in favor of vshCommandOptStringReq
---
tools/virsh-snapshot.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index c980b5b..ba57059 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -207,7 +207,7 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "xmlfile", &from) <= 0) {
+ if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0) {
buffer = vshStrdup(ctl, "<domainsnapshot/>");
} else {
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
@@ -431,11 +431,9 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
if (dom == NULL)
goto cleanup;
- if (vshCommandOptString(cmd, "name", &name) < 0 ||
- vshCommandOptString(cmd, "description", &desc) < 0) {
- vshError(ctl, _("argument must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0 ||
+ vshCommandOptStringReq(ctl, cmd, "description", &desc) < 0)
goto cleanup;
- }
virBufferAddLit(&buf, "<domainsnapshot>\n");
if (name)
@@ -443,10 +441,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
if (desc)
virBufferEscapeString(&buf, "
<description>%s</description>\n", desc);
- if (vshCommandOptString(cmd, "memspec", &memspec) < 0) {
- vshError(ctl, _("memspec argument must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "memspec", &memspec) < 0)
goto cleanup;
- }
if (memspec && vshParseSnapshotMemspec(ctl, &buf, memspec) < 0)
goto cleanup;
@@ -496,10 +492,8 @@ vshLookupSnapshot(vshControl *ctl, const vshCmd *cmd,
bool current = vshCommandOptBool(cmd, "current");
const char *snapname = NULL;
- if (vshCommandOptString(cmd, arg, &snapname) < 0) {
- vshError(ctl, _("invalid argument for --%s"), arg);
+ if (vshCommandOptStringReq(ctl, cmd, arg, &snapname) < 0)
return -1;
- }
if (exclusive && current && snapname) {
vshError(ctl, _("--%s and --current are mutually exclusive"), arg);
@@ -703,10 +697,9 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
if (dom == NULL)
goto cleanup;
- if (vshCommandOptString(cmd, "snapshotname", &snapshotname) < 0) {
- vshError(ctl, _("invalid snapshotname argument '%s'"),
snapshotname);
+ if (vshCommandOptStringReq(ctl, cmd, "snapshotname", &snapshotname)
< 0)
goto cleanup;
- }
+
if (snapshotname) {
virDomainSnapshotPtr snapshot2 = NULL;
flags = (VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
--
1.8.1.1
1:08 a.m.
New subject: [libvirt] [PATCH 15/17] virsh-snapshot: Update snapshot commands to use vshCommandOptStringReq
On 2013年01月22日 02:07, Peter Krempa wrote:
also avoids potential NULL pointer dereference:
$ virsh snapshot-current asdf ""
error: invalid snapshotname argument '(null)'
by removing the error message in favor of vshCommandOptStringReq
---
tools/virsh-snapshot.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index c980b5b..ba57059 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -207,7 +207,7 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (vshCommandOptString(cmd, "xmlfile",&from)<= 0) {
+ if (vshCommandOptStringReq(ctl, cmd, "xmlfile",&from)< 0) {
buffer = vshStrdup(ctl, "<domainsnapshot/>");
} else {
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0) {
@@ -431,11 +431,9 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
if (dom == NULL)
goto cleanup;
- if (vshCommandOptString(cmd, "name",&name)< 0 ||
- vshCommandOptString(cmd, "description",&desc)< 0) {
- vshError(ctl, _("argument must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "name",&name)< 0 ||
+ vshCommandOptStringReq(ctl, cmd, "description",&desc)< 0)
goto cleanup;
- }
virBufferAddLit(&buf, "<domainsnapshot>\n");
if (name)
@@ -443,10 +441,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
if (desc)
virBufferEscapeString(&buf,
"<description>%s</description>\n", desc);
- if (vshCommandOptString(cmd, "memspec",&memspec)< 0) {
- vshError(ctl, _("memspec argument must not be empty"));
+ if (vshCommandOptStringReq(ctl, cmd, "memspec",&memspec)< 0)
goto cleanup;
- }
if (memspec&& vshParseSnapshotMemspec(ctl,&buf, memspec)< 0)
goto cleanup;
@@ -496,10 +492,8 @@ vshLookupSnapshot(vshControl *ctl, const vshCmd *cmd,
bool current = vshCommandOptBool(cmd, "current");
const char *snapname = NULL;
- if (vshCommandOptString(cmd, arg,&snapname)< 0) {
- vshError(ctl, _("invalid argument for --%s"), arg);
+ if (vshCommandOptStringReq(ctl, cmd, arg,&snapname)< 0)
return -1;
- }
if (exclusive&& current&& snapname) {
vshError(ctl, _("--%s and --current are mutually exclusive"), arg);
@@ -703,10 +697,9 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
if (dom == NULL)
goto cleanup;
- if (vshCommandOptString(cmd, "snapshotname",&snapshotname)< 0) {
- vshError(ctl, _("invalid snapshotname argument '%s'"),
snapshotname);
+ if (vshCommandOptStringReq(ctl, cmd, "snapshotname",&snapshotname)<
0)
goto cleanup;
- }
+
if (snapshotname) {
virDomainSnapshotPtr snapshot2 = NULL;
flags = (VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
ACK
12:07 p.m.
New subject: [libvirt] [PATCH 16/17] virsh-nwfilter: Update nwfilter commands to use vshCommandOptStringReq
---
tools/virsh-nwfilter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c
index 61ec34c..9407d38 100644
--- a/tools/virsh-nwfilter.c
+++ b/tools/virsh-nwfilter.c
@@ -49,7 +49,7 @@ vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd,
if (!vshCmdHasOption(ctl, cmd, optname))
return NULL;
- if (vshCommandOptString(cmd, optname, &n) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0)
return NULL;
vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n",
@@ -103,7 +103,7 @@ cmdNWFilterDefine(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
--
1.8.1.1
1:09 a.m.
New subject: [libvirt] [PATCH 16/17] virsh-nwfilter: Update nwfilter commands to use vshCommandOptStringReq
On 2013年01月22日 02:07, Peter Krempa wrote:
---
tools/virsh-nwfilter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c
index 61ec34c..9407d38 100644
--- a/tools/virsh-nwfilter.c
+++ b/tools/virsh-nwfilter.c
@@ -49,7 +49,7 @@ vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd,
if (!vshCmdHasOption(ctl, cmd, optname))
return NULL;
- if (vshCommandOptString(cmd, optname,&n)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname,&n)< 0)
return NULL;
vshDebug(ctl, VSH_ERR_INFO, "%s: found option<%s>: %s\n",
@@ -103,7 +103,7 @@ cmdNWFilterDefine(vshControl *ctl, const vshCmd *cmd)
bool ret = true;
char *buffer;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
ACK
12:07 p.m.
New subject: [libvirt] [PATCH 17/17] virsh-secret: Refactor error paths
This patch switches string option retrieval to vshCommandOptStringReq
and refactors some error paths.
---
tools/virsh-secret.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 05bab85..a735498 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -48,7 +48,7 @@ vshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, const char
**name)
if (!vshCmdHasOption(ctl, cmd, optname))
return NULL;
- if (vshCommandOptString(cmd, optname, &n) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0)
return NULL;
vshDebug(ctl, VSH_ERR_DEBUG,
@@ -90,28 +90,31 @@ cmdSecretDefine(vshControl *ctl, const vshCmd *cmd)
char *buffer;
virSecretPtr res;
char uuid[VIR_UUID_STRING_BUFLEN];
+ bool ret = false;
- if (vshCommandOptString(cmd, "file", &from) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
return false;
- res = virSecretDefineXML(ctl->conn, buffer, 0);
- VIR_FREE(buffer);
-
- if (res == NULL) {
+ if (!(res = virSecretDefineXML(ctl->conn, buffer, 0))) {
vshError(ctl, _("Failed to set attributes from %s"), from);
- return false;
+ goto cleanup;
}
+
if (virSecretGetUUIDString(res, &(uuid[0])) < 0) {
vshError(ctl, "%s", _("Failed to get UUID of created
secret"));
- virSecretFree(res);
- return false;
+ goto cleanup;
}
+
vshPrint(ctl, _("Secret %s created\n"), uuid);
+ ret = true;
+
+cleanup:
+ VIR_FREE(buffer);
virSecretFree(res);
- return true;
+ return ret;
}
/*
@@ -188,11 +191,10 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
int res;
bool ret = false;
- secret = vshCommandOptSecret(ctl, cmd, NULL);
- if (secret == NULL)
+ if (!(secret = vshCommandOptSecret(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "base64", &base64) <= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "base64", &base64) < 0)
goto cleanup;
if (!base64_decode_alloc(base64, strlen(base64), &value, &value_size)) {
@@ -201,7 +203,7 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
}
if (value == NULL) {
vshError(ctl, "%s", _("Failed to allocate memory"));
- return false;
+ goto cleanup;
}
res = virSecretSetValue(secret, (unsigned char *)value, value_size, 0);
--
1.8.1.1
1:11 a.m.
New subject: [libvirt] [PATCH 17/17] virsh-secret: Refactor error paths
On 2013年01月22日 02:07, Peter Krempa wrote:
This patch switches string option retrieval to
vshCommandOptStringReq
and refactors some error paths.
---
tools/virsh-secret.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 05bab85..a735498 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -48,7 +48,7 @@ vshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, const char
**name)
if (!vshCmdHasOption(ctl, cmd, optname))
return NULL;
- if (vshCommandOptString(cmd, optname,&n)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, optname,&n)< 0)
return NULL;
vshDebug(ctl, VSH_ERR_DEBUG,
@@ -90,28 +90,31 @@ cmdSecretDefine(vshControl *ctl, const vshCmd *cmd)
char *buffer;
virSecretPtr res;
char uuid[VIR_UUID_STRING_BUFLEN];
+ bool ret = false;
- if (vshCommandOptString(cmd, "file",&from)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "file",&from)< 0)
return false;
if (virFileReadAll(from, VSH_MAX_XML_FILE,&buffer)< 0)
return false;
- res = virSecretDefineXML(ctl->conn, buffer, 0);
- VIR_FREE(buffer);
-
- if (res == NULL) {
+ if (!(res = virSecretDefineXML(ctl->conn, buffer, 0))) {
vshError(ctl, _("Failed to set attributes from %s"), from);
- return false;
+ goto cleanup;
}
+
if (virSecretGetUUIDString(res,&(uuid[0]))< 0) {
vshError(ctl, "%s", _("Failed to get UUID of created
secret"));
- virSecretFree(res);
- return false;
+ goto cleanup;
}
+
vshPrint(ctl, _("Secret %s created\n"), uuid);
+ ret = true;
+
+cleanup:
+ VIR_FREE(buffer);
virSecretFree(res);
- return true;
+ return ret;
}
/*
@@ -188,11 +191,10 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
int res;
bool ret = false;
- secret = vshCommandOptSecret(ctl, cmd, NULL);
- if (secret == NULL)
+ if (!(secret = vshCommandOptSecret(ctl, cmd, NULL)))
return false;
- if (vshCommandOptString(cmd, "base64",&base64)<= 0)
+ if (vshCommandOptStringReq(ctl, cmd, "base64",&base64)< 0)
goto cleanup;
if (!base64_decode_alloc(base64, strlen(base64),&value,&value_size)) {
@@ -201,7 +203,7 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd)
}
if (value == NULL) {
vshError(ctl, "%s", _("Failed to allocate memory"));
- return false;
+ goto cleanup;
Good to say this fixes a memory leak in commit msg, ACK.
7:34 a.m.
New subject: [libvirt] [PATCH 17/17] virsh-secret: Refactor error paths
On 01/31/13 08:11, Osier Yang wrote:
On 2013年01月22日 02:07, Peter Krempa wrote:
> This patch switches string option retrieval to vshCommandOptStringReq
> and refactors some error paths.
> ---
> tools/virsh-secret.c | 30 ++++++++++++++++--------------
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
[...]
Good to say this fixes a memory leak in commit msg, ACK.
I added that to the commit message and pushed the series.
Thanks for the review.
Peter
9:13 a.m.
On 01/21/13 19:07, Peter Krempa wrote:
This patchset fixes retrieval of string arguments in virsh to report
sane
errors in all the places it's used. A new string option retrieval function is
added that supports error reporting and changes appropriate places to use it.
Along with that this series cleans up a few places with old code and bad style.
Ping, now that it's after the release.
Peter
4309
days inactive
4323
days old
44 comments
2 participants
participants (2)
-
Osier Yang -
Peter Krempa