[daemon] let daemon choose mount target if not specified #3695
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3695 +/- ##
==========================================
- Coverage 88.93% 88.93% -0.01%
==========================================
Files 254 254
Lines 14299 14304 +5
==========================================
+ Hits 12717 12721 +4
- Misses 1582 1583 +1 ☔ View full report in Codecov by Sentry. |
include/multipass/utils.h
Outdated
| @@ -235,6 +235,8 @@ class Utils : public Singleton<Utils> | |||
| virtual QString make_uuid(const std::optional<std::string>& seed = std::nullopt) const; | |||
| virtual void sleep_for(const std::chrono::milliseconds& ms) const; | |||
| virtual bool is_ipv4_valid(const std::string& ipv4) const; | |||
|
|
|||
| virtual Path default_mount_target_from_source(const Path& source) const; | |||
There was a problem hiding this comment.
Could we have a shorter name, please? C++ is verbose enough as it is, and source is already mentioned in the parameter, so perhaps just:
| virtual Path default_mount_target_from_source(const Path& source) const; | |
| virtual Path default_mount_target(const Path& source) const; |
There was a problem hiding this comment.
Sure, makes sense. I'll change that everywhere.
| @@ -582,6 +582,11 @@ bool mp::Utils::is_ipv4_valid(const std::string& ipv4) const | |||
| return true; | |||
| } | |||
|
|
|||
| mp::Path mp::Utils::default_mount_target_from_source(const Path& source) const | |||
| { | |||
| return source.isEmpty() ? "" : QDir{QDir::cleanPath(source)}.dirName().prepend("/home/ubuntu/"); | |||
There was a problem hiding this comment.
Shouldn't we throw if this is empty? You probably want to do that in this function's client (daemon) and make it a pre-condition here that the directory isn't empty, perhaps with an assert.
There was a problem hiding this comment.
There is a purpose for returning an empty path if given an empty path, which is the GUI target preview. If the source field in the GUI is left empty, the target should also be empty. So this just simplifies that process. But I understand that taken in isolation, the better behavior for this method is to error if the source is empty.
There was a problem hiding this comment.
Hmm, well that could be done in the GUI itself, no? Anyway, I am OK keeping it like this as long as the daemon itself handles the case.
andrei-toterman
force-pushed
the
default-mount-target-in-daemon
branch
2 times, most recently
from
9da2922
to
e056ee9
Compare
There was a problem hiding this comment.
Thanks @andrei-toterman! Let's wait for a primary reviewer, but LGTM on secondary review.
andrei-toterman
force-pushed
the
default-mount-target-in-daemon
branch
2 times, most recently
from
57873ac
to
d830495
Compare
andrei-toterman
force-pushed
the
default-mount-target-in-daemon
branch
from
d830495
to
a7b3f43
Compare
This PR adds the functionality for the daemon to choose a mount target path if it is not given. Right now the logic is to take the last directory name in the source path and append it to
/home/ubuntue.g. given the source path/home/andrei/projects/multipass, the default target path would be/home/ubuntu/multipass