[libvirt] [PATCH] SetAutostart and GetAutostart in openvz driver
Daniel Veillard
veillard at redhat.com
Thu Jul 10 07:55:47 UTC 2008
On Wed, Jul 09, 2008 at 06:48:25PM +0400, Evgeniy Sokolov wrote:
> >
> >I realize you are just following the existing pattern used in OpenVZ
> >driver, but this piece of code is horrible.
> >
> >sprintf'ing into a string, then parsing that string and turning it back
> >into a list of argv[] strings, with no escaping of special characters,
> >or quoting. eg if the vm name had a space in it it'll mis-parse it.
> In fact, names of VMs are always numbers in OpenVZ.
> >
> >Just declare the command argv straight into a char*[], eg
> >
> > const char *prog[] = {
> > VZCTL,
> > "set",
> > vm->vmdef->name,
> > "--onboot",
> > autostart ? "yes" : "no",
> > "--save"
> > }
> > ret = virExec(conn, prog, &pid, -1, &outfd, &errfd)
> >
> >See the storage_backend_logical.c file for examples of this kind of
> >approach
> >
> >We should put other uses of convCmdbufExec() on the TODO list for removal
> >in the future.
> Ok. Moreover, many code is needed to rewrite.
>
> Daniel, thanks for review!
>
> fixed patch is attached.
Patch looks fine to me, applied and commited to CVS,
thanks !
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard at redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
More information about the libvir-list
mailing list