| Message ID | 1468499383-17840-4-git-send-email-den@openvz.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/14/2016 06:29 AM, Denis V. Lunev wrote: > From: Evgeny Yakovlev <eyakovlev@virtuozzo.com> > > Due to changes in flush behaviour clean disks stopped generating > flush_to_disk events and IDE and AHCI tests that test flush commands > started to fail. > > This change adds additional DMA writes to affected tests before sending > flush commands so that bdrv_flush actually generates flush_to_disk event. > > Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com> > Signed-off-by: Denis V. Lunev <den@openvz.org> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > CC: Kevin Wolf <kwolf@redhat.com> > CC: Max Reitz <mreitz@redhat.com> > CC: Stefan Hajnoczi <stefanha@redhat.com> > CC: Fam Zheng <famz@redhat.com> > CC: John Snow <jsnow@redhat.com> > --- > tests/ahci-test.c | 34 ++++++++++++++++++++++++++++++++-- > tests/ide-test.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 75 insertions(+), 2 deletions(-) > > diff --git a/tests/ahci-test.c b/tests/ahci-test.c > index 57dc44c..d707714 100644 > --- a/tests/ahci-test.c > +++ b/tests/ahci-test.c > @@ -1063,11 +1063,34 @@ static void test_dma_fragmented(void) > g_free(tx); > } > > +/* > + * Write sector 0 with random data to make AHCI storage dirty If we ever have a case where we open a disk without specifying -raw, the random data _might_ resemble some other format and cause probe to misbehave; as such, we also have code in the block layer that specifically prevents writes to sector 0 for some data. Should you pick a different sector than 0, so as to avoid any (remote) possibility that the random data could change probe results or be rejected?
On 14.07.2016 19:05, Eric Blake wrote: > On 07/14/2016 06:29 AM, Denis V. Lunev wrote: >> From: Evgeny Yakovlev <eyakovlev@virtuozzo.com> >> >> Due to changes in flush behaviour clean disks stopped generating >> flush_to_disk events and IDE and AHCI tests that test flush commands >> started to fail. >> >> This change adds additional DMA writes to affected tests before sending >> flush commands so that bdrv_flush actually generates flush_to_disk event. >> >> Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com> >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> CC: Kevin Wolf <kwolf@redhat.com> >> CC: Max Reitz <mreitz@redhat.com> >> CC: Stefan Hajnoczi <stefanha@redhat.com> >> CC: Fam Zheng <famz@redhat.com> >> CC: John Snow <jsnow@redhat.com> >> --- >> tests/ahci-test.c | 34 ++++++++++++++++++++++++++++++++-- >> tests/ide-test.c | 43 +++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 75 insertions(+), 2 deletions(-) >> >> diff --git a/tests/ahci-test.c b/tests/ahci-test.c >> index 57dc44c..d707714 100644 >> --- a/tests/ahci-test.c >> +++ b/tests/ahci-test.c >> @@ -1063,11 +1063,34 @@ static void test_dma_fragmented(void) >> g_free(tx); >> } >> >> +/* >> + * Write sector 0 with random data to make AHCI storage dirty > If we ever have a case where we open a disk without specifying -raw, the > random data _might_ resemble some other format and cause probe to > misbehave; as such, we also have code in the block layer that > specifically prevents writes to sector 0 for some data. Should you pick > a different sector than 0, so as to avoid any (remote) possibility that > the random data could change probe results or be rejected? > Not sure if i understand the problem you're referring to here. Those are blkdebug tests, those disks are created, emulated with blkdebug backend, flushed and then thrown away. So is there really any possibility for reopening the image and accidentally parsing a partition table in sector 0? Also, not sure what you mean by "code in the block layer that specifically prevents writes to sector 0 for some data". Can you explain that bit, because it sounds pretty scary. How can we deny guest VM to write anything to sector 0 on its emulated disk?
On 07/15/2016 02:08 AM, Evgeny Yakovlev wrote: >>> + * Write sector 0 with random data to make AHCI storage dirty >> If we ever have a case where we open a disk without specifying -raw, the >> random data _might_ resemble some other format and cause probe to >> misbehave; as such, we also have code in the block layer that >> specifically prevents writes to sector 0 for some data. Should you pick >> a different sector than 0, so as to avoid any (remote) possibility that >> the random data could change probe results or be rejected? >> > > Not sure if i understand the problem you're referring to here. Those are > blkdebug tests, those disks are created, emulated with blkdebug backend, > flushed and then thrown away. So is there really any possibility for > reopening the image and accidentally parsing a partition table in sector 0? > > Also, not sure what you mean by "code in the block layer that > specifically prevents writes to sector 0 for some data". Can you explain > that bit, because it sounds pretty scary. How can we deny guest VM to > write anything to sector 0 on its emulated disk? Read block/raw_bsd.c:raw_co_writev_flags() for the gory details. If the guest ever gets a raw format driver because the user forgot to say '--format $foo', then we prevent the guest from writing anything into sector 0 that would be probed as non-raw. It means there are only a handful of patterns that the guest cannot write into the first sector, but it IS a non-zero number of patterns. How the guest behaves if such a write is attempted depends on the error policy you have on that device; it might show up as an EIO error to the guest, or it might stop the guest from executing and raise a qemu event to the management application, but the point is that we actively prohibit some writes to sector 0 on a probed raw disk. Using any sector other than 0 doesn't have this limitation, or you can ensure that your test ALWAYS passes the appropriate --format $foo so that the disk is never probed as another way to avoid limitations on sector 0.
On 15.07.2016 20:23, Eric Blake wrote: > On 07/15/2016 02:08 AM, Evgeny Yakovlev wrote: > >>>> + * Write sector 0 with random data to make AHCI storage dirty >>> If we ever have a case where we open a disk without specifying -raw, the >>> random data _might_ resemble some other format and cause probe to >>> misbehave; as such, we also have code in the block layer that >>> specifically prevents writes to sector 0 for some data. Should you pick >>> a different sector than 0, so as to avoid any (remote) possibility that >>> the random data could change probe results or be rejected? >>> >> Not sure if i understand the problem you're referring to here. Those are >> blkdebug tests, those disks are created, emulated with blkdebug backend, >> flushed and then thrown away. So is there really any possibility for >> reopening the image and accidentally parsing a partition table in sector 0? >> >> Also, not sure what you mean by "code in the block layer that >> specifically prevents writes to sector 0 for some data". Can you explain >> that bit, because it sounds pretty scary. How can we deny guest VM to >> write anything to sector 0 on its emulated disk? > Read block/raw_bsd.c:raw_co_writev_flags() for the gory details. If the > guest ever gets a raw format driver because the user forgot to say > '--format $foo', then we prevent the guest from writing anything into > sector 0 that would be probed as non-raw. It means there are only a > handful of patterns that the guest cannot write into the first sector, > but it IS a non-zero number of patterns. How the guest behaves if such > a write is attempted depends on the error policy you have on that > device; it might show up as an EIO error to the guest, or it might stop > the guest from executing and raise a qemu event to the management > application, but the point is that we actively prohibit some writes to > sector 0 on a probed raw disk. Using any sector other than 0 doesn't > have this limitation, or you can ensure that your test ALWAYS passes the > appropriate --format $foo so that the disk is never probed as another > way to avoid limitations on sector 0. > > I think i get it now. Thanks!
diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 57dc44c..d707714 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1063,11 +1063,34 @@ static void test_dma_fragmented(void) g_free(tx); } +/* + * Write sector 0 with random data to make AHCI storage dirty + * Needed for flush tests so that flushes actually go though the block layer + */ +static void make_dirty(AHCIQState* ahci, uint8_t port) +{ + uint64_t ptr; + unsigned bufsize = 512; + + ptr = ahci_alloc(ahci, bufsize); + g_assert(ptr); + + ahci_guest_io(ahci, port, CMD_WRITE_DMA, ptr, bufsize, 0); + ahci_free(ahci, ptr); +} + static void test_flush(void) { AHCIQState *ahci; + uint8_t port; ahci = ahci_boot_and_enable(NULL); + + port = ahci_port_select(ahci); + ahci_port_clear(ahci, port); + + make_dirty(ahci, port); + ahci_test_flush(ahci); ahci_shutdown(ahci); } @@ -1087,10 +1110,13 @@ static void test_flush_retry(void) debug_path, tmp_path, imgfmt); - /* Issue Flush Command and wait for error */ port = ahci_port_select(ahci); ahci_port_clear(ahci, port); + /* Issue write so that flush actually goes to disk */ + make_dirty(ahci, port); + + /* Issue Flush Command and wait for error */ cmd = ahci_guest_io_halt(ahci, port, CMD_FLUSH_CACHE, 0, 0, 0); ahci_guest_io_resume(ahci, cmd); @@ -1343,9 +1369,13 @@ static void test_flush_migrate(void) set_context(src->parent); - /* Issue Flush Command */ px = ahci_port_select(src); ahci_port_clear(src, px); + + /* Dirty device so that flush reaches disk */ + make_dirty(src, px); + + /* Issue Flush Command */ cmd = ahci_command_create(CMD_FLUSH_CACHE); ahci_command_commit(src, cmd, px); ahci_command_issue_async(src, cmd); diff --git a/tests/ide-test.c b/tests/ide-test.c index fed1b2e..8466d0f 100644 --- a/tests/ide-test.c +++ b/tests/ide-test.c @@ -499,6 +499,39 @@ static void test_identify(void) ide_test_quit(); } +/* + * Write sector 0 with random data to make IDE storage dirty + * Needed for flush tests so that flushes actually go though the block layer + */ +static void make_dirty(uint8_t device) +{ + uint8_t status; + size_t len = 512; + uintptr_t guest_buf; + void* buf; + + guest_buf = guest_alloc(guest_malloc, len); + buf = g_malloc(len); + g_assert(guest_buf); + g_assert(buf); + + memwrite(guest_buf, buf, len); + + PrdtEntry prdt[] = { + { + .addr = cpu_to_le32(guest_buf), + .size = cpu_to_le32(len | PRDT_EOT), + }, + }; + + status = send_dma_request(CMD_WRITE_DMA, 0, 1, prdt, + ARRAY_SIZE(prdt), NULL); + g_assert_cmphex(status, ==, BM_STS_INTR); + assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR); + + g_free(buf); +} + static void test_flush(void) { uint8_t data; @@ -507,6 +540,11 @@ static void test_flush(void) "-drive file=blkdebug::%s,if=ide,cache=writeback,format=raw", tmp_path); + qtest_irq_intercept_in(global_qtest, "ioapic"); + + /* Dirty media so that CMD_FLUSH_CACHE will actually go to disk */ + make_dirty(0); + /* Delay the completion of the flush request until we explicitly do it */ g_free(hmp("qemu-io ide0-hd0 \"break flush_to_os A\"")); @@ -549,6 +587,11 @@ static void test_retry_flush(const char *machine) "rerror=stop,werror=stop", debug_path, tmp_path); + qtest_irq_intercept_in(global_qtest, "ioapic"); + + /* Dirty media so that CMD_FLUSH_CACHE will actually go to disk */ + make_dirty(0); + /* FLUSH CACHE command on device 0*/ outb(IDE_BASE + reg_device, 0); outb(IDE_BASE + reg_command, CMD_FLUSH_CACHE);