diff mbox

[22/32] qtest: Cover qdev properties for disk geometry

Message ID 1340984094-5451-23-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster June 29, 2012, 3:34 p.m. UTC
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/hd-geo-test.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

Comments

Kevin Wolf July 5, 2012, 11:33 a.m. UTC | #1
Am 29.06.2012 17:34, schrieb Markus Armbruster:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/hd-geo-test.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
> index 0ab573c..02eb5c2 100644
> --- a/tests/hd-geo-test.c
> +++ b/tests/hd-geo-test.c
> @@ -321,13 +321,15 @@ static void test_ide_drive_user(const char *dev, bool trans)
>      const chst expected_chst = { secs / (4 * 32) , 4, 32, trans };
>  
>      argc = setup_common(argv, ARRAY_SIZE(argv));
> -    opts = g_strdup_printf(",cyls=%d,heads=%d,secs=%d%s",
> +    opts = g_strdup_printf("%s,cyls=%d,heads=%d,secs=%d%s",
> +                           dev && !trans ? dev : "",
>                             expected_chst.cyls, expected_chst.heads,
>                             expected_chst.secs,
>                             trans ? ",trans=lba" : "");
>      cur_ide[0] = &expected_chst;
>      argc = setup_ide(argc, argv, ARRAY_SIZE(argv),
> -                     0, dev, backend_small, mbr_chs, opts);
> +                     0, dev && !trans ? opts : NULL, backend_small, mbr_chs,
> +                     dev && !trans ? "" : opts);
>      qtest_start(g_strjoinv(" ", argv));
>      test_cmos();
>      qtest_quit(global_qtest);

I've spent more time parsing this test code than I needed for the review
for most patches that touch actual code... Maybe an explicit if (dev &&
!trans) would help somewhat. don't know.

Kevin
Markus Armbruster July 5, 2012, 12:08 p.m. UTC | #2
Kevin Wolf <kwolf@redhat.com> writes:

> Am 29.06.2012 17:34, schrieb Markus Armbruster:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  tests/hd-geo-test.c |    6 ++++--
>>  1 files changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
>> index 0ab573c..02eb5c2 100644
>> --- a/tests/hd-geo-test.c
>> +++ b/tests/hd-geo-test.c
>> @@ -321,13 +321,15 @@ static void test_ide_drive_user(const char *dev, bool trans)
>>      const chst expected_chst = { secs / (4 * 32) , 4, 32, trans };
>>  
>>      argc = setup_common(argv, ARRAY_SIZE(argv));
>> -    opts = g_strdup_printf(",cyls=%d,heads=%d,secs=%d%s",
>> +    opts = g_strdup_printf("%s,cyls=%d,heads=%d,secs=%d%s",
>> +                           dev && !trans ? dev : "",
>>                             expected_chst.cyls, expected_chst.heads,
>>                             expected_chst.secs,
>>                             trans ? ",trans=lba" : "");
>>      cur_ide[0] = &expected_chst;
>>      argc = setup_ide(argc, argv, ARRAY_SIZE(argv),
>> -                     0, dev, backend_small, mbr_chs, opts);
>> +                     0, dev && !trans ? opts : NULL, backend_small, mbr_chs,
>> +                     dev && !trans ? "" : opts);
>>      qtest_start(g_strjoinv(" ", argv));
>>      test_cmos();
>>      qtest_quit(global_qtest);
>
> I've spent more time parsing this test code than I needed for the review
> for most patches that touch actual code... Maybe an explicit if (dev &&
> !trans) would help somewhat. don't know.

Sorry about that.  It gets better again in PATCH 26/32.
diff mbox

Patch

diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 0ab573c..02eb5c2 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -321,13 +321,15 @@  static void test_ide_drive_user(const char *dev, bool trans)
     const chst expected_chst = { secs / (4 * 32) , 4, 32, trans };
 
     argc = setup_common(argv, ARRAY_SIZE(argv));
-    opts = g_strdup_printf(",cyls=%d,heads=%d,secs=%d%s",
+    opts = g_strdup_printf("%s,cyls=%d,heads=%d,secs=%d%s",
+                           dev && !trans ? dev : "",
                            expected_chst.cyls, expected_chst.heads,
                            expected_chst.secs,
                            trans ? ",trans=lba" : "");
     cur_ide[0] = &expected_chst;
     argc = setup_ide(argc, argv, ARRAY_SIZE(argv),
-                     0, dev, backend_small, mbr_chs, opts);
+                     0, dev && !trans ? opts : NULL, backend_small, mbr_chs,
+                     dev && !trans ? "" : opts);
     qtest_start(g_strjoinv(" ", argv));
     test_cmos();
     qtest_quit(global_qtest);