Message ID | 1329713430-9209-1-git-send-email-zwu.kernel@gmail.com |
---|---|
State | New |
Headers | show |
Am 20.02.2012 05:50, schrieb zwu.kernel@gmail.com: > From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> > > If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first. > > Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected. > > Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> > --- > block.c | 29 ++++++++++++++++++++++------- > block_int.h | 1 + > 2 files changed, 23 insertions(+), 7 deletions(-) > > diff --git a/block.c b/block.c > index ae297bb..f78df78 100644 > --- a/block.c > +++ b/block.c > @@ -853,25 +853,40 @@ void bdrv_close_all(void) > } > } > > -/* > - * Wait for pending requests to complete across all BlockDriverStates > - * > - * This function does not flush data to disk, use bdrv_flush_all() for that > - * after calling this function. > - */ > -void bdrv_drain_all(void) > +void bdrv_drain_request(BlockDriverState *throttled_bs) > { > BlockDriverState *bs; > > + QTAILQ_FOREACH(bs, &bdrv_states, list) { > + if (throttled_bs && throttled_bs != bs) { > + continue; > + } > + qemu_co_queue_restart_all(&bs->throttled_reqs); > + } > + > qemu_aio_flush(); Why doesn't qemu_aio_flush() invoke whatever is needed? I think this is the real bug that should be fixed. Kevin
On Mon, Feb 20, 2012 at 5:26 PM, Kevin Wolf <kwolf@redhat.com> wrote: > Am 20.02.2012 05:50, schrieb zwu.kernel@gmail.com: >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first. >> >> Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected. >> >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> --- >> block.c | 29 ++++++++++++++++++++++------- >> block_int.h | 1 + >> 2 files changed, 23 insertions(+), 7 deletions(-) >> >> diff --git a/block.c b/block.c >> index ae297bb..f78df78 100644 >> --- a/block.c >> +++ b/block.c >> @@ -853,25 +853,40 @@ void bdrv_close_all(void) >> } >> } >> >> -/* >> - * Wait for pending requests to complete across all BlockDriverStates >> - * >> - * This function does not flush data to disk, use bdrv_flush_all() for that >> - * after calling this function. >> - */ >> -void bdrv_drain_all(void) >> +void bdrv_drain_request(BlockDriverState *throttled_bs) >> { >> BlockDriverState *bs; >> >> + QTAILQ_FOREACH(bs, &bdrv_states, list) { >> + if (throttled_bs && throttled_bs != bs) { >> + continue; >> + } >> + qemu_co_queue_restart_all(&bs->throttled_reqs); >> + } >> + >> qemu_aio_flush(); > > Why doesn't qemu_aio_flush() invoke whatever is needed? I think this is > the real bug that should be fixed. Do you mean that why qemu_aio_flush doesn't invoke those lines of codes above it? As what i said in above comments, when one VM has 2 multiple disks with enabling I/O throttling, if only one disk need to be flushed, only this disk's throttled request need to be drained, and another disk's throttled requests don't need. > > Kevin
For example, one disk of one guest is hot plugout, its other disks should not be affected if those disks also enable I/O throttling. But if one whole VM need to be stored, all throttled requests for all disks need to be drained. On Mon, Feb 20, 2012 at 5:26 PM, Kevin Wolf <kwolf@redhat.com> wrote: > Am 20.02.2012 05:50, schrieb zwu.kernel@gmail.com: >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first. >> >> Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected. >> >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> --- >> block.c | 29 ++++++++++++++++++++++------- >> block_int.h | 1 + >> 2 files changed, 23 insertions(+), 7 deletions(-) >> >> diff --git a/block.c b/block.c >> index ae297bb..f78df78 100644 >> --- a/block.c >> +++ b/block.c >> @@ -853,25 +853,40 @@ void bdrv_close_all(void) >> } >> } >> >> -/* >> - * Wait for pending requests to complete across all BlockDriverStates >> - * >> - * This function does not flush data to disk, use bdrv_flush_all() for that >> - * after calling this function. >> - */ >> -void bdrv_drain_all(void) >> +void bdrv_drain_request(BlockDriverState *throttled_bs) >> { >> BlockDriverState *bs; >> >> + QTAILQ_FOREACH(bs, &bdrv_states, list) { >> + if (throttled_bs && throttled_bs != bs) { >> + continue; >> + } >> + qemu_co_queue_restart_all(&bs->throttled_reqs); >> + } >> + >> qemu_aio_flush(); > > Why doesn't qemu_aio_flush() invoke whatever is needed? I think this is > the real bug that should be fixed. > > Kevin
Am 20.02.2012 10:29, schrieb Zhi Yong Wu: > On Mon, Feb 20, 2012 at 5:26 PM, Kevin Wolf <kwolf@redhat.com> wrote: >> Am 20.02.2012 05:50, schrieb zwu.kernel@gmail.com: >>> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >>> >>> If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first. >>> >>> Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected. >>> >>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >>> --- >>> block.c | 29 ++++++++++++++++++++++------- >>> block_int.h | 1 + >>> 2 files changed, 23 insertions(+), 7 deletions(-) >>> >>> diff --git a/block.c b/block.c >>> index ae297bb..f78df78 100644 >>> --- a/block.c >>> +++ b/block.c >>> @@ -853,25 +853,40 @@ void bdrv_close_all(void) >>> } >>> } >>> >>> -/* >>> - * Wait for pending requests to complete across all BlockDriverStates >>> - * >>> - * This function does not flush data to disk, use bdrv_flush_all() for that >>> - * after calling this function. >>> - */ >>> -void bdrv_drain_all(void) >>> +void bdrv_drain_request(BlockDriverState *throttled_bs) >>> { >>> BlockDriverState *bs; >>> >>> + QTAILQ_FOREACH(bs, &bdrv_states, list) { >>> + if (throttled_bs && throttled_bs != bs) { >>> + continue; >>> + } >>> + qemu_co_queue_restart_all(&bs->throttled_reqs); >>> + } >>> + >>> qemu_aio_flush(); >> >> Why doesn't qemu_aio_flush() invoke whatever is needed? I think this is >> the real bug that should be fixed. > Do you mean that why qemu_aio_flush doesn't invoke those lines of > codes above it? As what i said in above comments, when one VM has 2 > multiple disks with enabling I/O throttling, if only one disk need to > be flushed, only this disk's throttled request need to be drained, and > another disk's throttled requests don't need. So this is an optimisation rather than a fix? Would the right optimisation be a qemu_aio_flush_disk() that flushes the requests for a single disk? Kevin
On Mon, Feb 20, 2012 at 5:39 PM, Kevin Wolf <kwolf@redhat.com> wrote: > Am 20.02.2012 10:29, schrieb Zhi Yong Wu: >> On Mon, Feb 20, 2012 at 5:26 PM, Kevin Wolf <kwolf@redhat.com> wrote: >>> Am 20.02.2012 05:50, schrieb zwu.kernel@gmail.com: >>>> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >>>> >>>> If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first. >>>> >>>> Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected. >>>> >>>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >>>> --- >>>> block.c | 29 ++++++++++++++++++++++------- >>>> block_int.h | 1 + >>>> 2 files changed, 23 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/block.c b/block.c >>>> index ae297bb..f78df78 100644 >>>> --- a/block.c >>>> +++ b/block.c >>>> @@ -853,25 +853,40 @@ void bdrv_close_all(void) >>>> } >>>> } >>>> >>>> -/* >>>> - * Wait for pending requests to complete across all BlockDriverStates >>>> - * >>>> - * This function does not flush data to disk, use bdrv_flush_all() for that >>>> - * after calling this function. >>>> - */ >>>> -void bdrv_drain_all(void) >>>> +void bdrv_drain_request(BlockDriverState *throttled_bs) >>>> { >>>> BlockDriverState *bs; >>>> >>>> + QTAILQ_FOREACH(bs, &bdrv_states, list) { >>>> + if (throttled_bs && throttled_bs != bs) { >>>> + continue; >>>> + } >>>> + qemu_co_queue_restart_all(&bs->throttled_reqs); >>>> + } >>>> + >>>> qemu_aio_flush(); >>> >>> Why doesn't qemu_aio_flush() invoke whatever is needed? I think this is >>> the real bug that should be fixed. >> Do you mean that why qemu_aio_flush doesn't invoke those lines of >> codes above it? As what i said in above comments, when one VM has 2 >> multiple disks with enabling I/O throttling, if only one disk need to >> be flushed, only this disk's throttled request need to be drained, and >> another disk's throttled requests don't need. > > So this is an optimisation rather than a fix? Would the right It is a fix. i think that current handling is wrong when I/O throttling is enabled > optimisation be a qemu_aio_flush_disk() that flushes the requests for a > single disk? No. If bdrv_drain_request is invoked with specified bs, it will flush All the throttled requests in the throttled queue for this specific bs + pending emulated aio for all disks (in aio engine) But if bdrv_drain_request(NULL) is invoked, it will flush All the throttled request in the throttled queue for all disks + pending emulated aio for all disks (in aio engine) > > Kevin
On Mon, Feb 20, 2012 at 12:50:30PM +0800, zwu.kernel@gmail.com wrote: > From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> > > If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first. > > Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected. > > Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> > --- > block.c | 29 ++++++++++++++++++++++------- > block_int.h | 1 + > 2 files changed, 23 insertions(+), 7 deletions(-) > > diff --git a/block.c b/block.c > index ae297bb..f78df78 100644 > --- a/block.c > +++ b/block.c > @@ -853,25 +853,40 @@ void bdrv_close_all(void) > } > } > > -/* > - * Wait for pending requests to complete across all BlockDriverStates > - * > - * This function does not flush data to disk, use bdrv_flush_all() for that > - * after calling this function. > - */ > -void bdrv_drain_all(void) > +void bdrv_drain_request(BlockDriverState *throttled_bs) > { > BlockDriverState *bs; > > + QTAILQ_FOREACH(bs, &bdrv_states, list) { > + if (throttled_bs && throttled_bs != bs) { > + continue; > + } > + qemu_co_queue_restart_all(&bs->throttled_reqs); > + } > + > qemu_aio_flush(); Since I/O throttling is still enabled, the restarted requests could enqueue again if they exceed the limit. We could still hit the assert. If the semantics of bdrv_drain_request() are that no requests are pending when it returns then we need a loop here. BTW bdrv_drain() would be a shorter name for this function. Stefan
On Fri, Feb 24, 2012 at 4:49 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Mon, Feb 20, 2012 at 12:50:30PM +0800, zwu.kernel@gmail.com wrote: >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first. >> >> Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected. >> >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> --- >> block.c | 29 ++++++++++++++++++++++------- >> block_int.h | 1 + >> 2 files changed, 23 insertions(+), 7 deletions(-) >> >> diff --git a/block.c b/block.c >> index ae297bb..f78df78 100644 >> --- a/block.c >> +++ b/block.c >> @@ -853,25 +853,40 @@ void bdrv_close_all(void) >> } >> } >> >> -/* >> - * Wait for pending requests to complete across all BlockDriverStates >> - * >> - * This function does not flush data to disk, use bdrv_flush_all() for that >> - * after calling this function. >> - */ >> -void bdrv_drain_all(void) >> +void bdrv_drain_request(BlockDriverState *throttled_bs) >> { >> BlockDriverState *bs; >> >> + QTAILQ_FOREACH(bs, &bdrv_states, list) { >> + if (throttled_bs && throttled_bs != bs) { >> + continue; >> + } >> + qemu_co_queue_restart_all(&bs->throttled_reqs); >> + } >> + >> qemu_aio_flush(); > > Since I/O throttling is still enabled, the restarted requests could > enqueue again if they exceed the limit. We could still hit the assert. good catch. > > If the semantics of bdrv_drain_request() are that no requests are > pending when it returns then we need a loop here. OK. > > BTW bdrv_drain() would be a shorter name for this function. OK > > Stefan
On Fri, Feb 24, 2012 at 4:49 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Mon, Feb 20, 2012 at 12:50:30PM +0800, zwu.kernel@gmail.com wrote: >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first. >> >> Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected. >> >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> --- >> block.c | 29 ++++++++++++++++++++++------- >> block_int.h | 1 + >> 2 files changed, 23 insertions(+), 7 deletions(-) >> >> diff --git a/block.c b/block.c >> index ae297bb..f78df78 100644 >> --- a/block.c >> +++ b/block.c >> @@ -853,25 +853,40 @@ void bdrv_close_all(void) >> } >> } >> >> -/* >> - * Wait for pending requests to complete across all BlockDriverStates >> - * >> - * This function does not flush data to disk, use bdrv_flush_all() for that >> - * after calling this function. >> - */ >> -void bdrv_drain_all(void) >> +void bdrv_drain_request(BlockDriverState *throttled_bs) >> { >> BlockDriverState *bs; >> >> + QTAILQ_FOREACH(bs, &bdrv_states, list) { >> + if (throttled_bs && throttled_bs != bs) { >> + continue; >> + } >> + qemu_co_queue_restart_all(&bs->throttled_reqs); >> + } >> + >> qemu_aio_flush(); > > Since I/O throttling is still enabled, the restarted requests could > enqueue again if they exceed the limit. We could still hit the assert. > > If the semantics of bdrv_drain_request() are that no requests are > pending when it returns then we need a loop here. > > BTW bdrv_drain() would be a shorter name for this function. For this function's semantics, i have some concerns. Is it used to drain all requests of one single disk or all disks for one guest? which is more suitable? > > Stefan
On Fri, Feb 24, 2012 at 05:25:08PM +0800, Zhi Yong Wu wrote: > On Fri, Feb 24, 2012 at 4:49 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > > On Mon, Feb 20, 2012 at 12:50:30PM +0800, zwu.kernel@gmail.com wrote: > >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> > >> > >> If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first. > >> > >> Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected. > >> > >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> > >> --- > >> block.c | 29 ++++++++++++++++++++++------- > >> block_int.h | 1 + > >> 2 files changed, 23 insertions(+), 7 deletions(-) > >> > >> diff --git a/block.c b/block.c > >> index ae297bb..f78df78 100644 > >> --- a/block.c > >> +++ b/block.c > >> @@ -853,25 +853,40 @@ void bdrv_close_all(void) > >> } > >> } > >> > >> -/* > >> - * Wait for pending requests to complete across all BlockDriverStates > >> - * > >> - * This function does not flush data to disk, use bdrv_flush_all() for that > >> - * after calling this function. > >> - */ > >> -void bdrv_drain_all(void) > >> +void bdrv_drain_request(BlockDriverState *throttled_bs) > >> { > >> BlockDriverState *bs; > >> > >> + QTAILQ_FOREACH(bs, &bdrv_states, list) { > >> + if (throttled_bs && throttled_bs != bs) { > >> + continue; > >> + } > >> + qemu_co_queue_restart_all(&bs->throttled_reqs); > >> + } > >> + > >> qemu_aio_flush(); > > > > Since I/O throttling is still enabled, the restarted requests could > > enqueue again if they exceed the limit. We could still hit the assert. > > > > If the semantics of bdrv_drain_request() are that no requests are > > pending when it returns then we need a loop here. > > > > BTW bdrv_drain() would be a shorter name for this function. > For this function's semantics, i have some concerns. > Is it used to drain all requests of one single disk or all disks for one guest? > which is more suitable? Both are useful: /** * Complete all pending requests for a block device */ void bdrv_drain(BlockDriverState *bs); /** * Complete pending requests for all block devices */ void bdrv_drain_all(void); Stefan
On 02/24/2012 09:49 AM, Stefan Hajnoczi wrote: >> > +void bdrv_drain_request(BlockDriverState *throttled_bs) >> > { >> > BlockDriverState *bs; >> > >> > + QTAILQ_FOREACH(bs, &bdrv_states, list) { >> > + if (throttled_bs && throttled_bs != bs) { >> > + continue; >> > + } >> > + qemu_co_queue_restart_all(&bs->throttled_reqs); >> > + } >> > + >> > qemu_aio_flush(); > Since I/O throttling is still enabled, the restarted requests could > enqueue again if they exceed the limit. We could still hit the assert. Yes, and qemu_aio_flush() rightly doesn't know that there are pending requests, because these are "not there" until the timer fires. Perhaps the bug is simply that the assert is bogus. If there are throttled requests, we need to invoke qemu_aio_flush() again. The problem with this is that timers don't run inside qemu_aio_wait() so we have to restart them all and we're effectively busy waiting. Not a huge problem since qemu_aio_flush() is rare, but not too nice either. Paolo
On Fri, Feb 24, 2012 at 7:18 PM, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote: > On Fri, Feb 24, 2012 at 05:25:08PM +0800, Zhi Yong Wu wrote: >> On Fri, Feb 24, 2012 at 4:49 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: >> > On Mon, Feb 20, 2012 at 12:50:30PM +0800, zwu.kernel@gmail.com wrote: >> >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> >> >> If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first. >> >> >> >> Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected. >> >> >> >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> --- >> >> block.c | 29 ++++++++++++++++++++++------- >> >> block_int.h | 1 + >> >> 2 files changed, 23 insertions(+), 7 deletions(-) >> >> >> >> diff --git a/block.c b/block.c >> >> index ae297bb..f78df78 100644 >> >> --- a/block.c >> >> +++ b/block.c >> >> @@ -853,25 +853,40 @@ void bdrv_close_all(void) >> >> } >> >> } >> >> >> >> -/* >> >> - * Wait for pending requests to complete across all BlockDriverStates >> >> - * >> >> - * This function does not flush data to disk, use bdrv_flush_all() for that >> >> - * after calling this function. >> >> - */ >> >> -void bdrv_drain_all(void) >> >> +void bdrv_drain_request(BlockDriverState *throttled_bs) >> >> { >> >> BlockDriverState *bs; >> >> >> >> + QTAILQ_FOREACH(bs, &bdrv_states, list) { >> >> + if (throttled_bs && throttled_bs != bs) { >> >> + continue; >> >> + } >> >> + qemu_co_queue_restart_all(&bs->throttled_reqs); >> >> + } >> >> + >> >> qemu_aio_flush(); >> > >> > Since I/O throttling is still enabled, the restarted requests could >> > enqueue again if they exceed the limit. We could still hit the assert. >> > >> > If the semantics of bdrv_drain_request() are that no requests are >> > pending when it returns then we need a loop here. >> > >> > BTW bdrv_drain() would be a shorter name for this function. >> For this function's semantics, i have some concerns. >> Is it used to drain all requests of one single disk or all disks for one guest? >> which is more suitable? > > Both are useful: > > /** > * Complete all pending requests for a block device > */ > void bdrv_drain(BlockDriverState *bs); > > /** > * Complete pending requests for all block devices > */ > void bdrv_drain_all(void); Great, thanks. > > Stefan >
diff --git a/block.c b/block.c index ae297bb..f78df78 100644 --- a/block.c +++ b/block.c @@ -853,25 +853,40 @@ void bdrv_close_all(void) } } -/* - * Wait for pending requests to complete across all BlockDriverStates - * - * This function does not flush data to disk, use bdrv_flush_all() for that - * after calling this function. - */ -void bdrv_drain_all(void) +void bdrv_drain_request(BlockDriverState *throttled_bs) { BlockDriverState *bs; + QTAILQ_FOREACH(bs, &bdrv_states, list) { + if (throttled_bs && throttled_bs != bs) { + continue; + } + qemu_co_queue_restart_all(&bs->throttled_reqs); + } + qemu_aio_flush(); /* If requests are still pending there is a bug somewhere */ QTAILQ_FOREACH(bs, &bdrv_states, list) { assert(QLIST_EMPTY(&bs->tracked_requests)); + if (throttled_bs && throttled_bs != bs) { + continue; + } assert(qemu_co_queue_empty(&bs->throttled_reqs)); } } +/* + * Wait for pending requests to complete across all BlockDriverStates + * + * This function does not flush data to disk, use bdrv_flush_all() for that + * after calling this function. + */ +void bdrv_drain_all(void) +{ + bdrv_drain_request(NULL); +} + /* make a BlockDriverState anonymous by removing from bdrv_state list. Also, NULL terminate the device_name to prevent double remove */ void bdrv_make_anon(BlockDriverState *bs) diff --git a/block_int.h b/block_int.h index 7946cf6..1311288 100644 --- a/block_int.h +++ b/block_int.h @@ -323,6 +323,7 @@ void qemu_aio_release(void *p); void bdrv_set_io_limits(BlockDriverState *bs, BlockIOLimit *io_limits); +void bdrv_drain_request(BlockDriverState *bs); #ifdef _WIN32 int is_windows_drive(const char *filename);