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@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list