Nir Soffer has posted comments on this change.

Change subject: tests: Create storagetestutils module
......................................................................


Patch Set 3:

(4 comments)

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

Line 42:         metadata = FakeMetadata()
Line 43:     manifest = blockSD.BlockStorageDomainManifest(sduuid, metadata)
Line 44:     if tmpdir is not None:
Line 45:         manifest.domaindir = tmpdir
Line 46:         os.makedirs(os.path.join(manifest.domaindir, sduuid, 
sd.DOMAIN_IMAGES))
Can we have a manifest without a domain directory?
Line 47:     return manifest
Line 48: 
Line 49: 
Line 50: def get_random_devices(count=NR_PVS):


Line 59:     fake_lvm.createVG(vg_name, devices, 
blockSD.STORAGE_UNREADY_DOMAIN_TAG,
Line 60:                       blockSD.VG_METADATASIZE)
Line 61:     fake_lvm.createLV(vg_name, sd.METADATA, blockSD.SD_METADATA_SIZE)
Line 62: 
Line 63:     # Fake the PV information for our metadata LV
Can you add the output of lvm commands we are faking here?

I think lvs -o devices vgname/lvname will give this output.
Line 64:     fake_lvm.lvmd[vg_name][sd.METADATA]['devices'] = \
Line 65:         '{0}(0)'.format(devices[0])
Line 66:     return vg_name
Line 67: 


Line 74:     sduuid = str(uuid.uuid4())
Line 75:     domain_path = os.path.join(tmpdir, sduuid)
Line 76:     make_file(get_metafile_path(domain_path))
Line 77:     if metadata is None:
Line 78:         metadata = dict()
I think you mean FakeMetadata() instead of dict()
Line 79:     manifest = fileSD.FileStorageDomainManifest(domain_path, metadata)
Line 80:     os.makedirs(os.path.join(manifest.domaindir, sduuid, 
sd.DOMAIN_IMAGES))
Line 81:     return manifest
Line 82: 


https://gerrit.ovirt.org/#/c/43547/3/tests/testlib.py
File tests/testlib.py:

Line 148: 
Line 149: def make_file(path, size=0):
Line 150:     dirname = os.path.dirname(path)
Line 151:     if not os.path.exists(dirname):
Line 152:         os.makedirs(dirname)
This is racy and using more sycalls than needed.

Please use this pattern:

   try:
       os.makedirs(dirname)
   except OSError as e:
       if e.errno != errno.EEXIST:
           raise
Line 153:     with open(path, "w") as f:
Line 154:         f.truncate(size)
Line 155: 
Line 156: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I713e19473b45bc05da5a1c888a034306144228c5
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to