Merge lp://staging/~cwayne/ubuntu/trusty/systemd/symlink-support-hostnamed into lp://staging/ubuntu/trusty/systemd

Proposed by Chris Wayne
Status: Merged
Merge reported by: Martin Pitt
Merged at revision: not available
Proposed branch: lp://staging/~cwayne/ubuntu/trusty/systemd/symlink-support-hostnamed
Merge into: lp://staging/ubuntu/trusty/systemd
Diff against target: 188 lines (+112/-11)
2 files modified
debian/patches/support-phablet-etc-writable.patch (+85/-4)
src/hostname/hostnamed.c (+27/-7)
To merge this branch: bzr merge lp://staging/~cwayne/ubuntu/trusty/systemd/symlink-support-hostnamed
Reviewer Review Type Date Requested Status
Martin Pitt Approve
Review via email: mp+201540@code.staging.launchpad.net

Commit message

Follow symlinks to /etc/writable, to allow hostnamed to write the pretty-hostname. This is essentially copying what is done in timedated

Description of the change

Follow symlinks to /etc/writable, to allow hostnamed to write the pretty-hostname. This is essentially copying what is done in timedated

To post a comment you must log in.
Revision history for this message
Martin Pitt (pitti) wrote :

Looks good to me, thanks! I merged this into my local git repository, please don't actually use UDD for this.

review: Approve
Revision history for this message
Alex Chiang (achiang) wrote :

Hi pitti,

This patch came about because of the discussion here:

https://code.launchpad.net/~cwayne18/ubuntu/trusty/bluetooth-touch/bluetooth-touch_lp1266859/+merge/200699

Are we sure that carrying this diff (and patching who knows how many other programs need to write to /etc/hostname) is a better solution than the upstart job?

My thought is that we should try harder to make the upstart job work properly...

Opinions?

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Without doubt, this is a hack. The right way to handle this would be by an overlay on /etc...

However, while the upstart job would work, the right thing to do and recommended upstream (as well as what will continue to work well in BlueZ 5) is to handle the device naming, icon, and device class/chassis information via systemd-hostnamed.

Furthermore, while that particular patch doesn't change a thing on desktop, it does make it possible to use the same logic on Touch as on Desktop to device the device name, icon, and class.

The plan with this is to use hostnamectl calls from an upstart job for now to set that information, and later hook that to the control panel to let a user change the settings.

Revision history for this message
Martin Pitt (pitti) wrote :

If we can do without this additional patch, and that upstart job is sufficient for our needs that would be nice of course. I'm certainly not keen to pile up more of these hacks on top of hacks in Ubuntu Touch and Desktop. With the upstart job you'd get a nice device name, but you could still not change it, right?

Revision history for this message
Chris Wayne (cwayne) wrote :

Right, without this + the livecd-rootfs MR, the user wouldn't be able to change either the hostname or the pretty hostname/BT device name. This will also allow for customization in the hostname (as having the hostname hardcoded to ubuntu-phablet seems rather incorrect as well), as well as the chassis type and icon (both of which are useful for bluetooth).

Also just to point out that this is simply part of the same patch that we're already carrying for systemd (specifically support-phablet-etc-writable.patch), so there shouldn't be any more effort to remove this if we end up with an /etc overlay somewhere in the future. (i.e. removing the patch from timedated would also remove it from hostnamed)

Revision history for this message
Alex Chiang (achiang) wrote :

How important is it for the end-user to change the hostname / BT device name?

How important is it to set the chassis type and icon?

Are these real use cases? They need to be balanced against the maintenance of carrying these patches.

As far as what Mathieu says, I agree there is probably a more proper way to do things, which includes sending a patch to upstream BlueZ. But I thought you already did that and upstream rejected it?

Final thought - let's not let the perfect be the enemy of the good.

I'll let pitti and mathieu make the final call since they are the ones who have to maintain the code. My tendency is to go for simple and good enough which is why I'm asking these questions, but I defer to the maintainers.

Thanks.

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

It is obviously important for the end user to be able to change the hostname / BT device name, if at least so that we can show "Matt's Nexus 4" rather than a generic "Nexus 4" (if there are multiple), or worse yet, the current "ubuntu-phablet-0" which you can't change atm. People will definitely ask for it, altough it's not currently my primary focus.

Changing the chassis type and icon is what really makes the compelling argument for having a writable machine-info -- if it's not writable, we need to reapply the same change every boot, which is an obvious waste of time and resources. The chassis type and icon are what will display when you discover the Touch device from another system; rather than showing as an uncategorized computer, we do want it to show as a phone, or tablet, whatever the device might actually be. This is additionally going to help making things in bluetooth work right.

This is the proper, upstream-blessed way to do this. The patch I wrote was a nice idea, but was dismissed (and also has the disadvantage of not being applicable at all to BlueZ 5, when we'll want to switch over to that).

I want this to be done right, and that we don't have to mess with the feature more than once if possible; so while doing this patch is less than ideal, it's the best we can get for now until there is a proper way to overlay /etc.

Since the patch is pretty much self-contained, it feels to me like it will be easy enough to get rid of when necessary. Other applications normally shouldn't be writing to /etc/hostname or /etc/machine-info, if they do we ought to figure out why and whether that application really is necessary (at least as far as Touch goes). It's far more likely that they'll read rather than write, and won't be affected by the fact that it's a symlink.

TBH, it's also a good idea to play nice and not destroy things that a sysadmin might have done, so I'd even go as far as say that the patch ought to be forwarded upstream (if it hasn't already been dismissed in disgust ;)

Revision history for this message
Martin Pitt (pitti) wrote :

Mathieu Trudel-Lapierre [2014-01-15 16:50 -0000]:
> TBH, it's also a good idea to play nice and not destroy things that
> a sysadmin might have done, so I'd even go as far as say that the
> patch ought to be forwarded upstream (if it hasn't already been
> dismissed in disgust ;)

I hope you don't mean that (and the corresponding timedated) systemd
patch -- this is nothing but an embarassment, and should certainly not
go upstream :-)

Revision history for this message
Chris Wayne (cwayne) wrote :

Also, anything that's trying to /etc/hostname right now is failing anyway since it's R/O, so nothing should break there :)

Revision history for this message
Martin Pitt (pitti) wrote :

Uploaded this to trusty as per Chris' request.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches