| Message ID | 20200503164943.27215-1-mlevitsk@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix iotest 153 | expand |
On 03.05.20 18:49, Maxim Levitsky wrote: > Commit f62514b3def5fb2acbef64d0e053c0c31fa45aff made qemu-img reject -o "" but this test uses it > > Since this test only tries to do a dry-run run of qemu-img amend, replace the -o "" with > dummy -o "size=0" since due to the nature of the test, it is not going > to reach the actual amend operation anyway > > Fixes: f62514b3def5fb2acbef64d0e053c0c31fa45aff > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > tests/qemu-iotests/153 | 2 +- > tests/qemu-iotests/153.out | 12 ++++++------ > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153 > index 2b13111768..3f5029dd8f 100755 > --- a/tests/qemu-iotests/153 > +++ b/tests/qemu-iotests/153 > @@ -122,7 +122,7 @@ for opts1 in "" "read-only=on" "read-only=on,force-share=on"; do > _run_cmd $QEMU_IMG check $L "${TEST_IMG}" > _run_cmd $QEMU_IMG compare $L "${TEST_IMG}" "${TEST_IMG}" > _run_cmd $QEMU_IMG map $L "${TEST_IMG}" > - _run_cmd $QEMU_IMG amend -o "" $L "${TEST_IMG}" > + _run_cmd $QEMU_IMG amend -o "size=0" $L "${TEST_IMG}" AFAIU we don’t want this command to actually change the image (hence the empty options list, which would result in nothing being changed), so maybe "size=$size" would be more in the spirit of the test? Max
On Mon, 2020-05-04 at 11:22 +0200, Max Reitz wrote: > On 03.05.20 18:49, Maxim Levitsky wrote: > > Commit f62514b3def5fb2acbef64d0e053c0c31fa45aff made qemu-img reject -o "" but this test uses it > > > > Since this test only tries to do a dry-run run of qemu-img amend, replace the -o "" with > > dummy -o "size=0" since due to the nature of the test, it is not going > > to reach the actual amend operation anyway > > > > Fixes: f62514b3def5fb2acbef64d0e053c0c31fa45aff > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > tests/qemu-iotests/153 | 2 +- > > tests/qemu-iotests/153.out | 12 ++++++------ > > 2 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153 > > index 2b13111768..3f5029dd8f 100755 > > --- a/tests/qemu-iotests/153 > > +++ b/tests/qemu-iotests/153 > > @@ -122,7 +122,7 @@ for opts1 in "" "read-only=on" "read-only=on,force-share=on"; do > > _run_cmd $QEMU_IMG check $L "${TEST_IMG}" > > _run_cmd $QEMU_IMG compare $L "${TEST_IMG}" "${TEST_IMG}" > > _run_cmd $QEMU_IMG map $L "${TEST_IMG}" > > - _run_cmd $QEMU_IMG amend -o "" $L "${TEST_IMG}" > > + _run_cmd $QEMU_IMG amend -o "size=0" $L "${TEST_IMG}" > > AFAIU we don’t want this command to actually change the image (hence the > empty options list, which would result in nothing being changed), so > maybe "size=$size" would be more in the spirit of the test? This is a good idea! Should I resend the patch or you can add this change locally? Best regards, Maxim Levitsky > > Max >
On 04.05.20 11:41, Maxim Levitsky wrote: > On Mon, 2020-05-04 at 11:22 +0200, Max Reitz wrote: >> On 03.05.20 18:49, Maxim Levitsky wrote: >>> Commit f62514b3def5fb2acbef64d0e053c0c31fa45aff made qemu-img reject -o "" but this test uses it >>> >>> Since this test only tries to do a dry-run run of qemu-img amend, replace the -o "" with >>> dummy -o "size=0" since due to the nature of the test, it is not going >>> to reach the actual amend operation anyway >>> >>> Fixes: f62514b3def5fb2acbef64d0e053c0c31fa45aff >>> >>> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> >>> --- >>> tests/qemu-iotests/153 | 2 +- >>> tests/qemu-iotests/153.out | 12 ++++++------ >>> 2 files changed, 7 insertions(+), 7 deletions(-) >>> >>> diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153 >>> index 2b13111768..3f5029dd8f 100755 >>> --- a/tests/qemu-iotests/153 >>> +++ b/tests/qemu-iotests/153 >>> @@ -122,7 +122,7 @@ for opts1 in "" "read-only=on" "read-only=on,force-share=on"; do >>> _run_cmd $QEMU_IMG check $L "${TEST_IMG}" >>> _run_cmd $QEMU_IMG compare $L "${TEST_IMG}" "${TEST_IMG}" >>> _run_cmd $QEMU_IMG map $L "${TEST_IMG}" >>> - _run_cmd $QEMU_IMG amend -o "" $L "${TEST_IMG}" >>> + _run_cmd $QEMU_IMG amend -o "size=0" $L "${TEST_IMG}" >> >> AFAIU we don’t want this command to actually change the image (hence the >> empty options list, which would result in nothing being changed), so >> maybe "size=$size" would be more in the spirit of the test? > > This is a good idea! Should I resend the patch or you can add this change locally? Fixing it up locally should actually rather be the exception than the rule. It makes sense for larger series where resending it just for one little thing generates too much noise, but in this case I don’t think sending a v2 would hurt. Max
On Mon, 2020-05-04 at 15:08 +0200, Max Reitz wrote: > On 04.05.20 11:41, Maxim Levitsky wrote: > > On Mon, 2020-05-04 at 11:22 +0200, Max Reitz wrote: > > > On 03.05.20 18:49, Maxim Levitsky wrote: > > > > Commit f62514b3def5fb2acbef64d0e053c0c31fa45aff made qemu-img reject -o "" but this test uses it > > > > > > > > Since this test only tries to do a dry-run run of qemu-img amend, replace the -o "" with > > > > dummy -o "size=0" since due to the nature of the test, it is not going > > > > to reach the actual amend operation anyway > > > > > > > > Fixes: f62514b3def5fb2acbef64d0e053c0c31fa45aff > > > > > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > > > --- > > > > tests/qemu-iotests/153 | 2 +- > > > > tests/qemu-iotests/153.out | 12 ++++++------ > > > > 2 files changed, 7 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153 > > > > index 2b13111768..3f5029dd8f 100755 > > > > --- a/tests/qemu-iotests/153 > > > > +++ b/tests/qemu-iotests/153 > > > > @@ -122,7 +122,7 @@ for opts1 in "" "read-only=on" "read-only=on,force-share=on"; do > > > > _run_cmd $QEMU_IMG check $L "${TEST_IMG}" > > > > _run_cmd $QEMU_IMG compare $L "${TEST_IMG}" "${TEST_IMG}" > > > > _run_cmd $QEMU_IMG map $L "${TEST_IMG}" > > > > - _run_cmd $QEMU_IMG amend -o "" $L "${TEST_IMG}" > > > > + _run_cmd $QEMU_IMG amend -o "size=0" $L "${TEST_IMG}" > > > > > > AFAIU we don’t want this command to actually change the image (hence the > > > empty options list, which would result in nothing being changed), so > > > maybe "size=$size" would be more in the spirit of the test? > > > > This is a good idea! Should I resend the patch or you can add this change locally? > > Fixing it up locally should actually rather be the exception than the > rule. It makes sense for larger series where resending it just for one > little thing generates too much noise, but in this case I don’t think > sending a v2 would hurt. > > Max > V2 is on the way then! Best regards, Maxim Levitsky
diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153 index 2b13111768..3f5029dd8f 100755 --- a/tests/qemu-iotests/153 +++ b/tests/qemu-iotests/153 @@ -122,7 +122,7 @@ for opts1 in "" "read-only=on" "read-only=on,force-share=on"; do _run_cmd $QEMU_IMG check $L "${TEST_IMG}" _run_cmd $QEMU_IMG compare $L "${TEST_IMG}" "${TEST_IMG}" _run_cmd $QEMU_IMG map $L "${TEST_IMG}" - _run_cmd $QEMU_IMG amend -o "" $L "${TEST_IMG}" + _run_cmd $QEMU_IMG amend -o "size=0" $L "${TEST_IMG}" _run_cmd $QEMU_IMG commit $L "${TEST_IMG}" _run_cmd $QEMU_IMG resize $L "${TEST_IMG}" $size _run_cmd $QEMU_IMG rebase $L "${TEST_IMG}" -b "${TEST_IMG}.base" diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out index f7464dd8d3..9c01b750e0 100644 --- a/tests/qemu-iotests/153.out +++ b/tests/qemu-iotests/153.out @@ -56,7 +56,7 @@ _qemu_img_wrapper map TEST_DIR/t.qcow2 qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to get shared "write" lock Is another process using the image [TEST_DIR/t.qcow2]? -_qemu_img_wrapper amend -o TEST_DIR/t.qcow2 +_qemu_img_wrapper amend -o size=0 TEST_DIR/t.qcow2 qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to get "write" lock Is another process using the image [TEST_DIR/t.qcow2]? @@ -118,7 +118,7 @@ _qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2 _qemu_img_wrapper map -U TEST_DIR/t.qcow2 -_qemu_img_wrapper amend -o -U TEST_DIR/t.qcow2 +_qemu_img_wrapper amend -o size=0 -U TEST_DIR/t.qcow2 qemu-img: unrecognized option '-U' Try 'qemu-img --help' for more information @@ -187,7 +187,7 @@ _qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2 _qemu_img_wrapper map TEST_DIR/t.qcow2 -_qemu_img_wrapper amend -o TEST_DIR/t.qcow2 +_qemu_img_wrapper amend -o size=0 TEST_DIR/t.qcow2 qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to get "write" lock Is another process using the image [TEST_DIR/t.qcow2]? @@ -241,7 +241,7 @@ _qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2 _qemu_img_wrapper map -U TEST_DIR/t.qcow2 -_qemu_img_wrapper amend -o -U TEST_DIR/t.qcow2 +_qemu_img_wrapper amend -o size=0 -U TEST_DIR/t.qcow2 qemu-img: unrecognized option '-U' Try 'qemu-img --help' for more information @@ -303,7 +303,7 @@ _qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2 _qemu_img_wrapper map TEST_DIR/t.qcow2 -_qemu_img_wrapper amend -o TEST_DIR/t.qcow2 +_qemu_img_wrapper amend -o size=0 TEST_DIR/t.qcow2 _qemu_img_wrapper commit TEST_DIR/t.qcow2 @@ -345,7 +345,7 @@ _qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2 _qemu_img_wrapper map -U TEST_DIR/t.qcow2 -_qemu_img_wrapper amend -o -U TEST_DIR/t.qcow2 +_qemu_img_wrapper amend -o size=0 -U TEST_DIR/t.qcow2 qemu-img: unrecognized option '-U' Try 'qemu-img --help' for more information
Commit f62514b3def5fb2acbef64d0e053c0c31fa45aff made qemu-img reject -o "" but this test uses it Since this test only tries to do a dry-run run of qemu-img amend, replace the -o "" with dummy -o "size=0" since due to the nature of the test, it is not going to reach the actual amend operation anyway Fixes: f62514b3def5fb2acbef64d0e053c0c31fa45aff Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- tests/qemu-iotests/153 | 2 +- tests/qemu-iotests/153.out | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-)