| Message ID | 1467038869-11538-4-git-send-email-den@openvz.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/27/2016 10:47 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> > 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 > + * 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); > Reviewed-by: John Snow <jsnow@redhat.com> However, the patch-set needs to be re-ordered as 3-1-2. Intermediate patch commits cannot have failing tests.
On 06/28/2016 02:19 AM, John Snow wrote: > > On 06/27/2016 10:47 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> >> 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 >> + * 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); >> > Reviewed-by: John Snow <jsnow@redhat.com> > > However, the patch-set needs to be re-ordered as 3-1-2. Intermediate > patch commits cannot have failing tests. ok
On 28.06.2016 02:19, John Snow wrote: > > On 06/27/2016 10:47 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> >> 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 >> + * 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); >> > Reviewed-by: John Snow <jsnow@redhat.com> > > However, the patch-set needs to be re-ordered as 3-1-2. Intermediate > patch commits cannot have failing tests. Sure, although 2-3-1 would probably be better, since 2 fixes an assert in IDE that is triggered by new tests in 3?
On 06/28/2016 05:21 AM, Evgeny Yakovlev wrote: > > On 28.06.2016 02:19, John Snow wrote: >> >> On 06/27/2016 10:47 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> >>> 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 >>> + * 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); > With no link to allow us to view the keynote from another room or our > desks, most of us had not choice but to abandon our attempt to actually > listen Jim talk about the future of Red Hat. >>> + >>> + 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; >>> + > With no link to allow us to view the keynote from another room or our > desks, most of us had not choice but to abandon our attempt to actually > listen Jim talk about the future of Red Hat. >>> + 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); >>> >> Reviewed-by: John Snow <jsnow@redhat.com> >> >> However, the patch-set needs to be re-ordered as 3-1-2. Intermediate >> patch commits cannot have failing tests. > > Sure, although 2-3-1 would probably be better, since 2 fixes an assert > in IDE that is triggered by new tests in 3? > :) Yes, thanks! --js
On 06/28/2016 12:37 PM, John Snow wrote: > > > On 06/28/2016 05:21 AM, Evgeny Yakovlev wrote: >> >> On 28.06.2016 02:19, John Snow wrote: >>> >>> On 06/27/2016 10:47 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> >>>> 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 >>>> + * 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); >> With no link to allow us to view the keynote from another room or our >> desks, most of us had not choice but to abandon our attempt to actually >> listen Jim talk about the future of Red Hat. Apologies, random clipboard detritus escaped into my emails again. (Red Hat summit is ongoing right now and some people had concerns about access to the live feeds. Apologies.) Please pardon the noise. >>>> + >>>> + 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; >>>> + >> With no link to allow us to view the keynote from another room or our >> desks, most of us had not choice but to abandon our attempt to actually >> listen Jim talk about the future of Red Hat. My clipboard must have been really angry. >>>> + 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); >>>> >>> Reviewed-by: John Snow <jsnow@redhat.com> >>> >>> However, the patch-set needs to be re-ordered as 3-1-2. Intermediate >>> patch commits cannot have failing tests. >> >> Sure, although 2-3-1 would probably be better, since 2 fixes an assert >> in IDE that is triggered by new tests in 3? >> > > :) > > Yes, thanks! > > --js >
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);