Change in vdsm[master]: Adding Vdsm cron job check for available packages update

ybronhei at redhat.com ybronhei at redhat.com
Mon Mar 30 13:12:50 UTC 2015


Yaniv Bronhaim has posted comments on this change.

Change subject: Adding Vdsm cron job check for available packages update
......................................................................


Patch Set 10:

(4 comments)

https://gerrit.ovirt.org/#/c/38401/10/vdsm/Makefile.am
File vdsm/Makefile.am:

Line 73: dist_vdsm_SCRIPTS = \
Line 74: 	addNetwork \
Line 75: 	delNetwork \
Line 76: 	get-conf-item \
Line 77: 	updater-tool \
> the name is very generic; yet what it does is just poll for updates. The pr
It planed to be separate to vdsm at all. some plans talk about using it in other services as well as external tool. 

I don't know if it will happen but still vdsm-tool verbs belong to vdsm related scripts\tools and this one is too general.. no? 

(I can find more general verbs like service, but still im not sure about that)
Line 78: 	set-conf-item \
Line 79: 	supervdsmServer \
Line 80: 	vdsm \
Line 81: 	vdsm-restore-net-config \


https://gerrit.ovirt.org/#/c/38401/10/vdsm/updater-tool
File vdsm/updater-tool:

Line 84:             ups = self.update()
Line 85:             for trns in ups:
Line 86:                 if pkgs is None or trns.name in pkgs:
Line 87:                     pkgInfo = {}
Line 88:                     pkgInfo['fullname'] = "%s.%s %s-%s" % (trns.name,
> why do we need to report duplicate info? we have name, we have version, why
rpm reports that in this format as well so I assumed it could be relevant in this format. but you're right, I should split it to pkgInfo['arch'] and add the release to version. will do that.
Line 89:                                                            trns.arch,
Line 90:                                                            trns.version,
Line 91:                                                            trns.release,
Line 92:                                                            )


Line 144:                             'will check all available updates')
Line 145:                         )
Line 146:     parser.add_argument('--provider',
Line 147:                         default='yum',
Line 148:                         choices=('yum', 'dnf'),
> what are our plans regarding apt-get ?
later .. ? we don't plan to add apt-get support for 3.6 but dnf we do
Line 149:                         help=(
Line 150:                             'Specify package-management utility to use. \n'
Line 151:                             'Current available options - %s' %
Line 152:                             ('yum, dnf (default: yum)'))


Line 155:     return parser.parse_args(sys.argv[1:])
Line 156: 
Line 157: 
Line 158: if __name__ == '__main__':
Line 159:     if os.geteuid() != 0:
> don't you want to make it a vdsm-tool command? at least for proper placemen
replied on that in reply to the Makefile comment
Line 160:         outputPrint('updater-tool must run as root')
Line 161:         sys.exit(1)
Line 162: 
Line 163:     args = _parse_args()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1f6b83e1c18c05142da095b0b537fa6d9aecb59e
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag at redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list