On Thu, Oct 15, 2015 at 9:54 AM, Fabiano Fidêncio <fidencio@xxxxxxxxxx> wrote: > Jonathon, > > On Wed, Oct 14, 2015 at 9:26 PM, Jonathon Jongsma <jjongsma@xxxxxxxxxx> wrote: >> The file transfer progress dialog depends on new API provided by >> spice-gtk to display the progress of each individual file being >> transferred between the client and the guest. If we're not building >> against a new enough version of spice-gtk, the dialog will simply be >> disabled. >> >> The dialog also allows each transfer to be cancelled individually, or to >> cancel all ongoing transfers at once. >> --- >> configure.ac | 4 + >> src/Makefile.am | 7 + >> src/virt-viewer-file-transfer-dialog.c | 227 +++++++++++++++++++++++++++++++++ >> src/virt-viewer-file-transfer-dialog.h | 61 +++++++++ >> src/virt-viewer-session-spice.c | 35 +++++ >> 5 files changed, 334 insertions(+) >> create mode 100644 src/virt-viewer-file-transfer-dialog.c >> create mode 100644 src/virt-viewer-file-transfer-dialog.h >> >> diff --git a/configure.ac b/configure.ac >> index cf75f0e..4aad0ba 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -180,8 +180,12 @@ AS_IF([test "x$with_spice_gtk" = "xyes"], >> [PKG_CHECK_MODULES(SPICE_CONTROLLER, [spice-controller])] >> [PKG_CHECK_MODULES(SPICE_PROTOCOL, [spice-protocol >= $SPICE_PROTOCOL_REQUIRED])] >> [AC_DEFINE([HAVE_SPICE_GTK], 1, [Have spice-gtk?])] >> + [PKG_CHECK_EXISTS([spice-client-glib-2.0 >= 0.30.6], >> + [with_file_transfer=1], [with_file_transfer=0])] >> + [AC_DEFINE_UNQUOTED([HAVE_FILE_TRANSFER], [$with_file_transfer], ["Use spice-gtk file transfer capability"])] >> ) >> AM_CONDITIONAL([HAVE_SPICE_GTK], [test "x$with_spice_gtk" = "xyes"]) >> +AM_CONDITIONAL([HAVE_FILE_TRANSFER], [test $with_file_transfer -eq 1]) >> >> AC_ARG_WITH([ovirt], >> AS_HELP_STRING([--without-ovirt], [Ignore presence of librest and disable oVirt support])) >> diff --git a/src/Makefile.am b/src/Makefile.am >> index 1ebc24e..7d4d2cf 100644 >> --- a/src/Makefile.am >> +++ b/src/Makefile.am >> @@ -85,6 +85,13 @@ libvirt_viewer_la_SOURCES += \ >> virt-viewer-display-spice.h \ >> virt-viewer-display-spice.c \ >> $(NULL) >> + >> +if HAVE_FILE_TRANSFER >> +libvirt_viewer_la_SOURCES += \ >> + virt-viewer-file-transfer-dialog.h \ >> + virt-viewer-file-transfer-dialog.c \ >> + $(NULL) >> +endif >> endif >> > > This approach for detecting the spice-gtk version and set the > HAVE_FILE_TRANSFER is okay. But, personally, I would have gone for the > SPICE_GTK_CHECK_VERSION() in the files (considering it will work for > your purposes) :-) > >> if HAVE_OVIRT >> diff --git a/src/virt-viewer-file-transfer-dialog.c b/src/virt-viewer-file-transfer-dialog.c >> new file mode 100644 >> index 0000000..cdc4290 >> --- /dev/null >> +++ b/src/virt-viewer-file-transfer-dialog.c >> @@ -0,0 +1,227 @@ >> +/* >> + * Virt Viewer: A virtual machine console viewer >> + * >> + * Copyright (C) 2015 Red Hat, Inc. >> + * >> + * 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; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * 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, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA >> + */ >> + >> +#include "virt-viewer-file-transfer-dialog.h" >> +#include <glib/gi18n.h> >> + >> +G_DEFINE_TYPE(VirtViewerFileTransferDialog, virt_viewer_file_transfer_dialog, GTK_TYPE_DIALOG) >> + >> +#define FILE_TRANSFER_DIALOG_PRIVATE(o) \ >> + (G_TYPE_INSTANCE_GET_PRIVATE((o), VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG, VirtViewerFileTransferDialogPrivate)) >> + >> +struct _VirtViewerFileTransferDialogPrivate >> +{ >> + /* GHashTable<SpiceFileTransferTask, widgets> */ >> + GHashTable *file_transfers; >> +}; >> + >> + >> +static void >> +virt_viewer_file_transfer_dialog_dispose(GObject *object) >> +{ >> + VirtViewerFileTransferDialog *self = VIRT_VIEWER_FILE_TRANSFER_DIALOG(object); >> + >> + if (self->priv->file_transfers) { >> + g_hash_table_unref(self->priv->file_transfers); >> + self->priv->file_transfers = NULL; >> + } >> + >> + G_OBJECT_CLASS(virt_viewer_file_transfer_dialog_parent_class)->dispose(object); >> +} >> + >> +static void >> +virt_viewer_file_transfer_dialog_class_init(VirtViewerFileTransferDialogClass *klass) >> +{ >> + GObjectClass *object_class = G_OBJECT_CLASS(klass); >> + >> + g_type_class_add_private(klass, sizeof(VirtViewerFileTransferDialogPrivate)); >> + >> + object_class->dispose = virt_viewer_file_transfer_dialog_dispose; >> +} >> + >> +static void >> +dialog_response(GtkDialog *dialog, >> + gint response_id, >> + gpointer user_data G_GNUC_UNUSED) >> +{ >> + VirtViewerFileTransferDialog *self = VIRT_VIEWER_FILE_TRANSFER_DIALOG(dialog); >> + GHashTableIter iter; >> + gpointer key, value; >> + >> + switch (response_id) { >> + case GTK_RESPONSE_CANCEL: >> + /* cancel all current tasks */ >> + g_hash_table_iter_init(&iter, self->priv->file_transfers); >> + >> + while (g_hash_table_iter_next(&iter, &key, &value)) { >> + spice_file_transfer_task_cancel(SPICE_FILE_TRANSFER_TASK(key)); >> + } >> + break; >> + case GTK_RESPONSE_DELETE_EVENT: >> + /* silently ignore */ >> + break; >> + default: >> + g_warn_if_reached(); >> + } >> +} >> + >> +static void task_cancel_clicked(GtkButton *button G_GNUC_UNUSED, >> + gpointer user_data) >> +{ >> + SpiceFileTransferTask *task = SPICE_FILE_TRANSFER_TASK(user_data); > > No need to cast here ... > >> + spice_file_transfer_task_cancel(task); >> +} >> + >> +typedef struct { >> + GtkWidget *vbox; >> + GtkWidget *hbox; >> + GtkWidget *progress; >> + GtkWidget *label; >> + GtkWidget *cancel; >> +} TaskWidgets; >> + >> +static TaskWidgets *task_widgets_new(SpiceFileTransferTask *task) >> +{ >> + TaskWidgets *w = g_new0(TaskWidgets, 1); >> + >> +#if !GTK_CHECK_VERSION(3, 0, 0) >> + w->vbox = gtk_vbox_new(FALSE, 6); >> + w->hbox = gtk_hbox_new(FALSE, 12); >> +#else >> + w->vbox = gtk_box_new(GTK_ORIENTATION_VERTICAL, 6); >> + w->hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 12); >> +#endif >> + w->progress = gtk_progress_bar_new(); >> + w->label = gtk_label_new(spice_file_transfer_task_get_filename(task)); >> +#if !GTK_CHECK_VERSION(3, 0, 0) >> + w->cancel = gtk_button_new_from_stock(GTK_STOCK_CANCEL); >> +#else >> + w->cancel = gtk_button_new_from_icon_name("gtk-cancel", GTK_ICON_SIZE_SMALL_TOOLBAR); >> +#endif >> +#if GTK_CHECK_VERSION(3, 0, 0) >> + gtk_widget_set_hexpand(w->progress, TRUE); >> + gtk_widget_set_valign(w->progress, GTK_ALIGN_CENTER); >> + gtk_widget_set_hexpand(w->label, TRUE); >> + gtk_widget_set_valign(w->label, GTK_ALIGN_END); >> + gtk_widget_set_halign(w->label, GTK_ALIGN_START); >> + gtk_widget_set_hexpand(w->cancel, FALSE); >> + gtk_widget_set_valign(w->cancel, GTK_ALIGN_CENTER); >> +#endif >> + >> + g_signal_connect(w->cancel, "clicked", >> + G_CALLBACK(task_cancel_clicked), task); >> + >> + gtk_box_pack_start(GTK_BOX(w->hbox), w->progress, TRUE, TRUE, 0); >> + gtk_box_pack_start(GTK_BOX(w->hbox), w->cancel, FALSE, TRUE, 0); >> + gtk_box_pack_start(GTK_BOX(w->vbox), w->label, TRUE, TRUE, 0); >> + gtk_box_pack_start(GTK_BOX(w->vbox), w->hbox, TRUE, TRUE, 0); >> + >> + gtk_widget_show_all(w->vbox); >> + return w; >> +} >> + >> +static gboolean delete_event(GtkWidget *widget, >> + GdkEvent *event G_GNUC_UNUSED, >> + gpointer user_data G_GNUC_UNUSED) >> +{ >> + /* don't allow window to be deleted, just process the response signal, >> + * which may result in the window being hidden */ >> + gtk_dialog_response(GTK_DIALOG(widget), GTK_RESPONSE_CANCEL); >> + return TRUE; >> +} >> + >> +static void >> +virt_viewer_file_transfer_dialog_init(VirtViewerFileTransferDialog *self) >> +{ >> + GtkBox *content = GTK_BOX(gtk_dialog_get_content_area(GTK_DIALOG(self))); >> + >> + self->priv = FILE_TRANSFER_DIALOG_PRIVATE(self); >> + >> + gtk_widget_set_size_request(GTK_WIDGET(content), 400, -1); >> + gtk_container_set_border_width(GTK_CONTAINER(content), 12); >> + self->priv->file_transfers = g_hash_table_new_full(g_direct_hash, g_direct_equal, >> + g_object_unref, >> + (GDestroyNotify)g_free); >> + gtk_dialog_add_button(GTK_DIALOG(self), _("Cancel"), GTK_RESPONSE_CANCEL); >> + gtk_dialog_set_default_response(GTK_DIALOG(self), >> + GTK_RESPONSE_CANCEL); >> + g_signal_connect(self, "response", G_CALLBACK(dialog_response), NULL); >> + g_signal_connect(self, "delete-event", G_CALLBACK(delete_event), NULL); >> +} >> + >> +VirtViewerFileTransferDialog * >> +virt_viewer_file_transfer_dialog_new(GtkWindow *parent) >> +{ >> + return g_object_new(VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG, >> + "title", _("File Transfers"), >> + "transient-for", parent, >> + "resizable", FALSE, >> + NULL); >> +} >> + >> +static void task_progress_notify(GObject *object, >> + GParamSpec *pspec G_GNUC_UNUSED, >> + gpointer user_data) >> +{ >> + VirtViewerFileTransferDialog *self = VIRT_VIEWER_FILE_TRANSFER_DIALOG(user_data); >> + SpiceFileTransferTask *task = SPICE_FILE_TRANSFER_TASK(object); >> + TaskWidgets *w = g_hash_table_lookup(self->priv->file_transfers, task); >> + g_return_if_fail(w); >> + >> + double pct = spice_file_transfer_task_get_progress(task); >> + gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(w->progress), pct); >> +} >> + >> +static void task_finished(SpiceFileTransferTask *task, >> + GError *error, >> + gpointer user_data) >> +{ >> + VirtViewerFileTransferDialog *self = VIRT_VIEWER_FILE_TRANSFER_DIALOG(user_data); >> + TaskWidgets *w = g_hash_table_lookup(self->priv->file_transfers, task); >> + >> + if (error && !g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) >> + g_warning("File transfer task %p failed: %s", task, error->message); >> + >> + g_return_if_fail(w); >> + >> + gtk_widget_destroy(w->vbox); >> + >> + g_hash_table_remove(self->priv->file_transfers, task); >> + >> + /* if this is the last transfer, close the dialog */ >> + if (!g_hash_table_size(self->priv->file_transfers)) >> + gtk_widget_hide(GTK_WIDGET(self)); >> +} >> + >> +void virt_viewer_file_transfer_dialog_add_task(VirtViewerFileTransferDialog *self, >> + SpiceFileTransferTask *task) >> +{ >> + GtkBox *content = GTK_BOX(gtk_dialog_get_content_area(GTK_DIALOG(self))); >> + TaskWidgets *w = task_widgets_new(task); >> + >> + gtk_box_pack_start(content, >> + w->vbox, >> + TRUE, TRUE, 12); >> + g_hash_table_insert(self->priv->file_transfers, g_object_ref(task), w); >> + g_signal_connect(task, "notify::progress", G_CALLBACK(task_progress_notify), self); >> + g_signal_connect(task, "finished", G_CALLBACK(task_finished), self); >> + >> + gtk_widget_show(GTK_WIDGET(self)); >> +} >> diff --git a/src/virt-viewer-file-transfer-dialog.h b/src/virt-viewer-file-transfer-dialog.h >> new file mode 100644 >> index 0000000..c6b1034 >> --- /dev/null >> +++ b/src/virt-viewer-file-transfer-dialog.h >> @@ -0,0 +1,61 @@ >> +/* >> + * Virt Viewer: A virtual machine console viewer >> + * >> + * Copyright (C) 2015 Red Hat, Inc. >> + * >> + * 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; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * 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, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA >> + */ >> + >> +#ifndef __VIRT_VIEWER_FILE_TRANSFER_DIALOG_H__ >> +#define __VIRT_VIEWER_FILE_TRANSFER_DIALOG_H__ >> + >> +#include <gtk/gtk.h> >> +#include <spice-client.h> >> + >> +G_BEGIN_DECLS >> + >> +#define VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG virt_viewer_file_transfer_dialog_get_type() >> + >> +#define VIRT_VIEWER_FILE_TRANSFER_DIALOG(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG, VirtViewerFileTransferDialog)) >> +#define VIRT_VIEWER_FILE_TRANSFER_DIALOG_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST((klass), VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG, VirtViewerFileTransferDialogClass)) >> +#define VIRT_VIEWER_IS_FILE_TRANSFER_DIALOG(obj) (G_TYPE_CHECK_INSTANCE_TYPE((obj), VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG)) >> +#define VIRT_VIEWER_IS_FILE_TRANSFER_DIALOG_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass), VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG)) >> +#define VIRT_VIEWER_FILE_TRANSFER_DIALOG_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS((obj), VIRT_VIEWER_TYPE_FILE_TRANSFER_DIALOG, VirtViewerFileTransferDialogClass)) >> + >> +typedef struct _VirtViewerFileTransferDialog VirtViewerFileTransferDialog; >> +typedef struct _VirtViewerFileTransferDialogClass VirtViewerFileTransferDialogClass; >> +typedef struct _VirtViewerFileTransferDialogPrivate VirtViewerFileTransferDialogPrivate; >> + >> +struct _VirtViewerFileTransferDialog >> +{ >> + GtkDialog parent; >> + >> + VirtViewerFileTransferDialogPrivate *priv; >> +}; >> + >> +struct _VirtViewerFileTransferDialogClass >> +{ >> + GtkDialogClass parent_class; >> +}; >> + >> +GType virt_viewer_file_transfer_dialog_get_type(void) G_GNUC_CONST; >> + >> +VirtViewerFileTransferDialog *virt_viewer_file_transfer_dialog_new(GtkWindow *parent); >> +void virt_viewer_file_transfer_dialog_add_task(VirtViewerFileTransferDialog *self, >> + SpiceFileTransferTask *task); >> + >> +G_END_DECLS >> + >> +#endif /* __VIRT_VIEWER_FILE_TRANSFER_DIALOG_H__ */ >> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c >> index eb0761d..dfb106e 100644 >> --- a/src/virt-viewer-session-spice.c >> +++ b/src/virt-viewer-session-spice.c >> @@ -44,6 +44,10 @@ >> #include "gbinding.c" >> #endif >> >> +#if HAVE_FILE_TRANSFER >> +#include "virt-viewer-file-transfer-dialog.h" >> +#endif >> + >> G_DEFINE_TYPE (VirtViewerSessionSpice, virt_viewer_session_spice, VIRT_VIEWER_TYPE_SESSION) >> >> >> @@ -58,6 +62,10 @@ struct _VirtViewerSessionSpicePrivate { >> gboolean has_sw_smartcard_reader; >> guint pass_try; >> gboolean did_auto_conf; >> +#if HAVE_FILE_TRANSFER >> + VirtViewerFileTransferDialog *file_transfer_dialog; >> +#endif >> + >> }; >> >> #define VIRT_VIEWER_SESSION_SPICE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o), VIRT_VIEWER_TYPE_SESSION_SPICE, VirtViewerSessionSpicePrivate)) >> @@ -155,6 +163,12 @@ virt_viewer_session_spice_dispose(GObject *obj) >> spice->priv->audio = NULL; >> >> g_clear_object(&spice->priv->main_window); >> +#if HAVE_FILE_TRANSFER >> + if (spice->priv->file_transfer_dialog) { >> + gtk_widget_destroy(spice->priv->file_transfer_dialog); > > Please, do: > gtk_widget_destroy(GTK_WIDGET(spice->priv->file_transfer_dialog)); > > Otherwise we will have warnings in the build time: > virt-viewer-session-spice.c: In function 'virt_viewer_session_spice_dispose': > virt-viewer-session-spice.c:168:28: warning: passing argument 1 of > 'gtk_widget_destroy' from incompatible pointer type > [-Wincompatible-pointer-types] > gtk_widget_destroy(spice->priv->file_transfer_dialog); > ^ > In file included from /usr/include/gtk-3.0/gtk/gtkapplication.h:27:0, > from /usr/include/gtk-3.0/gtk/gtkwindow.h:33, > from /usr/include/gtk-3.0/gtk/gtkdialog.h:33, > from /usr/include/gtk-3.0/gtk/gtkaboutdialog.h:30, > from /usr/include/gtk-3.0/gtk/gtk.h:31, > from > /home/ffidenci/local/upstream/include/spice-client-gtk-3.0/usb-device-widget.h:28, > from virt-viewer-session-spice.c:34: > /usr/include/gtk-3.0/gtk/gtkwidget.h:639:9: note: expected 'GtkWidget > * {aka struct _GtkWidget *}' but argument is of type > 'VirtViewerFileTransferDialog * {aka struct > _VirtViewerFileTransferDialog *}' > void gtk_widget_destroy (GtkWidget *widget); > >> + spice->priv->file_transfer_dialog = NULL; >> + } >> +#endif >> >> G_OBJECT_CLASS(virt_viewer_session_spice_parent_class)->dispose(obj); >> } >> @@ -232,6 +246,11 @@ virt_viewer_session_spice_constructed(GObject *obj) >> G_CALLBACK(update_share_folder), self, >> G_CONNECT_SWAPPED); >> >> +#if HAVE_FILE_TRANSFER >> + self->priv->file_transfer_dialog = >> + virt_viewer_file_transfer_dialog_new(self->priv->main_window); >> +#endif >> + >> G_OBJECT_CLASS(virt_viewer_session_spice_parent_class)->constructed(obj); >> } >> >> @@ -897,6 +916,18 @@ virt_viewer_session_spice_display_monitors(SpiceChannel *channel, >> >> } >> >> +#if HAVE_FILE_TRANSFER >> +static void >> +on_new_file_transfer(SpiceMainChannel *channel G_GNUC_UNUSED, >> + SpiceFileTransferTask *task, >> + gpointer user_data) >> +{ >> + VirtViewerSessionSpice *self = VIRT_VIEWER_SESSION_SPICE(user_data); >> + virt_viewer_file_transfer_dialog_add_task(self->priv->file_transfer_dialog, >> + task); >> +} >> +#endif >> + >> static void >> virt_viewer_session_spice_channel_new(SpiceSession *s, >> SpiceChannel *channel, >> @@ -929,6 +960,10 @@ virt_viewer_session_spice_channel_new(SpiceSession *s, >> >> virt_viewer_signal_connect_object(channel, "notify::agent-connected", >> G_CALLBACK(agent_connected_changed), self, 0); >> +#if HAVE_FILE_TRANSFER >> + virt_viewer_signal_connect_object(channel, "new-file-transfer", >> + G_CALLBACK(on_new_file_transfer), self, 0); >> +#endif >> } >> >> if (SPICE_IS_DISPLAY_CHANNEL(channel)) { >> -- >> 2.4.3 >> >> _______________________________________________ >> virt-tools-list mailing list >> virt-tools-list@xxxxxxxxxx >> https://www.redhat.com/mailman/listinfo/virt-tools-list > > > While doing the review of these 2 patches I found a problem with your > spice-gtk patches that I would like to have fixed there before pushing > this series to virt-viewer. > The problem in spice-gtk is that we don't support folder's drag and > drop, but we still launch the dialog for this situation, ending up > with a "stuck" dialog, showing no progress, bot being able to be > canceled or closed. The test is simple, just try to transfer a folder > and you'll see what happens. s/bot/not > > Apart from the comments, series look good. > > Best Regards, > -- > Fabiano Fidêncio _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list