Change in vdsm[master]: py3: make network/conf_test pass

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: py3: make network/conf_test pass
..


Patch Set 1: Verified+1

still passes, on both py2 and py3

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcab2d6f1f88b04d849156915a89c4cc8589
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: wip: jsonrpc: introduce new api

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: wip: jsonrpc: introduce new api
..


Patch Set 4:

(19 comments)

https://gerrit.ovirt.org/#/c/64502/4/lib/vdsm/jsonrpcapi.py
File lib/vdsm/jsonrpcapi.py:

Line 15: # along with this program; if not, write to the Free Software
Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
02110-1301 USA
Line 17: #
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
How about vdsm.client?

We are not going to switch the protocol again, this was painful enough last 
time. And jsonrpc is not correct anyway, we are actually using stomp, not 
jsonrpc.
Line 20: 
Line 21: from __future__ import absolute_import
Line 22: 
Line 23: from uuid import uuid4


Line 19: #
Line 20: 
Line 21: from __future__ import absolute_import
Line 22: 
Line 23: from uuid import uuid4
Don't import functions, only modules. This make the dependencies easier to 
understand and allow reloading of modules in runtime.
Line 24: 
Line 25: from yajsonrpc import stompreactor
Line 26: from yajsonrpc import \
Line 27: JsonRpcRequest, \


Line 21: from __future__ import absolute_import
Line 22: 
Line 23: from uuid import uuid4
Line 24: 
Line 25: from yajsonrpc import stompreactor
This package depends on vdsm, we need to make the client separate from vdsm, it 
should be a separate standalone package, maybe depending on the api package.

We can resolve this issue later, our most important users (hosted-engine, mom) 
run on the same machine.
Line 26: from yajsonrpc import \
Line 27: JsonRpcRequest, \
Line 28: JsonRpcNoResponseError, \
Line 29: CALL_TIMEOUT


Line 25: from yajsonrpc import stompreactor
Line 26: from yajsonrpc import \
Line 27: JsonRpcRequest, \
Line 28: JsonRpcNoResponseError, \
Line 29: CALL_TIMEOUT
Better just to import yajsonrpc and use yajsonrpc.Xxx when needed.
Line 30: 
Line 31: from api import vdsmapi
Line 32: from .config import config
Line 33: 


Line 28: JsonRpcNoResponseError, \
Line 29: CALL_TIMEOUT
Line 30: 
Line 31: from api import vdsmapi
Line 32: from .config import config
Vdsm config may not be avilable.
Line 33: 
Line 34: 
Line 35: class _Server(object):
Line 36: 


Line 31: from api import vdsmapi
Line 32: from .config import config
Line 33: 
Line 34: 
Line 35: class _Server(object):
This is a client, not a server.
Line 36: 
Line 37: def __init__(self, client):
Line 38: api_strict_mode = config.getboolean('devel', 'api_strict_mode')
Line 39: self._schema = vdsmapi.Schema([vdsmapi.find_schema()],


Line 34: 
Line 35: class _Server(object):
Line 36: 
Line 37: def __init__(self, client):
Line 38: api_strict_mode = config.getboolean('devel', 'api_strict_mode')
You cannot use vdsm configuration here, the library may be used on another 
machine (another server, someone laptop), which does not have vdsm installed.
Line 39: self._schema = vdsmapi.Schema([vdsmapi.find_schema()],
Line 40:   api_strict_mode)
Line 41: self._client = client
Line 42: self._default_timeout = CALL_TIMEOUT


Line 36: 
Line 37: def __init__(self, client):
Line 38: api_strict_mode = config.getboolean('devel', 'api_strict_mode')
Line 39: self._schema = vdsmapi.Schema([vdsmapi.find_schema()],
Line 40:   api_strict_mode)
Please avoid using the schema here, we don't want to pay for parsing the schema 
on each client call.

Also, this is not needed to have use the client, maybe only when the client is 
run with --help, or with --verify flag. This library must provide control to 
the user, not force the shema on all users of the library.

Please send a very simple first version, we don't have lot of time for 4.1, and 
if we waste time on the schema this will not be ready in time and we will have 
to carry xmlrpc for another version.
Line 41: self._client = client
Line 42: self._default_timeout = CALL_TIMEOUT
Line 43: self._timeouts = {
Line 44: 'migrationCreate': config.getint(


Line 44: 'migrationCreate': config.getint(
Line 45: 'vars', 'migration_create_timeout'),
Line 46: }
Line 47: 
Line 48: def set_default_timeout(self, timeout):
We don't need setters like this in python. just make default_timeout public 
attribute.

If we need to do handle or control changes, we can convert it to a property.
Line 49: self._default_timeout = timeout
Line 50: 
Line 51: def _prepare_args(self, className, methodName, args, kwargs):
Line 52: try:


Line 57: params = dict(zip(allargs, args))
Line 58: params.update(kwargs)
Line 59: return params
Line 60: 
Line 61: def call(self, method, *args, **kwargs):
Can you explain why we need both *args and **kwargs? To send a request we need:
- The 

Change in vdsm[master]: storage: Support generation in sdm.copy_data

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Support generation in sdm.copy_data
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I701e220f5c275dccaa3767768e2d1433fe033839
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Increment generation when completing operation

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Increment generation when completing operation
..


Patch Set 4: Code-Review+1

(1 comment)

This is next generation patch :-)

https://gerrit.ovirt.org/#/c/64487/4/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 512
Line 513
Line 514
Line 515
Line 516
It would be helpful to comment here that we intentionally do not use 
try-finally block, so volume will become legal only if the operation succeeded.

I can see the patch trying to "fix" this code...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9a62289378b11c3b2f2f520c894336fc89c1fdc
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: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Report generation in volume.getInfo

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Report generation in volume.getInfo
..


Patch Set 4: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/64485/4/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 6964: type: *VolumeLeaseStatus
Line 6965: 
Line 6966: -   description: A monotonically increasing number, 
incremented each
Line 6967:  time a volume operation is completed 
successfully.
Line 6968:  The maximum value is 2^63-1 and the next 
increment
999
Line 6969:  will reset the value to 0.
Line 6970: name: generation
Line 6971: type: uint
Line 6972: type: object


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a6bb44c5f789acf3c63953f4b87c72585becc1
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: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 4: Code-Review+2

(1 comment)

https://gerrit.ovirt.org/#/c/64484/4/tests/storage_volume_metadata_test.py
File tests/storage_volume_metadata_test.py:

Line 188: 
Line 189: def test_generation_default(self):
Line 190: lines = make_lines(GEN=None)
Line 191: md = volume.VolumeMetadata.from_lines(lines)
Line 192: self.assertEqual(sc.DEFAULT_GENERATION, md.generation)
Neat


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
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: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: SDM.copy_data test for abort while copying

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: SDM.copy_data test for abort while copying
..


Patch Set 5: Code-Review-1

(4 comments)

https://gerrit.ovirt.org/#/c/64479/5/tests/storage_sdm_copy_data_test.py
File tests/storage_sdm_copy_data_test.py:

Line 274
Line 275
Line 276
Line 277
Line 278
Lets use here the started event owned by the FakeQemuConvertChecker.

One problem with this, is that all operations shared the same event. Since we 
cannot really support multiple operations without reces, we can return self  in 
the checker, and have the checker implement the operation interface. Another 
option, that may be cleaner, generated the fake operation in the test and send 
it to the checker.


Line 278
Line 279
Line 280
Line 281
Line 282
So we remove this...


Line 296: 
Line 297: class FakeQemuImgOperation(object):
Line 298: def __init__(self, error, wait_for_abort):
Line 299: self.error = error
Line 300: self.wait_for_abort = wait_for_abort
Lets have an event here...
Line 301: self.running = True
Line 302: 
Line 303: def _wait(self):
Line 304: while self.running:


Line 310: def wait_for_completion(self):
Line 311: if self.error:
Line 312: raise self.error()
Line 313: if self.wait_for_abort:
Line 314: self._wait()
And we can wait on the event here:

if not self.wait_for_abort.wait(1):
raise RuntimeError("Timeout waiting for abort, broken test?")


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I740d9ba42e3bd70865eadcb024ce6d9d8da0af95
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: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: file_volume: Add qcow2_compat on create

[automation]

gerrit-hooks has posted comments on this change.

Change subject: file_volume: Add qcow2_compat on create
..


Patch Set 16:

* update_tracker: OK
* Set MODIFIED::IGNORE, no Bug-Url found.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I81fd3a6e263980c1eabeea7f619c9a6b48991938
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: GenerationMismatch exception

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: GenerationMismatch exception
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2eb9ffb326f343d89fa17af462c3bbfb18697a04
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: file_volume: Add qcow2_compat on create

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: file_volume: Add qcow2_compat on create
..


file_volume: Add qcow2_compat on create

Pass qcow2 compat on create.

Change-Id: I81fd3a6e263980c1eabeea7f619c9a6b48991938
Signed-off-by: Maor Lipchuk 
Reviewed-on: https://gerrit.ovirt.org/64374
Reviewed-by: Nir Soffer 
Continuous-Integration: Nir Soffer 
---
M vdsm/storage/fileVolume.py
1 file changed, 2 insertions(+), 1 deletion(-)

Approvals:
  Nir Soffer: Looks good to me, approved; Passed CI tests
  Maor Lipchuk: Verified

Objections:
  Jenkins CI: Failed CI tests



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I81fd3a6e263980c1eabeea7f619c9a6b48991938
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: file_volume: Add qcow2_compat on create

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: file_volume: Add qcow2_compat on create
..


Patch Set 15: Continuous-Integration+1

Network tests errors again, ignoring.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I81fd3a6e263980c1eabeea7f619c9a6b48991938
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Introduce VolumeManifest.operation context

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Introduce VolumeManifest.operation context
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30a3ac2971411778d24e007aac9fcb3009edb4c4
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Introduce VolumeManifest.operation context

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: storage: Introduce VolumeManifest.operation context
..


storage: Introduce VolumeManifest.operation context

When performing datapath operations on a volume (eg. copying data) we
mark the volume ILLEGAL before starting the operation and only mark the
volume LEGAL again once the operation is finished.  As long as this is
all done with the volume lease held the engine can poll the volume from
any host to determine if the operation is running (lease held) and can
detect an interrupted/failed operation (lease free and volume ILLEGAL).

Later this contextmanager will be expanded to support volume generation
incrementation when exiting successfully which will also allow engine to
determine if an operation was completed successfully.

All metadata updates must be performed to a single block with one write
in order to ensure atomicity.

Change-Id: I30a3ac2971411778d24e007aac9fcb3009edb4c4
Signed-off-by: Adam Litke 
Reviewed-on: https://gerrit.ovirt.org/64362
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer 
Tested-by: Nir Soffer 
---
M tests/storage_volume_test.py
M vdsm/storage/volume.py
2 files changed, 59 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I30a3ac2971411778d24e007aac9fcb3009edb4c4
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Introduce VolumeManifest.operation context

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Introduce VolumeManifest.operation context
..


Patch Set 4: Verified+1

Verified by the tests.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30a3ac2971411778d24e007aac9fcb3009edb4c4
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add make_uuid test helper

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Add make_uuid test helper
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Use make_uuid in storage tests

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Use make_uuid in storage tests
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie86551f26d5cdd12e66516af3b069344c99588e8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Use make_uuid in storage tests

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: tests: Use make_uuid in storage tests
..


tests: Use make_uuid in storage tests

Change-Id: Ie86551f26d5cdd12e66516af3b069344c99588e8
Signed-off-by: Adam Litke 
Reviewed-on: https://gerrit.ovirt.org/64943
Reviewed-by: Nir Soffer 
Continuous-Integration: Jenkins CI
Tested-by: Nir Soffer 
---
M tests/storageMailboxTests.py
M tests/storage_hsm_test.py
M tests/storage_sdm_copy_data_test.py
M tests/storage_sdm_create_volume_test.py
M tests/storage_volume_artifacts_test.py
M tests/storage_volume_metadata_test.py
M tests/storage_workarounds_test.py
M tests/storagetestlib.py
M tests/storagetestlibTests.py
9 files changed, 49 insertions(+), 50 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie86551f26d5cdd12e66516af3b069344c99588e8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add make_uuid test helper

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Add make_uuid test helper
..


Patch Set 2: Verified+1

Verified by the next patch.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add make_uuid test helper

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: tests: Add make_uuid test helper
..


tests: Add make_uuid test helper

The test code is sprinkled with lots of boilerplate code to generate
UUIDs in the proper format.  Add a helper to testlib so we can reduce
this duplicated logic in tests.

Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
Signed-off-by: Adam Litke 
Reviewed-on: https://gerrit.ovirt.org/64542
Reviewed-by: Nir Soffer 
Continuous-Integration: Jenkins CI
Tested-by: Nir Soffer 
---
M tests/testlib.py
1 file changed, 8 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
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 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Use make_uuid in storage tests

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Use make_uuid in storage tests
..


Patch Set 1: Verified+1

Verified by the tests.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie86551f26d5cdd12e66516af3b069344c99588e8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: blockVolume: Add qcow2_compat on create

[automation]

gerrit-hooks has posted comments on this change.

Change subject: blockVolume: Add qcow2_compat on create
..


Patch Set 16:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fe6bf59b76b68ed8f0e60f66057c07b56d2d942
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: blockVolume: Add qcow2_compat on create

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: blockVolume: Add qcow2_compat on create
..


blockVolume: Add qcow2_compat on create

Pass qcow2 compat on create.

Change-Id: I9fe6bf59b76b68ed8f0e60f66057c07b56d2d942
Signed-off-by: Maor Lipchuk 
Reviewed-on: https://gerrit.ovirt.org/64376
Reviewed-by: Nir Soffer 
Continuous-Integration: Nir Soffer 
---
M vdsm/storage/blockVolume.py
1 file changed, 2 insertions(+), 1 deletion(-)

Approvals:
  Nir Soffer: Looks good to me, approved; Passed CI tests
  Maor Lipchuk: Verified

Objections:
  Jenkins CI: Failed CI tests



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I9fe6bf59b76b68ed8f0e60f66057c07b56d2d942
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: volume_artifacts: Add qcow2_compat on create.

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: volume_artifacts: Add qcow2_compat on create.
..


Patch Set 15:

Adam, do we need to update this code? it is basically dead code now.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2dd2d53fba0dd69cdb4f60e152cf6d254cfb863a
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: vm: Add qcow2_compat on create

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
..


Patch Set 15: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/64377/15/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2701: try:
Line 2702: qemuimg.create(transientPath, 
format=qemuimg.FORMAT.QCOW2,
Line 2703:backing=diskParams['path'],
Line 2704:backingFormat=driveFormat,
Line 2705:qcow2Compat=self._dom.qcow2_compat())
I commented before - you need a storage domain manifest object, vm._dom is a 
libvirt.virDomain object.
Line 2706: os.fchmod(transientHandle, 0o660)
Line 2707: except Exception:
Line 2708: os.unlink(transientPath)  # Closing after deletion is 
correct
Line 2709: self.log.exception("Failed to create the transient disk 
for "


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: copy_data: Add qcow2_compat on convert.

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: copy_data: Add qcow2_compat on convert.
..


Patch Set 15: Code-Review-1

We must have tests for this.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie26e5dcba6fc493b32ea7764889df2918c4dfdd3
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: blockVolume: Add qcow2_compat on create

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: blockVolume: Add qcow2_compat on create
..


Patch Set 15: Continuous-Integration+1

Network tests failing on python 3, could not be more irrelevant to this patch.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fe6bf59b76b68ed8f0e60f66057c07b56d2d942
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: blockVolume: Add qcow2_compat on create

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: blockVolume: Add qcow2_compat on create
..


Patch Set 15: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fe6bf59b76b68ed8f0e60f66057c07b56d2d942
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: volume: Add qcow2_compat on create

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: volume: Add qcow2_compat on create
..


Patch Set 15: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/64375/15/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 690
Line 691
Line 692
Line 693
Line 694
Should be here, the compat belongs to the image, not the backing image.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8655be6da0b4cbd1c286b0c40288681a991aff19
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 12:

(1 comment)

https://gerrit.ovirt.org/#/c/63027/12/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 5654: 
Line 5655: -   defaultvalue: null
Line 5656: description: The GUID of the first device containing the 
domain
Line 5657: metadata lv for block storage domains (optional)
Line 5658: name: metadataDevice
> I am wondering if this is too much detail for this API.  What engine really
+1
Line 5659: type: string
Line 5660: type: object
Line 5661: 
Line 5662: StorageDomainStatus: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[alitke]

Adam Litke has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 12:

(2 comments)

https://gerrit.ovirt.org/#/c/63027/12/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

PS12, Line 5658: metadataDevice
I am wondering if this is too much detail for this API.  What engine really 
wants to know is "Does this VG have any immutable devices?".  Immutable devices 
must be excluded from VG modifications.  If we do it this way, then block 
domains can add here the first PV of the metadata LV and file domains can 
return an empty list.  If we need to restrict other PVs later we can easily add 
them to the list.


https://gerrit.ovirt.org/#/c/63027/12/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

PS12, Line 448: getMetadataLVDevice
This can be an internal function.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: image: Pass compat version.

[automation]

gerrit-hooks has posted comments on this change.

Change subject: image: Pass compat version.
..


Patch Set 19:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I56e01923aba982e20bda72af61db83df50373127
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: file_volume: Add qcow2_compat on create

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: file_volume: Add qcow2_compat on create
..


Patch Set 15: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I81fd3a6e263980c1eabeea7f619c9a6b48991938
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: image: Pass compat version.

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: image: Pass compat version.
..


image: Pass compat version.

Pass sd compat version on convert and create image.

Change-Id: I56e01923aba982e20bda72af61db83df50373127
Signed-off-by: Maor Lipchuk 
Reviewed-on: https://gerrit.ovirt.org/64293
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer 
---
M vdsm/storage/image.py
1 file changed, 5 insertions(+), 2 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, approved
  Jenkins CI: Passed CI tests
  Maor Lipchuk: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I56e01923aba982e20bda72af61db83df50373127
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: image: Pass compat version.

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: image: Pass compat version.
..


Patch Set 18: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I56e01923aba982e20bda72af61db83df50373127
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: blockSD: Storage domain life cycle management

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: blockSD: Storage domain life cycle management
..


Patch Set 6:

(1 comment)

If we move this to activateStorageDomain and deactivateStorageDomain, we may 
block these flows when storage is not available. This is the reason that we do 
everything in the domain monitor thread.

https://gerrit.ovirt.org/#/c/56876/6/vdsm/storage/monitor.py
File vdsm/storage/monitor.py:

Line 364: @utils.cancelpoint
Line 365: def _produceDomain(self):
Line 366: log.debug("Producing domain %s", self.sdUUID)
Line 367: domain = sdCache.produce(self.sdUUID)
Line 368: domain.setup()
> Why not put this in _setupMonitor()?  It seems kind of hidden in here.
If this fail, we want to retry - setting self.domain only when the domain was 
setup is a nice way to get this.

But I can add a flag for setup to make it more clear.
Line 369: self.domain = domain
Line 370: 
Line 371: @utils.cancelpoint
Line 372: def _setIsoDomainInfo(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7227bb43c2e1ee67a6239956aae48173a27f566e
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Simone Tiraboschi 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: blockSD: Storage domain life cycle management

[alitke]

Adam Litke has posted comments on this change.

Change subject: blockSD: Storage domain life cycle management
..


Patch Set 6:

(1 comment)

Seems straightforward.  Why did you put this in the monitor instead of 
activateStorageDomain and deactivateStorageDomain?

https://gerrit.ovirt.org/#/c/56876/6/vdsm/storage/monitor.py
File vdsm/storage/monitor.py:

PS6, Line 368: domain.setup()
Why not put this in _setupMonitor()?  It seems kind of hidden in here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7227bb43c2e1ee67a6239956aae48173a27f566e
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Simone Tiraboschi 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 12: Continuous-Integration+1

Unrelated network tests failing.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report vg metadata device for block sd

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: StorageDomain.getInfo - report vg metadata device for block sd
..


Patch Set 7:

Please fix pep8 errors:

19:19:38 ./vdsm/storage/lvm.py:1342:80: E501 line too long (80 > 79 characters)
19:19:38 ./vdsm/storage/lvm.py:1348:1: E302 expected 2 blank lines, found 1
19:19:38 ./vdsm/storage/lvm.py:1351:80: E501 line too long (84 > 79 characters)
19:19:38 ./vdsm/storage/lvm.py:1352:80: E501 line too long (80 > 79 characters)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 18:

(2 comments)

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 346
Line 347
Line 348
Line 349
Line 350
> Right, I missed this. Will fix.
You can fix the places which changed the semantics, or split them to the next 
patch, I guess most need the same fix like:

   vm = self.vm  # First check that the vm is running

Then the current code will probably work without change.


Line 263: """
Line 264: Lock user session in guest operating system using guest agent.
Line 265: """
Line 266: self.vm.guestAgent.desktopLock()
Line 267: if self.vm.guestAgent.isResponsive():
> Let's keep this version as-is, I have further (minor) improvements in the p
+1 for current code.
Line 268: return {'status': doneCode}
Line 269: else:
Line 270: return errCode['nonresp']
Line 271: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Use Volume.operation in SDM.copy_data

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: Use Volume.operation in SDM.copy_data
..


Patch Set 4: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/64478/4/tests/storage_sdm_copy_data_test.py
File tests/storage_sdm_copy_data_test.py:

Line 268: assert sc.ILLEGAL_VOL == self.dst_vol.getLegality()
Line 269: try:
Line 270: return FakeQemuImgOperation(self.error)
Line 271: finally:
Line 272: self.started.set()
We talked today about fixing the race here - do you plan to address this in 
this patch?
Line 273: 
Line 274: 
Line 275: class FakeQemuImgOperation(object):
Line 276: def __init__(self, error):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I574cea3387ab5b99368e0317aed73683d398a596
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Introduce VolumeManifest.operation context

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Introduce VolumeManifest.operation context
..


Patch Set 4: Code-Review+2

(1 comment)

https://gerrit.ovirt.org/#/c/64362/4/tests/storage_volume_test.py
File tests/storage_volume_test.py:

Line 106: 
Line 107: with fake_env('file') as env:
Line 108: env.make_volume(MB, img_id, vol_id)
Line 109: vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 110: vol.setMetadata = CountedInstanceMethod(vol.setMetadata)
I don't think we need this, this will only make the test break if we change the 
implementation later to read another value, but lets take it as is.
Line 111: self.assertEqual(sc.LEGAL_VOL, vol.getLegality())
Line 112: with vol.operation():
Line 113: self.assertEqual(sc.ILLEGAL_VOL, vol.getLegality())
Line 114: self.assertEqual(1, vol.setMetadata.nr_calls)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30a3ac2971411778d24e007aac9fcb3009edb4c4
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: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: API: modernize VM methods

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: API: modernize VM methods
..


Patch Set 19: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Use make_uuid in storage tests

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Use make_uuid in storage tests
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie86551f26d5cdd12e66516af3b069344c99588e8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add make_uuid test helper

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Add make_uuid test helper
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: core: Expose API for qemuimg commit

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: core: Expose API for qemuimg commit
..


Patch Set 8:

(1 comment)

https://gerrit.ovirt.org/#/c/64222/8/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 193: else:
Line 194: # We don't want to empty the volume as a result of the commit
Line 195: # operation. To do so, we provide the '-d' option when the 
base
Line 196: # isn't provided. When the base volume is provided, the top 
volume
Line 197: # will not be emptied after the commit operation.
Can we shorten this to one line?
Line 198: # This is important mainly when we want to wipe the volume 
before
Line 199: # deleting it. Emptying it may leave the data on the 
underlying
Line 200: # storage.
Line 201: cmd.append("-d")


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If7a13be40541fb268541bd8614a642263b96b487
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: core: Expose API for qemuimg commit

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: core: Expose API for qemuimg commit
..


Patch Set 8:

(11 comments)

Tests are nice, but we are reinventing storagetestlib.write_qemu_chain and 
storagetestlib.verify_qemu_chain.

I think we should instead fix these methods so they work with generic tuples 
instead of using volume objects, so we can reuse them for other tests.

https://gerrit.ovirt.org/#/c/64222/8/tests/qemuimg_test.py
File tests/qemuimg_test.py:

Line 334: 
Line 335: @expandPermutations
Line 336: class TestCommit(TestCaseBase):
Line 337: 
Line 338: @MonkeyPatch(qemuimg, 'config', CONFIG)
We can remove the config mocking.

Instead, I would add qcow2 compat to permutations - note that qeumimg.create 
supports now qcow2Compat argument - we should test with both compat="0.10" and 
"1.1" to make sure commit works for both old volumes and new volumes.
Line 339: @permutations([
Line 340: # chain_len, base_format, use_base
Line 341: (1, qemuimg.FORMAT.RAW, False),
Line 342: (1, qemuimg.FORMAT.RAW, True),


Line 343: (1, qemuimg.FORMAT.QCOW2, False),
Line 344: (1, qemuimg.FORMAT.QCOW2, True),
Line 345: (3, qemuimg.FORMAT.RAW, True),
Line 346: (3, qemuimg.FORMAT.QCOW2, True)
Line 347: ])
Nice!
Line 348: def test_commit(self, chain_len, base_format, use_base=True):
Line 349: size = 1048576
Line 350: with namedTemporaryDir() as tmpdir:
Line 351: # create base


Line 347: ])
Line 348: def test_commit(self, chain_len, base_format, use_base=True):
Line 349: size = 1048576
Line 350: with namedTemporaryDir() as tmpdir:
Line 351: # create base
It would be nice to create base in the same loop as the rest of the volumes. We 
can call it "vol0.img".

To set the base format, we can do:

for i, name in enumerate(chain):
   ...
   make_image(..., format=base_format if i == 0 else format, ...)
Line 352: base_vol = os.path.join(tmpdir, "base.img")
Line 353: self.make_image(base_vol, size, base_format, 0)
Line 354: 
Line 355: # create chain


Line 349: size = 1048576
Line 350: with namedTemporaryDir() as tmpdir:
Line 351: # create base
Line 352: base_vol = os.path.join(tmpdir, "base.img")
Line 353: self.make_image(base_vol, size, base_format, 0)
We can add qcow2_compat argument here...
Line 354: 
Line 355: # create chain
Line 356: chain = []
Line 357: parent = base_vol


Line 355: # create chain
Line 356: chain = []
Line 357: parent = base_vol
Line 358: top_format = qemuimg.FORMAT.QCOW2
Line 359: for i in range(0, chain_len):
This can and should be written as:

for i in range(chain_len):
Line 360: vol = os.path.join(tmpdir, "vol%d.img" % i)
Line 361: self.make_image(vol, size, top_format, i+1, parent)
Line 362: chain.append(vol)
Line 363: parent = vol


Line 363: parent = vol
Line 364: 
Line 365: base = None
Line 366: if use_base:
Line 367: base = base_vol
This works, but we can use more modern conditional expression:

base = base_vol if use_base else None
Line 368: 
Line 369: op = qemuimg.commit(chain[-1], topFormat=top_format,
Line 370: base=base)
Line 371: op.wait_for_completion()


Line 366: if use_base:
Line 367: base = base_vol
Line 368: 
Line 369: op = qemuimg.commit(chain[-1], topFormat=top_format,
Line 370: base=base)
Or we can just do:

op = qemuimg.commit(chain[-1], topFormat=top_format,
base=base if use_base else None)
Line 371: op.wait_for_completion()
Line 372: 
Line 373: # verify base data at offset 0 after commit
Line 374: qemu_pattern_verify(base_vol, base_format, offset='0',


Line 371: op.wait_for_completion()
Line 372: 
Line 373: # verify base data at offset 0 after commit
Line 374: qemu_pattern_verify(base_vol, base_format, offset='0',
Line 375: len='1k', pattern=0xf0)
If you include base in chain, can verify all volume in the same loop.
Line 376: for i, vol in enumerate(chain):
Line 377: offset = "{}k".format(i+1)
Line 378: pattern = 0xf0 + (i+1)
Line 379: qemu_pattern_verify(base_vol, base_format, 
offset=offset,


Line 385: @permutations([
Line 386: # base_format
Line 387: (qemuimg.FORMAT.RAW,),
Line 388: (qemuimg.FORMAT.QCOW2,)
Line 389: ])
We don't need permutations for this test, and we don't care about the 

Change in vdsm[master]: tests: Use make_uuid in storage tests

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Use make_uuid in storage tests
..


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/64943
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie86551f26d5cdd12e66516af3b069344c99588e8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: GenerationMismatch exception

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: GenerationMismatch exception
..


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/64543
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2eb9ffb326f343d89fa17af462c3bbfb18697a04
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Use Volume.operation in SDM.copy_data

[automation]

gerrit-hooks has posted comments on this change.

Change subject: Use Volume.operation in SDM.copy_data
..


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/64478
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I574cea3387ab5b99368e0317aed73683d398a596
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: SDM.copy_data test for abort while copying

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: SDM.copy_data test for abort while copying
..


Patch Set 5:

* 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/64479
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I740d9ba42e3bd70865eadcb024ce6d9d8da0af95
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Support generation in sdm.copy_data

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Support generation in sdm.copy_data
..


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/64488
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I701e220f5c275dccaa3767768e2d1433fe033839
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Remove TODO for SDM.copy_data abort before copy test

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Remove TODO for SDM.copy_data abort before copy test
..


Patch Set 5:

* 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/64480
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I26e724560e9cb485fd4766ebcc04f644ad6431e4
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add make_uuid test helper

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Add make_uuid test helper
..


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/64542
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Introduce VolumeManifest.operation context

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Introduce VolumeManifest.operation context
..


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/64362
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I30a3ac2971411778d24e007aac9fcb3009edb4c4
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Report generation in volume.getInfo

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Report generation in volume.getInfo
..


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/64485
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a6bb44c5f789acf3c63953f4b87c72585becc1
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


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/64484
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Validate generation in volume.operation context

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Validate generation in volume.operation context
..


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/64486
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I77d9c6cb46053ab32c59c77599b7c1366e1c8196
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Increment generation when completing operation

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Increment generation when completing operation
..


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/64487
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9a62289378b11c3b2f2f520c894336fc89c1fdc
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Use make_uuid in storage tests

[alitke]

Adam Litke has uploaded a new change for review.

Change subject: tests: Use make_uuid in storage tests
..

tests: Use make_uuid in storage tests

Change-Id: Ie86551f26d5cdd12e66516af3b069344c99588e8
Signed-off-by: Adam Litke 
---
M tests/storageMailboxTests.py
M tests/storage_hsm_test.py
M tests/storage_sdm_copy_data_test.py
M tests/storage_sdm_create_volume_test.py
M tests/storage_volume_artifacts_test.py
M tests/storage_volume_metadata_test.py
M tests/storage_workarounds_test.py
M tests/storagetestlib.py
M tests/storagetestlibTests.py
9 files changed, 49 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/43/64943/1

diff --git a/tests/storageMailboxTests.py b/tests/storageMailboxTests.py
index 1d5b45a..5de9652 100644
--- a/tests/storageMailboxTests.py
+++ b/tests/storageMailboxTests.py
@@ -18,11 +18,11 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
-from uuid import uuid4
 import threading
 import os
 import shutil
 
+from testlib import make_uuid
 from testlib import VdsmTestCase as TestCaseBase
 
 import storage.storage_mailbox as sm
@@ -33,7 +33,7 @@
 
 class StoragePoolStub(object):
 def __init__(self):
-self.spUUID = str(uuid4())
+self.spUUID = make_uuid()
 self.storage_repository = tempfile.mkdtemp(dir='/var/tmp')
 self.__masterDir = os.path.join(self.storage_repository, self.spUUID,
 "mastersd", DOMAIN_META_DATA)
diff --git a/tests/storage_hsm_test.py b/tests/storage_hsm_test.py
index e9e0448..b7e0e28 100644
--- a/tests/storage_hsm_test.py
+++ b/tests/storage_hsm_test.py
@@ -18,11 +18,11 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
-import uuid
 from contextlib import contextmanager
 
 from monkeypatch import MonkeyPatchScope
 from testlib import make_config
+from testlib import make_uuid
 from testlib import VdsmTestCase
 from testlib import permutations, expandPermutations
 from storagetestlib import fake_file_env
@@ -112,8 +112,8 @@
 @contextmanager
 def fake_volume(self, vol_fmt):
 with fake_file_env() as env:
-img_id = str(uuid.uuid4())
-vol_id = str(uuid.uuid4())
+img_id = make_uuid()
+vol_id = make_uuid()
 make_file_volume(env.sd_manifest, self.SIZE, img_id, vol_id,
  vol_format=vol_fmt)
 yield env.sd_manifest.produceVolume(img_id, vol_id)
diff --git a/tests/storage_sdm_copy_data_test.py 
b/tests/storage_sdm_copy_data_test.py
index c0ec533..e770b7f 100644
--- a/tests/storage_sdm_copy_data_test.py
+++ b/tests/storage_sdm_copy_data_test.py
@@ -19,7 +19,6 @@
 #
 from __future__ import absolute_import
 
-import uuid
 from contextlib import contextmanager
 
 from fakelib import FakeScheduler
@@ -28,6 +27,7 @@
 from storagetestlib import fake_env
 from storagetestlib import make_qemu_chain, write_qemu_chain, verify_qemu_chain
 from storagetestlib import ChainVerificationError
+from testlib import make_uuid
 from testlib import VdsmTestCase, expandPermutations, permutations
 from testlib import wait_for_job
 
@@ -130,7 +130,7 @@
 def test_intra_domain_copy(self, env_type, src_fmt, dst_fmt):
 src_fmt = sc.name2type(src_fmt)
 dst_fmt = sc.name2type(dst_fmt)
-job_id = str(uuid.uuid4())
+job_id = make_uuid()
 
 with self.get_vols(env_type, src_fmt, dst_fmt) as (src_chain,
dst_chain):
@@ -173,7 +173,7 @@
  dst_chain):
 write_qemu_chain(src_chain)
 for index in copy_seq:
-job_id = str(uuid.uuid4())
+job_id = make_uuid()
 src_vol = src_chain[index]
 dst_vol = dst_chain[index]
 source = dict(endpoint_type='div', sd_id=src_vol.sdUUID,
@@ -193,7 +193,7 @@
 volume format may be set incorrectly due to an old bug.  Check that the
 workaround we have in place allows the copy to proceed without error.
 """
-job_id = str(uuid.uuid4())
+job_id = make_uuid()
 vm_conf_size = workarounds.VM_CONF_SIZE_BLK * sc.BLOCK_SIZE
 vm_conf_data = "VM Configuration"
 
diff --git a/tests/storage_sdm_create_volume_test.py 
b/tests/storage_sdm_create_volume_test.py
index ebc2db0..f62f325 100644
--- a/tests/storage_sdm_create_volume_test.py
+++ b/tests/storage_sdm_create_volume_test.py
@@ -20,10 +20,10 @@
 from __future__ import absolute_import
 
 from contextlib import contextmanager
-import uuid
 
 from monkeypatch import MonkeyPatchScope
 from storagefakelib import FakeResourceManager
+from testlib import make_uuid
 from testlib import VdsmTestCase, recorded, expandPermutations, permutations
 from testlib import wait_for_job
 
@@ -84,17 

Change in vdsm[master]: sdm: Introduce new SDM.merge verb

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: sdm: Introduce new SDM.merge verb
..


Patch Set 10: -Code-Review

We need to see the implementation first.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I96d57a5b9f21153ce1de2cd5619c7f9f78bbe75b
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Increment generation id when completing operation

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Increment generation id when completing operation
..


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/64487/2/tests/storage_volume_test.py
File tests/storage_volume_test.py:

Line 170: vol.setMetaParam(sc.GENERATION, generation)
Line 171: self.assertEqual(generation, vol.getInfo()['generation'])
Line 172: 
Line 173: @permutations((
Line 174: (sc.MAX_GENERATION, 0),
> I think we need only this test for wrapping, previous test verified increas
Keeping first two as they catch off by one errors.
Line 175: (sc.MAX_GENERATION - 1, sc.MAX_GENERATION),
Line 176: (0, 1),
Line 177: (1, 2),
Line 178: ))


https://gerrit.ovirt.org/#/c/64487/2/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 55: # domains) we do not need to use the image dir reference anymore.
Line 56: return volUUID
Line 57: 
Line 58: 
Line 59: def next_generation(current_generation):
> Do we need it public?
no.  Making private.
Line 60: # Increment a generation value and wrap to 0 after MAX_GENERATION
Line 61: return (current_generation + 1) % (sc.MAX_GENERATION + 1)
Line 62: 
Line 63: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9a62289378b11c3b2f2f520c894336fc89c1fdc
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Validate generation id in volume.operation context

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Validate generation id in volume.operation context
..


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/64486/1/tests/storage_volume_test.py
File tests/storage_volume_test.py:

Line 134: vol_id = gen_uuid()
Line 135: generation = 100
Line 136: 
Line 137: with fake_env('file') as env:
Line 138: env.make_volume(MB, img_id, vol_id)
> Sure, most test do not care about generation, but they may need other metad
Let's keep it as-is for now.
Line 139: vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 140: vol.setMetaParam(sc.GENERATION, 100)
Line 141: with vol.operation(generation):
Line 142: pass


https://gerrit.ovirt.org/#/c/64486/2/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 507: marked ILLEGAL prior to the first modification of data and 
subsequently
Line 508: marked LEGAL again once the operation has completed.  Thus, 
if an
Line 509: interruption occurs the volume will remain in an ILLEGAL 
state.
Line 510: 
Line 511: If generation is provided we check that the volume's 
generation matches
> Missing period at the end.
Done
Line 512: """
Line 513: actual_gen = self.getMetaParam(sc.GENERATION)
Line 514: if requested_gen is not None and actual_gen != requested_gen:
Line 515: raise se.GenerationMismatch(requested_gen, actual_gen)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77d9c6cb46053ab32c59c77599b7c1366e1c8196
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Report generation id in volume.getInfo

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Report generation id in volume.getInfo
..


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/64485/2/tests/storage_volume_test.py
File tests/storage_volume_test.py:

Line 138: vol = env.sd_manifest.produceVolume(img_id, vol_id)
Line 139: vol.getLeaseStatus = lambda: 'unused'
Line 140: vol.setMetaParam(sc.GENERATION, generation)
Line 141: self.assertEqual(generation, vol.getInfo()['generation'])
Line 142: 
> Do we need a test for volume without generation?
I'll permutate this test so we can check that case explicitly.
Line 143: 
Line 144: class CountedInstanceMethod(object):
Line 145: def __init__(self, method):
Line 146: self._method = method


https://gerrit.ovirt.org/#/c/64485/2/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 215
Line 216
Line 217
Line 218
Line 219
> This always uses VolumeMetadata, right?
correct.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a6bb44c5f789acf3c63953f4b87c72585becc1
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 7: Continuous-Integration+1

CI is broken, we will ignore it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Remove volume run link after live merge

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: Live Merge: Remove volume run link after live merge
..


Patch Set 13: Code-Review+1

Waiting for more reviews.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 7: Code-Review+1

Waiting for more reviews.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/64484/3/lib/vdsm/storage/constants.py
File lib/vdsm/storage/constants.py:

Line 165: # ascii values, but limit non-ascii values, which are encoded by 
engine
Line 166: # using 4 bytes per character.
Line 167: DESCRIPTION_SIZE = 210
Line 168: DEFAULT_GENERATION = 0
Line 169: MAX_GENERATION = 2 ** 63 - 1
> Yes, better - don't forget to update the worst case example and the number 
Done
Line 170: 
Line 171: # Block volume metadata tags
Line 172: TAG_PREFIX_MD = "MD_"
Line 173: TAG_PREFIX_MDNUMBLKS = "MS_"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 2:

(4 comments)

https://gerrit.ovirt.org/#/c/64484/2//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2016-09-27 15:19:01 -0400
Line 6: 
Line 7: storage: Add support for generation to VolumeMetadata
Line 8: 
Line 9: We would like to add support for a new generation id key in the volume
> remove id.
Done
Line 10: metadata.  GEN is a monotonically increasing integer that can be used
Line 11: to determine whether certain operations have been completed on a volume
Line 12: and to prevent two hosts from sequentially performing the same 
operation
Line 13: in the event of double scheduling.


https://gerrit.ovirt.org/#/c/64484/2/lib/vdsm/storage/constants.py
File lib/vdsm/storage/constants.py:

Line 161
Line 162
Line 163
Line 164
Line 165
> Lets separate DESCRIPTION_SIZE from the generation constants, and maybe add
Done


Line 124: IMAGE = "IMAGE"
Line 125: DESCRIPTION = "DESCRIPTION"
Line 126: LEGALITY = "LEGALITY"
Line 127: MTIME = "MTIME"
Line 128: GENERATION = "GEN"
> Lets add a comment:
Done
Line 129: POOL = MDK_POOLS  # Deprecated
Line 130: 
Line 131: # In block storage, metadata size is limited to BLOCK_SIZE (512), to
Line 132: # ensure that metadata is written atomically. This is big enough for 
the


https://gerrit.ovirt.org/#/c/64484/2/tests/storage_volume_metadata_test.py
File tests/storage_volume_metadata_test.py:

Line 188: 
Line 189: def test_generation_default(self):
Line 190: data = make_md_dict()
Line 191: lines = make_lines(generation=sc.DEFAULT_GENERATION + 1, 
**data)
Line 192: lines.remove("{}={}".format(sc.GENERATION, 
sc.DEFAULT_GENERATION + 1))
> Why not:
Simplified.
Line 193: md = volume.VolumeMetadata.from_lines(lines)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/64484/3/lib/vdsm/storage/constants.py
File lib/vdsm/storage/constants.py:

Line 165: # ascii values, but limit non-ascii values, which are encoded by 
engine
Line 166: # using 4 bytes per character.
Line 167: DESCRIPTION_SIZE = 210
Line 168: DEFAULT_GENERATION = 0
Line 169: MAX_GENERATION = 2 ** 63 - 1
> Let's reduce this to 999
Yes, better - don't forget to update the worst case example and the number 
above.
Line 170: 
Line 171: # Block volume metadata tags
Line 172: TAG_PREFIX_MD = "MD_"
Line 173: TAG_PREFIX_MDNUMBLKS = "MS_"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report vg metadata device for block sd

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: StorageDomain.getInfo - report vg metadata device for block sd
..


Patch Set 7:

(5 comments)

https://gerrit.ovirt.org/#/c/64433/7/lib/vdsm/storage/exception.py
File lib/vdsm/storage/exception.py:

Line 1558: 
Line 1559: 
Line 1560: class UnexpectedVolumeGroupMetadata(StorageException):
Line 1561: def __init__(self, err):
Line 1562: self.value = "err=%s" % err
reason=?
Line 1563: code = 616
Line 1564: message = "Volume Group metadata isn't as expected"
Line 1565: 
Line 1566: 


https://gerrit.ovirt.org/#/c/64433/7/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 451: """
Line 452: dev, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
Line 453: return os.path.basename(dev)
Line 454: 
Line 455: def getVgMetadataDevice(self):
Add docstring similar to getMetadataLVDevice (see my comment in the previous 
patch).
Line 456: return os.path.basename(lvm.getVgMetadataPv(self.sdUUID))
Line 457: 
Line 458: @classmethod
Line 459: def getMetaDataMapping(cls, vgName, oldMapping={}):


https://gerrit.ovirt.org/#/c/64433/7/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 588: return vgs.values()
Line 589: 
Line 590: def getVgPvs(self, vgName):
Line 591: """
Line 592: Returns the pvs of the given vg
... reloading pvs only once if some of the pvs are missing or stale.

Hopefully we can use this elsewhere instead of calling multiple getPv calls.

I wonder if this should be be renamed to getPvs and moved next to getPv and 
getAllPvs - The common behavior is method returning pv objects.
Line 593: """
Line 594: stalepvs = []
Line 595: vgpvs = []
Line 596: vg = self.getVg(vgName)


Line 594: stalepvs = []
Line 595: vgpvs = []
Line 596: vg = self.getVg(vgName)
Line 597: for pvName in vg.pv_name:
Line 598: pv = self._pvs.get(pvName)
One issue here, you are accessing a dict which may be modified by other theads 
on both spm/hsm. The methods in the cache accessing this dict typically take 
the lock (see reload pvs).

Not all code is thread safe here, but when adding new code we better be thread 
safe.

Can be easily fixed by locking the part getting stale pvs:

with self._lock:
# find vgpvs and stalepvs here
  
# reload outside of the lock (otherwise it will deadlock)
Line 599: if pv is None or isinstance(pv, Stub):
Line 600: stalepvs.append(pvName)
Line 601: else:
Line 602: vgpvs.append(pv)


Line 1341: pvs = _lvminfo.getVgPvs(vgName)
Line 1342: mdpvs = [pv for pv in pvs if not isinstance(pv, Stub) and 
_isMetadataPv(pv)]
Line 1343: if len(mdpvs) != 1:
Line 1344: se.UnexpectedVolumeGroupMetadata("Expected one metadata pv 
in vg: %s, "
Line 1345:  "vg pvs: %s" % (vgName, 
pvs))
Errors looks fine, raising it would be even better :-)
Line 1346: return mdpvs[0]
Line 1347: 
Line 1348: def _isMetadataPv(pv):
Line 1349: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/64484/1/lib/vdsm/storage/volumemetadata.py
File lib/vdsm/storage/volumemetadata.py:

Line 148: constants.DESCRIPTION: self.description,
Line 149: constants.PUUID: self.puuid,
Line 150: constants.MTIME: str(self.mtime),
Line 151: constants.LEGALITY: self.legality,
Line 152: constants.GENERATION: self.generation,
> If you think it is less work to check old code, why not.
Just checked 3.6 code.  We don't try to filter out unknown keys in getMetadata. 
 We just read lines, split each line around '=' and populate a dict with the 
pairs.  This is verified to my satisfaction.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 12:

(2 comments)

https://gerrit.ovirt.org/#/c/63027/12/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 5653: type: *UUID
Line 5654: 
Line 5655: -   defaultvalue: null
Line 5656: description: The GUID of the first device containing the 
domain
Line 5657: metadata lv for block storage domains (optional)
Same as next patch, this is available on block storage, and never available on 
file storage.
Line 5658: name: metadataDevice
Line 5659: type: string
Line 5660: type: object
Line 5661: 


https://gerrit.ovirt.org/#/c/63027/12/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 446: lvm.extendLV(self.sdUUID, volumeUUID, size)  # , 
isShuttingDown)
Line 447: 
Line 448: def getMetadataLVDevice(self):
Line 449: """
Line 450: Returns the first device of the domain metadata lv
Returns the devices the metadata lv was created on.

And add a note that this may not be the same device containing the vg metadata.
Line 451: """
Line 452: dev, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
Line 453: return os.path.basename(dev)
Line 454: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add gen_uuid test helper

[alitke]

Adam Litke has posted comments on this change.

Change subject: tests: Add gen_uuid test helper
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/64542/1/tests/testlib.py
File tests/testlib.py:

Line 590: while job.active:
Line 591: time.sleep(1)
Line 592: 
Line 593: 
Line 594: def gen_uuid():
> uuid_string? uuid_str? make_uuid?
make_uuid
Line 595: """
Line 596: Return a new UUID in the format used for all vdsm APIs that 
accept UUIDs.
Line 597: """


Line 592: 
Line 593: 
Line 594: def gen_uuid():
Line 595: """
Line 596: Return a new UUID in the format used for all vdsm APIs that 
accept UUIDs.
> Return a new UUID version 4 string...
Done
Line 597: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report vg metadata device for block sd

[automation]

gerrit-hooks has posted comments on this change.

Change subject: StorageDomain.getInfo - report vg metadata device for block sd
..


Patch Set 7:

* 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/64433
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[automation]

gerrit-hooks has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 12:

* 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/63027
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[automation]

gerrit-hooks has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 11:

* 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/63027
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report vg metadata device for block sd

[automation]

gerrit-hooks has posted comments on this change.

Change subject: StorageDomain.getInfo - report vg metadata device for block sd
..


Patch Set 6:

* 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/64433
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Validate generation id in volume.operation context

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Validate generation id in volume.operation context
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/64486/2/tests/storage_volume_test.py
File tests/storage_volume_test.py:

Line 142: pass
Line 143: 
Line 144: @permutations(((100, 99), (100, 101)))
Line 145: def test_operation_invalid_generation_raises(self, 
actual_generation,
Line 146:  
requested_generation):
> test_operation_generation_mismatch?
Ignore, its fine.
Line 147: img_id = gen_uuid()
Line 148: vol_id = gen_uuid()
Line 149: 
Line 150: with fake_env('file') as env:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77d9c6cb46053ab32c59c77599b7c1366e1c8196
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Report generation id in volume.getInfo

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storage: Report generation id in volume.getInfo
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/64485/2/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 205: "image": self.getImage(),
Line 206: "ctime": meta.get(sc.CTIME, ""),
Line 207: "mtime": "0",
Line 208: "legality": meta.get(sc.LEGALITY, ""),
Line 209: "generation": meta.get(sc.GENERATION, 
sc.DEFAULT_GENERATION)
> This should work, but since we always add a default generation in VolumeMet
Ignore this.
Line 210: }
Line 211: 
Line 212: def getInfo(self):
Line 213: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a6bb44c5f789acf3c63953f4b87c72585becc1
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Add support for generation to VolumeMetadata

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: Add support for generation to VolumeMetadata
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/64484/3/lib/vdsm/storage/constants.py
File lib/vdsm/storage/constants.py:

Line 165: # ascii values, but limit non-ascii values, which are encoded by 
engine
Line 166: # using 4 bytes per character.
Line 167: DESCRIPTION_SIZE = 210
Line 168: DEFAULT_GENERATION = 0
Line 169: MAX_GENERATION = 2 ** 63 - 1
Let's reduce this to 999
Line 170: 
Line 171: # Block volume metadata tags
Line 172: TAG_PREFIX_MD = "MD_"
Line 173: TAG_PREFIX_MDNUMBLKS = "MS_"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb71e1fc78f6c1e411e725b26c48411ffd04d0b6
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add gen_uuid test helper

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Add gen_uuid test helper
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/64542/1/tests/testlib.py
File tests/testlib.py:

Line 590: while job.active:
Line 591: time.sleep(1)
Line 592: 
Line 593: 
Line 594: def gen_uuid():
uuid_string? uuid_str? make_uuid?
Line 595: """
Line 596: Return a new UUID in the format used for all vdsm APIs that 
accept UUIDs.
Line 597: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I622928911c3f07739fd61f61a58cee2e692c7eeb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[automation]

gerrit-hooks has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 10:

* 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/63027
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd

[laravot]

Liron Aravot has posted comments on this change.

Change subject: StorageDomain.getInfo - report lvm metadata device for block sd
..


Patch Set 5:

(8 comments)

https://gerrit.ovirt.org/#/c/64433/5/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 5659: type: string
Line 5660: 
Line 5661: -   defaultvalue: null
Line 5662: description: The GUID of the device containing the 
domain lvm metadata (optional)
Line 5663: name: lvmmetadatadevice
> vgMetadataDevice
Done
Line 5664: type: string
Line 5665: type: object
Line 5666: 
Line 5667: StorageDomainStatus: 


https://gerrit.ovirt.org/#/c/64433/5/lib/vdsm/storage/exception.py
File lib/vdsm/storage/exception.py:

Line 1558: 
Line 1559: 
Line 1560: class CannotGetVGMetadataPV(StorageException):
Line 1561: def __init__(self, vgname):
Line 1562: self.value = "vgname=%s" % (vgname)
> Expected one pv with pv metadata, vgname=%s, pvs=%s
As this error is intended to be "general" - the code that raises will pass the 
exact error.
Line 1563: code = 616
Line 1564: message = "Cannot get Volume Group metadata PV"
Line 1565: 
Line 1566: 


https://gerrit.ovirt.org/#/c/64433/5/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 1109
Line 1110
Line 
Line 1112
Line 1113
> Take this
Done


Line 1116: <<< HEAD
Line 1117: info['metadatadevice'] = self._manifest.getMetadataLVDevice()
Line 1118: ===
Line 1119: info['metadatadevice'] = 
self._manifest.getMetadataLVFirstDevice()
Line 1120: info['lvmmetadatadevice'] = 
self._manifest.getVgMetadataDevice()
> Use mixed  case name, already used in this verb response.
Done
Line 1121: >>> 7da51c9... StorageDomain.getInfo - report lvm metadata 
device for block sd
Line 1122: return info
Line 1123: 
Line 1124: def getStats(self):


https://gerrit.ovirt.org/#/c/64433/5/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 585
Line 586
Line 587
Line 588
Line 589
> Let put the new method here, more specific.
Done


Line 502: def _invalidatevgs(self, vgNames):
Line 503: vgNames = _normalizeargs(vgNames)
Line 504: with self._lock:
Line 505: for vgName in vgNames:
Line 506: self._vgs[vgName] = aStub(vgName, True)
> Not related.
Done
Line 507: 
Line 508: def _invalidateAllVgs(self):
Line 509: with self._lock:
Line 510: self._stalevg = True


Line 555: reloaded = self._reloadpvs(stalepvs)
Line 556: pvs.update(reloaded)
Line 557: return pvs.values()
Line 558: 
Line 559: def getVgPvs(self, vgName):
> Add docsting explaining this new function compared with getPv.
Done
Line 560: stalepvs = []
Line 561: vgpvs = []
Line 562: vg = self.getVg(vgName)
Line 563: for pvName in vg.pv_name:


Line 1337: def getVgMetadataPv(vgName):
Line 1338: pvs = _lvminfo.getVgPvs(vgName)
Line 1339: mdpvs = [pv for pv in pvs if not isinstance(pv, Stub) and 
_isMetadataPv(pv)]
Line 1340: if len(mdpvs) != 1:
Line 1341: se.CannotGetVGMetadataPV(vgName)
> "Expected one metadata pv in vg: %s, found: %s" % (vg.name, pvs)
Done
Line 1342: return mdpvs[0]
Line 1343: 
Line 1344: def _isMetadataPv(pv):
Line 1345: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[laravot]

Liron Aravot has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 9:

(2 comments)

https://gerrit.ovirt.org/#/c/63027/9/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 5653: type: *UUID
Line 5654: 
Line 5655: -   defaultvalue: null
Line 5656: description: The GUID of the first device containing the 
domain
Line 5657: metadata lv (optional)
> Always available only on block storage.
Done
Line 5658: name: metadatadevice
Line 5659: type: string
Line 5660: type: object
Line 5661: 


Line 5654: 
Line 5655: -   defaultvalue: null
Line 5656: description: The GUID of the first device containing the 
domain
Line 5657: metadata lv (optional)
Line 5658: name: metadatadevice
> metadataDevice
Done
Line 5659: type: string
Line 5660: type: object
Line 5661: 
Line 5662: StorageDomainStatus: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: implementing SDM.move_domain_device

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: implementing SDM.move_domain_device
..


Patch Set 8:

(2 comments)

https://gerrit.ovirt.org/#/c/63270/8/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 453: return os.path.basename(lvm.getVgMetadataPv(self.sdUUID))
Line 454: 
Line 455: def validateNotFirstMetadataLVDevice(self, guid):
Line 456: if self.getMetadataLVFirstDevice() == guid:
Line 457: raise se.CouldNotPerformOnMetadataLVFirstPV(guid)
Use same exception - maybe ForibddenPhisicalVolumeOperation

With text like "PV %s is used as metadata lv pv"
Line 458: 
Line 459: def validateNotVgMetadataDevice(self, guid):
Line 460: if self.getVgMetadataDevice() == guid:
Line 461: raise se.CouldNotPerformOnVgMetadataPV(guid)


Line 457: raise se.CouldNotPerformOnMetadataLVFirstPV(guid)
Line 458: 
Line 459: def validateNotVgMetadataDevice(self, guid):
Line 460: if self.getVgMetadataDevice() == guid:
Line 461: raise se.CouldNotPerformOnVgMetadataPV(guid)
Same with text for this case.
Line 462: 
Line 463: @classmethod
Line 464: def getMetaDataMapping(cls, vgName, oldMapping={}):
Line 465: firstDev, firstExtent = lvm.getFirstExt(vgName, sd.METADATA)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: StorageDomain.getInfo - report lvm metadata device for block sd
..


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/64433/5/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 5659: type: string
Line 5660: 
Line 5661: -   defaultvalue: null
Line 5662: description: The GUID of the device containing the 
domain lvm metadata (optional)
Line 5663: name: lvmmetadatadevice
> Update names
vgMetadataDevice
Line 5664: type: string
Line 5665: type: object
Line 5666: 
Line 5667: StorageDomainStatus: 


https://gerrit.ovirt.org/#/c/64433/5/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 1116: <<< HEAD
Line 1117: info['metadatadevice'] = self._manifest.getMetadataLVDevice()
Line 1118: ===
Line 1119: info['metadatadevice'] = 
self._manifest.getMetadataLVFirstDevice()
Line 1120: info['lvmmetadatadevice'] = 
self._manifest.getVgMetadataDevice()
> And this, but
Use mixed  case name, already used in this verb response.
Line 1121: >>> 7da51c9... StorageDomain.getInfo - report lvm metadata 
device for block sd
Line 1122: return info
Line 1123: 
Line 1124: def getStats(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 9:

(1 comment)

https://gerrit.ovirt.org/#/c/63027/9/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 5654: 
Line 5655: -   defaultvalue: null
Line 5656: description: The GUID of the first device containing the 
domain
Line 5657: metadata lv (optional)
Line 5658: name: metadatadevice
metadataDevice
Line 5659: type: string
Line 5660: type: object
Line 5661: 
Line 5662: StorageDomainStatus: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: StorageDomain.getInfo - report lvm metadata device for block sd
..


Patch Set 5:

(8 comments)

https://gerrit.ovirt.org/#/c/64433/5/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 5659: type: string
Line 5660: 
Line 5661: -   defaultvalue: null
Line 5662: description: The GUID of the device containing the 
domain lvm metadata (optional)
Line 5663: name: lvmmetadatadevice
Update names
Line 5664: type: string
Line 5665: type: object
Line 5666: 
Line 5667: StorageDomainStatus: 


https://gerrit.ovirt.org/#/c/64433/5/lib/vdsm/storage/exception.py
File lib/vdsm/storage/exception.py:

Line 1558: 
Line 1559: 
Line 1560: class CannotGetVGMetadataPV(StorageException):
Line 1561: def __init__(self, vgname):
Line 1562: self.value = "vgname=%s" % (vgname)
Expected one pv with pv metadata, vgname=%s, pvs=%s
Line 1563: code = 616
Line 1564: message = "Cannot get Volume Group metadata PV"
Line 1565: 
Line 1566: 


https://gerrit.ovirt.org/#/c/64433/5/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 1109
Line 1110
Line 
Line 1112
Line 1113
Take this


Line 1116: <<< HEAD
Line 1117: info['metadatadevice'] = self._manifest.getMetadataLVDevice()
Line 1118: ===
Line 1119: info['metadatadevice'] = 
self._manifest.getMetadataLVFirstDevice()
Line 1120: info['lvmmetadatadevice'] = 
self._manifest.getVgMetadataDevice()
And this, but
- use vgmetadatadevice
- use separate words metadata_device, vg_metadata_device
Line 1121: >>> 7da51c9... StorageDomain.getInfo - report lvm metadata 
device for block sd
Line 1122: return info
Line 1123: 
Line 1124: def getStats(self):


https://gerrit.ovirt.org/#/c/64433/5/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 585
Line 586
Line 587
Line 588
Line 589
Let put the new method here, more specific.


Line 502: def _invalidatevgs(self, vgNames):
Line 503: vgNames = _normalizeargs(vgNames)
Line 504: with self._lock:
Line 505: for vgName in vgNames:
Line 506: self._vgs[vgName] = aStub(vgName, True)
Not related.
Line 507: 
Line 508: def _invalidateAllVgs(self):
Line 509: with self._lock:
Line 510: self._stalevg = True


Line 555: reloaded = self._reloadpvs(stalepvs)
Line 556: pvs.update(reloaded)
Line 557: return pvs.values()
Line 558: 
Line 559: def getVgPvs(self, vgName):
Add docsting explaining this new function compared with getPv.
Line 560: stalepvs = []
Line 561: vgpvs = []
Line 562: vg = self.getVg(vgName)
Line 563: for pvName in vg.pv_name:


Line 1337: def getVgMetadataPv(vgName):
Line 1338: pvs = _lvminfo.getVgPvs(vgName)
Line 1339: mdpvs = [pv for pv in pvs if not isinstance(pv, Stub) and 
_isMetadataPv(pv)]
Line 1340: if len(mdpvs) != 1:
Line 1341: se.CannotGetVGMetadataPV(vgName)
"Expected one metadata pv in vg: %s, found: %s" % (vg.name, pvs)

Can we use an error name about the issue an not about the operation?

e,g UnexpectedVolumeGroupMetadata?
Line 1342: return mdpvs[0]
Line 1343: 
Line 1344: def _isMetadataPv(pv):
Line 1345: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 9: Code-Review+1

(1 comment)

https://gerrit.ovirt.org/#/c/63027/9/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 5653: type: *UUID
Line 5654: 
Line 5655: -   defaultvalue: null
Line 5656: description: The GUID of the first device containing the 
domain
Line 5657: metadata lv (optional)
Always available only on block storage.
Line 5658: name: metadatadevice
Line 5659: type: string
Line 5660: type: object
Line 5661: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: py3: make network/conf_test pass

[automation]

gerrit-hooks has posted comments on this change.

Change subject: py3: make network/conf_test pass
..


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/64928
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcab2d6f1f88b04d849156915a89c4cc8589
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: py3: fix remaining references to monitor.MonitorError as ite...

[automation]

gerrit-hooks has posted comments on this change.

Change subject: py3: fix remaining references to monitor.MonitorError as 
iterable
..


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/64929
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic991c1a017f09431d4b38f58e529d0a6ec45a896
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: py3: fix remaining references to monitor.MonitorError as ite...

[danken]

Dan Kenigsberg has uploaded a new change for review.

Change subject: py3: fix remaining references to monitor.MonitorError as 
iterable
..

py3: fix remaining references to monitor.MonitorError as iterable

Change-Id: Ic991c1a017f09431d4b38f58e529d0a6ec45a896
Signed-off-by: Dan Kenigsberg 
---
M init/wait_for_ipv4s
M tests/network/nettestlib.py
2 files changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/64929/1

diff --git a/init/wait_for_ipv4s b/init/wait_for_ipv4s
index c1e7f32..ab119de 100755
--- a/init/wait_for_ipv4s
+++ b/init/wait_for_ipv4s
@@ -51,7 +51,7 @@
 if not ipless_devs:
 return
 except monitor.MonitorError as e:
-if e[0] == monitor.E_TIMEOUT:
+if e.args[0] == monitor.E_TIMEOUT:
 sys.stderr.write('IP addresses has not been caught within the '
  'given timeout.\n')
 else:
diff --git a/tests/network/nettestlib.py b/tests/network/nettestlib.py
index bf32ea3..a5fc461 100644
--- a/tests/network/nettestlib.py
+++ b/tests/network/nettestlib.py
@@ -464,7 +464,7 @@
 return
 
 except monitor.MonitorError as e:
-if e[0] == monitor.E_TIMEOUT:
+if e.args[0] == monitor.E_TIMEOUT:
 raise Exception(
 'IPv6 addresses has not been caught within 20sec.\n'
 'Event log: {}\n'.format(logevents))


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic991c1a017f09431d4b38f58e529d0a6ec45a896
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: py3: make network/conf_test pass

[danken]

Dan Kenigsberg has uploaded a new change for review.

Change subject: py3: make network/conf_test pass
..

py3: make network/conf_test pass

re.sub should not mix strings and byte arrays.

Change-Id: Idcab2d6f1f88b04d849156915a89c4cc8589
Signed-off-by: Dan Kenigsberg 
---
M tests/Makefile.am
M tests/network/conf_test.py
2 files changed, 2 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/64928/1

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 132be62..fb3d7c3 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -191,7 +191,6 @@
momTests.py \
monkeypatchTests.py \
mountTests.py \
-   network/conf_test.py \
network/hook_ovs_test.py \
network/iproute2_test.py \
network/ipwrapper_test.py \
diff --git a/tests/network/conf_test.py b/tests/network/conf_test.py
index d94b5e6..c230a3c 100644
--- a/tests/network/conf_test.py
+++ b/tests/network/conf_test.py
@@ -73,8 +73,8 @@
 aXml = ET.tostring(ET.fromstring(a))
 bXml = ET.tostring(ET.fromstring(b))
 
-aXmlNrml = re.sub('>\s*\n\s*<', '><', aXml).strip()
-bXmlNrml = re.sub('>\s*\n\s*<', '><', bXml).strip()
+aXmlNrml = re.sub(b'>\s*\n\s*<', b'><', aXml).strip()
+bXmlNrml = re.sub(b'>\s*\n\s*<', b'><', bXml).strip()
 
 self.assertEqual(aXmlNrml, bXmlNrml, msg)
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idcab2d6f1f88b04d849156915a89c4cc8589
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: GenerationMismatch exception

[alitke]

Adam Litke has posted comments on this change.

Change subject: storage: GenerationMismatch exception
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/64543/1/lib/vdsm/storage/exception.py
File lib/vdsm/storage/exception.py:

Line 1789: 
Line 1790: 
Line 1791: class GenerationMismatch(StorageException):
Line 1792: code = 911
Line 1793: message = "The provided generation does not match the actual 
generation."
> Actual number is 6 messages ending with period, 284 without.
Ok.  Removed.
Line 1794: 
Line 1795: def __init__(self, requested, actual):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2eb9ffb326f343d89fa17af462c3bbfb18697a04
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd

[laravot]

Liron Aravot has posted comments on this change.

Change subject: StorageDomain.getInfo - report lvm metadata device for block sd
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64433/4/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 556: pvs.update(reloaded)
Line 557: return pvs.values()
Line 558: 
Line 559: def getVgPvNames(self, vgName):
Line 560: return self.getVg(vgName).pv_name
> We can work on refactoring this class later if you think such helper is nee
Well, i use this method in the following patch - so i assume i'll return it 
(see line 913 in https://gerrit.ovirt.org/#/c/63270/8/vdsm/storage/lvm.py)
Line 561: 
Line 562: def getVgPvs(self, vgName):
Line 563: stalepvs = []
Line 564: vgpvs = []


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: sdm: Introduce new SDM.merge verb

[frolland]

Freddy Rolland has posted comments on this change.

Change subject: sdm: Introduce new SDM.merge verb
..


Patch Set 10: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I96d57a5b9f21153ce1de2cd5619c7f9f78bbe75b
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv

[automation]

gerrit-hooks has posted comments on this change.

Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..


Patch Set 9:

* update_tracker: OK
* 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/63027
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd

[automation]

gerrit-hooks has posted comments on this change.

Change subject: StorageDomain.getInfo - report lvm metadata device for block sd
..


Patch Set 5:

* update_tracker: OK
* 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/64433
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd

[laravot]

Liron Aravot has posted comments on this change.

Change subject: StorageDomain.getInfo - report lvm metadata device for block sd
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64433/4/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 1343: return pv
Line 1344: except AttributeError:
Line 1345: pass
Line 1346: 
Line 1347: raise se.CannotGetVGMetadataPV(vgName)
Do we want to have a general exception or to use that specific one?
Line 1348: 
Line 1349: 
Line 1350: def _isMetadataPv(pv):
Line 1351: return pv.mda_used_count == '2'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: StorageDomain.getInfo - report lvm metadata device for block sd
..


Patch Set 4:

(2 comments)

https://gerrit.ovirt.org/#/c/64433/4/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 556: pvs.update(reloaded)
Line 557: return pvs.values()
Line 558: 
Line 559: def getVgPvNames(self, vgName):
Line 560: return self.getVg(vgName).pv_name
> I don't agree as this code is already repeated, but Done.
We can work on refactoring this class later if you think such helper is needed, 
lets focus on adding the new feature.
Line 561: 
Line 562: def getVgPvs(self, vgName):
Line 563: stalepvs = []
Line 564: vgpvs = []


Line 563: stalepvs = []
Line 564: vgpvs = []
Line 565: for pvName in self.getVgPvNames(vgName):
Line 566: pv = self._pvs.get(pvName)
Line 567: if pv is None or isinstance(pv, Stub):
> That's the standard way we check the pvs in this file.
OK
Line 568: stalepvs.append(pvName)
Line 569: else:
Line 570: vgpvs.append(pv)
Line 571: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org