Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-20 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-20 Thread nsoffer 
Nir Soffer has submitted this change and it was merged.

Change subject: storagetestlib: qemu chain verification
..


storagetestlib: qemu chain verification

Add utilities for:
 - creating a volume chain with properly initialized qcow2 headers
 - Writing a poisoned pattern to each layer of the chain
 - Verifying the correctness of the chain by reading through the leaf volume

Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Signed-off-by: Adam Litke 
Reviewed-on: https://gerrit.ovirt.org/60999
Reviewed-by: Nir Soffer 
Continuous-Integration: Jenkins CI
---
M tests/storagetestlib.py
M tests/storagetestlibTests.py
2 files changed, 111 insertions(+), 6 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, approved
  Adam Litke: Verified
  Jenkins CI: Passed CI tests



-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-20 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 4: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-20 Thread alitke 
Adam Litke has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 4: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-20 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-20 Thread alitke 
Adam Litke has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/60999/3/tests/storagetestlib.py
File tests/storagetestlib.py:

Line 267: desc,
Line 268: sc.LEGAL_VOL)
Line 269: 
Line 270: 
Line 271: class ChainVerificationError(Exception):
> Maybe inherit from AssertionError, so verification failures would be consid
Done
Line 272: pass
Line 273: 
Line 274: 
Line 275: def qemu_pattern_write(path, format, offset='512', len='1k', 
pattern=5):


-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 3: Code-Review+1

(1 comment)

https://gerrit.ovirt.org/#/c/60999/3/tests/storagetestlib.py
File tests/storagetestlib.py:

Line 267: desc,
Line 268: sc.LEGAL_VOL)
Line 269: 
Line 270: 
Line 271: class ChainVerificationError(Exception):
Maybe inherit from AssertionError, so verification failures would be considered 
as failure (code is broken) instead of an error (test is broken).
Line 272: pass
Line 273: 
Line 274: 
Line 275: def qemu_pattern_write(path, format, offset='512', len='1k', 
pattern=5):


-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread alitke 
Adam Litke has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 3: Verified+1

(5 comments)

https://gerrit.ovirt.org/#/c/60999/2/tests/storagetestlib.py
File tests/storagetestlib.py:

Line 290: raise ChainVerificationError("Verification of volume %s 
failed. "
Line 291:  "Pattern 0x%x not found at 
offset %s" %
Line 292:  (path, pattern, offset))
Line 293: 
Line 294: 
> This fails if you rebase on master:
Done
Line 295: def write_qemu_chain(vol_list):
Line 296: # Starting with the base volume in vol_list, write to the chain 
in a
Line 297: # pattern like the following:
Line 298: #


Line 290: raise ChainVerificationError("Verification of volume %s 
failed. "
Line 291:  "Pattern 0x%x not found at 
offset %s" %
Line 292:  (path, pattern, offset))
Line 293: 
Line 294: 
> pattern must be a number now, so just use 0xf0 + i
Done
Line 295: def write_qemu_chain(vol_list):
Line 296: # Starting with the base volume in vol_list, write to the chain 
in a
Line 297: # pattern like the following:
Line 298: #


Line 307: offset = "{}k".format(i)
Line 308: pattern = 0xf0 + i
Line 309: qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset,
Line 310:len='1k', pattern=pattern)
Line 311: 
> Same, remove str()
Done
Line 312: 
Line 313: def verify_qemu_chain(vol_list):
Line 314: # Check the integrity of a volume chain by reading the leaf volume
Line 315: # and verifying the pattern written by write_chain.  Also, check 
each


Line 315: # and verifying the pattern written by write_chain.  Also, check 
each
Line 316: # volume in the chain to ensure it contains the correct data.
Line 317: top_vol = vol_list[-1]
Line 318: top_vol_fmt = sc.fmt2str(top_vol.getFormat())
Line 319: for i, vol in enumerate(vol_list):
> This works only because we tested that the pattern does not exists in the p
Done
Line 320: offset = "{}k".format(i)
Line 321: pattern = 0xf0 + i
Line 322: 
Line 323: # Check that the correct pattern can be read through the top 
volume


Line 322: 
Line 323: # Check that the correct pattern can be read through the top 
volume
Line 324: qemu_pattern_verify(top_vol.volumePath, top_vol_fmt, 
offset=offset,
Line 325: len='1k', pattern=pattern)
Line 326: 
> Nice!
Thanks!
Line 327: # Check the volume where the pattern was originally written
Line 328: vol_fmt = sc.fmt2str(vol.getFormat())
Line 329: qemu_pattern_verify(vol.volumePath, vol_fmt, offset=offset, 
len='1k',
Line 330: pattern=pattern)


-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/60999/2/tests/storagetestlib.py
File tests/storagetestlib.py:

Line 290: # This allows us to verify the integrity of the whole chain.
Line 291: for i, vol in enumerate(vol_list):
Line 292: vol_fmt = sc.fmt2str(vol.getFormat())
Line 293: offset = "{}k".format(i)
Line 294: pattern = str(0xf0 + i)
> pattern must be a number now, so just use 0xf0 + i
This fails if you rebase on master:

 ==
 ERROR: test_volume_chain_copy('file', 5, 5, (1, 0)) 
(storage_sdm_copy_data_test.CopyDataTests)
 --
 Traceback (most recent call last):
  File "/home/nsoffer/src/vdsm/tests/testlib.py", line 82, in wrapper
return f(self, *args)
  File "/home/nsoffer/src/vdsm/tests/storage_sdm_copy_data_test.py", line 119, 
in test_volume_chain_copy
write_qemu_chain(src_chain)
  File "/home/nsoffer/src/vdsm/tests/storagetestlib.py", line 312, in 
write_qemu_chain
len='1k', pattern=pattern)
  File "/home/nsoffer/src/vdsm/tests/storagetestlib.py", line 281, in 
qemu_pattern_write
write_cmd = 'write -P %d %s %s' % (pattern, offset, len)
 TypeError: %d format: a number is required, not str
Line 295: qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset,
Line 296:len='1k', pattern=pattern)
Line 297: 
Line 298: 


-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 2:

(5 comments)

https://gerrit.ovirt.org/#/c/60999/2/tests/storagetestlib.py
File tests/storagetestlib.py:

Line 290: # This allows us to verify the integrity of the whole chain.
Line 291: for i, vol in enumerate(vol_list):
Line 292: vol_fmt = sc.fmt2str(vol.getFormat())
Line 293: offset = "{}k".format(i)
Line 294: pattern = str(0xf0 + i)
pattern must be a number now, so just use 0xf0 + i
Line 295: qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset,
Line 296:len='1k', pattern=pattern)
Line 297: 
Line 298: 


Line 307: top_vol = vol_list[-1]
Line 308: top_vol_fmt = sc.fmt2str(top_vol.getFormat())
Line 309: for i, vol in enumerate(vol_list):
Line 310: offset = "{}k".format(i)
Line 311: pattern = str(0xf0 + i)
Same, remove str()
Line 312: 
Line 313: # Check that the correct pattern can be read through the top 
volume
Line 314: if not qemu_pattern_verify(top_vol.volumePath, top_vol_fmt,
Line 315:offset=offset, len='1k', 
pattern=pattern):


Line 315:offset=offset, len='1k', 
pattern=pattern):
Line 316: raise ChainVerificationError("Verification of top volume 
failed "
Line 317:  "at offset %s" % offset)
Line 318: 
Line 319: # Check the volume where the pattern was originally written
This works only because we tested that the pattern does not exists in the 
previous volume at this offset (the step bellow). Lets add a comment explaining 
this logic.
Line 320: vol_fmt = sc.fmt2str(vol.getFormat())
Line 321: if not qemu_pattern_verify(vol.volumePath, vol_fmt,
Line 322:offset=offset, len='1k', 
pattern=pattern):
Line 323: raise ChainVerificationError("Verification of volume %d 
failed "


Line 322:offset=offset, len='1k', 
pattern=pattern):
Line 323: raise ChainVerificationError("Verification of volume %d 
failed "
Line 324:  "at offset %s" % (i, offset))
Line 325: 
Line 326: # Check that the next offset contains zeroes
Nice!
Line 327: next_offset = "{}K".format(i + 1)
Line 328: if not qemu_pattern_verify(vol.volumePath, vol_fmt,
Line 329:offset=next_offset, len='1k', 
pattern='0'):
Line 330: raise ChainVerificationError("Verification of zeroes in 
volume %d "


Line 327: next_offset = "{}K".format(i + 1)
Line 328: if not qemu_pattern_verify(vol.volumePath, vol_fmt,
Line 329:offset=next_offset, len='1k', 
pattern='0'):
Line 330: raise ChainVerificationError("Verification of zeroes in 
volume %d "
Line 331:  "failed at offset %s" % (i, 
offset))
Wouldn't it be useful if we raise inside qemu_pattern_verify? We are repeating 
the same error 3 times.

The exception can include the volume name, expected pattern, and offset
Line 332: 
Line 333: 
Line 334: def make_qemu_chain(env, size, base_vol_fmt, chain_len):
Line 335: vol_list = []


-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread alitke 
Adam Litke has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/60999/1/tests/storagetestlib.py
File tests/storagetestlib.py:

Line 292: # vol chain index: 0K  1K  2K  3K
Line 293: #   0: 
Line 294: #   1: 
Line 295: #   2: 
Line 296: #   3: 
> Good point, another pattern is str(0xF0 + i) - does qemu accept  values fro
Yes.  I like str(0xf0 + i) so I have changed to that.
Line 297: # This allows us to verify the integrity of the whole chain.
Line 298: byte_generator = _gen_pattern_bytes()
Line 299: for i, vol in list(enumerate(vol_list)):
Line 300: vol_fmt = sc.fmt2str(vol.getFormat())


-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/60999/1/tests/storagetestlib.py
File tests/storagetestlib.py:

Line 292: # vol chain index: 0K  1K  2K  3K
Line 293: #   0: 
Line 294: #   1: 
Line 295: #   2: 
Line 296: #   3: 
> I specifically avoided zeroes because a zero pattern is what we would have 
Good point, another pattern is str(0xF0 + i) - does qemu accept  values from 0 
to 255 for the pattern?
Line 297: # This allows us to verify the integrity of the whole chain.
Line 298: byte_generator = _gen_pattern_bytes()
Line 299: for i, vol in list(enumerate(vol_list)):
Line 300: vol_fmt = sc.fmt2str(vol.getFormat())


Line 298: byte_generator = _gen_pattern_bytes()
Line 299: for i, vol in list(enumerate(vol_list)):
Line 300: vol_fmt = sc.fmt2str(vol.getFormat())
Line 301: offset = "{}K".format(i)
Line 302: pattern = byte_generator.next()
> I'll use str(i + 1) in order to start with 1.
OK
Line 303: qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset,
Line 304:len='1k', pattern=pattern)
Line 305: 
Line 306: 


Line 337: vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 338: if vol_fmt == sc.COW_FORMAT:
Line 339: backing = parent_vol_id if parent_vol_id != sc.BLANK_UUID 
else None
Line 340: qemuimg.create(vol.volumePath, size=size,
Line 341:format=qemuimg.FORMAT.QCOW2, 
backing=backing)
> No, because I might want to use this helper for other tests without doing t
OK
Line 342: vol_list.append(vol)
Line 343: parent_vol_id = vol_id


-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread alitke 
Adam Litke has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 1:

(11 comments)

https://gerrit.ovirt.org/#/c/60999/1/tests/storagetestlib.py
File tests/storagetestlib.py:

Line 281: def _gen_pattern_bytes():
Line 282: # Produce a predictable sequence of pattern bytes (eg. '1', '2', 
'3')
Line 283: byte_list = ['1', '2', '3', '4', '5', '6', '7', '8', '9']
Line 284: while byte_list:
Line 285: yield byte_list.pop(0)
> We can simplify this to:
Removed.
Line 286: 
Line 287: 
Line 288: def write_qemu_chain(vol_list):
Line 289: # Starting with the base volume in vol_list, write to the chain 
in a


Line 292: # vol chain index: 0K  1K  2K  3K
Line 293: #   0: 
Line 294: #   1: 
Line 295: #   2: 
Line 296: #   3: 
> We should use zero-based patterns (000, 111...)
I specifically avoided zeroes because a zero pattern is what we would have 
before writing anyway.  So how would we detect the difference between not 
writing anything and writing the correct pattern?
Line 297: # This allows us to verify the integrity of the whole chain.
Line 298: byte_generator = _gen_pattern_bytes()
Line 299: for i, vol in list(enumerate(vol_list)):
Line 300: vol_fmt = sc.fmt2str(vol.getFormat())


Line 295: #   2: 
Line 296: #   3: 
Line 297: # This allows us to verify the integrity of the whole chain.
Line 298: byte_generator = _gen_pattern_bytes()
Line 299: for i, vol in list(enumerate(vol_list)):
> We can simplify this to:
Ok, but I want vol so I'll keep it as:
 for i, vol in enumerate(vol_list):
Line 300: vol_fmt = sc.fmt2str(vol.getFormat())
Line 301: offset = "{}K".format(i)
Line 302: pattern = byte_generator.next()
Line 303: qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset,


Line 298: byte_generator = _gen_pattern_bytes()
Line 299: for i, vol in list(enumerate(vol_list)):
Line 300: vol_fmt = sc.fmt2str(vol.getFormat())
Line 301: offset = "{}K".format(i)
Line 302: pattern = byte_generator.next()
> pattern is str(i)
I'll use str(i + 1) in order to start with 1.
Line 303: qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset,
Line 304:len='1k', pattern=pattern)
Line 305: 
Line 306: 


Line 313: # and verifying the pattern written by write_chain.
Line 314: byte_generator = _gen_pattern_bytes()
Line 315: vol = vol_list[-1]
Line 316: vol_fmt = sc.fmt2str(vol.getFormat())
Line 317: for i in list(range(len(vol_list))):
> We can simplify this to:
same as above.
Line 318: offset = "{}K".format(i)
Line 319: pattern = byte_generator.next()
Line 320: if not qemu_pattern_verify(vol.volumePath, vol_fmt, 
offset=offset,
Line 321:len='1k', pattern=pattern):


Line 315: vol = vol_list[-1]
Line 316: vol_fmt = sc.fmt2str(vol.getFormat())
Line 317: for i in list(range(len(vol_list))):
Line 318: offset = "{}K".format(i)
Line 319: pattern = byte_generator.next()
> The pattern is str(i) we don't need the byte_generator.
str(i + 1)
Line 320: if not qemu_pattern_verify(vol.volumePath, vol_fmt, 
offset=offset,
Line 321:len='1k', pattern=pattern):
Line 322: raise ChainVerificationError("Verification failed at 
offset %s" %
Line 323:  offset)


Line 319: pattern = byte_generator.next()
Line 320: if not qemu_pattern_verify(vol.volumePath, vol_fmt, 
offset=offset,
Line 321:len='1k', pattern=pattern):
Line 322: raise ChainVerificationError("Verification failed at 
offset %s" %
Line 323:  offset)
> Since we check only the top volume, we will see all patterns written to any
Done
Line 324: 
Line 325: 
Line 326: def make_qemu_chain(env, size, base_vol_fmt, chain_len):
Line 327: vol_list = []


Line 327: vol_list = []
Line 328: img_id = str(uuid.uuid4())
Line 329: parent_vol_id = sc.BLANK_UUID
Line 330: vol_fmt = base_vol_fmt
Line 331: for i in range(chain_len):
> The pattern we should write here is str(i)
str(i+1)
Line 332: vol_id = str(uuid.uuid4())
Line 333: if parent_vol_id != sc.BLANK_UUID:
Line 334: vol_fmt = sc.COW_FORMAT
Line 335: env.make_volume(size, img_id, vol_id,


Line 337: vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 338: if vol_fmt == sc.COW_FORMAT:
Line 339: backing = parent_vol_id if parent_vol_id != sc.BLANK_UUID 
else None
Line 340: qemuimg.create(vol.volumePath, size=size,
Line

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 1: Code-Review-1

(3 comments)

https://gerrit.ovirt.org/#/c/60999/1/tests/storagetestlib.py
File tests/storagetestlib.py:

Line 292: # vol chain index: 0K  1K  2K  3K
Line 293: #   0: 
Line 294: #   1: 
Line 295: #   2: 
Line 296: #   3: 
We should use zero-based patterns (000, 111...)

Will simplify the code later (using index of volume as the pattern).
Line 297: # This allows us to verify the integrity of the whole chain.
Line 298: byte_generator = _gen_pattern_bytes()
Line 299: for i, vol in list(enumerate(vol_list)):
Line 300: vol_fmt = sc.fmt2str(vol.getFormat())


Line 319: pattern = byte_generator.next()
Line 320: if not qemu_pattern_verify(vol.volumePath, vol_fmt, 
offset=offset,
Line 321:len='1k', pattern=pattern):
Line 322: raise ChainVerificationError("Verification failed at 
offset %s" %
Line 323:  offset)
Since we check only the top volume, we will see all patterns written to any 
volume in the chain.

How can this detect copying images to the wrong target volume?

For example:

- create source volume 0 with pattern  in range 0-1K
- create source volume 1 with pattern  in range 1K-2K
- create destination volumes
- copy data from source volume 0 to destination volume 1
- copy data from source volume 1 to destination volume 0

When checking top destination volume, we still have both 
patterns, right?

I think we should verify each volume in the chain, making sure that each one 
has the required pattern.
Line 324: 
Line 325: 
Line 326: def make_qemu_chain(env, size, base_vol_fmt, chain_len):
Line 327: vol_list = []


https://gerrit.ovirt.org/#/c/60999/1/tests/storagetestlibTests.py
File tests/storagetestlibTests.py:

Line 249: def test_bad_chain_raises(self, storage_type):
Line 250: with fake_env(storage_type) as env:
Line 251: vol_list = make_qemu_chain(env, MB, sc.RAW_FORMAT, 2)
Line 252: write_qemu_chain(vol_list[:1])  # Write the same pattern 
byte
Line 253: write_qemu_chain(vol_list[1:])  # to both volumes in the 
chain
We can do one write like this:

write_qemu_chain(reversed(vol_list))
Line 254: self.assertRaises(ChainVerificationError,
Line 255:   verify_qemu_chain, vol_list)
Line 256: 
Line 257: 


-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-19 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 1:

(8 comments)

https://gerrit.ovirt.org/#/c/60999/1/tests/storagetestlib.py
File tests/storagetestlib.py:

Line 281: def _gen_pattern_bytes():
Line 282: # Produce a predictable sequence of pattern bytes (eg. '1', '2', 
'3')
Line 283: byte_list = ['1', '2', '3', '4', '5', '6', '7', '8', '9']
Line 284: while byte_list:
Line 285: yield byte_list.pop(0)
We can simplify this to:

for byte in range(1, 10):
yield str(byte)

But I think this helper is not needed.
Line 286: 
Line 287: 
Line 288: def write_qemu_chain(vol_list):
Line 289: # Starting with the base volume in vol_list, write to the chain 
in a


Line 295: #   2: 
Line 296: #   3: 
Line 297: # This allows us to verify the integrity of the whole chain.
Line 298: byte_generator = _gen_pattern_bytes()
Line 299: for i, vol in list(enumerate(vol_list)):
We can simplify this to:

for i, _ in enumerate(vol_list):
...
Line 300: vol_fmt = sc.fmt2str(vol.getFormat())
Line 301: offset = "{}K".format(i)
Line 302: pattern = byte_generator.next()
Line 303: qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset,


Line 298: byte_generator = _gen_pattern_bytes()
Line 299: for i, vol in list(enumerate(vol_list)):
Line 300: vol_fmt = sc.fmt2str(vol.getFormat())
Line 301: offset = "{}K".format(i)
Line 302: pattern = byte_generator.next()
pattern is str(i)
Line 303: qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset,
Line 304:len='1k', pattern=pattern)
Line 305: 
Line 306: 


Line 313: # and verifying the pattern written by write_chain.
Line 314: byte_generator = _gen_pattern_bytes()
Line 315: vol = vol_list[-1]
Line 316: vol_fmt = sc.fmt2str(vol.getFormat())
Line 317: for i in list(range(len(vol_list))):
We can simplify this to:

for i, _ in enumerate(vol_list):
...
Line 318: offset = "{}K".format(i)
Line 319: pattern = byte_generator.next()
Line 320: if not qemu_pattern_verify(vol.volumePath, vol_fmt, 
offset=offset,
Line 321:len='1k', pattern=pattern):


Line 315: vol = vol_list[-1]
Line 316: vol_fmt = sc.fmt2str(vol.getFormat())
Line 317: for i in list(range(len(vol_list))):
Line 318: offset = "{}K".format(i)
Line 319: pattern = byte_generator.next()
The pattern is str(i) we don't need the byte_generator.
Line 320: if not qemu_pattern_verify(vol.volumePath, vol_fmt, 
offset=offset,
Line 321:len='1k', pattern=pattern):
Line 322: raise ChainVerificationError("Verification failed at 
offset %s" %
Line 323:  offset)


Line 327: vol_list = []
Line 328: img_id = str(uuid.uuid4())
Line 329: parent_vol_id = sc.BLANK_UUID
Line 330: vol_fmt = base_vol_fmt
Line 331: for i in range(chain_len):
The pattern we should write here is str(i)
Line 332: vol_id = str(uuid.uuid4())
Line 333: if parent_vol_id != sc.BLANK_UUID:
Line 334: vol_fmt = sc.COW_FORMAT
Line 335: env.make_volume(size, img_id, vol_id,


Line 337: vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 338: if vol_fmt == sc.COW_FORMAT:
Line 339: backing = parent_vol_id if parent_vol_id != sc.BLANK_UUID 
else None
Line 340: qemuimg.create(vol.volumePath, size=size,
Line 341:format=qemuimg.FORMAT.QCOW2, 
backing=backing)
Writing the pattern here would be better.
Line 342: vol_list.append(vol)
Line 343: parent_vol_id = vol_id


https://gerrit.ovirt.org/#/c/60999/1/tests/storagetestlibTests.py
File tests/storagetestlibTests.py:

Line 237: qemu_pattern_write(path, img_format, pattern='2')
Line 238: self.assertFalse(qemu_pattern_verify(path, img_format,
Line 239:  pattern='4'))
Line 240: 
Line 241: @permutations((('file',), ('block',)))
This tests both file_env and block_env, but the underlying writes are always 
done to a regular file. Maybe add a comment about this.
Line 242: def test_verify_chain(self, storage_type):
Line 243: with fake_env(storage_type) as env:
Line 244: vol_list = make_qemu_chain(env, MB, sc.RAW_FORMAT, 2)
Line 245: write_qemu_chain(vol_list)


-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-18 Thread alitke 
Adam Litke has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 1: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-18 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storagetestlib: qemu chain verification
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/60999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: qemu chain verification

2016-07-18 Thread alitke 
Adam Litke has uploaded a new change for review.

Change subject: storagetestlib: qemu chain verification
..

storagetestlib: qemu chain verification

Add utilities for:
 - creating a volume chain with properly initialized qcow2 headers
 - Writing a poisoned pattern to each layer of the chain
 - Verifying the correctness of the chain by reading through the leaf volume

Change-Id: I88008b26f340a87466e9fa98ffb34b5df7509390
Signed-off-by: Adam Litke 
---
M tests/storagetestlib.py
M tests/storagetestlibTests.py
2 files changed, 86 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/60999/1

diff --git a/tests/storagetestlib.py b/tests/storagetestlib.py
index f685403..16f7c31 100644
--- a/tests/storagetestlib.py
+++ b/tests/storagetestlib.py
@@ -27,6 +27,7 @@
 
 from vdsm import cmdutils
 from vdsm import commands
+from vdsm import qemuimg
 from vdsm import utils
 from vdsm.storage import constants as sc
 
@@ -275,3 +276,69 @@
 if rc != 0:
 raise cmdutils.Error(cmd, rc, out, err)
 return "Pattern verification failed" not in out
+
+
+def _gen_pattern_bytes():
+# Produce a predictable sequence of pattern bytes (eg. '1', '2', '3')
+byte_list = ['1', '2', '3', '4', '5', '6', '7', '8', '9']
+while byte_list:
+yield byte_list.pop(0)
+
+
+def write_qemu_chain(vol_list):
+# Starting with the base volume in vol_list, write to the chain in a
+# pattern like the following:
+#
+# vol chain index: 0K  1K  2K  3K
+#   0: 
+#   1: 
+#   2: 
+#   3: 
+# This allows us to verify the integrity of the whole chain.
+byte_generator = _gen_pattern_bytes()
+for i, vol in list(enumerate(vol_list)):
+vol_fmt = sc.fmt2str(vol.getFormat())
+offset = "{}K".format(i)
+pattern = byte_generator.next()
+qemu_pattern_write(vol.volumePath, vol_fmt, offset=offset,
+   len='1k', pattern=pattern)
+
+
+class ChainVerificationError(Exception):
+pass
+
+
+def verify_qemu_chain(vol_list):
+# Check the integrity of a volume chain by reading the leaf volume
+# and verifying the pattern written by write_chain.
+byte_generator = _gen_pattern_bytes()
+vol = vol_list[-1]
+vol_fmt = sc.fmt2str(vol.getFormat())
+for i in list(range(len(vol_list))):
+offset = "{}K".format(i)
+pattern = byte_generator.next()
+if not qemu_pattern_verify(vol.volumePath, vol_fmt, offset=offset,
+   len='1k', pattern=pattern):
+raise ChainVerificationError("Verification failed at offset %s" %
+ offset)
+
+
+def make_qemu_chain(env, size, base_vol_fmt, chain_len):
+vol_list = []
+img_id = str(uuid.uuid4())
+parent_vol_id = sc.BLANK_UUID
+vol_fmt = base_vol_fmt
+for i in range(chain_len):
+vol_id = str(uuid.uuid4())
+if parent_vol_id != sc.BLANK_UUID:
+vol_fmt = sc.COW_FORMAT
+env.make_volume(size, img_id, vol_id,
+parent_vol_id=parent_vol_id, vol_format=vol_fmt)
+vol = env.sd_manifest.produceVolume(img_id, vol_id)
+if vol_fmt == sc.COW_FORMAT:
+backing = parent_vol_id if parent_vol_id != sc.BLANK_UUID else None
+qemuimg.create(vol.volumePath, size=size,
+   format=qemuimg.FORMAT.QCOW2, backing=backing)
+vol_list.append(vol)
+parent_vol_id = vol_id
+return vol_list
diff --git a/tests/storagetestlibTests.py b/tests/storagetestlibTests.py
index 0012c83..964c887 100644
--- a/tests/storagetestlibTests.py
+++ b/tests/storagetestlibTests.py
@@ -24,11 +24,14 @@
 from testlib import namedTemporaryDir
 from testlib import VdsmTestCase
 from testlib import TEMPDIR
+from storagetestlib import fake_env
 from storagetestlib import fake_block_env
 from storagetestlib import fake_file_env
 from storagetestlib import make_block_volume
 from storagetestlib import make_file_volume
 from storagetestlib import qemu_pattern_write, qemu_pattern_verify
+from storagetestlib import make_qemu_chain, write_qemu_chain, verify_qemu_chain
+from storagetestlib import ChainVerificationError
 
 from storage import blockSD, fileSD, fileVolume, sd
 
@@ -235,6 +238,22 @@
 self.assertFalse(qemu_pattern_verify(path, img_format,
  pattern='4'))
 
+@permutations((('file',), ('block',)))
+def test_verify_chain(self, storage_type):
+with fake_env(storage_type) as env:
+vol_list = make_qemu_chain(env, MB, sc.RAW_FORMAT, 2)
+write_qemu_chain(vol_list)
+verify_qemu_chain(vol_list)
+
+@permutations((('file',), ('block',)))
+def test_bad_chain_raises(self, storage_type):