vdsm-patches October 2013

vdsm-patches@lists.fedorahosted.org

51 participants
500 discussions

Change in vdsm[master]: jsonrpc: Create the java bindings and fix bugs
by ybronhei＠redhat.com 02 Oct '13

02 Oct '13

Yaniv Bronhaim has posted comments on this change. Change subject: jsonrpc: Create the java bindings and fix bugs ...................................................................... Patch Set 5: (18 comments) .................................................... File lib/yajsonrpc/__init__.py Line 282: if cb is None or cb == eventcb: Line 283: try: Line 284: self._eventcbs.remove(r) Line 285: except ValueError: Line 286: pass report? print r? Line 287: Line 288: def emit(self, client, event, params): Line 289: for r in self._eventcbs[:]: Line 290: cb = r() Line 315: try: Line 316: mobj = json.loads(message) Line 317: isResponse = self._isResponse(mobj) Line 318: except: Line 319: self.log.warning("Problem parsing message from client") better to print the message for debugging, no? Line 320: transport.close() Line 321: del self._clients[transport] Line 322: continue Line 323: Line 333: if v is None: Line 334: v = res Line 335: Line 336: if v != res: Line 337: raise TypeError("batch is mixed") print also v and res for more details Line 338: Line 339: return v Line 340: else: Line 341: return ("result" in obj or "error" in obj) Line 340: else: Line 341: return ("result" in obj or "error" in obj) Line 342: Line 343: def close(self): Line 344: self._inbox.put(None) why not using a flag and stop serving immediately when close is called? in this case you continue to serve until _inbox gets to None object, instead of having boolean for the while loop that 'close' changes to false. Line 345: Line 346: Line 347: class JsonRpcCall(object): Line 348: def __init__(self): .................................................... File lib/yajsonrpc/asyncoreReactor.py Line 100: if sslctx is None: Line 101: try: Line 102: sslctx = sock.get_context() Line 103: except AttributeError: Line 104: pass please report. Line 105: else: Line 106: sslctx = _SSLContextFactory(sslctx) Line 107: Line 108: if sslctx is None: Line 177: self.log.debug("Accepting connection from client " Line 178: "at tcp://%s:%s", addr[0], addr[1]) Line 179: Line 180: clientWrapper = AsyncoreClient(sock, self._reactor, addr) Line 181: self._acceptHandler(self, clientWrapper) no need for the Wrapper suffix Line 182: Line 183: def writable(self, dispatcher): Line 184: return False Line 185: .................................................... File lib/yajsonrpc/betterAsyncore.py Line 105: class AsyncChat(object): Line 106: # these are overridable defaults Line 107: Line 108: ac_in_buffer_size = 4096 Line 109: ac_out_buffer_size = 4096 can be constants? Line 110: Line 111: def __init__(self, impl): Line 112: self._fifoLock = Lock() Line 113: self._impl = impl Line 147: Line 148: # grab some more data from the socket, Line 149: # throw it to the collector method, Line 150: # check for the terminator, Line 151: # if found, transition to the next state. is this comment refer to something around? Line 152: Line 153: def handle_read(self, dispatcher): Line 154: Line 155: try: Line 154: Line 155: try: Line 156: data = dispatcher.recv(self.ac_in_buffer_size) Line 157: except socket.error: Line 158: dispatcher.handle_error() verify that it prints log here, otherwise add such print Line 159: return Line 160: Line 161: self.ac_in_buffer = self.ac_in_buffer + data Line 162: Line 208: else: Line 209: # check for a prefix of the terminator Line 210: index = self._find_prefix_at_end(self.ac_in_buffer, Line 211: terminator) Line 212: if index: index > 0 please. index=-1 gets in also Line 213: if index != lb: Line 214: # we found a prefix, collect up to the prefix Line 215: self.collect_incoming_data( Line 216: self.ac_in_buffer[:-index]) Line 222: self.ac_in_buffer = '' Line 223: Line 224: def _find_prefix_at_end(haystack, needle): Line 225: l = len(needle) - 1 Line 226: while l and not haystack.endswith(needle[:l]): while l > 0 , if it gets below zero by mistake somehow you can end up with infinite loop Line 227: l -= 1 Line 228: return l Line 229: Line 230: def handle_write(self, dispatcher): Line 318: Line 319: try: Line 320: impl.init(self) Line 321: except AttributeError: Line 322: pass better to report before ignoring, no? Line 323: Line 324: def __invoke(self, name, *args, **kwargs): Line 325: if hasattr(self.__impl, name): Line 326: return getattr(self.__impl, name)(self, *args, **kwargs) Line 373: Line 374: def connect(self, addr): Line 375: self.connected = False Line 376: self.connecting = True Line 377: socket = self.socket why not using self.socket directly ? Line 378: socket.setblocking(1) Line 379: socket.connect(addr) Line 380: socket.setblocking(0) Line 381: self.addr = addr .................................................... File lib/yajsonrpc/protonReactor.py Line 62: self._reactor = reactor Line 63: self._connected = False Line 64: Line 65: def setTimeout(self, timeout): Line 66: # TODO maybe a log print that it is being used without appropriate implementation ? Line 67: pass Line 68: Line 69: def closed(self): Line 70: return (self.connector is None or Line 85: Line 86: self.connector = proton.pn_connector(self._reactor._driver, Line 87: host, str(port), None) Line 88: if self.connector is None: Line 89: # TODO: proper exception Why delaying it? add proper exception here. Line 90: raise Exception("Could not create connector") Line 91: Line 92: self.connection = proton.pn_connection() Line 93: proton.pn_connector_set_connection(self.connector, self.connection) .................................................... File tests/jsonRpcTests.py Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: import threading Line 21: import socket Line 22: import logging Any logging config somewhere? Line 23: from Queue import Queue Line 24: from contextlib import contextmanager Line 25: from testValidation import brokentest Line 26: .................................................... File tests/jsonRpcUtils.py Line 57: ("127.0.0.1", port)) Line 58: Line 59: Line 60: REACTOR_CONSTRUCTORS = {"tcp": _tcpServerConstructor, Line 61: "amqp": _protonServerConstructor} why changing the proton key to amqp? and if so, change also _protonServerConstructor to _amqp... Line 62: REACTOR_TYPE_PERMUTATIONS = [[r] for r in REACTOR_CONSTRUCTORS.iterkeys()] Line 63: SSL_OPTIONS = (True, False) Line 64: CONNECTION_PERMUTATIONS = tuple(product(REACTOR_CONSTRUCTORS.iterkeys(), Line 65: SSL_OPTIONS)) Line 59: Line 60: REACTOR_CONSTRUCTORS = {"tcp": _tcpServerConstructor, Line 61: "amqp": _protonServerConstructor} Line 62: REACTOR_TYPE_PERMUTATIONS = [[r] for r in REACTOR_CONSTRUCTORS.iterkeys()] Line 63: SSL_OPTIONS = (True, False) can you add short comment about the usage of this tupple ? what True and False mean?. it isn't being used in this file, so its hard to know by reading it here Line 64: CONNECTION_PERMUTATIONS = tuple(product(REACTOR_CONSTRUCTORS.iterkeys(), Line 65: SSL_OPTIONS)) Line 66: Line 67: CERT_DIR = os.path.abspath(os.path.dirname(__file__)) -- To view, visit http://gerrit.ovirt.org/19497 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If828355b7efe28fe6a2e784069425fefd2f3f25c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com> Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com> Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes

1 0

Change in vdsm[master]: jsonrpc: Create the java bindings and fix bugs
by ybronhei＠redhat.com 02 Oct '13

02 Oct '13

1 0

Change in vdsm[master]: jsonrpc: Create the java bindings and fix bugs
by ybronhei＠redhat.com 02 Oct '13

02 Oct '13

Yaniv Bronhaim has posted comments on this change. Change subject: jsonrpc: Create the java bindings and fix bugs ...................................................................... Patch Set 5: (7 comments) You have tabs all around in the java files, I comment on some of them. .................................................... Commit Message Line 3: AuthorDate: 2013-02-07 15:26:39 -0500 Line 4: Commit: Saggi Mizrahi <smizrahi(a)redhat.com> Line 5: CommitDate: 2013-09-24 18:04:18 +0300 Line 6: Line 7: jsonrpc: Create the java bindings and fix bugs You can detail a bit more, or forward to wiki page. I mean, detail shortly about yajsonrpc missing parts that you extend here, what changes do you do in the scheme and I guess you can add more information about the java part. Line 8: Line 9: Change-Id: If828355b7efe28fe6a2e784069425fefd2f3f25c .................................................... File client/java/vdsm-api/src/main/java/org/ovirt/vdsm/VDSMClientBase.java Line 24: return _transport.closed(); Line 25: } Line 26: Line 27: protected <T> Future<VDSMResponse<T>> _call(RequestBuilder rb, Line 28: TypeReference<T> tr) { tabs Line 29: Line 30: final JsonRpcRequest __req = rb.build(); Line 31: final Future<JsonRpcResponse> __call = _transport.call(__req); Line 32: return new FutureResultWrapper<>( .................................................... File client/java/vdsm-api/src/main/java/org/ovirt/vdsm/VDSMClientPool.java Line 36: Line 37: // package private Line 38: Future<JsonRpcClient> connectToClient(URI uri) { Line 39: final Future<ReactorClient> client = createReactorClient(uri); Line 40: return new FutureResultWrapper<>(client, tabs Line 41: new ResultWrapper<JsonRpcClient, ReactorClient>() { Line 42: @Override Line 43: public JsonRpcClient wrap(ReactorClient value) { Line 44: if (value == null) { .................................................... File client/java/vdsm-api/src/main/java/org/ovirt/vdsm/VDSMObject.java Line 37: } Line 38: Line 39: @Override Line 40: public boolean equals(Object obj) { Line 41: if (this == obj) { tabs and redundant indentation Line 42: return true; Line 43: } Line 44: if ((obj == null) || (obj.getClass() != this.getClass())) { Line 45: return false; .................................................... File client/java/vdsm-json-rpc/src/test/java/org/ovirt/vdsm/jsonrpc/TcpSSLReactorTestHelper.java Line 5: Line 6: public class TcpSSLReactorTestHelper implements ReactorTestHelper { Line 7: Line 8: public static final String KEYSTORE_PATH = Line 9: "/home/saggi/projects/vdsm/tests/jsonrpc-tests.p12"; is this path valid...? Line 10: Line 11: @Override Line 12: public Reactor createReactor(JsonRpcServer server) throws Exception { Line 13: return new NioReactor(server.createSSLContext()); .................................................... File lib/yajsonrpc/__init__.py Line 88 Line 89 Line 90 Line 91 Line 92 why removing it? Line 449: self._workQueue = Queue() Line 450: self._threadFactory = threadFactory Line 451: Line 452: def queueRequest(self, req): Line 453: print "DSAD" debug Line 454: self._workQueue.put_nowait(req) Line 455: print "DSAD" Line 456: Line 457: def _serveRequest(self, ctx, req): -- To view, visit http://gerrit.ovirt.org/19497 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If828355b7efe28fe6a2e784069425fefd2f3f25c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com> Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com> Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com> Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes

1 0

Change in vdsm[master]: Text: add 'dstqemu' to vds client migration command description
by mtayer＠redhat.com 02 Oct '13

02 Oct '13

mooli tayer has uploaded a new change for review. Change subject: Text: add 'dstqemu' to vds client migration command description ...................................................................... Text: add 'dstqemu' to vds client migration command description Change-Id: Ib160ef79f842fb2dc75cfb229f281806bc801464 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1007686 Signed-off-by: Mooli Tayer <mtayer(a)redhat.com> --- M client/vdsClient.py 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/77/19277/1 diff --git a/client/vdsClient.py b/client/vdsClient.py index 590b1e7..bddfaa1 100644 --- a/client/vdsClient.py +++ b/client/vdsClient.py @@ -1957,8 +1957,8 @@ ' name:value,name:value' )), 'migrate': (serv.do_migrate, - ('vmId=<id> method=<offline|online> src=<host:[port]> ' - 'dst=<host:[port]>', + ('vmId=<id> method=<offline|online> src=<host[:port]> ' + 'dst=<host[:port]> dstqemu=<host[:port]>', 'Migrate a desktop from src machine to dst host using ' 'the specified ports' )), -- To view, visit http://gerrit.ovirt.org/19277 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib160ef79f842fb2dc75cfb229f281806bc801464 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer <mtayer(a)redhat.com>

5 14

Change in vdsm[master]: readme: how to configure libvirt after manual installation
by Nir Soffer 02 Oct '13

02 Oct '13

Nir Soffer has uploaded a new change for review. Change subject: readme: how to configure libvirt after manual installation ...................................................................... readme: how to configure libvirt after manual installation When vdsm is installed automatically, libvirt is configured during the bootstap flow. However, when installing vdsm manually the user has configure and restart libvirt using vdsm-tool. Change-Id: I208a752993c0e714a065026155da1f7efa29eeeb Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M README 1 file changed, 9 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/61/19761/1 diff --git a/README b/README index e649da5..7ddce56 100644 --- a/README +++ b/README @@ -25,9 +25,17 @@ make sudo make install +Before starting the VDSM service in the first time, you must configure and +restart libvirt service: + + vdsm-tool libvirt-configure + vdsm-tool libvirt-configure-services-restart + +Packaging +========= + The 'vdsm.spec' file demonstrates how to distribute VDSM as an RPM package. - Getting Help ============ -- To view, visit http://gerrit.ovirt.org/19761 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I208a752993c0e714a065026155da1f7efa29eeeb Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

3 4

Change in vdsm[master]: Fix tokenizing of unsupported data in vdsmapi-schema.json
by Nir Soffer 01 Oct '13

01 Oct '13

Nir Soffer has uploaded a new change for review. Change subject: Fix tokenizing of unsupported data in vdsmapi-schema.json ...................................................................... Fix tokenizing of unsupported data in vdsmapi-schema.json vdmapi tokenizer got into an endless loop when unsupported token was found. Example input trigering this are invalid tokens such as unquoted string, or valid JSON tokens like null, true and false, which are not supported by the parser. Now the tokenizer raise ValueError with the offending data. This should be good enough for developers usage. New unittests cover tokenizing both valid, invalid and unspported tokens. Change-Id: I7d29686d3189fd9e64e26c56dcd9da370f855406 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M tests/schemaTests.py M vdsm_api/vdsmapi.py 2 files changed, 70 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/19697/1 diff --git a/tests/schemaTests.py b/tests/schemaTests.py index ae82b9f..9b709ae 100644 --- a/tests/schemaTests.py +++ b/tests/schemaTests.py @@ -22,5 +22,73 @@ class SchemaTest(TestCaseBase): + def testSchemaParse(self): self.assertTrue(isinstance(vdsmapi.get_api(), dict)) + + ## Supported JSON syntax + + def testTokenizeEmpty(self): + tokens = list(vdsmapi.tokenize('')) + self.assertEqual(tokens, []) + + def testTokenizeString(self): + tokens = list(vdsmapi.tokenize("'string'")) + self.assertEqual(tokens, ['string']) + + def testTokenizeStringWithWhitespace(self): + tokens = list(vdsmapi.tokenize("'s1 s2'")) + self.assertEqual(tokens, ['s1 s2']) + + def testTokenizeStringEmpty(self): + tokens = list(vdsmapi.tokenize("''")) + self.assertEqual(tokens, ['']) + + def testTokenizeArray(self): + tokens = list(vdsmapi.tokenize("['i1', 'i2']")) + self.assertEqual(tokens, ['[', 'i1', ',', 'i2', ']']) + + def testTokenizeArrayEmpty(self): + tokens = list(vdsmapi.tokenize("[]")) + self.assertEqual(tokens, ['[', ']']) + + def testTokenizeObject(self): + tokens = list(vdsmapi.tokenize("{'a': 'b', 'c': 'd'}")) + self.assertEqual(tokens, ['{', 'a', ':', 'b', ',', 'c', ':', 'd', '}']) + + def testTokenizeObjectEmpty(self): + tokens = list(vdsmapi.tokenize("{}")) + self.assertEqual(tokens, ['{', '}']) + + def testTokenizeMixed(self): + tokens = list(vdsmapi.tokenize("{'a': {'b': ['c']}}")) + self.assertEqual(tokens, ['{', 'a', ':', '{', 'b', ':', '[', 'c', ']', + '}', '}']) + + def testTokenizeSkipWhitespaceBetweenTokens(self): + tokens = list(vdsmapi.tokenize(" { 'a': \n 'b' , 'c'\n\n : 'd' } \n")) + self.assertEqual(tokens, ['{', 'a', ':', 'b', ',', 'c', ':', 'd', '}']) + + ## Unsupported JSON syntax + + def testTokenizeRaiseOnNumber(self): + generator = vdsmapi.tokenize("1") + self.assertRaises(ValueError, list, generator) + + def testTokenizeRaiseOnTrue(self): + generator = vdsmapi.tokenize("true") + self.assertRaises(ValueError, list, generator) + + def testTokenizeRaiseOnFalse(self): + generator = vdsmapi.tokenize("false") + self.assertRaises(ValueError, list, generator) + + def testTokenizeRaiseOnNull(self): + generator = vdsmapi.tokenize("null") + self.assertRaises(ValueError, list, generator) + + ## Invalid JSON + + def testTokenizeRaiseOnInvalidData(self): + generator = vdsmapi.tokenize("{'a': invalid, 'b': 'c'}") + self.assertRaises(ValueError, list, generator) diff --git a/vdsm_api/vdsmapi.py b/vdsm_api/vdsmapi.py index 4747dcb..db29c13 100644 --- a/vdsm_api/vdsmapi.py +++ b/vdsm_api/vdsmapi.py @@ -49,6 +49,8 @@ data = data[1:] data = data[1:] yield string + else: + raise ValueError('Invalid data: %r' % data) def parse(tokens): -- To view, visit http://gerrit.ovirt.org/19697 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7d29686d3189fd9e64e26c56dcd9da370f855406 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

5 9

Change in vdsm[master]: Alphabetize test modules list that has gone out of order.
by mtayer＠redhat.com 01 Oct '13

01 Oct '13

mooli tayer has uploaded a new change for review. Change subject: Alphabetize test modules list that has gone out of order. ...................................................................... Alphabetize test modules list that has gone out of order. Change-Id: I9f9a8bdc1a16a172d8a3cabb1b7005accb32ced6 Signed-off-by: Mooli Tayer <mtayer(a)redhat.com> --- M tests/Makefile.am 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/19512/1 diff --git a/tests/Makefile.am b/tests/Makefile.am index 30bca46..ad8c6cf 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -36,12 +36,8 @@ guestIFTests.py \ hooksTests.py \ ipwrapperTests.py \ - netconfTests.py \ - netmodelsTests.py \ jsonRpcTests.py \ jsonRpcUtils.py \ - vmTests.py \ - vmTestsData.py \ libvirtconnectionTests.py \ lsblkTests.py \ lvmTests.py \ @@ -50,7 +46,9 @@ miscTests.py \ mkimageTests.py \ mountTests.py \ + netconfTests.py \ netinfoTests.py \ + netmodelsTests.py \ outOfProcessTests.py \ parted_utils_tests.py \ permutationTests.py \ @@ -63,6 +61,8 @@ tcTests.py \ utilsTests.py \ vdsClientTests.py \ + vmTestsData.py \ + vmTests.py \ volumeTests.py \ zombieReaperTests.py \ $(NULL) -- To view, visit http://gerrit.ovirt.org/19512 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9f9a8bdc1a16a172d8a3cabb1b7005accb32ced6 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer <mtayer(a)redhat.com>

5 9

Change in vdsm[master]: Adding call to svdsm.ping from vdsm ping verb
by ybronhei＠redhat.com 01 Oct '13

01 Oct '13

Yaniv Bronhaim has uploaded a new change for review. Change subject: Adding call to svdsm.ping from vdsm ping verb ...................................................................... Adding call to svdsm.ping from vdsm ping verb In additional to adding supervdsm ping call, this also allows calling to ping verb from vdsClient Change-Id: Ifc4caf642ce6d8281fa97f7f26ec39575304f339 Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com> --- M client/vdsClient.py M vdsm/API.py 2 files changed, 15 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/18329/1 diff --git a/client/vdsClient.py b/client/vdsClient.py index c52256a..38a09e8 100644 --- a/client/vdsClient.py +++ b/client/vdsClient.py @@ -1744,6 +1744,10 @@ return status['status']['code'], status['status']['message'] + def do_ping(self, args): + return self.ExecAndExit(self.s.ping()) + + if __name__ == '__main__': if _glusterEnabled: serv = ge.GlusterService() @@ -2489,6 +2493,11 @@ '<vmId> <spUUID> <sdUUID> <imgUUID> <volUUID> <newSize>', 'Extends the virtual size of a disk' )), + 'ping': ( + serv.do_ping, ( + '', + 'Check connectivity with vdsm and supervdsm' + )), } if _glusterEnabled: commands.update(ge.getGlusterCmdDict(serv)) diff --git a/vdsm/API.py b/vdsm/API.py index 37bb908..58d6815 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -1152,6 +1152,12 @@ def ping(self): "Ping the server. Useful for tests" + try: + supervdsm.getProxy().ping() + except: + return {'status': + {'code': 1, 'message': "Couldn't ping supervdsm"}} + return {'status': doneCode} def getCapabilities(self): -- To view, visit http://gerrit.ovirt.org/18329 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ifc4caf642ce6d8281fa97f7f26ec39575304f339 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>

4 6

Change in vdsm[master]: libvirtconnection: ping libvirt upon disconnection
by mtayer＠redhat.com 01 Oct '13

01 Oct '13

mooli tayer has uploaded a new change for review. Change subject: libvirtconnection: ping libvirt upon disconnection ...................................................................... libvirtconnection: ping libvirt upon disconnection Upon recieving an error from libvirt we should ping it (using getLibVersion) and only upon failure treat it as down. Libvirt error codes do not distinguish recoverable from non-recoverable libvirt errors. Change-Id: Ia489e46dd8ce4c70c888988d17b86311d3c4b935 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=982634 Signed-off-by: Mooli Tayer <mtayer(a)redhat.com> --- M lib/vdsm/libvirtconnection.py A tests/libvirtconnectionTests.py 2 files changed, 136 insertions(+), 13 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/44/19444/1 diff --git a/lib/vdsm/libvirtconnection.py b/lib/vdsm/libvirtconnection.py index cdba57c..b129c03 100644 --- a/lib/vdsm/libvirtconnection.py +++ b/lib/vdsm/libvirtconnection.py @@ -70,15 +70,15 @@ Wrap methods of connection object so that they catch disconnection, and take the current process down. """ - def wrapMethod(f): - def wrapper(*args, **kwargs): + def wrapMethod(conn, f): + def wrapper(vdsmLibvirtPing=False, *args, **kwargs): try: ret = f(*args, **kwargs) if isinstance(ret, libvirt.virDomain): for name in dir(ret): method = getattr(ret, name) if callable(method) and name[0] != '_': - setattr(ret, name, wrapMethod(method)) + setattr(ret, name, wrapMethod(conn, method)) return ret except libvirt.libvirtError as e: edom = e.get_error_domain() @@ -90,15 +90,19 @@ libvirt.VIR_ERR_NO_CONNECT, libvirt.VIR_ERR_INVALID_CONN) if edom in EDOMAINS and ecode in ECODES: - log.error('connection to libvirt broken.' - ' ecode: %d edom: %d', ecode, edom) - if killOnFailure: - log.error('taking calling process down.') - os.kill(os.getpid(), signal.SIGTERM) - else: - log.debug('Unknown libvirterror: ecode: %d edom: %d ' - 'level: %d message: %s', ecode, edom, - e.get_error_level(), e.get_error_message()) + if not vdsmLibvirtPing: + conn.getLibVersion(vdsmLibvirtPing=True) + else: + log.error('connection to libvirt broken.' + ' ecode: %d edom: %d', ecode, edom) + if killOnFailure: + log.error('taking calling process down.') + os.kill(os.getpid(), signal.SIGTERM) + else: + raise + log.debug('Unknown libvirterror: ecode: %d edom: %d ' + 'level: %d message: %s', ecode, edom, + e.get_error_level(), e.get_error_message()) raise wrapper.__name__ = f.__name__ wrapper.__doc__ = f.__doc__ @@ -128,7 +132,7 @@ for name in dir(libvirt.virConnect): method = getattr(conn, name) if callable(method) and name[0] != '_': - setattr(conn, name, wrapMethod(method)) + setattr(conn, name, wrapMethod(conn, method)) if target is not None: for ev in (libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE, libvirt.VIR_DOMAIN_EVENT_ID_REBOOT, diff --git a/tests/libvirtconnectionTests.py b/tests/libvirtconnectionTests.py new file mode 100644 index 0000000..280dad9 --- /dev/null +++ b/tests/libvirtconnectionTests.py @@ -0,0 +1,119 @@ +# +# Copyright 2012 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# + +from vdsm import libvirtconnection +from testrunner import VdsmTestCase as TestCaseBase +import os +from monkeypatch import MonkeyPatch + + +class TerminationException(Exception): + pass + + +class LibvirtMock: + VIR_CRED_AUTHNAME = 1 + VIR_CRED_PASSPHRASE = 2 + VIR_FROM_RPC = 3 + VIR_FROM_REMOTE = 4 + VIR_ERR_SYSTEM_ERROR = 5 + VIR_ERR_INTERNAL_ERROR = 6 + VIR_ERR_NO_CONNECT = 7 + VIR_ERR_INVALID_CONN = 8 + + class libvirtError(Exception): + def get_error_code(self): + return LibvirtMock.VIR_ERR_SYSTEM_ERROR + + def get_error_domain(self): + return LibvirtMock.VIR_FROM_RPC + + def get_error_level(self): + return 3 + + def get_error_message(self): + return '' + + class virConnect: + failGetLibVersion = False + failNodeDeviceLookupByName = False + + def nodeDeviceLookupByName(self): + if LibvirtMock.virConnect.failNodeDeviceLookupByName: + print('nodeDeviceLookupByName called, exception') + + raise LibvirtMock.libvirtError() + else: + print('nodeDeviceLookupByName called, success') + return '' + + def getLibVersion(self): + if LibvirtMock.virConnect.failGetLibVersion: + print('getLibVersion called, exception') + raise LibvirtMock.libvirtError() + else: + print('getLibVersion called, success') + return '' + + class virDomain(): + pass + + def openAuth(self, *args): + return LibvirtMock.virConnect() + + +def _kill(*args): + raise TerminationException() + + +class testLibvirtconnection(TestCaseBase): + @MonkeyPatch(libvirtconnection, 'libvirt', LibvirtMock()) + def testCallSucceeded(self): + """Positive test - libvirtMock does not raise any errors""" + LibvirtMock.virConnect.failGetLibVersion = False + LibvirtMock.virConnect.failNodeDeviceLookupByName = False + connection = libvirtconnection.get() + connection.nodeDeviceLookupByName() + + @MonkeyPatch(libvirtconnection, 'libvirt', LibvirtMock()) + def testCallFailedConnectionUp(self): + """ + libvirtMock will raise an error when nodeDeviceLookupByName is called. + When getLibVersion is called (used by libvirtconnection to recognize disconnections) + it will not raise an error -> in that case an error should be raised ('Unknown libvirterror'). + """ + connection = libvirtconnection.get(killOnFailure=True) + LibvirtMock.virConnect.failNodeDeviceLookupByName = True + LibvirtMock.virConnect.failGetLibVersion = False + self.assertRaises(LibvirtMock.libvirtError, connection.nodeDeviceLookupByName) + + @MonkeyPatch(libvirtconnection, 'libvirt', LibvirtMock()) + @MonkeyPatch(os, 'kill', _kill) + def testCallFailedConnectionDown(self): + """ + libvirtMock will raise an error when nodeDeviceLookupByName is called. + When getLibVersion is called (used by libvirtconnection to recognize disconnections) + it will also raise an error -> in that case os.kill should be called('connection to libvirt broken.'). + """ + connection = libvirtconnection.get(killOnFailure=True) + LibvirtMock.virConnect.failNodeDeviceLookupByName = True + LibvirtMock.virConnect.failGetLibVersion = True + self.assertRaises(TerminationException, connection.nodeDeviceLookupByName) \ No newline at end of file -- To view, visit http://gerrit.ovirt.org/19444 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia489e46dd8ce4c70c888988d17b86311d3c4b935 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer <mtayer(a)redhat.com>

6 33

Change in vdsm[master]: vdsClient: hibernate now sends required hiberVolHandle
by mpoledni＠redhat.com 01 Oct '13

01 Oct '13

Martin Polednik has uploaded a new change for review. Change subject: vdsClient: hibernate now sends required hiberVolHandle ...................................................................... vdsClient: hibernate now sends required hiberVolHandle - vdsClient did not pass hiberVolHandle, causing it to fail any attempt to hibernate - BindigsXMLRPC defaulted required parameter to None Change-Id: Id703a349bac7d75d3acb7e124392ac05d25e2869 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=802446 Signed-off-by: Martin Polednik <mpoledni(a)redhat.com> --- M client/vdsClient.py M vdsm/BindingXMLRPC.py 2 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/80/18280/1 diff --git a/client/vdsClient.py b/client/vdsClient.py index c52256a..cf93f00 100644 --- a/client/vdsClient.py +++ b/client/vdsClient.py @@ -382,8 +382,8 @@ return self.ExecAndExit(self.s.sendkeys(vmId, args[1:])) def hibernate(self, args): - vmId = args[0] - response = self.s.hibernate(vmId) + vmId, hiberVolHandle = args[0], args[1] + response = self.s.hibernate(vmId, hiberVolHandle) print response['status']['message'] sys.exit(response['status']['code']) diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py index fb65ad4..20aaee1 100644 --- a/vdsm/BindingXMLRPC.py +++ b/vdsm/BindingXMLRPC.py @@ -360,7 +360,7 @@ vm = API.VM(vmId) return vm.desktopSendHcCommand(message) - def vmHibernate(self, vmId, hiberVolHandle=None): + def vmHibernate(self, vmId, hiberVolHandle): vm = API.VM(vmId) return vm.hibernate(hiberVolHandle) -- To view, visit http://gerrit.ovirt.org/18280 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id703a349bac7d75d3acb7e124392ac05d25e2869 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <mpoledni(a)redhat.com>

4 9

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches October 2013