On 08/31/2012 10:49 AM, Kevin Wolf wrote: > Am 31.08.2012 16:42, schrieb Jeff Cody: >> On 08/30/2012 06:15 PM, Eric Blake wrote: >>> On 08/30/2012 11:47 AM, Jeff Cody wrote: >>>> This is derived from the Supriya Kannery's reopen patches. >>>> >>>> This contains the raw-posix driver changes for the bdrv_reopen_* >>>> functions. All changes are staged into a temporary scratch buffer >>>> during the prepare() stage, and copied over to the live structure >>>> during commit(). Upon abort(), all changes are abandoned, and the >>>> live structures are unmodified. >>>> >>>> The _prepare() will create an extra fd - either by means of a dup, >>>> if possible, or opening a new fd if not (for instance, access >>>> control changes). Upon _commit(), the original fd is closed and >>>> the new fd is used. Upon _abort(), the duplicate/new fd is closed. >>>> >>> >>>> + if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & >>>> ~fcntl_flags)) { >>>> + /* dup the original fd */ >>>> + /* TODO: use qemu fcntl wrapper */ >>>> + raw_s->fd = fcntl(s->fd, F_DUPFD_CLOEXEC, 0); >>> >>> I assume this TODO has to be fixed to allow compilation on systems that >>> lack F_DUPFD_CLOEXEC. >> >> Yes, either that or add the logic here. > > Would qemu_dup_flags() from osdep.c be the right thing here? It was > introduces with Corey's fd passing series.
I think so - that is the one I was thinking about. It would just need to be made non-static. > >>>> + if (raw_s->fd == -1) { >>>> + ret = -1; >>>> + goto error; >>>> + } >>>> + ret = fcntl_setfl(raw_s->fd, raw_s->open_flags); >>>> + } else { >>>> + raw_s->fd = qemu_open(state->bs->filename, raw_s->open_flags, >>>> 0644); >>> >>> Is raw_s->open_flags every going to contain O_CREAT, or is the 0644 mode >>> argument spurious? >> >> Thanks, you are right, it is spurious. The raw_s->open_flags are >> explicitly set via raw_parse_flags(), so we know it will never contain >> O_CREAT. > > We can probably assert it. > OK