[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 04/23] libxc: add support for FreeBSD
Roger Pau Monne writes ("[PATCH RFC 04/23] libxc: add support for FreeBSD"): > Add the FreeBSD implementation of the privcmd and evtchn devices > interface. > > The evtchn device interface is the same as the Linux one, while the > privcmd map interface is simplified because FreeBSD only supports > IOCTL_PRIVCMD_MMAPBATCH. ... > +/* Optionally flush file to disk and discard page cache */ > +void discard_file_cache(xc_interface *xch, int fd, int flush) > +{ My main criticism of this is that much of this file is a (nearly) verbatim copy of the Linux functions. I think we should have a conversation about other approaches that would allow us to share more of the code. Two options I can think of (which could perhaps be combined): xc_posix.c containing some "standard" versions #ifdeffery Now, prompted by your patch to look at this function: > + off_t cur = 0; > + int saved_errno = errno; > + > + if ( flush && (fsync(fd) < 0) ) > + goto out; This function has truly demented error behaviour, but this is not your fault - you're just copying what the Linux one does. I'm pretty sure that many of the callers expect this function to do its thing and that if it doesn't work we might get corruption in the guest. > + /* Discard from the buffer cache. */ > + if ( posix_fadvise(fd, 0, cur, POSIX_FADV_DONTNEED) < 0 ) > + goto out; Which also means that the use of posix_fadvise is hardly safe. fadvise can quite legally be a noop. > +static xc_osdep_handle freebsd_privcmd_open(xc_interface *xch) > +{ > + int flags, saved_errno; > + int fd = open(PRIVCMD_DEV, O_RDWR); > + > + if ( fd == -1 ) > + { > + PERROR("Could not obtain handle on privileged command interface " > + PRIVCMD_DEV); > + return XC_OSDEP_OPEN_ERROR; I think it would be better to use the "goto out" error handling style. Of course this function is a clone so really I'm complaining about an existing function. Maybe I should stop looking under this rock before I discover any more rotting yak hair. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |