Merge #812 #814

812: launch: Clean up instance data if an error occurs during image prep r=townsend2010,ricab a=townsend2010 Fixes #680 814: Teach fmtlib to deal with QString, can remove lots of toStdString() calls r=ricab,townsend2010 a=gerboland RFC. Do we want? Co-authored-by: Chris Townsend <[email protected]> Co-authored-by: Ricardo Abreu <[email protected]> Co-authored-by: Gerry Boland <[email protected]>
canonical · May 31, 2019 · 69cfb24 · 69cfb24
3 parents 50fe23b + cc5ae88 + e86e6d1
commit 69cfb24
Show file tree

Hide file tree

Showing 44 changed files with 144 additions and 91 deletions.
diff --git a/include/multipass/cli/command.h b/include/multipass/cli/command.h
@@ -20,15 +20,14 @@
 
 #include <multipass/callable_traits.h>
 #include <multipass/cli/return_codes.h>
+#include <multipass/format.h>
 #include <multipass/rpc/multipass.grpc.pb.h>
 #include <multipass/terminal.h>
 #include <multipass/utils.h>
 
 #include <QLocalSocket>
 #include <QString>
 
-#include <fmt/format.h>
-
 #include <grpc++/grpc++.h>
 
 namespace multipass

diff --git a/include/multipass/format.h b/include/multipass/format.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2019 Canonical, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef MULTIPASS_FORMAT_H
+#define MULTIPASS_FORMAT_H
+
+#include <fmt/format.h>
+#include <fmt/ostream.h>
+#include <QString>
+
+namespace fmt
+{
+
+template <>
+struct formatter<QString>
+{
+    template <typename ParseContext>
+    constexpr auto parse(ParseContext& ctx)
+    {
+        return ctx.begin();
+    }
+
+    template <typename FormatContext>
+    auto format(const QString& a, FormatContext& ctx)
+    {
+        return format_to(ctx.begin(), "{}", a.toStdString()); // TODO: remove the copy?
+    }
+};
+
+} // namespace fmt
+
+
+#endif // MULTIPASS_FORMAT_H
diff --git a/src/cert/ssl_cert_provider.cpp b/src/cert/ssl_cert_provider.cpp
@@ -24,7 +24,7 @@
 #include <openssl/rand.h>
 #include <openssl/x509.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QFile>
 
@@ -45,8 +45,7 @@ class WritableFile
     explicit WritableFile(const QString& name) : fp{fopen(name.toStdString().c_str(), "wb"), fclose}
     {
         if (fp == nullptr)
-            throw std::runtime_error(
-                fmt::format("failed to open file '{}': {}({})", name.toStdString(), strerror(errno), errno));
+            throw std::runtime_error(fmt::format("failed to open file '{}': {}({})", name, strerror(errno), errno));
     }
 
     FILE* get() const
@@ -94,8 +93,7 @@ class EVPKey
     {
         WritableFile file{name};
         if (!PEM_write_PrivateKey(file.get(), key.get(), nullptr, nullptr, 0, nullptr, nullptr))
-            throw std::runtime_error(
-                fmt::format("Failed writing certificate private key to file '{}'", name.toStdString()));
+            throw std::runtime_error(fmt::format("Failed writing certificate private key to file '{}'", name));
 
         QFile::setPermissions(name, QFile::ReadOwner);
     }
@@ -179,7 +177,7 @@ class X509Cert
     {
         WritableFile file{name};
         if (!PEM_write_X509(file.get(), x509.get()))
-            throw std::runtime_error(fmt::format("Failed writing certificate to file '{}'", name.toStdString()));
+            throw std::runtime_error(fmt::format("Failed writing certificate to file '{}'", name));
     }
 
 private:

diff --git a/src/client/cli/argparser.cpp b/src/client/cli/argparser.cpp
@@ -17,7 +17,7 @@
 
 #include <multipass/cli/argparser.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QFileInfo>
 #include <QRegExp>

diff --git a/src/client/cli/cmd/launch.cpp b/src/client/cli/cmd/launch.cpp
@@ -23,7 +23,7 @@
 #include <multipass/cli/client_platform.h>
 #include <multipass/constants.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <yaml-cpp/yaml.h>
 

diff --git a/src/client/cli/cmd/mount.cpp b/src/client/cli/cmd/mount.cpp
@@ -24,7 +24,7 @@
 #include <multipass/logging/log.h>
 #include <multipass/sshfs_mount/sftp_server.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QDir>
 #include <QFileInfo>

diff --git a/src/client/cli/cmd/transfer.cpp b/src/client/cli/cmd/transfer.cpp
@@ -205,7 +205,7 @@ mp::ParseCode cmd::Transfer::parse_sources(mp::ArgParser* parser)
             QFileInfo source(source_path);
             if (!source.exists())
             {
-                cerr << fmt::format("Source path \"{}\" does not exist\n", source_path.toStdString());
+                cerr << fmt::format("Source path \"{}\" does not exist\n", source_path);
                 return ParseCode::CommandLineError;
             }
 
@@ -217,7 +217,7 @@ mp::ParseCode cmd::Transfer::parse_sources(mp::ArgParser* parser)
 
             if (!source.isReadable())
             {
-                cerr << fmt::format("Source path \"{}\" is not readable\n", source_path.toStdString());
+                cerr << fmt::format("Source path \"{}\" is not readable\n", source_path);
                 return ParseCode::CommandLineError;
             }
         }
@@ -254,15 +254,15 @@ mp::ParseCode cmd::Transfer::parse_destination(mp::ArgParser* parser)
         {
             if (!destination_file.isWritable())
             {
-                cerr << fmt::format("Destination path \"{}\" is not writable\n", destination_path.toStdString());
+                cerr << fmt::format("Destination path \"{}\" is not writable\n", destination_path);
                 return ParseCode::CommandLineError;
             }
         }
         else
         {
             if (!QFileInfo(destination_file.dir().absolutePath()).isWritable())
             {
-                cerr << fmt::format("Destination path \"{}\" is not writable\n", destination_path.toStdString());
+                cerr << fmt::format("Destination path \"{}\" is not writable\n", destination_path);
                 return ParseCode::CommandLineError;
             }
             else if (sources.size() > 1)

diff --git a/src/client/cli/formatter/csv_formatter.cpp b/src/client/cli/formatter/csv_formatter.cpp
@@ -19,7 +19,7 @@
 
 #include <multipass/cli/format_utils.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 namespace mp = multipass;
 

diff --git a/src/client/cli/formatter/table_formatter.cpp b/src/client/cli/formatter/table_formatter.cpp
@@ -19,7 +19,7 @@
 
 #include <multipass/cli/format_utils.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 namespace mp = multipass;
 

diff --git a/src/client/cli/formatter/yaml_formatter.cpp b/src/client/cli/formatter/yaml_formatter.cpp
@@ -19,7 +19,7 @@
 #include <multipass/cli/format_utils.h>
 #include <multipass/utils.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <yaml-cpp/yaml.h>
 

diff --git a/src/daemon/cli.cpp b/src/daemon/cli.cpp
@@ -21,7 +21,7 @@
 #include <multipass/platform.h>
 #include <multipass/utils.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QCommandLineOption>
 #include <QCommandLineParser>
@@ -80,7 +80,7 @@ mp::DaemonConfigBuilder mp::cli::parse(const QCoreApplication& app)
         else if (logger == "stderr")
             builder.logger = std::make_unique<mpl::StandardLogger>(builder.verbosity_level);
         else
-            throw std::runtime_error(fmt::format("invalid logger option '{}'", logger.toStdString()));
+            throw std::runtime_error(fmt::format("invalid logger option '{}'", logger));
     }
 
     if (parser.isSet(address_option))

diff --git a/src/daemon/common_image_host.cpp b/src/daemon/common_image_host.cpp
@@ -19,7 +19,7 @@
 
 #include <multipass/logging/log.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 namespace mp = multipass;
 namespace mpl = multipass::logging;

diff --git a/src/daemon/custom_image_host.cpp b/src/daemon/custom_image_host.cpp
@@ -22,7 +22,7 @@
 
 #include <multipass/exceptions/download_exception.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QMap>
 #include <QUrl>

diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp
@@ -41,7 +41,7 @@
 #include <multipass/vm_image_host.h>
 #include <multipass/vm_image_vault.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 #include <yaml-cpp/yaml.h>
 
 #include <QDir>
@@ -1925,6 +1925,9 @@ void mp::Daemon::create_vm(const CreateRequest* request, grpc::ServerWriter<Crea
             catch (const std::exception& e)
             {
                 preparing_instances.erase(name);
+                release_resources(name);
+                vm_instances.erase(name);
+                persist_instances();
                 status_promise->set_value(grpc::Status(grpc::StatusCode::FAILED_PRECONDITION, e.what(), ""));
             }
 

diff --git a/src/daemon/daemon_main.cpp b/src/daemon/daemon_main.cpp
@@ -33,7 +33,7 @@
 #include <multipass/vm_image_host.h>
 #include <multipass/vm_image_vault.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QCoreApplication>
 

diff --git a/src/daemon/daemon_rpc.cpp b/src/daemon/daemon_rpc.cpp
@@ -22,7 +22,7 @@
 #include <multipass/virtual_machine_factory.h>
 #include <multipass/vm_image_host.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <chrono>
 #include <stdexcept>

diff --git a/src/daemon/default_vm_image_vault.cpp b/src/daemon/default_vm_image_vault.cpp
@@ -28,7 +28,7 @@
 #include <multipass/vm_image.h>
 #include <multipass/xz_image_decoder.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QCryptographicHash>
 #include <QJsonArray>
@@ -179,7 +179,7 @@ QString copy(const QString& file_name, const QDir& output_dir)
         return {};
 
     if (!QFileInfo::exists(file_name))
-        throw std::runtime_error(fmt::format("{} missing", file_name.toStdString()));
+        throw std::runtime_error(fmt::format("{} missing", file_name));
 
     QFileInfo info{file_name};
     const auto source_name = info.fileName();
@@ -288,7 +288,7 @@ mp::VMImage mp::DefaultVMImageVault::fetch_image(const FetchType& fetch_type, co
         VMImage source_image, vm_image;
 
         if (!QFile::exists(image_url.path()))
-            throw std::runtime_error(fmt::format("Custom image `{}` does not exist.", image_url.path().toStdString()));
+            throw std::runtime_error(fmt::format("Custom image `{}` does not exist.", image_url.path()));
 
         source_image.image_path = image_url.path();
 

diff --git a/src/daemon/delayed_shutdown_timer.cpp b/src/daemon/delayed_shutdown_timer.cpp
@@ -18,7 +18,7 @@
 #include <multipass/delayed_shutdown_timer.h>
 #include <multipass/logging/log.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 namespace mp = multipass;
 namespace mpl = multipass::logging;

diff --git a/src/daemon/ubuntu_image_host.cpp b/src/daemon/ubuntu_image_host.cpp
@@ -23,7 +23,7 @@
 
 #include <multipass/exceptions/download_exception.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QUrl>
 

diff --git a/src/logging/log.cpp b/src/logging/log.cpp
@@ -17,7 +17,7 @@
 
 #include <multipass/logging/log.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QByteArray>
 #include <QString>

diff --git a/src/logging/standard_logger.cpp b/src/logging/standard_logger.cpp
@@ -19,7 +19,7 @@
 
 #include <multipass/utils.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 namespace mp = multipass;
 namespace mpl = multipass::logging;

diff --git a/src/metrics/metrics_provider.cpp b/src/metrics/metrics_provider.cpp
@@ -19,7 +19,7 @@
 #include <multipass/metrics_provider.h>
 #include <multipass/utils.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QDateTime>
 #include <QEventLoop>
@@ -61,13 +61,12 @@ void post_request(const QUrl& metrics_url, const QByteArray& body)
     {
         auto error_msg = QJsonDocument::fromJson(buff).object();
         mpl::log(mpl::Level::info, category,
-                 fmt::format("Metrics error: {} - {}", error_msg["code"].toString().toStdString(),
-                             error_msg["message"].toString().toStdString()));
+                 fmt::format("Metrics error: {} - {}", error_msg["code"].toString(), error_msg["message"].toString()));
     }
     else if (reply->error() != QNetworkReply::NoError)
     {
         QObject::disconnect(reply.get(), 0, 0, 0);
-        throw std::runtime_error(fmt::format("Metrics error: {}", reply->errorString().toStdString()));
+        throw std::runtime_error(fmt::format("Metrics error: {}", reply->errorString()));
     }
 }
 

diff --git a/src/network/url_downloader.cpp b/src/network/url_downloader.cpp
@@ -20,7 +20,7 @@
 #include <multipass/exceptions/download_exception.h>
 #include <multipass/logging/log.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <QDir>
 #include <QEventLoop>
@@ -134,8 +134,7 @@ void mp::URLDownloader::download_to(const QUrl& url, const QString& file_name, i
 
         if (file.write(reply->readAll()) < 0)
         {
-            mpl::log(mpl::Level::error, category,
-                     fmt::format("error writing image: {}", file.errorString().toStdString()));
+            mpl::log(mpl::Level::error, category, fmt::format("error writing image: {}", file.errorString()));
             reply->abort();
         }
         download_timeout.start();

diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp
@@ -29,7 +29,7 @@
 #include <QDir>
 #include <QXmlStreamReader>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 #include <libvirt/virterror.h>
 
@@ -128,7 +128,7 @@ auto generate_xml_config_for(const mp::VirtualMachineDescription& desc, const st
     {
         auto snap_path = QDir(snap);
         snap_path.cd("../current");
-        qemu_path = fmt::format("{}{}", snap_path.path().toStdString(), qemu_path);
+        qemu_path = fmt::format("{}{}", snap_path.path(), qemu_path);
     }
 
     return fmt::format(

diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine_factory.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine_factory.cpp
@@ -22,7 +22,7 @@
 #include <multipass/virtual_machine_description.h>
 #include <shared/linux/backend_utils.h>
 
-#include <fmt/format.h>
+#include <multipass/format.h>
 
 namespace mp = multipass;