On 05/19/2013 11:52 AM, Michal Privoznik wrote:
> Currently, the openvzDomainSetNetwork function constructs an
> array of strings representing a command line for VZCTL binary.
> This is a overkill since our virCommand APIs can cover all the
> functionality.
> ---
> src/openvz/openvz_driver.c | 56 ++++++++++------------------------------------
> 1 file changed, 12 insertions(+), 44 deletions(-)
>
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index 129e328..db41d72 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c
snip snip snip
> @@ -922,37 +892,35 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid,
> if (!(opt = virBufferContentAndReset(&buf)))
> goto no_memory;
>
> - ADD_ARG_LIT(opt) ;
> + /* --netif_add ifname[,mac,host_ifname,host_mac] */
> + virCommandAddArgList(cmd, "--netif_add", opt, NULL);
> VIR_FREE(opt);
> } else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
> net->data.ethernet.ipaddr != NULL) {
> /* --ipadd ip */
> - ADD_ARG_LIT("--ipadd") ;
> - ADD_ARG_LIT(net->data.ethernet.ipaddr) ;
> + virCommandAddArgList(cmd, "--ipadd", net->data.ethernet.ipaddr, NULL);
> }
>
> /* TODO: processing NAT and physical device */
>
> - if (prog[0] != NULL) {
> - ADD_ARG_LIT("--save");
> - if (virRun(prog, NULL) < 0) {
> + if (cmd) {
cmd may be NULL here on OOM too.
> + virCommandAddArg(cmd, "--save");
> + if (virCommandRun(cmd, NULL) < 0) {
> rc = -1;
> goto exit;
Redundant goto (unless you initialize rc to -1 and use it to skip over
"rc = 0").
> }
> }
>
> exit:
> - cmdExecFree(prog);
> + virCommandFree(cmd);
> return rc;
>
> no_memory:
> VIR_FREE(opt);
opt is NULL here.
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not put argument to %s"), VZCTL);
I think a standard "out of memory" message would be more fitting here.
You could also rename 'exit' to 'cleanup' and get rid of the 'no_memory' label
completely.
Jan
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list