Bug 1260747

Some more details are given here: http://0pointer.net/blog/projects/stateless.html

Dan, any historical reason why we create those folders on first run and not as part of the installation?


task_mkdirs(){                                                                  

    _mk_data_center - /rhev - ? any reason we don't create it in installation time?
    _mk_core_path   - /var/log/core ? we do ""install -dDm 1777 %{buildroot}%{_localstatedir}/log/core"" only in rhel. why not always?

    _mk_dom_backup  - /var/log/vdsm/backup - we create it in spec - this can be removed (line 1057) no?

    _mk_run_path    - we do create those subfolders /var/run/vdsm subfolders as part of installation - we can remove the mkdir, no?

    _mk_console_path - /var/run/ovirt-vmconsole-console - isn't ovirt-vmconsole should take care of creating this folder? how can it not be there? why do we have this condition there?

    "/usr/bin/chmod" 1777 /dev/shm                                              
}

(In reply to Yaniv Bronhaim from comment #2)
> Dan, any historical reason why we create those folders on first run and not
> as part of the installation?

There might have been special considerations on ovirt-node. Placing mkdir in startup gives a little bit more protection against someone deleting a required directory by mistake, or chaning its permission. It also provides a more precise error log in case the directory does not exist and cannot be created.

However, these are mostly excuses. We should conform to standards. At most, we can check (and fail startup) if these directories are missing.


> task_mkdirs(){                                                              
> 
> 
>     _mk_data_center - /rhev - ? any reason we don't create it in
> installation time?
>     _mk_core_path   - /var/log/core ? we do ""install -dDm 1777
> %{buildroot}%{_localstatedir}/log/core"" only in rhel. why not always?

commit 670503889: conform to Fedora standards. We need to remove this dropbox completely.

> 
>     _mk_dom_backup  - /var/log/vdsm/backup - we create it in spec - this can
> be removed (line 1057) no?
> 
>     _mk_run_path    - we do create those subfolders /var/run/vdsm subfolders
> as part of installation - we can remove the mkdir, no?

Just double check they exist on boot time (with /var/run being tmpfs) and on the node.

> 
>     _mk_console_path - /var/run/ovirt-vmconsole-console - isn't
> ovirt-vmconsole should take care of creating this folder? how can it not be
> there? why do we have this condition there?

I believe it was only Francesco trying to be nicer and not recreate an existing directory.

> 
>     "/usr/bin/chmod" 1777 /dev/shm                                          

Long long ago this was required by spice server. I think it can (and should!) be gone.

it mostly about cleanup and arrangement of vdsm init script. moving that to 4.1

(In reply to Yaniv Bronhaim from comment #4)
> it mostly about cleanup and arrangement of vdsm init script. moving that to
> 4.1

What's the latest?

I wouldn't try to achieve this in 4.1, I missed that request for awhile.. 
it touches few aspects that require deeper check - probably most of it is for node setup, so it requires some verification, depends of our support back, but that's basically it. I believe it can be done for 4.2 if we want to remove that code, but it's only for the cleanup, if we do those mkdirs during installation already and it works on node.

(In reply to Yaniv Bronhaim from comment #6)
> I wouldn't try to achieve this in 4.1, I missed that request for awhile.. 
> it touches few aspects that require deeper check - probably most of it is
> for node setup, so it requires some verification, depends of our support
> back, but that's basically it. I believe it can be done for 4.2 if we want
> to remove that code, but it's only for the cleanup, if we do those mkdirs
> during installation already and it works on node.

We missed 4.1 with this. I'm wondering if we are going to do it in 4.2.
If we do, now's the time.

Technically we don't need this anymore. It would still help Node NG a little, but it's not as urgent as before.

(In reply to Dan Kenigsberg from comment #3)
> >     _mk_console_path - /var/run/ovirt-vmconsole-console - isn't
> > ovirt-vmconsole should take care of creating this folder? how can it not be
> > there? why do we have this condition there?
> 
> I believe it was only Francesco trying to be nicer and not recreate an
> existing directory.

The problem here is who - between Vdsm and ovirt-vmconsole owns this directory.
Vdsm has optional support for ovirt-vmconsole, so it takes care of setting up the directory as it needs.

In order to remove the snippet from init_common.sh we need to fix Vdsm to
1. assume the directory is present and with right attributes (permissions, labels) if the support for ovirt-vmconsole is enabled
2. use a different path - and set it once? if not

BTW, back in time, I remember we discussed that checking paths, permissions and labels every startup added a bit of security; otherwise Vdsm will blindly trust what happened at install time. Not sure if this is relevant anymore.

The status is:

1. /rhev - we won't touch (see nir's comment on the abandoned patch - http://gerrit.ovirt.org/81898)

2. For /var/run/vdsm  /var/log/backup and /var/log/core we have:

https://gerrit.ovirt.org/#/c/81899/ - Dan suggests something not related to the removal. still wait for approval on that change

Just posted: https://gerrit.ovirt.org/#/c/83473/ and https://gerrit.ovirt.org/83474 - for the rest. 

This was verified with ost using https://gerrit.ovirt.org/82424

3. /var/run/ovirt-vmconsole-console - we won't touch (see comment #9)
"/usr/bin/chmod" 1777 /dev/shm - already removed


So basically we wait to merge https://gerrit.ovirt.org/#/c/81899/ https://gerrit.ovirt.org/#/c/83473/ and ttps://gerrit.ovirt.org/83474

https://gerrit.ovirt.org/81899 - was changed to remove all current vdsm handling with /var/log/core , basically after abrt integration abrt service configures the system where to output core dumps.
Rest of the patches are merged. Moving to MODIFIED

adding CodeChange, running AutomationSanity on this one -> moving to VERIFIED

This bugzilla is included in oVirt 4.2.0 release, published on Dec 20th 2017.

Since the problem described in this bug report should be
resolved in oVirt 4.2.0 release, published on Dec 20th 2017, it has been closed with a resolution of CURRENT RELEASE.

If the solution does not work for you, please open a new bug report.