Nir Soffer has posted comments on this change.

Change subject: multipath: use a backup file instead of rotateFiles
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/34077/1/vdsm/storage/multipath.py
File vdsm/storage/multipath.py:

Line 174:                 shutil.copyfile(MPATH_CONF, CONF_BACKUP)
Line 175:             except IOError:
Line 176:                 raise se.MultipathSetupError()
Line 177:             else:
Line 178:                 utils.persist(CONF_BACKUP)
This logic is not correct. There is no such thing as most important backup file.

When installing on fresh system, the original file is the default file vdsm 3.4 
and 3.5 has created. This file is useless for backup, since it just disable all 
devices.

When installing on a system which used multipath before, the original file may 
be important. But if the user has changed multipath configuration after that, 
and then run vdsm-tool configure again (we have a bug on this), the most 
important file is the current one.

So if we want to move away from rotating files, the correct solution would be 
to backup the file on every change with a timestamp.

This is what rpm? does on rhel7 - this is my current /etc/vdsm:

    [root@voodoo6 vdsm]# ls -lht
    total 60K
    -rw-r--r--. 1 root root 7.9K Oct 12 20:58 vdsm.conf
    -rw-r--r--. 1 root root   56 Oct  5 18:35 vdsm.conf.rpmsave
    -rw-r--r--. 1 root root 7.9K Oct  5 16:46 vdsm.conf.20141005183511
    -rw-r--r--. 1 root root 7.8K Sep 11 00:41 vdsm.conf.20140915004721
    -rw-r--r--. 1 root root 7.8K Sep 11 00:41 vdsm.conf.20140925233025

So on every configure, we create this file:

    /etc/multipath.conf.backup-yyyymmddHHMM

And add this note when we configure:

    /etc/multipath.conf was updated. The previous content was saved
    in /etc/multipath.conf.backup-yyyymmddHHMM

Since the user run configure, she can remove the file if it is not needed.

If people will complain about these backups, we can provide a --nobackup flag.
Line 179: 
Line 180: 
Line 181:     with tempfile.NamedTemporaryFile() as f:
Line 182:         f.write(MPATH_CONF_TEMPLATE % {'scsi_id_path': _scsi_id.cmd})


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I96a9251ebc852fcdf39239ba50f43dd2343b83a6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to