[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] SetAutostart and GetAutostart in openvz driver



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 redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]