Re: [libvirt] [PATCH] [3/6] Add hook utilities

On Fri, Mar 26, 2010 at 10:53:01AM -0600, Eric Blake wrote: > On 03/26/2010 09:43 AM, Daniel Veillard wrote: > > + if (stat(path, &sb) < 0) { > > + ret = 0; > > + VIR_DEBUG("No hook script %s", path); > > + } else { > > + if (access(path, X_OK) != 0) { > > Should we also check for !S_ISDIR(&sb.st_mode), so that we explicitly > reject directories here, rather than failing later when trying to > execute them? Or go one step further and require regular files, with > the stricter check for S_ISREG(&sb.st_mode)? (Note: symlinks to regular > files would still be okay, given that you used stat().) Right, I'm adding this > > + * virHookInitialize: > > + * > > + * Initialize syncronous hooks support. > > s/syncronous/synchronous/ > > > +/** > > + * virHookPresent: > > + * @driver: the driver number (from virHookDriver enum) > > + * > > + * Check if a hook exists for the given driver, this is needed > > + * to avoid unecessary work if the hook is not present > > s/unecessary/unnecessary/ > > > +/* > > + * virHookCall: > > + * @driver: the driver number (from virHookDriver enum) > > + * @id: an id for the object '-' if non available for example on daemon hooks > > + * @op: the operation on the id e.g. VIR_HOOK_QEMU_OP_START > > + * @sub_op: a sub_operation, currently unused > > + * @extra: optional string informations > > s/informations/information/ (multiple times) > > > + * @input: extra input given to the script on stdin > > + * > > + * Implement an Hook call, where the external script for the driver is > > s/an Hook/a Hook/ > English is funny on 'a' vs. 'an' before a word starting in 'h'; > sometimes you just have to use a native speaker to get it right ;) fixed the typos, thanks for raising them, btw I would normally spot 'an hook' as a problem, but I guess I didn't reread thoise comments, thanks for spotting those !