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