Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

virtme-ng: Set a default for --name #34

Merged
merged 2 commits into from
Nov 20, 2023
Merged

Conversation

zevweiss
Copy link
Contributor

With virtme-ng defaulting to running as the current user instead of root, I've been finding it easy to get confused about whether my shell prompt (which includes the hostname) is from the host or the guest (since they end up identical). Providing a recognizable default for the --name flag makes it easier to tell them apart.

@arighi
Copy link
Owner

arighi commented Nov 18, 2023

I've been thinking to do something like this in the past, but one issue that I've found changing the hostname by default is that some commands may complain about the hostname change, for example:

10:59 arighi@virtme$ sudo su -
sudo: unable to resolve host virtme: Name or service not known
root@virtme:~# 

This doesn't happen if the hostname is identical, so that's why by default I decided to not change the hostname, but provide the --name option to set a different hostname if someone wants to.

Also this is the reason why now we're printing the virtme-ng banner at boot, so at least we can see that a virtme-ng session has started, otherwise it would be very confusing to distinguish between host and guest.

Maybe we should try to set something in the bash prompt? Like adding some a "virtme-ng" string somewhere to the PS1 env variable when the virtme-ng session is started?

@zevweiss
Copy link
Contributor Author

zevweiss commented Nov 18, 2023

Hmm, are there other instances of things that a changed hostname causes problems with? On my system I don't see that behavior, so I wonder if it might be some distro configuration or version-specific sudo behavior that we could just add a little workaround for if it's just that...

[zev@hatter: virtme-ng]% ./vng -r
          _      _
   __   _(_)_ __| |_ _ __ ___   ___       _ __   __ _
   \ \ / / |  __| __|  _   _ \ / _ \_____|  _ \ / _  |
    \ V /| | |  | |_| | | | | |  __/_____| | | | (_| |
     \_/ |_|_|   \__|_| |_| |_|\___|     |_| |_|\__  |
                                                |___/
   kernel version: 6.3.13_1 x86_64

[zev@virtme: virtme-ng]% sudo su -
-sh-5.2# whoami
root

My sudo version, FWIW:

[zev@hatter: virtme-ng]% sudo --version
Sudo version 1.9.14p3
Sudoers policy plugin version 1.9.14p3
Sudoers file grammar version 50
Sudoers I/O plugin version 1.9.14p3
Sudoers audit plugin version 1.9.14p3

Maybe something like this in virtme-ng-init?

diff --git a/src/main.rs b/src/main.rs
index a436b2c83f33..415607cae673 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -317,10 +317,21 @@ fn generate_sudoers() -> io::Result<()> {
     Ok(())
 }
 
+fn generate_hosts() -> io::Result<()> {
+    if let Ok(hostname) = env::var("virtme_hostname") {
+        std::fs::copy("/etc/hosts", "/tmp/hosts")?;
+        let mut h = OpenOptions::new().write(true).append(true).open("/tmp/hosts")?;
+        writeln!(h, "127.0.0.1 {}\n::1 {}", hostname, hostname)?;
+        utils::do_mount("/tmp/hosts", "/etc/hosts", "", libc::MS_BIND as usize, "");
+    }
+    Ok(())
+}
+
 fn override_system_files() {
     generate_fstab().ok();
     generate_shadow().ok();
     generate_sudoers().ok();
+    generate_hosts().ok();
 }

Or possibly just some other little bit of /etc/sudo* stuff we could mask in the guest?

@arighi
Copy link
Owner

arighi commented Nov 19, 2023

....

Maybe something like this in virtme-ng-init?

diff --git a/src/main.rs b/src/main.rs
index a436b2c83f33..415607cae673 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -317,10 +317,21 @@ fn generate_sudoers() -> io::Result<()> {
     Ok(())
 }
 
+fn generate_hosts() -> io::Result<()> {
+    if let Ok(hostname) = env::var("virtme_hostname") {
+        std::fs::copy("/etc/hosts", "/tmp/hosts")?;
+        let mut h = OpenOptions::new().write(true).append(true).open("/tmp/hosts")?;
+        writeln!(h, "127.0.0.1 {}\n::1 {}", hostname, hostname)?;
+        utils::do_mount("/tmp/hosts", "/etc/hosts", "", libc::MS_BIND as usize, "");
+    }
+    Ok(())
+}
+
 fn override_system_files() {
     generate_fstab().ok();
     generate_shadow().ok();
     generate_sudoers().ok();
+    generate_hosts().ok();
 }

Adjusting /etc/hosts seems to do the trick. I'm trying to think if there's any other potential regression that can happen by changing the hostname, but as long as we fix /etc/hosts we should be fine. Can you send a patch for virtme-ng-init with this change? And ideally also changing virtme/guest/virtme-init accordingly (we still support the old bash init script viia --no-virtme-ng-init)?

With that in place I'll be happy to apply also this PR, thanks!

@zevweiss
Copy link
Contributor Author

Sure, init PR here: arighi/virtme-ng-init#4

I figure once that's merged I'll add a commit to this branch doing both a submodule update to incorporate that and the corresponding virtme-init tweak to make it a single atomic change.

@arighi
Copy link
Owner

arighi commented Nov 19, 2023

Sure, init PR here: arighi/virtme-ng-init#4

Applied and ran all my usual tests also with this PR applied, all good!

I figure once that's merged I'll add a commit to this branch doing both a submodule update to incorporate that and the corresponding virtme-init tweak to make it a single atomic change.

Sure, go for it, I was thinking to resync the virtme-ng-init submodule anyway, so we can do it with this PR since we need the hostname change.

Thank you so much for working on this, showing virtme as hostname is really helpful, I like it. BTW do you see any problem if we use virtme-ng as default hostname? I try to be consistent with the virtme-ng naming for all the new features and use virtme only for the legacy stuff.

The update of the virtme_ng_init submodule pulls in a larger batch of
changes; while most of them are non-behavioral code cleanups, also
included is commit a1eb25c ("Add guest hostname to /etc/hosts"),
which makes the corresponding change to this one so that the two inits
remain behaviorally in sync with each other.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
This is basically just to help make it easier to distinguish a shell
prompt within the guest from one on the host (as long as your prompt
incorporates the hostname).

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
@zevweiss
Copy link
Contributor Author

Sure, no problem -- now updated with all of the above.

@arighi arighi merged commit ca6ee9e into arighi:main Nov 20, 2023
3 checks passed
@arighi
Copy link
Owner

arighi commented Nov 20, 2023

Applied, thanks, this one is really useful!

@zevweiss zevweiss deleted the default-name branch November 20, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants