Change in vdsm[master]: API: setHaMaintenanceMode command

gpadgett at redhat.com gpadgett at redhat.com
Thu Feb 6 14:55:47 UTC 2014


Greg Padgett has posted comments on this change.

Change subject: API: setHaMaintenanceMode command
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.ovirt.org/#/c/23264/4/client/vdsClient.py
File client/vdsClient.py:

Line 1446: 
Line 1447:     def do_setHaMaintenanceMode(self, args):
Line 1448:         mode = args[0]
Line 1449:         enabled = utils.tobool(args[1])
Line 1450:         assert len(args) == 2
> the assert before the usage, otherwise you'll get IndexError. btw, i don't 
Ahh of course, done.  Agree that it's a worthy addition.
Line 1451:         stats = self.s.setHaMaintenanceMode(mode, enabled)
Line 1452:         if stats['status']['code']:
Line 1453:             return stats['status']['code'], stats['status']['message']
Line 1454:         return 0, ''


Line 2356:         'setHaMaintenanceMode': (serv.do_setHaMaintenanceMode,
Line 2357:                                  ('<type = global/local>'
Line 2358:                                   ' <enabled = true/false>',
Line 2359:                                   'Enable or disable Hosted Engine HA'
Line 2360:                                   ' maintenance')),
> not sure, but wouldn't it be nice to explain what is global and what is loc
I could go either way.  It's the same idiom from engine ui and hosted engine agent, so if they are setting it here I hope they would be familiar with it.  The explanation is a bit longer than the typical succinct usage message.
Line 2361:         'deleteImage': (serv.deleteImage,
Line 2362:                         ('<sdUUID> <spUUID> <imgUUID> [<postZero>] [<force>]',
Line 2363:                          'Delete Image folder with all volumes.',
Line 2364:                          )),


http://gerrit.ovirt.org/#/c/23264/4/vdsm/API.py
File vdsm/API.py:

Line 1507:         try:
Line 1508:             haClient.HAClient().set_maintenance_mode(mm, enabled)
Line 1509:         except:
Line 1510:             self.log.exception("error setting HA maintenance mode")
Line 1511:             return errCode['haErr']
> still I don't except general catching, and I want to have the exception's i
I checked to see if I could find specific exceptions to catch and found a 'throw Exception', so I think 'except Exception' is the best we can do now anyway.  For logging, logger.exception() will print the exception and a backtrace, basically a shortcut to logger.error(..., exc_info=True).
Line 1512:         return {'status': doneCode}
Line 1513: 
Line 1514:     # take a rough estimate on how much free mem is available for new vm
Line 1515:     # memTotal = memFree + memCached + mem_used_by_non_qemu + resident  .


http://gerrit.ovirt.org/#/c/23264/4/vdsm_api/vdsmapi-schema.json
File vdsm_api/vdsmapi-schema.json:

Line 468: #
Line 469: # @LOCAL:   Suspend Hosted Engine agent on this host, after migrating
Line 470: #           engine VM, if necessary.
Line 471: #
Line 472: # Since: 4.14.0
> shouldn't it be 4.15? we already published 4.14 beta without it. also not s
Not sure either.  We (hosted engine) were hoping to get this feature in 3.4, so I'm not sure how that correlates to vdsm releases.  I'll change it to whichever version fits before it's merged.
Line 473: ##
Line 474: {'enum': 'HaMaintenanceMode',
Line 475:  'data': ['GLOBAL', 'LOCAL']}
Line 476: 


-- 
To view, visit http://gerrit.ovirt.org/23264
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic08c5edb0e9b8cc11eb70ef6a66301335c42aad3
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett at redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list