| Message ID | 1290665220-26478-14-git-send-email-tamura.yoshiaki@lab.ntt.co.jp |
|---|---|
| State | New |
| Headers | show |
On Thu, Nov 25, 2010 at 03:06:52PM +0900, Yoshiaki Tamura wrote: > Replace bdrv_aio_writev() with bdrv_aio_writev_proxy() to let > event-tap capture events from dma-helpers. > > Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> Same comment as -net here: it's not clear when should a device use bdrv_aio_writev_proxy and when bdrv_aio_writev. If all devices should just use _proxy, let's just make bdrv_aio_writev DTRT instead. > --- > dma-helpers.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dma-helpers.c b/dma-helpers.c > index 712ed89..8ab2c26 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -117,8 +117,8 @@ static void dma_bdrv_cb(void *opaque, int ret) > } > > if (dbs->is_write) { > - dbs->acb = bdrv_aio_writev(dbs->bs, dbs->sector_num, &dbs->iov, > - dbs->iov.size / 512, dma_bdrv_cb, dbs); > + dbs->acb = bdrv_aio_writev_proxy(dbs->bs, dbs->sector_num, &dbs->iov, > + dbs->iov.size / 512, dma_bdrv_cb, dbs); > } else { > dbs->acb = bdrv_aio_readv(dbs->bs, dbs->sector_num, &dbs->iov, > dbs->iov.size / 512, dma_bdrv_cb, dbs); > -- > 1.7.1.2 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
2010/11/28 Michael S. Tsirkin <mst@redhat.com>: > On Thu, Nov 25, 2010 at 03:06:52PM +0900, Yoshiaki Tamura wrote: >> Replace bdrv_aio_writev() with bdrv_aio_writev_proxy() to let >> event-tap capture events from dma-helpers. >> >> Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> > > Same comment as -net here: it's not clear when should > a device use bdrv_aio_writev_proxy and when bdrv_aio_writev. > If all devices should just use _proxy, let's > just make bdrv_aio_writev DTRT instead. Same as I replied to the net layer question. However, I had troubles with inserting event-tap functions into block.c before. block.c gets linked with utils like qemu-img, but they don't get linked with emulators code which event-tap uses in it. So I want to avoid linking block and event-tap for utils, but I guess we don't want to use ifdefs for this. I'm wondering how I can solve this problem cleanly. Kevin, do you have suggestions here? Yoshi > >> --- >> dma-helpers.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/dma-helpers.c b/dma-helpers.c >> index 712ed89..8ab2c26 100644 >> --- a/dma-helpers.c >> +++ b/dma-helpers.c >> @@ -117,8 +117,8 @@ static void dma_bdrv_cb(void *opaque, int ret) >> } >> >> if (dbs->is_write) { >> - dbs->acb = bdrv_aio_writev(dbs->bs, dbs->sector_num, &dbs->iov, >> - dbs->iov.size / 512, dma_bdrv_cb, dbs); >> + dbs->acb = bdrv_aio_writev_proxy(dbs->bs, dbs->sector_num, &dbs->iov, >> + dbs->iov.size / 512, dma_bdrv_cb, dbs); >> } else { >> dbs->acb = bdrv_aio_readv(dbs->bs, dbs->sector_num, &dbs->iov, >> dbs->iov.size / 512, dma_bdrv_cb, dbs); >> -- >> 1.7.1.2 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Sun, Nov 28, 2010 at 08:55:28PM +0900, Yoshiaki Tamura wrote: > 2010/11/28 Michael S. Tsirkin <mst@redhat.com>: > > On Thu, Nov 25, 2010 at 03:06:52PM +0900, Yoshiaki Tamura wrote: > >> Replace bdrv_aio_writev() with bdrv_aio_writev_proxy() to let > >> event-tap capture events from dma-helpers. > >> > >> Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> > > > > Same comment as -net here: it's not clear when should > > a device use bdrv_aio_writev_proxy and when bdrv_aio_writev. > > If all devices should just use _proxy, let's > > just make bdrv_aio_writev DTRT instead. > > Same as I replied to the net layer question. However, I had > troubles with inserting event-tap functions into block.c before. > block.c gets linked with utils like qemu-img, but they don't get > linked with emulators code which event-tap uses in it. So I want > to avoid linking block and event-tap for utils, but I guess we > don't want to use ifdefs for this. I'm wondering how I can solve > this problem cleanly. Add stubs same as we have for other functions. > Kevin, do you have suggestions here? > > Yoshi > > > > >> --- > >> dma-helpers.c | 4 ++-- > >> 1 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/dma-helpers.c b/dma-helpers.c > >> index 712ed89..8ab2c26 100644 > >> --- a/dma-helpers.c > >> +++ b/dma-helpers.c > >> @@ -117,8 +117,8 @@ static void dma_bdrv_cb(void *opaque, int ret) > >> } > >> > >> if (dbs->is_write) { > >> - dbs->acb = bdrv_aio_writev(dbs->bs, dbs->sector_num, &dbs->iov, > >> - dbs->iov.size / 512, dma_bdrv_cb, dbs); > >> + dbs->acb = bdrv_aio_writev_proxy(dbs->bs, dbs->sector_num, &dbs->iov, > >> + dbs->iov.size / 512, dma_bdrv_cb, dbs); > >> } else { > >> dbs->acb = bdrv_aio_readv(dbs->bs, dbs->sector_num, &dbs->iov, > >> dbs->iov.size / 512, dma_bdrv_cb, dbs); > >> -- > >> 1.7.1.2 > >> > >> -- > >> To unsubscribe from this list: send the line "unsubscribe kvm" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > >
Am 28.11.2010 12:55, schrieb Yoshiaki Tamura: > 2010/11/28 Michael S. Tsirkin <mst@redhat.com>: >> On Thu, Nov 25, 2010 at 03:06:52PM +0900, Yoshiaki Tamura wrote: >>> Replace bdrv_aio_writev() with bdrv_aio_writev_proxy() to let >>> event-tap capture events from dma-helpers. >>> >>> Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> >> >> Same comment as -net here: it's not clear when should >> a device use bdrv_aio_writev_proxy and when bdrv_aio_writev. >> If all devices should just use _proxy, let's >> just make bdrv_aio_writev DTRT instead. > > Same as I replied to the net layer question. However, I had > troubles with inserting event-tap functions into block.c before. > block.c gets linked with utils like qemu-img, but they don't get > linked with emulators code which event-tap uses in it. So I want > to avoid linking block and event-tap for utils, but I guess we > don't want to use ifdefs for this. I'm wondering how I can solve > this problem cleanly. > > Kevin, do you have suggestions here? Michael's stubs (probably in qemu-tool.c) seem to be the right solution. Which requests do you actually want to intercept? I assume you're aware that for example qcow2 internally calls another bdrv_aio_readv/writev that accesses the image file. So if you only want to have the requests that come directly from devices, maybe you'll have to restrict it to BlockDriverStates that belongs to a drive. I think this is the case if it has a non-empty device name. Kevin
2010/11/29 Kevin Wolf <kwolf@redhat.com>: > Am 28.11.2010 12:55, schrieb Yoshiaki Tamura: >> 2010/11/28 Michael S. Tsirkin <mst@redhat.com>: >>> On Thu, Nov 25, 2010 at 03:06:52PM +0900, Yoshiaki Tamura wrote: >>>> Replace bdrv_aio_writev() with bdrv_aio_writev_proxy() to let >>>> event-tap capture events from dma-helpers. >>>> >>>> Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> >>> >>> Same comment as -net here: it's not clear when should >>> a device use bdrv_aio_writev_proxy and when bdrv_aio_writev. >>> If all devices should just use _proxy, let's >>> just make bdrv_aio_writev DTRT instead. >> >> Same as I replied to the net layer question. However, I had >> troubles with inserting event-tap functions into block.c before. >> block.c gets linked with utils like qemu-img, but they don't get >> linked with emulators code which event-tap uses in it. So I want >> to avoid linking block and event-tap for utils, but I guess we >> don't want to use ifdefs for this. I'm wondering how I can solve >> this problem cleanly. >> >> Kevin, do you have suggestions here? > > Michael's stubs (probably in qemu-tool.c) seem to be the right solution. Same here. I noticed kvm-stub to be a good example. > Which requests do you actually want to intercept? I assume you're aware > that for example qcow2 internally calls another bdrv_aio_readv/writev > that accesses the image file. > > So if you only want to have the requests that come directly from > devices, maybe you'll have to restrict it to BlockDriverStates that > belongs to a drive. I think this is the case if it has a non-empty > device name. Yes, exactly. I noticed that a little while ago. Thanks for making it clear. > > Kevin > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
diff --git a/dma-helpers.c b/dma-helpers.c index 712ed89..8ab2c26 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -117,8 +117,8 @@ static void dma_bdrv_cb(void *opaque, int ret) } if (dbs->is_write) { - dbs->acb = bdrv_aio_writev(dbs->bs, dbs->sector_num, &dbs->iov, - dbs->iov.size / 512, dma_bdrv_cb, dbs); + dbs->acb = bdrv_aio_writev_proxy(dbs->bs, dbs->sector_num, &dbs->iov, + dbs->iov.size / 512, dma_bdrv_cb, dbs); } else { dbs->acb = bdrv_aio_readv(dbs->bs, dbs->sector_num, &dbs->iov, dbs->iov.size / 512, dma_bdrv_cb, dbs);
Replace bdrv_aio_writev() with bdrv_aio_writev_proxy() to let event-tap capture events from dma-helpers. Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> --- dma-helpers.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)